Message-ID: <ebd98acb-1b54-cf8f-19cf-13ba3d575c27@fb.com>
Date:   Tue, 6 Jul 2021 09:10:11 -0700
From:   Yonghong Song <yhs@...com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>
CC:     <bpf@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next] libbpf: ignore .eh_frame sections when parsing
 elf files



On 7/6/21 4:51 AM, Toke Høiland-Jørgensen wrote:
> Daniel Borkmann <daniel@...earbox.net> writes:
> 
>> On 7/5/21 12:33 PM, Toke Høiland-Jørgensen wrote:
>>> Daniel Borkmann <daniel@...earbox.net> writes:
>>>> On 6/29/21 1:09 PM, Toke Høiland-Jørgensen wrote:
>>>>> The .eh_frame and .rel.eh_frame sections will be present in BPF object
>>>>> files when compiled using a multi-stage compile pipe like in samples/bpf.
>>>>> This produces errors when loading such a file with libbpf. While the errors
>>>>> are technically harmless, they look odd and confuse users. So add .eh_frame
>>>>> sections to is_sec_name_dwarf() so they will also be ignored by libbpf
>>>>> processing. This gets rid of output like this from samples/bpf:
>>>>>
>>>>> libbpf: elf: skipping unrecognized data section(32) .eh_frame
>>>>> libbpf: elf: skipping relo section(33) .rel.eh_frame for section(32) .eh_frame
>>>>>
>>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
>>>>
>>>> For the samples/bpf case, could we instead just add a -fno-asynchronous-unwind-tables
>>>> to clang as cflags to avoid .eh_frame generation in the first place?
>>>
>>> Ah, great suggestion! Was trying, but failed, to figure out how to do
>>> that. Just tested it, and yeah, that does fix samples; will send a
>>> separate patch to add that.
>>
>> Sounds good, just applied.
> 
> Awesome, thanks!
> 
>>> I still think filtering this section name in libbpf is worthwhile,
>>> though, as the error message is really just noise... WDYT?
>>
>> No strong opinion from my side, I can also see the argument that
>> Andrii made some time ago [0] in that normally you should never see
>> these in a BPF object file. But then ... there's BPF samples giving a
>> wrong sample. ;( And I bet some users might have copied from there,
>> and it's generally confusing from a user experience in libbpf on
>> whether it's harmless or not.
> 
> Yeah, they "shouldn't" be there, but they clearly can be. So given that
> it's pretty trivial to filter it, IMO, that would be the friendly thing
> to do. Let's see what Andrii thinks.
> 
>> Side-question: Did you check if it is still necessary in general to
>> have this multi-stage compile pipe in samples with the native clang
>> frontend invocation (instead of bpf target one)? (Maybe it's time to
>> get rid of it in general.)
> 
> I started looking into this, but chickened out of actually changing it.
> The comment above the rule mentions LLVM 12, so it seems like it has
> been updated fairly recently, specifically in:
> 9618bde489b2 ("samples/bpf: Change Makefile to cope with latest llvm")
> 
> OTOH, that change does seem to be a fix to the native-compilation mode;
> so maybe it would be viable to just change it to straight bpf-target
> clang compilation? Yonghong, any opinion?

Right, the fix is to fix a native-compilation for frontend with using 
bpf target as the backend.

I think it is possible to use bpf-target clang compilation. You need
to generate vmlinux.h (similar to selftests/bpf) and change Makefile
etc.

> 
>> Anyway, would be nice to add further context/description about it to
>> the commit message at least for future reference on what the .eh_frame
>> sections contain exactly and why it's harmless. (Right now it only
>> states that it is but without more concrete rationale, would be good
>> to still add.)
> 
> Sure, can add that and send a v2 :)
> 
> -Toke
> 

Powered by blists - more mailing lists