bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Enum relocations against zero values
@ 2021-02-26 17:47 Lorenz Bauer
  2021-02-26 18:08 ` Yonghong Song
  0 siblings, 1 reply; 7+ messages in thread
From: Lorenz Bauer @ 2021-02-26 17:47 UTC (permalink / raw)
  To: bpf, Andrii Nakryiko, Yonghong Song

Hi Andrii and Yonghong,

I'm playing around with enum CO-RE relocations, and hit the following snag:

    enum e { TWO };
    bpf_core_enum_value_exists(enum e, TWO);

Compiling this with clang-12
(12.0.0-++20210225092616+e0e6b1e39e7e-1~exp1~20210225083321.50) gives
me the following:

internal/btf/testdata/relocs.c:66:2: error:
__builtin_preserve_enum_value argument 1 invalid
        enum_value_exists(enum e, TWO);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
internal/btf/testdata/relocs.c:53:8: note: expanded from macro
'enum_value_exists'
                if (!bpf_core_enum_value_exists(t, v)) { \
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
internal/btf/testdata/bpf_core_read.h:168:32: note: expanded from
macro 'bpf_core_enum_value_exists'
        __builtin_preserve_enum_value(*(typeof(enum_type)
*)enum_value, BPF_ENUMVAL_EXISTS)
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Changing the definition of the enum to

    enum e { TWO = 1 }

compiles successfully. I get the same result for any enum value that
is zero. Is this expected?

Best
Lorenz

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Enum relocations against zero values
  2021-02-26 17:47 Enum relocations against zero values Lorenz Bauer
@ 2021-02-26 18:08 ` Yonghong Song
  2021-02-26 20:43   ` Andrii Nakryiko
  0 siblings, 1 reply; 7+ messages in thread
From: Yonghong Song @ 2021-02-26 18:08 UTC (permalink / raw)
  To: Lorenz Bauer, bpf, Andrii Nakryiko



On 2/26/21 9:47 AM, Lorenz Bauer wrote:
> Hi Andrii and Yonghong,
> 
> I'm playing around with enum CO-RE relocations, and hit the following snag:
> 
>      enum e { TWO };
>      bpf_core_enum_value_exists(enum e, TWO);
> 
> Compiling this with clang-12
> (12.0.0-++20210225092616+e0e6b1e39e7e-1~exp1~20210225083321.50) gives
> me the following:
> 
> internal/btf/testdata/relocs.c:66:2: error:
> __builtin_preserve_enum_value argument 1 invalid
>          enum_value_exists(enum e, TWO);
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> internal/btf/testdata/relocs.c:53:8: note: expanded from macro
> 'enum_value_exists'
>                  if (!bpf_core_enum_value_exists(t, v)) { \
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> internal/btf/testdata/bpf_core_read.h:168:32: note: expanded from
> macro 'bpf_core_enum_value_exists'
>          __builtin_preserve_enum_value(*(typeof(enum_type)
> *)enum_value, BPF_ENUMVAL_EXISTS)
>                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Andrii can comment on MACRO failures.

> 
> Changing the definition of the enum to
> 
>      enum e { TWO = 1 }
> 
> compiles successfully. I get the same result for any enum value that
> is zero. Is this expected?

IIRC, libbpf will try to do relocation against vmlinux BTF.
So here, "enum e" probably does not exist in vmlinux BTF, so
the builtin will return 0. You can try some enum type
existing in vmlinux BTF to see what happens.

> 
> Best
> Lorenz
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Enum relocations against zero values
  2021-02-26 18:08 ` Yonghong Song
@ 2021-02-26 20:43   ` Andrii Nakryiko
  2021-02-27  3:31     ` Yonghong Song
  0 siblings, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2021-02-26 20:43 UTC (permalink / raw)
  To: Yonghong Song; +Cc: Lorenz Bauer, bpf, Andrii Nakryiko

On Fri, Feb 26, 2021 at 10:08 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 2/26/21 9:47 AM, Lorenz Bauer wrote:
> > Hi Andrii and Yonghong,
> >
> > I'm playing around with enum CO-RE relocations, and hit the following snag:
> >
> >      enum e { TWO };
> >      bpf_core_enum_value_exists(enum e, TWO);
> >
> > Compiling this with clang-12
> > (12.0.0-++20210225092616+e0e6b1e39e7e-1~exp1~20210225083321.50) gives
> > me the following:
> >
> > internal/btf/testdata/relocs.c:66:2: error:
> > __builtin_preserve_enum_value argument 1 invalid
> >          enum_value_exists(enum e, TWO);
> >          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > internal/btf/testdata/relocs.c:53:8: note: expanded from macro
> > 'enum_value_exists'
> >                  if (!bpf_core_enum_value_exists(t, v)) { \
> >                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > internal/btf/testdata/bpf_core_read.h:168:32: note: expanded from
> > macro 'bpf_core_enum_value_exists'
> >          __builtin_preserve_enum_value(*(typeof(enum_type)
> > *)enum_value, BPF_ENUMVAL_EXISTS)
> >                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Andrii can comment on MACRO failures.

Yeah, I ran into this a long time ago as well...

I don't actually know why this doesn't work for zeroes. I've tried to
write that macro in a bit different way, but Clang rejects it:

__builtin_preserve_enum_value(({typeof(enum_type) ___xxx =
(enum_value); *(typeof(enum_type)*)&___xxx;}), BPF_ENUMVAL_EXISTS)

And something as straightforward as

__builtin_preserve_enum_value((typeof(enum_type))(enum_value),
BPF_ENUMVAL_EXISTS)

doesn't work as well.

Yonghong, any idea how to write such a macro to work in all cases? Or
why those alternatives don't work? I only get " error:
__builtin_preserve_enum_value argument 1 invalid" with no more
details, so hard to do anything about this.


>
> >
> > Changing the definition of the enum to
> >
> >      enum e { TWO = 1 }
> >
> > compiles successfully. I get the same result for any enum value that
> > is zero. Is this expected?
>
> IIRC, libbpf will try to do relocation against vmlinux BTF.
> So here, "enum e" probably does not exist in vmlinux BTF, so
> the builtin will return 0. You can try some enum type
> existing in vmlinux BTF to see what happens.
>
> >
> > Best
> > Lorenz
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Enum relocations against zero values
  2021-02-26 20:43   ` Andrii Nakryiko
@ 2021-02-27  3:31     ` Yonghong Song
  2021-03-02  4:19       ` Yonghong Song
  0 siblings, 1 reply; 7+ messages in thread
From: Yonghong Song @ 2021-02-27  3:31 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Lorenz Bauer, bpf, Andrii Nakryiko



On 2/26/21 12:43 PM, Andrii Nakryiko wrote:
> On Fri, Feb 26, 2021 at 10:08 AM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 2/26/21 9:47 AM, Lorenz Bauer wrote:
>>> Hi Andrii and Yonghong,
>>>
>>> I'm playing around with enum CO-RE relocations, and hit the following snag:
>>>
>>>       enum e { TWO };
>>>       bpf_core_enum_value_exists(enum e, TWO);
>>>
>>> Compiling this with clang-12
>>> (12.0.0-++20210225092616+e0e6b1e39e7e-1~exp1~20210225083321.50) gives
>>> me the following:
>>>
>>> internal/btf/testdata/relocs.c:66:2: error:
>>> __builtin_preserve_enum_value argument 1 invalid
>>>           enum_value_exists(enum e, TWO);
>>>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> internal/btf/testdata/relocs.c:53:8: note: expanded from macro
>>> 'enum_value_exists'
>>>                   if (!bpf_core_enum_value_exists(t, v)) { \
>>>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> internal/btf/testdata/bpf_core_read.h:168:32: note: expanded from
>>> macro 'bpf_core_enum_value_exists'
>>>           __builtin_preserve_enum_value(*(typeof(enum_type)
>>> *)enum_value, BPF_ENUMVAL_EXISTS)
>>>                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Andrii can comment on MACRO failures.
> 
> Yeah, I ran into this a long time ago as well...
> 
> I don't actually know why this doesn't work for zeroes. I've tried to
> write that macro in a bit different way, but Clang rejects it:
> 
> __builtin_preserve_enum_value(({typeof(enum_type) ___xxx =
> (enum_value); *(typeof(enum_type)*)&___xxx;}), BPF_ENUMVAL_EXISTS)
> 
> And something as straightforward as
> 
> __builtin_preserve_enum_value((typeof(enum_type))(enum_value),
> BPF_ENUMVAL_EXISTS)
> 
> doesn't work as well.
> 
> Yonghong, any idea how to write such a macro to work in all cases? Or
> why those alternatives don't work? I only get " error:
> __builtin_preserve_enum_value argument 1 invalid" with no more
> details, so hard to do anything about this.

This is a clang BPF bug. In certain number classification system,
clang considers 0 as NULL and non-0 as INTEGER. I only checked
INTEGER and hence only non-0 works. All my tests has non-zero
enum values :-(

Will fix the issue soon. Thanks for reporting!

> 
> 
>>
>>>
>>> Changing the definition of the enum to
>>>
>>>       enum e { TWO = 1 }
>>>
>>> compiles successfully. I get the same result for any enum value that
>>> is zero. Is this expected?
>>
>> IIRC, libbpf will try to do relocation against vmlinux BTF.
>> So here, "enum e" probably does not exist in vmlinux BTF, so
>> the builtin will return 0. You can try some enum type
>> existing in vmlinux BTF to see what happens.
>>
>>>
>>> Best
>>> Lorenz
>>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Enum relocations against zero values
  2021-02-27  3:31     ` Yonghong Song
@ 2021-03-02  4:19       ` Yonghong Song
  2021-03-02  5:08         ` Andrii Nakryiko
  2021-03-02  9:51         ` Lorenz Bauer
  0 siblings, 2 replies; 7+ messages in thread
From: Yonghong Song @ 2021-03-02  4:19 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Lorenz Bauer, bpf, Andrii Nakryiko



On 2/26/21 7:31 PM, Yonghong Song wrote:
> 
> 
> On 2/26/21 12:43 PM, Andrii Nakryiko wrote:
>> On Fri, Feb 26, 2021 at 10:08 AM Yonghong Song <yhs@fb.com> wrote:
>>>
>>>
>>>
>>> On 2/26/21 9:47 AM, Lorenz Bauer wrote:
>>>> Hi Andrii and Yonghong,
>>>>
>>>> I'm playing around with enum CO-RE relocations, and hit the 
>>>> following snag:
>>>>
>>>>       enum e { TWO };
>>>>       bpf_core_enum_value_exists(enum e, TWO);
>>>>
>>>> Compiling this with clang-12
>>>> (12.0.0-++20210225092616+e0e6b1e39e7e-1~exp1~20210225083321.50) gives
>>>> me the following:
>>>>
>>>> internal/btf/testdata/relocs.c:66:2: error:
>>>> __builtin_preserve_enum_value argument 1 invalid
>>>>           enum_value_exists(enum e, TWO);
>>>>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> internal/btf/testdata/relocs.c:53:8: note: expanded from macro
>>>> 'enum_value_exists'
>>>>                   if (!bpf_core_enum_value_exists(t, v)) { \
>>>>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> internal/btf/testdata/bpf_core_read.h:168:32: note: expanded from
>>>> macro 'bpf_core_enum_value_exists'
>>>>           __builtin_preserve_enum_value(*(typeof(enum_type)
>>>> *)enum_value, BPF_ENUMVAL_EXISTS)
>>>>                                         
>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Andrii can comment on MACRO failures.
>>
>> Yeah, I ran into this a long time ago as well...
>>
>> I don't actually know why this doesn't work for zeroes. I've tried to
>> write that macro in a bit different way, but Clang rejects it:
>>
>> __builtin_preserve_enum_value(({typeof(enum_type) ___xxx =
>> (enum_value); *(typeof(enum_type)*)&___xxx;}), BPF_ENUMVAL_EXISTS)
>>
>> And something as straightforward as
>>
>> __builtin_preserve_enum_value((typeof(enum_type))(enum_value),
>> BPF_ENUMVAL_EXISTS)
>>
>> doesn't work as well.
>>
>> Yonghong, any idea how to write such a macro to work in all cases? Or
>> why those alternatives don't work? I only get " error:
>> __builtin_preserve_enum_value argument 1 invalid" with no more
>> details, so hard to do anything about this.
> 
> This is a clang BPF bug. In certain number classification system,
> clang considers 0 as NULL and non-0 as INTEGER. I only checked
> INTEGER and hence only non-0 works. All my tests has non-zero
> enum values :-(
> 
> Will fix the issue soon. Thanks for reporting!

Just pushed the fix (https://reviews.llvm.org/D97659) to llvm trunk
this morning. Also filed a request 
(https://bugs.llvm.org/show_bug.cgi?id=49391) to backport the fix to 
12.0.1 release.
it is too late to be included in 12.0.0 release.
Thanks!

> 
>>
>>
>>>
>>>>
>>>> Changing the definition of the enum to
>>>>
>>>>       enum e { TWO = 1 }
>>>>
>>>> compiles successfully. I get the same result for any enum value that
>>>> is zero. Is this expected?
>>>
>>> IIRC, libbpf will try to do relocation against vmlinux BTF.
>>> So here, "enum e" probably does not exist in vmlinux BTF, so
>>> the builtin will return 0. You can try some enum type
>>> existing in vmlinux BTF to see what happens.
>>>
>>>>
>>>> Best
>>>> Lorenz
>>>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Enum relocations against zero values
  2021-03-02  4:19       ` Yonghong Song
@ 2021-03-02  5:08         ` Andrii Nakryiko
  2021-03-02  9:51         ` Lorenz Bauer
  1 sibling, 0 replies; 7+ messages in thread
From: Andrii Nakryiko @ 2021-03-02  5:08 UTC (permalink / raw)
  To: Yonghong Song; +Cc: Lorenz Bauer, bpf, Andrii Nakryiko

On Mon, Mar 1, 2021 at 8:19 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 2/26/21 7:31 PM, Yonghong Song wrote:
> >
> >
> > On 2/26/21 12:43 PM, Andrii Nakryiko wrote:
> >> On Fri, Feb 26, 2021 at 10:08 AM Yonghong Song <yhs@fb.com> wrote:
> >>>
> >>>
> >>>
> >>> On 2/26/21 9:47 AM, Lorenz Bauer wrote:
> >>>> Hi Andrii and Yonghong,
> >>>>
> >>>> I'm playing around with enum CO-RE relocations, and hit the
> >>>> following snag:
> >>>>
> >>>>       enum e { TWO };
> >>>>       bpf_core_enum_value_exists(enum e, TWO);
> >>>>
> >>>> Compiling this with clang-12
> >>>> (12.0.0-++20210225092616+e0e6b1e39e7e-1~exp1~20210225083321.50) gives
> >>>> me the following:
> >>>>
> >>>> internal/btf/testdata/relocs.c:66:2: error:
> >>>> __builtin_preserve_enum_value argument 1 invalid
> >>>>           enum_value_exists(enum e, TWO);
> >>>>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>> internal/btf/testdata/relocs.c:53:8: note: expanded from macro
> >>>> 'enum_value_exists'
> >>>>                   if (!bpf_core_enum_value_exists(t, v)) { \
> >>>>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>> internal/btf/testdata/bpf_core_read.h:168:32: note: expanded from
> >>>> macro 'bpf_core_enum_value_exists'
> >>>>           __builtin_preserve_enum_value(*(typeof(enum_type)
> >>>> *)enum_value, BPF_ENUMVAL_EXISTS)
> >>>>
> >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>
> >>> Andrii can comment on MACRO failures.
> >>
> >> Yeah, I ran into this a long time ago as well...
> >>
> >> I don't actually know why this doesn't work for zeroes. I've tried to
> >> write that macro in a bit different way, but Clang rejects it:
> >>
> >> __builtin_preserve_enum_value(({typeof(enum_type) ___xxx =
> >> (enum_value); *(typeof(enum_type)*)&___xxx;}), BPF_ENUMVAL_EXISTS)
> >>
> >> And something as straightforward as
> >>
> >> __builtin_preserve_enum_value((typeof(enum_type))(enum_value),
> >> BPF_ENUMVAL_EXISTS)
> >>
> >> doesn't work as well.
> >>
> >> Yonghong, any idea how to write such a macro to work in all cases? Or
> >> why those alternatives don't work? I only get " error:
> >> __builtin_preserve_enum_value argument 1 invalid" with no more
> >> details, so hard to do anything about this.
> >
> > This is a clang BPF bug. In certain number classification system,
> > clang considers 0 as NULL and non-0 as INTEGER. I only checked
> > INTEGER and hence only non-0 works. All my tests has non-zero
> > enum values :-(
> >
> > Will fix the issue soon. Thanks for reporting!
>
> Just pushed the fix (https://reviews.llvm.org/D97659) to llvm trunk
> this morning. Also filed a request
> (https://bugs.llvm.org/show_bug.cgi?id=49391) to backport the fix to
> 12.0.1 release.
> it is too late to be included in 12.0.0 release.
> Thanks!
>

Thanks, Yonghong!

> >
> >>
> >>
> >>>
> >>>>
> >>>> Changing the definition of the enum to
> >>>>
> >>>>       enum e { TWO = 1 }
> >>>>
> >>>> compiles successfully. I get the same result for any enum value that
> >>>> is zero. Is this expected?
> >>>
> >>> IIRC, libbpf will try to do relocation against vmlinux BTF.
> >>> So here, "enum e" probably does not exist in vmlinux BTF, so
> >>> the builtin will return 0. You can try some enum type
> >>> existing in vmlinux BTF to see what happens.
> >>>
> >>>>
> >>>> Best
> >>>> Lorenz
> >>>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Enum relocations against zero values
  2021-03-02  4:19       ` Yonghong Song
  2021-03-02  5:08         ` Andrii Nakryiko
@ 2021-03-02  9:51         ` Lorenz Bauer
  1 sibling, 0 replies; 7+ messages in thread
From: Lorenz Bauer @ 2021-03-02  9:51 UTC (permalink / raw)
  To: Yonghong Song; +Cc: Andrii Nakryiko, bpf, Andrii Nakryiko

On Tue, 2 Mar 2021 at 04:19, Yonghong Song <yhs@fb.com> wrote:
>
> Just pushed the fix (https://reviews.llvm.org/D97659) to llvm trunk
> this morning. Also filed a request
> (https://bugs.llvm.org/show_bug.cgi?id=49391) to backport the fix to
> 12.0.1 release.
> it is too late to be included in 12.0.0 release.
> Thanks!

Thank you for the quick fix :)

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-02 10:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-26 17:47 Enum relocations against zero values Lorenz Bauer
2021-02-26 18:08 ` Yonghong Song
2021-02-26 20:43   ` Andrii Nakryiko
2021-02-27  3:31     ` Yonghong Song
2021-03-02  4:19       ` Yonghong Song
2021-03-02  5:08         ` Andrii Nakryiko
2021-03-02  9:51         ` Lorenz Bauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).