Re: libbpf/bpftool inconsistent handling og .data and .bss ?

[Luigi Rizzo <lrizzo@google.com>]

TL;DR; there seems to be a compiler bug with clang-10 and -O2
when struct are in .data -- details below.

On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> >
> > I am experiencing some weirdness in global variables handling
> > in bpftool and libbpf, as described below.
...
> > 2. .bss overrides from userspace are not seen in bpf at runtime
> >
> >     In foo_bpf.c I have "int x = 0;"
> >     In the userspace program, before foo_bpf__load(), I do
> >        obj->bss->x = 1
> >     but after attach, the bpf code does not see the change, ie
> >         "if (x == 0) { .. } else { .. }"
> >     always takes the first branch.
> >
> >     If I initialize "int x = 2" and then do
> >        obj->data->x = 1
> >     the update is seen correctly ie
> >           "if (x == 2) { .. } else { .. }"
> >      takes one or the other depending on whether userspace overrides
> >      the value before foo_bpf__load()
>
> This is quite surprising, given we have explicit selftests validating
> that all this works. And it seems to work. Please check
> prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> it and confirm that it works in your setup?

Ah, this was non intuitive but obvious in hindsight:

.bss is zeroed by the kernel after load(), and since my program
changed the value before foo_bpf__load() , the memory was overwritten
with 0s. I could confirm this by printing the value after load.

If I update obj->data-><something> after __load(),
or even after __attach() given that userspace mmaps .bss and .data,
everything works as expected both for scalars and structs.

> >
> > 3. .data overrides do not seem to work for non-scalar types
> >     In foo_bpf.c I have
> >           struct one { int a; }; // type also visible to userspace
> >           struct one x { .a = 2 }; // avoid bugs #1 and #2
> >     If in userspace I do
> >           obj->data->x.a = 1
> >     the update is not seen in the kernel, ie
> >             "if (x.a == 2) { .. } else { .. }"
> >      always takes the first branch
> >
>
> Similarly, the same skeleton selftest tests this situation. So please
> check selftests first and report if selftests for some reason don't
> work in your case.

Actually test_skeleton.c does _not_ test for struct in .data,
only in .rodata and .bss

There seems to be a compiler error, at least with clang-10 and -O2

Note how the struct case the compiler uses '2' as immediate value
when reading, whereas in the scalar case it correctly dereferences
the pointer to the variable


Disassembly of section fexit/bar:

0000000000000000 foo_struct:
; int BPF_PROG(foo_struct) {
       0:       b7 01 00 00 02 00 00 00 r1 = 2
;   if (x.a == 2 || x.a > 10) { x.a += 10; }
       1:       15 01 02 00 02 00 00 00 if r1 == 2 goto +2 <LBB1_2>
       2:       b7 02 00 00 0b 00 00 00 r2 = 11
       3:       2d 12 04 00 00 00 00 00 if r2 > r1 goto +4 <LBB1_3>

0000000000000020 LBB1_2:
       4:       07 01 00 00 0a 00 00 00 r1 += 10
       5:       18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0 ll
       7:       63 12 00 00 00 00 00 00 *(u32 *)(r2 + 0) = r1

0000000000000040 LBB1_3:
; int BPF_PROG(foo_struct) {
       8:       b7 00 00 00 00 00 00 00 r0 = 0
       9:       95 00 00 00 00 00 00 00 exit

Disassembly of section fexit/baz:

0000000000000000 foo_scalar:
;   if (count_off == 2 || count_off > 10) { count_off += 10; }
       0:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
       2:       61 12 00 00 00 00 00 00 r2 = *(u32 *)(r1 + 0)
       3:       15 02 02 00 02 00 00 00 if r2 == 2 goto +2 <LBB2_2>
       4:       b7 03 00 00 0b 00 00 00 r3 = 11
       5:       2d 23 02 00 00 00 00 00 if r3 > r2 goto +2 <LBB2_3>

0000000000000030 LBB2_2:
       6:       07 02 00 00 0a 00 00 00 r2 += 10
       7:       63 21 00 00 00 00 00 00 *(u32 *)(r1 + 0) = r2

0000000000000040 LBB2_3:
; int BPF_PROG(foo_scalar) {
       8:       b7 00 00 00 00 00 00 00 r0 = 0
       9:       95 00 00 00 00 00 00 00 exit

------------

If I put the struct in .bss then it gets translated correctly:

Disassembly of section fexit/bar:

0000000000000000 foo_struct:
;   if (x.a == 2 || x.a > 10) { x.a += 10; }
       0:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
       2:       61 12 00 00 00 00 00 00 r2 = *(u32 *)(r1 + 0)
       3:       15 02 02 00 02 00 00 00 if r2 == 2 goto +2 <LBB1_2>
       4:       b7 03 00 00 0b 00 00 00 r3 = 11
       5:       2d 23 02 00 00 00 00 00 if r3 > r2 goto +2 <LBB1_3>

0000000000000030 LBB1_2:
       6:       07 02 00 00 0a 00 00 00 r2 += 10
       7:       63 21 00 00 00 00 00 00 *(u32 *)(r1 + 0) = r2

0000000000000040 LBB1_3:
; int BPF_PROG(foo_struct) {
       8:       b7 00 00 00 00 00 00 00 r0 = 0
       9:       95 00 00 00 00 00 00 00 exit