* [PATCH bpf-next] libbpf: preserve empty DATASEC BTFs during static linking
@ 2021-03-25 5:11 Andrii Nakryiko
2021-03-25 15:31 ` Yonghong Song
0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2021-03-25 5:11 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team, Alexei Starovoitov
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.
Reported-by: Alexei Starovoitov <ast@kernel.org>
Fixes: 8fd27bf69b86 ("libbpf: Add BPF static linker BTF and BTF.ext support")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
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
+ * 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);
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH bpf-next] libbpf: preserve empty DATASEC BTFs during static linking
2021-03-25 5:11 [PATCH bpf-next] libbpf: preserve empty DATASEC BTFs during static linking Andrii Nakryiko
@ 2021-03-25 15:31 ` Yonghong Song
2021-03-26 4:17 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Yonghong Song @ 2021-03-25 15:31 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, netdev, ast, daniel; +Cc: kernel-team, Alexei Starovoitov
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@kernel.org>
> Fixes: 8fd27bf69b86 ("libbpf: Add BPF static linker BTF and BTF.ext support")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Ack with a nit below.
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> 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.
> + * 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);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH bpf-next] libbpf: preserve empty DATASEC BTFs during static linking
2021-03-25 15:31 ` Yonghong Song
@ 2021-03-26 4:17 ` Andrii Nakryiko
0 siblings, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2021-03-26 4:17 UTC (permalink / raw)
To: Yonghong Song
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, Alexei Starovoitov
On Thu, Mar 25, 2021 at 8:31 AM Yonghong Song <yhs@fb.com> 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@kernel.org>
> > Fixes: 8fd27bf69b86 ("libbpf: Add BPF static linker BTF and BTF.ext support")
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> Ack with a nit below.
>
> Acked-by: Yonghong Song <yhs@fb.com>
>
> > ---
> > 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);
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-26 4:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-25 5:11 [PATCH bpf-next] libbpf: preserve empty DATASEC BTFs during static linking Andrii Nakryiko
2021-03-25 15:31 ` Yonghong Song
2021-03-26 4:17 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox