| Message ID | 20240408192651.11034-1-david.faust@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | btf: emit symbol refs in DATASEC entries only for BPF [PR114608] | expand |
On 4/8/24 12:26, David Faust wrote: > The behavior introduced in > fa60ac54964 btf: Emit labels in DATASEC bts_offset entries. > > is only fully correct when compiling for the BPF target with BPF CO-RE > enabled. In other cases, depending on optimizations, it can result in > an incorrect symbol reference in the entry bts_offset field for a symbol > which may not be emitted at all, causing link-time undefined symbol > reference errors like in PR114608. > > The offending bts_offset field of BTF_KIND_DATASEC entries is in reality > only currently useful to consumers of BTF information for BPF programs > anyway. Correct the regression by only emitting symbol references in > these entries when compiling for the BPF target. For other targets, the > behavior returns to that prior to fa60ac54964. > > The underlying cause is related to PR 113566 "btf: incorrect > BTF_KIND_DATASEC entries for variables which are optimized out." A > complete fix for 113566 is more involved and unsuitable for stage 4, > but will be addressed in the near future. > > Tested on x86_64-linux-gnu and on x86_64-linux-gnu host for > bpf-unknown-none target. > > OK? > Thanks. > LGTM. I think adding a comment in the bugzilla for 114431 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114431 indicating a further patch would not hurt either. Thanks > gcc/ > PR debug/114608 > * btfout.cc (btf_asm_datasec_entry): Only emit a symbol reference when > generating BTF for BPF CO-RE target. > > gcc/testsuite/ > PR debug/114608 > * gcc.dg/debug/btf/btf-datasec-1.c: Check bts_offset symbol references > only for BPF target. > * gcc.dg/debug/btf/btf-datasec-2.c: Likewise. > * gcc.dg/debug/btf/btf-pr106773.c: Likewise. > --- > gcc/btfout.cc | 2 +- > gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c | 10 ++++++---- > gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c | 7 ++++--- > gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c | 3 ++- > 4 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/gcc/btfout.cc b/gcc/btfout.cc > index 2e2b3524e83..4a8ec4d1ff0 100644 > --- a/gcc/btfout.cc > +++ b/gcc/btfout.cc > @@ -1045,7 +1045,7 @@ btf_asm_datasec_entry (ctf_container_ref ctfc, struct btf_var_secinfo info) > { > const char *symbol_name = get_name_for_datasec_entry (ctfc, info.type); > btf_asm_type_ref ("bts_type", ctfc, info.type); > - if (symbol_name == NULL) > + if (!btf_with_core_debuginfo_p () || symbol_name == NULL) > dw2_asm_output_data (4, info.offset, "bts_offset"); > else > dw2_asm_output_offset (4, symbol_name, NULL, "bts_offset"); > diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c > index c8ebe5d07ca..15b76259218 100644 > --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c > +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c > @@ -19,10 +19,12 @@ > /* { dg-final { scan-assembler-times "0xf000003\[\t \]+\[^\n\]*btt_info" 2 } } */ > /* { dg-final { scan-assembler-times "0xf000001\[\t \]+\[^\n\]*btt_info" 1 } } */ > > -/* The offset entry for each variable in a DATSEC should contain a label. */ > -/* { dg-final { scan-assembler-times "(?:(?:\\.4byte|\\.long|data4\\.ua|\\.ualong|\\.uaword|\\.dword|long|dc\\.l|\\.word)\[\t \]|\\.vbyte\t4,\[\t \]?)_?\[a-e\]\[\t \]+\[^\n\]*bts_offset" 5 } } */ > -/* { dg-final { scan-assembler-times "my_cstruct\[\t \]+\[^\n\]*bts_offset" 1 } } */ > -/* { dg-final { scan-assembler-times "bigarr\[\t \]+\[^\n\]*bts_offset" 1 } } */ > +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 7 { target { ! bpf-*-* } } } } */ > + > +/* For BPF target the offset entry for each variable in a DATSEC should contain a label. */ > +/* { dg-final { scan-assembler-times ".4byte\[\t \]\[a-e\]\[\t \]+\[^\n\]*bts_offset" 5 { target bpf-*-* } } } */ > +/* { dg-final { scan-assembler-times "my_cstruct\[\t \]+\[^\n\]*bts_offset" 1 { target bpf-*-* } } } */ > +/* { dg-final { scan-assembler-times "bigarr\[\t \]+\[^\n\]*bts_offset" 1 { target bpf-*-* } } } */ > > /* Check that strings for each DATASEC have been added to the BTF string table. */ > /* { dg-final { scan-assembler-times "ascii \".data.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */ > diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c > index 857d830e446..a89a239a504 100644 > --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c > +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c > @@ -10,9 +10,10 @@ > /* { dg-final { scan-assembler-times " BTF_KIND_DATASEC '.bar_sec'\[\\r\\n\]+\[^\\r\\n\]*0xf000002\[\t \]+\[^\n\]*btt_info" 1 } } */ > > /* Function entries should have offset with a label and size of 0 at compile time. */ > -/* { dg-final { scan-assembler-times "chacha\[\t \]+\[^\n\]*bts_offset" 1 } } */ > -/* { dg-final { scan-assembler-times "bar\[\t \]+\[^\n\]*bts_offset" 1 } } */ > -/* { dg-final { scan-assembler-times "foo\[\t \]+\[^\n\]*bts_offset" 1 } } */ > +/* { dg-final { scan-assembler-times "chacha\[\t \]+\[^\n\]*bts_offset" 1 { target { bpf-*-* } }} } */ > +/* { dg-final { scan-assembler-times "bar\[\t \]+\[^\n\]*bts_offset" 1 { target { bpf-*-* } } } } */ > +/* { dg-final { scan-assembler-times "foo\[\t \]+\[^\n\]*bts_offset" 1 { target { bpf-*-* } } } } */ > +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 3 { target { ! bpf-*-* } } } } */ > > /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_size" 3 } } */ > > diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c > index c06220eb520..4e3da27d20d 100644 > --- a/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c > +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c > @@ -11,7 +11,8 @@ > > /* { dg-final { scan-assembler-times "ascii \"foo.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */ > > -/* { dg-final { scan-assembler-times "foo\[\t \]+\[^\n\]*bts_offset" 1 } } */ > +/* { dg-final { scan-assembler-times "foo\[\t \]+\[^\n\]*bts_offset" 1 { target { bpf-*-* } } } } */ > +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 1 { target { ! bpf-*-* } } } } */ > /* { dg-final { scan-assembler-times "1\[\t \]+\[^\n\]*bts_size" 1 } } */ > > extern const void foo __attribute__((weak)) __attribute__((section (".ksyms")));
diff --git a/gcc/btfout.cc b/gcc/btfout.cc index 2e2b3524e83..4a8ec4d1ff0 100644 --- a/gcc/btfout.cc +++ b/gcc/btfout.cc @@ -1045,7 +1045,7 @@ btf_asm_datasec_entry (ctf_container_ref ctfc, struct btf_var_secinfo info) { const char *symbol_name = get_name_for_datasec_entry (ctfc, info.type); btf_asm_type_ref ("bts_type", ctfc, info.type); - if (symbol_name == NULL) + if (!btf_with_core_debuginfo_p () || symbol_name == NULL) dw2_asm_output_data (4, info.offset, "bts_offset"); else dw2_asm_output_offset (4, symbol_name, NULL, "bts_offset"); diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c index c8ebe5d07ca..15b76259218 100644 --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c @@ -19,10 +19,12 @@ /* { dg-final { scan-assembler-times "0xf000003\[\t \]+\[^\n\]*btt_info" 2 } } */ /* { dg-final { scan-assembler-times "0xf000001\[\t \]+\[^\n\]*btt_info" 1 } } */ -/* The offset entry for each variable in a DATSEC should contain a label. */ -/* { dg-final { scan-assembler-times "(?:(?:\\.4byte|\\.long|data4\\.ua|\\.ualong|\\.uaword|\\.dword|long|dc\\.l|\\.word)\[\t \]|\\.vbyte\t4,\[\t \]?)_?\[a-e\]\[\t \]+\[^\n\]*bts_offset" 5 } } */ -/* { dg-final { scan-assembler-times "my_cstruct\[\t \]+\[^\n\]*bts_offset" 1 } } */ -/* { dg-final { scan-assembler-times "bigarr\[\t \]+\[^\n\]*bts_offset" 1 } } */ +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 7 { target { ! bpf-*-* } } } } */ + +/* For BPF target the offset entry for each variable in a DATSEC should contain a label. */ +/* { dg-final { scan-assembler-times ".4byte\[\t \]\[a-e\]\[\t \]+\[^\n\]*bts_offset" 5 { target bpf-*-* } } } */ +/* { dg-final { scan-assembler-times "my_cstruct\[\t \]+\[^\n\]*bts_offset" 1 { target bpf-*-* } } } */ +/* { dg-final { scan-assembler-times "bigarr\[\t \]+\[^\n\]*bts_offset" 1 { target bpf-*-* } } } */ /* Check that strings for each DATASEC have been added to the BTF string table. */ /* { dg-final { scan-assembler-times "ascii \".data.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */ diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c index 857d830e446..a89a239a504 100644 --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c @@ -10,9 +10,10 @@ /* { dg-final { scan-assembler-times " BTF_KIND_DATASEC '.bar_sec'\[\\r\\n\]+\[^\\r\\n\]*0xf000002\[\t \]+\[^\n\]*btt_info" 1 } } */ /* Function entries should have offset with a label and size of 0 at compile time. */ -/* { dg-final { scan-assembler-times "chacha\[\t \]+\[^\n\]*bts_offset" 1 } } */ -/* { dg-final { scan-assembler-times "bar\[\t \]+\[^\n\]*bts_offset" 1 } } */ -/* { dg-final { scan-assembler-times "foo\[\t \]+\[^\n\]*bts_offset" 1 } } */ +/* { dg-final { scan-assembler-times "chacha\[\t \]+\[^\n\]*bts_offset" 1 { target { bpf-*-* } }} } */ +/* { dg-final { scan-assembler-times "bar\[\t \]+\[^\n\]*bts_offset" 1 { target { bpf-*-* } } } } */ +/* { dg-final { scan-assembler-times "foo\[\t \]+\[^\n\]*bts_offset" 1 { target { bpf-*-* } } } } */ +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 3 { target { ! bpf-*-* } } } } */ /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_size" 3 } } */ diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c index c06220eb520..4e3da27d20d 100644 --- a/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c @@ -11,7 +11,8 @@ /* { dg-final { scan-assembler-times "ascii \"foo.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */ -/* { dg-final { scan-assembler-times "foo\[\t \]+\[^\n\]*bts_offset" 1 } } */ +/* { dg-final { scan-assembler-times "foo\[\t \]+\[^\n\]*bts_offset" 1 { target { bpf-*-* } } } } */ +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 1 { target { ! bpf-*-* } } } } */ /* { dg-final { scan-assembler-times "1\[\t \]+\[^\n\]*bts_size" 1 } } */ extern const void foo __attribute__((weak)) __attribute__((section (".ksyms")));