On Wed, Apr 20, 2022 at 12:53:55AM IST, Joanne Koong wrote:
> > [...]
> > There is another issue I noticed while basing other work on this. You have
> > declared bpf_dynptr in UAPI header as:
> >
> > struct bpf_dynptr {
> > __u64 :64;
> > __u64 :64;
> > } __attribute__((aligned(8)));
> >
> > Sadly, in C standard, the compiler is under no obligation to initialize padding
> > bits when the object is zero initialized (using = {}). It is worse, when
> > unrelated struct fields are assigned the padding bits are assumed to attain
> > unspecified values, but compilers are usually conservative in that case (C11
> > 6.2.6.1 p6).
> Thanks for noting this. By "padding bits", you are referring to the
> unnamed fields, correct?
>
> From the commit message in 5eaed6eedbe9, I see:
>
> INTERNATIONAL STANDARD ©ISO/IEC ISO/IEC 9899:201x
> Programming languages — C
> http://www.open-std.org/Jtc1/sc22/wg14/www/docs/n1547.pdf
> page 157:
> Except where explicitly stated otherwise, for the purposes of
> this subclause unnamed members of objects of structure and union
> type do not participate in initialization. Unnamed members of
> structure objects have indeterminate value even after initialization.
>
> so it seems like the best way to address that here is to just have the
> fields be explicitly named, like something like
>
> struct bpf_dynptr {
> __u64 anon1;
> __u64 anon2;
> } __attribute__((aligned(8)))
>
> Do you agree with this assessment?
>
Yes, this should work. Also, maybe 'variable not initialized error' shouldn't be
'verifier internal error', since it would quite common for user to hit it.
--
Kartikeya
