Re: [PATCH bpf-next] libbpf: ignore .eh_frame sections when parsing elf files

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Yonghong Song <yhs@fb.com> writes:

> On 7/6/21 4:51 AM, Toke Høiland-Jørgensen wrote:
>> Daniel Borkmann <daniel@iogearbox.net> writes:
>> 
>>> On 7/5/21 12:33 PM, Toke Høiland-Jørgensen wrote:
>>>> Daniel Borkmann <daniel@iogearbox.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@redhat.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.

Alright, cool. Probably won't get around to looking further at that
before my vacation, but possibly sometime after unless someone beats me
to it :)

-Toke