[PATCH] btf: emit symbol refs in DATASEC entries only for BPF [PR114608]

[PATCH] btf: emit symbol refs in DATASEC entries only for BPF [PR114608]

[David Faust]

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.

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")));
-- 
2.43.0

Re: [PATCH] btf: emit symbol refs in DATASEC entries only for BPF [PR114608]

[Indu Bhagat]

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")));