Enum relocations against zero values

Enum relocations against zero values

[Lorenz Bauer]

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

Re: Enum relocations against zero values

[Yonghong Song]

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
> 

Re: Enum relocations against zero values

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

Re: Enum relocations against zero values

[Yonghong Song]

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

Re: Enum relocations against zero values

[Yonghong Song]

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

Re: Enum relocations against zero values

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

Re: Enum relocations against zero values

[Lorenz Bauer]

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