[PATCH 0/2] Fix PR debug/112878 and a BTF issue

[PATCH 0/2] Fix PR debug/112878 and a BTF issue

[Indu Bhagat]

Hi,

The patch series includes two patches: first one is a fix for PR
debug/112878 and the second one is for an existing BTF generation issue.

Testing Notes:
 - Regression tested on x86_64-linux-gnu
 - Tested btf.exp, ctf.exp, bpf.exp for --target=bpf-unknown-none

Thanks,
Indu Bhagat (2):
  ctf: fix PR debug/112878
  btf: do not skip members of data type with type id BTF_VOID_TYPEID

 gcc/btfout.cc                                   |  5 -----
 gcc/dwarf2ctf.cc                                | 15 ++++++++++-----
 .../gcc.dg/debug/btf/btf-bitfields-4.c          |  6 +++---
 gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c   |  9 +++++----
 .../gcc.dg/debug/ctf/ctf-bitfields-5.c          | 17 +++++++++++++++++
 5 files changed, 35 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c

-- 
2.43.0

[PATCH 1/2] ctf: fix PR debug/112878

[Indu Bhagat]

PR debug/112878: ICE: in ctf_add_slice, at ctfc.cc:499 with _BitInt > 255 in a struct and -gctf1

The CTF generation in GCC does not have a mechanism to roll-back an
already added type.  In this testcase presented in the PR, we hit a
representation limit in CTF slices (for a member of a struct) and ICE,
after the type for struct (CTF_K_STRUCT) has already been added to the
container.

To exit gracefully instead, we now check for both the offset and size of
the bitfield to be explicitly <= 255.  If the check fails, we emit the
member with type CTF_K_UNKNOWN.  Note that, the value 255 stems from the
existing binutils libctf checks which were motivated to guard against
malformed inputs.

Although it is not accurate to say that this is a CTF representation
limit, mark the code with TBD_CTF_REPRESENTATION_LIMIT for now so that
this can be taken care of with the next format version bump, when
libctf's checks for the slice data can be lifted as well.

gcc/ChangeLog:
	PR debug/112878
	* dwarf2ctf.cc (gen_ctf_sou_type): Check for conditions before
	call to ctf_add_slice.  Use CTF_K_UNKNOWN type if fail.

gcc/testsuite/ChangeLog:
	PR debug/112878
	* gcc.dg/debug/ctf/ctf-bitfields-5.c: New test.
---
 gcc/dwarf2ctf.cc                                | 15 ++++++++++-----
 .../gcc.dg/debug/ctf/ctf-bitfields-5.c          | 17 +++++++++++++++++
 2 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c

diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index 77d6bf89689..dc59569fe56 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -606,11 +606,16 @@ gen_ctf_sou_type (ctf_container_ref ctfc, dw_die_ref sou, uint32_t kind)
 	      if (attr)
 		bitpos += AT_unsigned (attr);
 
-	      field_type_id = ctf_add_slice (ctfc, CTF_ADD_NONROOT,
-					     field_type_id,
-					     bitpos - field_location,
-					     bitsize,
-					     c);
+	      /* This is not precisely a TBD_CTF_REPRESENTATION_LIMIT, but
+		 surely something to look at for the next format version bump
+		 for CTF.  */
+	      if (bitsize <= 255 && (bitpos - field_location) <= 255)
+		field_type_id = ctf_add_slice (ctfc, CTF_ADD_NONROOT,
+					       field_type_id,
+					       bitpos - field_location,
+					       bitsize, c);
+	      else
+		field_type_id = gen_ctf_unknown_type (ctfc);
 	    }
 
 	  /* Add the field type to the struct or union type.  */
diff --git a/gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c b/gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c
new file mode 100644
index 00000000000..fee8228647c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c
@@ -0,0 +1,17 @@
+/* Bitfield where the bit offset is > 255 is not allowed in CTF.
+
+   PR debug/112878.
+   This testcase is to ensure graceful handling. No slices are expected.  */
+
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-O0 -gctf -dA" } */
+
+/* No slices are expected, but a struct with one member is expected.
+   CTF_K_UNKNOWN is also expected.  */
+/* { dg-final { scan-assembler-times "cts_type" 0 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x1a000001\[\t \]+\[^\n\]*ctt_info" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"unknown.0\"\[\t \]+\[^\n\]*ctf_string" 1 } } */
+
+struct {
+  _BitInt(282) a : 280;
+} b;
-- 
2.43.0

Re: [PATCH 1/2] ctf: fix PR debug/112878

[David Faust]

On 4/10/24 11:25, Indu Bhagat wrote:
> PR debug/112878: ICE: in ctf_add_slice, at ctfc.cc:499 with _BitInt > 255 in a struct and -gctf1
> 
> The CTF generation in GCC does not have a mechanism to roll-back an
> already added type.  In this testcase presented in the PR, we hit a
> representation limit in CTF slices (for a member of a struct) and ICE,
> after the type for struct (CTF_K_STRUCT) has already been added to the
> container.
> 
> To exit gracefully instead, we now check for both the offset and size of
> the bitfield to be explicitly <= 255.  If the check fails, we emit the
> member with type CTF_K_UNKNOWN.  Note that, the value 255 stems from the
> existing binutils libctf checks which were motivated to guard against
> malformed inputs.
> 
> Although it is not accurate to say that this is a CTF representation
> limit, mark the code with TBD_CTF_REPRESENTATION_LIMIT for now so that
> this can be taken care of with the next format version bump, when
> libctf's checks for the slice data can be lifted as well.

OK.

> 
> gcc/ChangeLog:
> 	PR debug/112878
> 	* dwarf2ctf.cc (gen_ctf_sou_type): Check for conditions before
> 	call to ctf_add_slice.  Use CTF_K_UNKNOWN type if fail.
> 
> gcc/testsuite/ChangeLog:
> 	PR debug/112878
> 	* gcc.dg/debug/ctf/ctf-bitfields-5.c: New test.
> ---
>  gcc/dwarf2ctf.cc                                | 15 ++++++++++-----
>  .../gcc.dg/debug/ctf/ctf-bitfields-5.c          | 17 +++++++++++++++++
>  2 files changed, 27 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c
> 
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index 77d6bf89689..dc59569fe56 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -606,11 +606,16 @@ gen_ctf_sou_type (ctf_container_ref ctfc, dw_die_ref sou, uint32_t kind)
>  	      if (attr)
>  		bitpos += AT_unsigned (attr);
>  
> -	      field_type_id = ctf_add_slice (ctfc, CTF_ADD_NONROOT,
> -					     field_type_id,
> -					     bitpos - field_location,
> -					     bitsize,
> -					     c);
> +	      /* This is not precisely a TBD_CTF_REPRESENTATION_LIMIT, but
> +		 surely something to look at for the next format version bump
> +		 for CTF.  */
> +	      if (bitsize <= 255 && (bitpos - field_location) <= 255)
> +		field_type_id = ctf_add_slice (ctfc, CTF_ADD_NONROOT,
> +					       field_type_id,
> +					       bitpos - field_location,
> +					       bitsize, c);
> +	      else
> +		field_type_id = gen_ctf_unknown_type (ctfc);
>  	    }
>  
>  	  /* Add the field type to the struct or union type.  */
> diff --git a/gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c b/gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c
> new file mode 100644
> index 00000000000..fee8228647c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/ctf/ctf-bitfields-5.c
> @@ -0,0 +1,17 @@
> +/* Bitfield where the bit offset is > 255 is not allowed in CTF.
> +
> +   PR debug/112878.
> +   This testcase is to ensure graceful handling. No slices are expected.  */
> +
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-O0 -gctf -dA" } */
> +
> +/* No slices are expected, but a struct with one member is expected.
> +   CTF_K_UNKNOWN is also expected.  */
> +/* { dg-final { scan-assembler-times "cts_type" 0 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x1a000001\[\t \]+\[^\n\]*ctt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"unknown.0\"\[\t \]+\[^\n\]*ctf_string" 1 } } */
> +
> +struct {
> +  _BitInt(282) a : 280;
> +} b;

[PATCH 2/2] btf: do not skip members of data type with type id BTF_VOID_TYPEID

[Indu Bhagat]

Testing the previous fix in gen_ctf_sou_type () reveals an issue in BTF
generation, however: BTF emission was currently decrementing the vlen
(indicating the number of members) to skip members of type CTF_K_UNKNOWN
altogether, but still emitting the BTF for the corresponding member (in
output_asm_btf_sou_fields ()).

One can see malformed BTF by executing the newly added CTF testcase
(gcc.dg/debug/ctf/ctf-bitfields-5.c) with -gbtf instead or even existing
btf-struct-2.c without this patch.

To fix the issue, it makes sense to rather _not_ skip members of data
type of type id BTF_VOID_TYPEID.

gcc/ChangeLog:
	* btfout.cc (btf_asm_type): Do not skip emitting members of
	unknown type.

gcc/testsuite/ChangeLog:
	* btf-bitfields-4.c: Update the vlen check.
	* btf-struct-2.c: Check that member named 'f' with void data
	type is emitted.
---
 gcc/btfout.cc                                    | 5 -----
 gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c | 6 +++---
 gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c    | 9 +++++----
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index 4a8ec4d1ff0..ab491f0297f 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -820,11 +820,6 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
 	  /* Set kflag if this member is a representable bitfield.  */
 	  if (btf_dmd_representable_bitfield_p (ctfc, dmd))
 	    btf_kflag = 1;
-
-	  /* Struct members that refer to unsupported types or bitfield formats
-	     shall be skipped. These are marked during preprocessing.  */
-	  else if (!btf_emit_id_p (dmd->dmd_type))
-	    btf_vlen -= 1;
 	}
     }
 
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c b/gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c
index c00c8b3d87f..d4a6ef6a1eb 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c
@@ -6,14 +6,14 @@
    In this test, we construct a structure such that the bitfield will have
    an offset so large as to be unrepresentable in BTF. We expect that the
    resulting BTF will describe the rest of the structure, ignoring the
-   non-representable bitfield.  */
+   non-representable bitfield by simply using void data type for the same.  */
 
 /* { dg-do compile } */
 /* { dg-options "-O0 -gbtf -dA" } */
 /* { dg-require-effective-target size32plus } */
 
-/* Struct with 3 members and no bitfield (kind_flag not set).  */
-/* { dg-final { scan-assembler-times "\[\t \]0x4000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* Struct with 4 members and no bitfield (kind_flag not set).  */
+/* { dg-final { scan-assembler-times "\[\t \]0x4000004\[\t \]+\[^\n\]*btt_info" 1 } } */
 
 struct bigly
 {
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c
index e9ff06883db..fa7231be75c 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c
@@ -2,14 +2,15 @@
    unsupported type.
 
    BTF does not support vector types (among other things). When
-   generating BTF for a struct (or union) type, members which refer to
-   unsupported types should be skipped.  */
+   generating BTF for a struct (or union) type.  Members which refer to
+   unsupported types should not be skipped, however.  */
 
 /* { dg-do compile } */
 /* { dg-options "-O0 -gbtf -dA" } */
 
-/* Expect a struct with only 2 members - 'f' should not be present.  */
-/* { dg-final { scan-assembler-times "\[\t \]0x4000002\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* Expect a struct with 3 members - 'f' is present but is of data type void.  */
+/* { dg-final { scan-assembler-times "\[\t \]0x4000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times " MEMBER 'f' idx=1\[\\r\\n\]+\[^\\r\\n\]*0\[\t \]+\[^\n\]*btm_type: void" 1 } } */
 
 struct with_float
 {
-- 
2.43.0

Re: [PATCH 2/2] btf: do not skip members of data type with type id BTF_VOID_TYPEID

[David Faust]

Hi Indu,

On 4/10/24 11:25, Indu Bhagat wrote:
> Testing the previous fix in gen_ctf_sou_type () reveals an issue in BTF
> generation, however: BTF emission was currently decrementing the vlen
> (indicating the number of members) to skip members of type CTF_K_UNKNOWN
> altogether, but still emitting the BTF for the corresponding member (in
> output_asm_btf_sou_fields ()).
> 
> One can see malformed BTF by executing the newly added CTF testcase
> (gcc.dg/debug/ctf/ctf-bitfields-5.c) with -gbtf instead or even existing
> btf-struct-2.c without this patch.
> 
> To fix the issue, it makes sense to rather _not_ skip members of data
> type of type id BTF_VOID_TYPEID.

Thank you for catching and fixing this.

FWIW, what to do in such cases for a struct with a member that has no
representation is undefined behavior in BTF.  I certainly agree it's
better not to emit something malformed, and using 'void' is a good
choice.  Better to know there was a member there that could not be
represented than to skip it altogether, and the total struct size
shall still be correct.

OK.
Thanks!

> 
> gcc/ChangeLog:
> 	* btfout.cc (btf_asm_type): Do not skip emitting members of
> 	unknown type.
> 
> gcc/testsuite/ChangeLog:
> 	* btf-bitfields-4.c: Update the vlen check.
> 	* btf-struct-2.c: Check that member named 'f' with void data
> 	type is emitted.
> ---
>  gcc/btfout.cc                                    | 5 -----
>  gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c | 6 +++---
>  gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c    | 9 +++++----
>  3 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index 4a8ec4d1ff0..ab491f0297f 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -820,11 +820,6 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>  	  /* Set kflag if this member is a representable bitfield.  */
>  	  if (btf_dmd_representable_bitfield_p (ctfc, dmd))
>  	    btf_kflag = 1;
> -
> -	  /* Struct members that refer to unsupported types or bitfield formats
> -	     shall be skipped. These are marked during preprocessing.  */
> -	  else if (!btf_emit_id_p (dmd->dmd_type))
> -	    btf_vlen -= 1;
>  	}
>      }
>  
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c b/gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c
> index c00c8b3d87f..d4a6ef6a1eb 100644
> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-bitfields-4.c
> @@ -6,14 +6,14 @@
>     In this test, we construct a structure such that the bitfield will have
>     an offset so large as to be unrepresentable in BTF. We expect that the
>     resulting BTF will describe the rest of the structure, ignoring the
> -   non-representable bitfield.  */
> +   non-representable bitfield by simply using void data type for the same.  */
>  
>  /* { dg-do compile } */
>  /* { dg-options "-O0 -gbtf -dA" } */
>  /* { dg-require-effective-target size32plus } */
>  
> -/* Struct with 3 members and no bitfield (kind_flag not set).  */
> -/* { dg-final { scan-assembler-times "\[\t \]0x4000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* Struct with 4 members and no bitfield (kind_flag not set).  */
> +/* { dg-final { scan-assembler-times "\[\t \]0x4000004\[\t \]+\[^\n\]*btt_info" 1 } } */
>  
>  struct bigly
>  {
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c
> index e9ff06883db..fa7231be75c 100644
> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-struct-2.c
> @@ -2,14 +2,15 @@
>     unsupported type.
>  
>     BTF does not support vector types (among other things). When
> -   generating BTF for a struct (or union) type, members which refer to
> -   unsupported types should be skipped.  */
> +   generating BTF for a struct (or union) type.  Members which refer to
> +   unsupported types should not be skipped, however.  */
>  
>  /* { dg-do compile } */
>  /* { dg-options "-O0 -gbtf -dA" } */
>  
> -/* Expect a struct with only 2 members - 'f' should not be present.  */
> -/* { dg-final { scan-assembler-times "\[\t \]0x4000002\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* Expect a struct with 3 members - 'f' is present but is of data type void.  */
> +/* { dg-final { scan-assembler-times "\[\t \]0x4000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times " MEMBER 'f' idx=1\[\\r\\n\]+\[^\\r\\n\]*0\[\t \]+\[^\n\]*btm_type: void" 1 } } */
>  
>  struct with_float
>  {