On Thu, Mar 25, 2021 at 8:31 AM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 3/24/21 10:11 PM, Andrii Nakryiko wrote:
> > Ensure that BPF static linker preserves all DATASEC BTF types, even if some of
> > them might not have any variable information at all. It's not completely clear
> > in which cases Clang chooses to not emit variable information, so adding
> > reliable repro is hard. But manual testing showed that this work correctly.
>
> This may happen if the compiler promotes local initialized variable
> contents into .rodata section and there are no global or static
> functions in the program.
>
> For example,
> $ cat t.c
> struct t { char a; char b; char c; };
> void bar(struct t*);
> void find() {
> struct t tmp = {1, 2, 3};
> bar(&tmp);
> }
>
> $ clang -target bpf -O2 -g -S t.c
> you will find:
>
> .long 104 # BTF_KIND_DATASEC(id = 8)
> .long 251658240 # 0xf000000
> .long 0
>
> .ascii ".rodata" # string offset=104
>
> $ clang -target bpf -O2 -g -c t.c
> $ readelf -S t.o | grep data
> [ 4] .rodata PROGBITS 0000000000000000 00000090
>
> >
> > Reported-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> > Fixes: 8fd27bf69b86 ("libbpf: Add BPF static linker BTF and BTF.ext support")
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> Ack with a nit below.
>
> Acked-by: Yonghong Song <yhs@xxxxxx>
>
> > ---
> > tools/lib/bpf/linker.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
> > index 5e0aa2f2c0ca..2c43943da30c 100644
> > --- a/tools/lib/bpf/linker.c
> > +++ b/tools/lib/bpf/linker.c
> > @@ -94,6 +94,7 @@ struct dst_sec {
> > int sec_sym_idx;
> >
> > /* section's DATASEC variable info, emitted on BTF finalization */
> > + bool has_btf;
> > int sec_var_cnt;
> > struct btf_var_secinfo *sec_vars;
> >
> > @@ -1436,6 +1437,15 @@ static int linker_append_btf(struct bpf_linker *linker, struct src_obj *obj)
> > continue;
> > dst_sec = &linker->secs[src_sec->dst_id];
> >
> > + /* Mark section as having BTF regardless of the presence of
> > + * variables. It seems to happen sometimes when BPF object
> > + * file has only static variables inside functions, not
> > + * globally, that DATASEC BTF with zero variables will be
> > + * emitted by Clang. We need to preserve such empty BTF and
>
> Maybe give a more specific example here, e.g.,
> For example, these static variables may be generated by the compiler
> by promoting local array/structure variable initial values.
Yep, thanks for explanations, I'll update the comment and commit message.
>
> > + * just set correct section size.
> > + */
> > + dst_sec->has_btf = true;
> > +
> > t = btf__type_by_id(obj->btf, src_sec->sec_type_id);
> > src_var = btf_var_secinfos(t);
> > n = btf_vlen(t);
> > @@ -1717,7 +1727,7 @@ static int finalize_btf(struct bpf_linker *linker)
> > for (i = 1; i < linker->sec_cnt; i++) {
> > struct dst_sec *sec = &linker->secs[i];
> >
> > - if (!sec->sec_var_cnt)
> > + if (!sec->has_btf)
> > continue;
> >
> > id = btf__add_datasec(btf, sec->sec_name, sec->sec_sz);
> >
