Re: CO-RE: Weird immediate for bpf_core_field_exists

[Yonghong Song <yhs@fb.com>]

On 4/29/21 3:29 AM, Lorenz Bauer wrote:
> Hi Andrii and Yonghong,
> 
> This is probably a case of me holding it wrong, but I figured I would
> share this nonetheless. Given the following C:
> 
> struct s {
>      int _1;
>      char _2;
> };
> 
> typedef struct s s_t;
> 
> union u {
>      int *_1;
>      char *_2;
> };
> 
> __section("socket_filter/fields") int fields() {
>      struct t {
>          union {
>              s_t s[10];
>          };
>          struct {
>              union u u;
>          };
>      } bar;
>      return bpf_core_field_exists((&bar)[1]);
> }
> 
> clang-12 generates the following instructions:
> 
> 0000000000000000 <fields>:
> ;     return bpf_core_field_exists((&bar)[1]);
>         0:    b7 00 00 00 58 00 00 00    r0 = 88
>         1:    95 00 00 00 00 00 00 00    exit
> 
> The weird bit is that the immediate for instruction 0 isn't 1 but 88.
> Coincidentally sizeof(bar) is also 88 bytes.

Thanks for the reporting. This is a compiler issue which didn't handle
invalid case nicely. The following is an explanation.

After macro expansion,
   bpf_core_field_exists((&bar)[1]);
is actually
   __builtin_preserve_field_info((&bar)[1], BPF_FIELD_EXISTS);

For BPF_FIELD_EXISTS, the first argument should be a field access.
But in the above, we got an array access, (&bar)[1], internally,
the compiler keeps track of the offset from the base address, and
this offset is used for most other builtin kinds like
FIELD_BYPE_OFFSET etc.

For relative to &bar, (&bar)[1] has an offset 88. In your
particular case, the code never went inside the routine to
generate correct "patch_imm" (i.e., r0 = <patch_imm> in the above)
since we didn't get a field access. So we got a wrong result.

I will fix this in the compiler by issuing an error so people
can correct their usage. Thanks for reporting!

> 
> $ clang-12 --version
> Ubuntu clang version
> 12.0.0-++20210126113614+510b3d4b3e02-1~exp1~20210126104320.178
> 
> I've tried clang-13 as well, same result.
> 
> Best,
> Lorenz
>