diff mbox series

btf: change encoding of forward-declared enums [PR111735]

Message ID 20231212223511.15390-1-david.faust@oracle.com
State Committed
Commit 1502d724df85163b14b04e8f67072ca88eac411d
Headers
Series btf: change encoding of forward-declared enums [PR111735] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

David Faust Dec. 12, 2023, 10:35 p.m. UTC 
  The BTF specification does not formally define a representation for
forward-declared enum types such as:

  enum Foo;

Forward-declarations for struct and union types are represented by
BTF_KIND_FWD, which has a 1-bit flag distinguishing the two.

The de-facto standard format used by other tools like clang and pahole
is to represent forward-declared enums as BTF_KIND_ENUM with vlen=0,
i.e. as a regular enum type with no enumerators.  This patch changes
GCC to adopt that format, and makes a couple of minor cleanups in
btf_asm_type ().

Bootstrapped and tested on x86_64-linux-gnu.
Also tested on x86_64-linux-gnu host for bpf-unknown-none target.

gcc/

	PR debug/111735
	* btfout.cc (btf_fwd_to_enum_p): New.
	(btf_asm_type_ref): Special case references to enum forwards.
	(btf_asm_type): Special case enum forwards. Rename btf_size_type to
	btf_size, and change chained ifs switching on btf_kind into else ifs.

gcc/testsuite/

	PR debug/111735
	* gcc.dg/debug/btf/btf-forward-2.c: New test.
---
 gcc/btfout.cc                                 | 46 ++++++++++++++-----
 .../gcc.dg/debug/btf/btf-forward-2.c          | 18 ++++++++
 2 files changed, 53 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-forward-2.c

Comments

Indu Bhagat Dec. 19, 2023, 5:29 p.m. UTC | #1 
On 12/12/23 14:35, David Faust wrote:
> The BTF specification does not formally define a representation for
> forward-declared enum types such as:
> 
>    enum Foo;
> 
> Forward-declarations for struct and union types are represented by
> BTF_KIND_FWD, which has a 1-bit flag distinguishing the two.
> 
> The de-facto standard format used by other tools like clang and pahole
> is to represent forward-declared enums as BTF_KIND_ENUM with vlen=0,
> i.e. as a regular enum type with no enumerators.  This patch changes
> GCC to adopt that format, and makes a couple of minor cleanups in
> btf_asm_type ().
> 
> Bootstrapped and tested on x86_64-linux-gnu.
> Also tested on x86_64-linux-gnu host for bpf-unknown-none target.
> 

LGTM.

Thanks

> gcc/
> 
> 	PR debug/111735
> 	* btfout.cc (btf_fwd_to_enum_p): New.
> 	(btf_asm_type_ref): Special case references to enum forwards.
> 	(btf_asm_type): Special case enum forwards. Rename btf_size_type to
> 	btf_size, and change chained ifs switching on btf_kind into else ifs.
> 
> gcc/testsuite/
> 
> 	PR debug/111735
> 	* gcc.dg/debug/btf/btf-forward-2.c: New test.
> ---
>   gcc/btfout.cc                                 | 46 ++++++++++++++-----
>   .../gcc.dg/debug/btf/btf-forward-2.c          | 18 ++++++++
>   2 files changed, 53 insertions(+), 11 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-forward-2.c
> 
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index db4f1084f85..3ec938874b6 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -268,6 +268,17 @@ btf_emit_id_p (ctf_id_t id)
>   	  && (btf_id_map[id] <= BTF_MAX_TYPE));
>   }
>   
> +/* Return true if DTD is a forward-declared enum.  The BTF representation
> +   of forward declared enums is not formally defined.  */
> +
> +static bool
> +btf_fwd_to_enum_p (ctf_dtdef_ref dtd)
> +{
> +  uint32_t btf_kind = get_btf_kind (CTF_V2_INFO_KIND (dtd->dtd_data.ctti_info));
> +
> +  return (btf_kind == BTF_KIND_FWD && dtd->dtd_data.ctti_type == CTF_K_ENUM);
> +}
> +
>   /* Each BTF type can be followed additional, variable-length information
>      completing the description of the type. Calculate the number of bytes
>      of variable information required to encode a given type.  */
> @@ -753,8 +764,12 @@ btf_asm_type_ref (const char *prefix, ctf_container_ref ctfc, ctf_id_t ref_id)
>         uint32_t ref_kind
>   	= get_btf_kind (CTF_V2_INFO_KIND (ref_type->dtd_data.ctti_info));
>   
> +      const char *kind_name = btf_fwd_to_enum_p (ref_type)
> +	? btf_kind_name (BTF_KIND_ENUM)
> +	: btf_kind_name (ref_kind);
> +
>         dw2_asm_output_data (4, ref_id, "%s: (BTF_KIND_%s '%s')",
> -			   prefix, btf_kind_name (ref_kind),
> +			   prefix, kind_name,
>   			   get_btf_type_name (ref_type));
>       }
>   }
> @@ -765,11 +780,11 @@ btf_asm_type_ref (const char *prefix, ctf_container_ref ctfc, ctf_id_t ref_id)
>   static void
>   btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>   {
> -  uint32_t btf_kind, btf_kflag, btf_vlen, btf_size_type;
> +  uint32_t btf_kind, btf_kflag, btf_vlen, btf_size;
>     uint32_t ctf_info = dtd->dtd_data.ctti_info;
>   
>     btf_kind = get_btf_kind (CTF_V2_INFO_KIND (ctf_info));
> -  btf_size_type = dtd->dtd_data.ctti_type;
> +  btf_size = dtd->dtd_data.ctti_size;
>     btf_vlen = CTF_V2_INFO_VLEN (ctf_info);
>   
>     /* By now any unrepresentable types have been removed.  */
> @@ -777,7 +792,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>   
>     /* Size 0 integers are redundant definitions of void. None should remain
>        in the types list by this point.  */
> -  gcc_assert (btf_kind != BTF_KIND_INT || btf_size_type >= 1);
> +  gcc_assert (btf_kind != BTF_KIND_INT || btf_size >= 1);
>   
>     /* Re-encode the ctti_info to BTF.  */
>     /* kflag is 1 for structs/unions with a bitfield member.
> @@ -810,16 +825,26 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>        structs and forwards to unions. The dwarf2ctf conversion process stores
>        the kind of the forward in ctti_type, but for BTF this must be 0 for
>        forwards, with only the KIND_FLAG to distinguish.
> -     At time of writing, BTF forwards to enums are unspecified.  */
> -  if (btf_kind == BTF_KIND_FWD)
> +     Forwards to enum types are special-cased below.  */
> +  else if (btf_kind == BTF_KIND_FWD)
>       {
>         if (dtd->dtd_data.ctti_type == CTF_K_UNION)
>   	btf_kflag = 1;
>   
> -      btf_size_type = 0;
> +      /* PR debug/111735.  Encode foward-declared enums as BTF_KIND_ENUM
> +	 with vlen=0.  A representation for these is not formally defined;
> +	 this is the de-facto standard used by other tools like clang
> +	 and pahole.  */
> +      else if (dtd->dtd_data.ctti_type == CTF_K_ENUM)
> +	{
> +	  btf_kind = BTF_KIND_ENUM;
> +	  btf_vlen = 0;
> +	}
> +
> +      btf_size = 0;
>       }
>   
> -  if (btf_kind == BTF_KIND_ENUM)
> +  else if (btf_kind == BTF_KIND_ENUM)
>       {
>         btf_kflag = dtd->dtd_enum_unsigned
>   		    ? BTF_KF_ENUM_UNSIGNED
> @@ -829,7 +854,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>      }
>   
>     /* PR debug/112656.  BTF_KIND_FUNC_PROTO is always anonymous.  */
> -  if (btf_kind == BTF_KIND_FUNC_PROTO)
> +  else if (btf_kind == BTF_KIND_FUNC_PROTO)
>       dtd->dtd_data.ctti_name = 0;
>   
>     dw2_asm_output_data (4, dtd->dtd_data.ctti_name,
> @@ -848,8 +873,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>       case BTF_KIND_ENUM:
>       case BTF_KIND_DATASEC:
>       case BTF_KIND_ENUM64:
> -      dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
> -			   dtd->dtd_data.ctti_size);
> +      dw2_asm_output_data (4, btf_size, "btt_size: %uB", btf_size);
>         return;
>       case BTF_KIND_ARRAY:
>       case BTF_KIND_FWD:
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-forward-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-forward-2.c
> new file mode 100644
> index 00000000000..318c4220ec3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-forward-2.c
> @@ -0,0 +1,18 @@
> +/* PR111735.  Test BTF generation for forward-declared enum.
> +
> +   The BTF representation for forward-declared enums is not formally
> +   defined, but the de-facto representation used by various tools is
> +   to encode them as a BTF_KIND_ENUM type with vlen=0 and size=0.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA" } */
> +
> +enum Foo;
> +enum Foo *pfoo;
> +
> +/* Check that there is one BTF_KIND_ENUM with vlen=0, and no BTF_KIND_FWD.  */
> +/* { dg-final { scan-assembler-times "\[\t \]0x6000000\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x7000000\[\t \]+\[^\n\]*btt_info" 0 } } */
> +
> +/* Verify the reference to the enum-forward.  */
> +/* { dg-final { scan-assembler-times "btt_type: \\(BTF_KIND_ENUM 'Foo'\\)" 1 } } */
diff mbox series

Patch

diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index db4f1084f85..3ec938874b6 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -268,6 +268,17 @@  btf_emit_id_p (ctf_id_t id)
 	  && (btf_id_map[id] <= BTF_MAX_TYPE));
 }
 
+/* Return true if DTD is a forward-declared enum.  The BTF representation
+   of forward declared enums is not formally defined.  */
+
+static bool
+btf_fwd_to_enum_p (ctf_dtdef_ref dtd)
+{
+  uint32_t btf_kind = get_btf_kind (CTF_V2_INFO_KIND (dtd->dtd_data.ctti_info));
+
+  return (btf_kind == BTF_KIND_FWD && dtd->dtd_data.ctti_type == CTF_K_ENUM);
+}
+
 /* Each BTF type can be followed additional, variable-length information
    completing the description of the type. Calculate the number of bytes
    of variable information required to encode a given type.  */
@@ -753,8 +764,12 @@  btf_asm_type_ref (const char *prefix, ctf_container_ref ctfc, ctf_id_t ref_id)
       uint32_t ref_kind
 	= get_btf_kind (CTF_V2_INFO_KIND (ref_type->dtd_data.ctti_info));
 
+      const char *kind_name = btf_fwd_to_enum_p (ref_type)
+	? btf_kind_name (BTF_KIND_ENUM)
+	: btf_kind_name (ref_kind);
+
       dw2_asm_output_data (4, ref_id, "%s: (BTF_KIND_%s '%s')",
-			   prefix, btf_kind_name (ref_kind),
+			   prefix, kind_name,
 			   get_btf_type_name (ref_type));
     }
 }
@@ -765,11 +780,11 @@  btf_asm_type_ref (const char *prefix, ctf_container_ref ctfc, ctf_id_t ref_id)
 static void
 btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
 {
-  uint32_t btf_kind, btf_kflag, btf_vlen, btf_size_type;
+  uint32_t btf_kind, btf_kflag, btf_vlen, btf_size;
   uint32_t ctf_info = dtd->dtd_data.ctti_info;
 
   btf_kind = get_btf_kind (CTF_V2_INFO_KIND (ctf_info));
-  btf_size_type = dtd->dtd_data.ctti_type;
+  btf_size = dtd->dtd_data.ctti_size;
   btf_vlen = CTF_V2_INFO_VLEN (ctf_info);
 
   /* By now any unrepresentable types have been removed.  */
@@ -777,7 +792,7 @@  btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
 
   /* Size 0 integers are redundant definitions of void. None should remain
      in the types list by this point.  */
-  gcc_assert (btf_kind != BTF_KIND_INT || btf_size_type >= 1);
+  gcc_assert (btf_kind != BTF_KIND_INT || btf_size >= 1);
 
   /* Re-encode the ctti_info to BTF.  */
   /* kflag is 1 for structs/unions with a bitfield member.
@@ -810,16 +825,26 @@  btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
      structs and forwards to unions. The dwarf2ctf conversion process stores
      the kind of the forward in ctti_type, but for BTF this must be 0 for
      forwards, with only the KIND_FLAG to distinguish.
-     At time of writing, BTF forwards to enums are unspecified.  */
-  if (btf_kind == BTF_KIND_FWD)
+     Forwards to enum types are special-cased below.  */
+  else if (btf_kind == BTF_KIND_FWD)
     {
       if (dtd->dtd_data.ctti_type == CTF_K_UNION)
 	btf_kflag = 1;
 
-      btf_size_type = 0;
+      /* PR debug/111735.  Encode foward-declared enums as BTF_KIND_ENUM
+	 with vlen=0.  A representation for these is not formally defined;
+	 this is the de-facto standard used by other tools like clang
+	 and pahole.  */
+      else if (dtd->dtd_data.ctti_type == CTF_K_ENUM)
+	{
+	  btf_kind = BTF_KIND_ENUM;
+	  btf_vlen = 0;
+	}
+
+      btf_size = 0;
     }
 
-  if (btf_kind == BTF_KIND_ENUM)
+  else if (btf_kind == BTF_KIND_ENUM)
     {
       btf_kflag = dtd->dtd_enum_unsigned
 		    ? BTF_KF_ENUM_UNSIGNED
@@ -829,7 +854,7 @@  btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
    }
 
   /* PR debug/112656.  BTF_KIND_FUNC_PROTO is always anonymous.  */
-  if (btf_kind == BTF_KIND_FUNC_PROTO)
+  else if (btf_kind == BTF_KIND_FUNC_PROTO)
     dtd->dtd_data.ctti_name = 0;
 
   dw2_asm_output_data (4, dtd->dtd_data.ctti_name,
@@ -848,8 +873,7 @@  btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
     case BTF_KIND_ENUM:
     case BTF_KIND_DATASEC:
     case BTF_KIND_ENUM64:
-      dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
-			   dtd->dtd_data.ctti_size);
+      dw2_asm_output_data (4, btf_size, "btt_size: %uB", btf_size);
       return;
     case BTF_KIND_ARRAY:
     case BTF_KIND_FWD:
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-forward-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-forward-2.c
new file mode 100644
index 00000000000..318c4220ec3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-forward-2.c
@@ -0,0 +1,18 @@ 
+/* PR111735.  Test BTF generation for forward-declared enum.
+
+   The BTF representation for forward-declared enums is not formally
+   defined, but the de-facto representation used by various tools is
+   to encode them as a BTF_KIND_ENUM type with vlen=0 and size=0.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -gbtf -dA" } */
+
+enum Foo;
+enum Foo *pfoo;
+
+/* Check that there is one BTF_KIND_ENUM with vlen=0, and no BTF_KIND_FWD.  */
+/* { dg-final { scan-assembler-times "\[\t \]0x6000000\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x7000000\[\t \]+\[^\n\]*btt_info" 0 } } */
+
+/* Verify the reference to the enum-forward.  */
+/* { dg-final { scan-assembler-times "btt_type: \\(BTF_KIND_ENUM 'Foo'\\)" 1 } } */