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

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

[Toke Høiland-Jørgensen]

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>
---
 tools/lib/bpf/libbpf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 1e04ce724240..676af6be5961 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2906,7 +2906,8 @@ static Elf_Data *elf_sec_data(const struct bpf_object *obj, Elf_Scn *scn)
 static bool is_sec_name_dwarf(const char *name)
 {
 	/* approximation, but the actual list is too long */
-	return strncmp(name, ".debug_", sizeof(".debug_") - 1) == 0;
+	return (strncmp(name, ".debug_", sizeof(".debug_") - 1) == 0 ||
+		strncmp(name, ".eh_frame", sizeof(".eh_frame") - 1) == 0);
 }
 
 static bool ignore_elf_section(GElf_Shdr *hdr, const char *name)
-- 
2.32.0

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

[Daniel Borkmann]

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?

> ---
>   tools/lib/bpf/libbpf.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 1e04ce724240..676af6be5961 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2906,7 +2906,8 @@ static Elf_Data *elf_sec_data(const struct bpf_object *obj, Elf_Scn *scn)
>   static bool is_sec_name_dwarf(const char *name)
>   {
>   	/* approximation, but the actual list is too long */
> -	return strncmp(name, ".debug_", sizeof(".debug_") - 1) == 0;
> +	return (strncmp(name, ".debug_", sizeof(".debug_") - 1) == 0 ||
> +		strncmp(name, ".eh_frame", sizeof(".eh_frame") - 1) == 0);
>   }
>   
>   static bool ignore_elf_section(GElf_Shdr *hdr, const char *name)
> 

Thanks,
Daniel

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

[Toke Høiland-Jørgensen]

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.

I still think filtering this section name in libbpf is worthwhile,
though, as the error message is really just noise... WDYT?

-Toke

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

[Daniel Borkmann]

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.

> 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.

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.)

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.)

Thanks,
Daniel

   [0] https://lore.kernel.org/bpf/CAEf4BzbCiMkQazSe2hky=Jx6QXZiZ2jyf+AuzMJEyAv+_B7vug@mail.gmail.com/

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

[Toke Høiland-Jørgensen]

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?

> 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

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

[Yonghong Song]

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.

> 
>> 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
> 

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

[Toke Høiland-Jørgensen]

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