diff mbox series

[v2,5/9] gas: Print DWARF call frame insn name in SFrame warning message

Message ID 20240223170800.3993092-6-jremus@linux.ibm.com
State Unresolved
Headers
Series s390: Initial support to generate SFrame info from CFI directives in assembler |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

Jens Remus Feb. 23, 2024, 5:07 p.m. UTC 
  SFrame generation prints the DWARF call frame instruction opcode in
hexadecimal. Leverage get_DW_CFA_name to additionally print the
DWARF call frame instruction name in human readable form, while also
respecting fake CFI types. Use "(unknown)", if the DWARF call frame
instruction name is not known.

This changes the following assembler SFrame generation warning message
as follows:

Old:
Warning: skipping SFrame FDE due to DWARF CFI op 0x<hexnum>

New:
Warning: skipping SFrame FDE due to DWARF CFI op <name> (0x<hexnum>)

gas/
	* gen-sframe.c (sframe_get_cfi_name): New function to get the
	  DWARF call frame instruction name for a DWARF call frame
	  instruction opcode.
	  (sframe_do_cfi_insn): Use sframe_get_cfi_name to print the
	  DWARF call frame instruction name for the DWARF call frame
	  instruction opcode in the warning message.

gas/testsuite/
	* gas/cfi-sframe/common-empty-1.d: Update expected SFrame
	  warning message text for DWARF call frame insn name.
	* gas/cfi-sframe/common-empty-2.d: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-1.l: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-2.l: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-3.l: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-4.l: Likewise.

Reviewed-by: Andreas Krebbel <krebbel@linux.ibm.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 gas/gen-sframe.c                              | 49 ++++++++++++++++++-
 gas/testsuite/gas/cfi-sframe/common-empty-1.d |  2 +-
 gas/testsuite/gas/cfi-sframe/common-empty-2.d |  2 +-
 3 files changed, 50 insertions(+), 3 deletions(-)

Comments

Indu Bhagat Feb. 24, 2024, 7:56 a.m. UTC | #1 
On 2/23/24 09:07, Jens Remus wrote:
> SFrame generation prints the DWARF call frame instruction opcode in
> hexadecimal. Leverage get_DW_CFA_name to additionally print the
> DWARF call frame instruction name in human readable form, while also
> respecting fake CFI types. Use "(unknown)", if the DWARF call frame
> instruction name is not known.
> 
> This changes the following assembler SFrame generation warning message
> as follows:
> 
> Old:
> Warning: skipping SFrame FDE due to DWARF CFI op 0x<hexnum>
> 
> New:
> Warning: skipping SFrame FDE due to DWARF CFI op <name> (0x<hexnum>)
> 
> gas/
> 	* gen-sframe.c (sframe_get_cfi_name): New function to get the
> 	  DWARF call frame instruction name for a DWARF call frame
> 	  instruction opcode.
> 	  (sframe_do_cfi_insn): Use sframe_get_cfi_name to print the
> 	  DWARF call frame instruction name for the DWARF call frame
> 	  instruction opcode in the warning message.
> 
> gas/testsuite/
> 	* gas/cfi-sframe/common-empty-1.d: Update expected SFrame
> 	  warning message text for DWARF call frame insn name.
> 	* gas/cfi-sframe/common-empty-2.d: Likewise.
> 	* gas/cfi-sframe/cfi-sframe-s390-err-1.l: Likewise.
> 	* gas/cfi-sframe/cfi-sframe-s390-err-2.l: Likewise.
> 	* gas/cfi-sframe/cfi-sframe-s390-err-3.l: Likewise.
> 	* gas/cfi-sframe/cfi-sframe-s390-err-4.l: Likewise.
> 

Stale entries in ChangeLog for cfi-sframe-s390* files.  These are not 
included in this patch.

Otherwise, LGTM.

Thanks

> Reviewed-by: Andreas Krebbel <krebbel@linux.ibm.com>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>   gas/gen-sframe.c                              | 49 ++++++++++++++++++-
>   gas/testsuite/gas/cfi-sframe/common-empty-1.d |  2 +-
>   gas/testsuite/gas/cfi-sframe/common-empty-2.d |  2 +-
>   3 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index 75781fc8ccbd..d35baaac54b2 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -1197,6 +1197,46 @@ sframe_xlate_do_gnu_window_save (struct sframe_xlate_ctx *xlate_ctx,
>     return SFRAME_XLATE_OK;
>   }
>   
> +/* Returns the DWARF call frame instruction name or fake CFI name for the
> +   specified CFI opcode, or NULL if the value is not recognized.  */
> +
> +static const char *
> +sframe_get_cfi_name (int cfi_opc)
> +{
> +  const char *cfi_name;
> +
> +  switch (cfi_opc)
> +    {
> +      /* Fake CFI type; outside the byte range of any real CFI insn.  */
> +      /* See gas/dw2gencfi.h.  */
> +      case CFI_adjust_cfa_offset:
> +	cfi_name = "CFI_adjust_cfa_offset";
> +	break;
> +      case CFI_return_column:
> +	cfi_name = "CFI_return_column";
> +	break;
> +      case CFI_rel_offset:
> +	cfi_name = "CFI_rel_offset";
> +	break;
> +      case CFI_escape:
> +	cfi_name = "CFI_escape";
> +	break;
> +      case CFI_signal_frame:
> +	cfi_name = "CFI_signal_frame";
> +	break;
> +      case CFI_val_encoded_addr:
> +	cfi_name = "CFI_val_encoded_addr";
> +	break;
> +      case CFI_label:
> +	cfi_name = "CFI_label";
> +	break;
> +      default:
> +	cfi_name = get_DW_CFA_name (cfi_opc);
> +    }
> +
> +  return cfi_name;
> +}
> +
>   /* Process CFI_INSN and update the translation context with the FRE
>      information.
>   
> @@ -1272,7 +1312,14 @@ sframe_do_cfi_insn (struct sframe_xlate_ctx *xlate_ctx,
>     /* An error here will cause no SFrame FDE later.  Warn the user because this
>        will affect the overall coverage and hence, asynchronicity.  */
>     if (err)
> -    as_warn (_("skipping SFrame FDE due to DWARF CFI op %#x"), op);
> +    {
> +      const char *cfi_name = sframe_get_cfi_name (op);
> +
> +      if (!cfi_name)
> +	cfi_name = _("(unknown)");
> +      as_warn (_("skipping SFrame FDE due to DWARF CFI op %s (%#x)"),
> +	       cfi_name, op);
> +    }
>   
>     return err;
>   }
> diff --git a/gas/testsuite/gas/cfi-sframe/common-empty-1.d b/gas/testsuite/gas/cfi-sframe/common-empty-1.d
> index 125612ff841f..d7756302b559 100644
> --- a/gas/testsuite/gas/cfi-sframe/common-empty-1.d
> +++ b/gas/testsuite/gas/cfi-sframe/common-empty-1.d
> @@ -1,5 +1,5 @@
>   #as: --gsframe
> -#warning: skipping SFrame FDE due to DWARF CFI op 0xa
> +#warning: skipping SFrame FDE due to DWARF CFI op DW_CFA_remember_state \(0xa\)
>   #objdump: --sframe=.sframe
>   #name: Uninteresting cfi directives generate an empty SFrame section
>   #...
> diff --git a/gas/testsuite/gas/cfi-sframe/common-empty-2.d b/gas/testsuite/gas/cfi-sframe/common-empty-2.d
> index 59328fc1033f..20282c7854e8 100644
> --- a/gas/testsuite/gas/cfi-sframe/common-empty-2.d
> +++ b/gas/testsuite/gas/cfi-sframe/common-empty-2.d
> @@ -1,5 +1,5 @@
>   #as: --gsframe
> -#warning: skipping SFrame FDE due to DWARF CFI op 0xe
> +#warning: skipping SFrame FDE due to DWARF CFI op DW_CFA_def_cfa_offset \(0xe\)
>   #objdump: --sframe=.sframe
>   #name: SFrame supports only FP/SP based CFA
>   #...
Jens Remus Feb. 26, 2024, 3:37 p.m. UTC | #2 
Am 24.02.2024 um 08:56 schrieb Indu Bhagat:
> On 2/23/24 09:07, Jens Remus wrote:
>> SFrame generation prints the DWARF call frame instruction opcode in
>> hexadecimal. Leverage get_DW_CFA_name to additionally print the
>> DWARF call frame instruction name in human readable form, while also
>> respecting fake CFI types. Use "(unknown)", if the DWARF call frame
>> instruction name is not known.
>>
>> This changes the following assembler SFrame generation warning message
>> as follows:
>>
>> Old:
>> Warning: skipping SFrame FDE due to DWARF CFI op 0x<hexnum>
>>
>> New:
>> Warning: skipping SFrame FDE due to DWARF CFI op <name> (0x<hexnum>)
>>
>> gas/
>>     * gen-sframe.c (sframe_get_cfi_name): New function to get the
>>       DWARF call frame instruction name for a DWARF call frame
>>       instruction opcode.
>>       (sframe_do_cfi_insn): Use sframe_get_cfi_name to print the
>>       DWARF call frame instruction name for the DWARF call frame
>>       instruction opcode in the warning message.
>>
>> gas/testsuite/
>>     * gas/cfi-sframe/common-empty-1.d: Update expected SFrame
>>       warning message text for DWARF call frame insn name.
>>     * gas/cfi-sframe/common-empty-2.d: Likewise.
>>     * gas/cfi-sframe/cfi-sframe-s390-err-1.l: Likewise.
>>     * gas/cfi-sframe/cfi-sframe-s390-err-2.l: Likewise.
>>     * gas/cfi-sframe/cfi-sframe-s390-err-3.l: Likewise.
>>     * gas/cfi-sframe/cfi-sframe-s390-err-4.l: Likewise.
>>
> 
> Stale entries in ChangeLog for cfi-sframe-s390* files.  These are not 
> included in this patch.

Good catch! Is there a (git pre-commit) script I could use to validate 
those? Googling for "GNU changelog git commit message validate" did not 
return anything useful.

> Otherwise, LGTM.

Thank you for the review!

> Thanks
> 
>> Reviewed-by: Andreas Krebbel <krebbel@linux.ibm.com>
>> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
>> ---
>>   gas/gen-sframe.c                              | 49 ++++++++++++++++++-
>>   gas/testsuite/gas/cfi-sframe/common-empty-1.d |  2 +-
>>   gas/testsuite/gas/cfi-sframe/common-empty-2.d |  2 +-
>>   3 files changed, 50 insertions(+), 3 deletions(-)
>>
>> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
>> index 75781fc8ccbd..d35baaac54b2 100644
>> --- a/gas/gen-sframe.c
>> +++ b/gas/gen-sframe.c
>> @@ -1197,6 +1197,46 @@ sframe_xlate_do_gnu_window_save (struct 
>> sframe_xlate_ctx *xlate_ctx,
>>     return SFRAME_XLATE_OK;
>>   }
>> +/* Returns the DWARF call frame instruction name or fake CFI name for 
>> the
>> +   specified CFI opcode, or NULL if the value is not recognized.  */
>> +
>> +static const char *
>> +sframe_get_cfi_name (int cfi_opc)
>> +{
>> +  const char *cfi_name;
>> +
>> +  switch (cfi_opc)
>> +    {
>> +      /* Fake CFI type; outside the byte range of any real CFI insn.  */
>> +      /* See gas/dw2gencfi.h.  */
>> +      case CFI_adjust_cfa_offset:
>> +    cfi_name = "CFI_adjust_cfa_offset";
>> +    break;
>> +      case CFI_return_column:
>> +    cfi_name = "CFI_return_column";
>> +    break;
>> +      case CFI_rel_offset:
>> +    cfi_name = "CFI_rel_offset";
>> +    break;
>> +      case CFI_escape:
>> +    cfi_name = "CFI_escape";
>> +    break;
>> +      case CFI_signal_frame:
>> +    cfi_name = "CFI_signal_frame";
>> +    break;
>> +      case CFI_val_encoded_addr:
>> +    cfi_name = "CFI_val_encoded_addr";
>> +    break;
>> +      case CFI_label:
>> +    cfi_name = "CFI_label";
>> +    break;
>> +      default:
>> +    cfi_name = get_DW_CFA_name (cfi_opc);
>> +    }
>> +
>> +  return cfi_name;
>> +}
>> +
>>   /* Process CFI_INSN and update the translation context with the FRE
>>      information.
>> @@ -1272,7 +1312,14 @@ sframe_do_cfi_insn (struct sframe_xlate_ctx 
>> *xlate_ctx,
>>     /* An error here will cause no SFrame FDE later.  Warn the user 
>> because this
>>        will affect the overall coverage and hence, asynchronicity.  */
>>     if (err)
>> -    as_warn (_("skipping SFrame FDE due to DWARF CFI op %#x"), op);
>> +    {
>> +      const char *cfi_name = sframe_get_cfi_name (op);
>> +
>> +      if (!cfi_name)
>> +    cfi_name = _("(unknown)");
>> +      as_warn (_("skipping SFrame FDE due to DWARF CFI op %s (%#x)"),
>> +           cfi_name, op);
>> +    }
>>     return err;
>>   }
>> diff --git a/gas/testsuite/gas/cfi-sframe/common-empty-1.d 
>> b/gas/testsuite/gas/cfi-sframe/common-empty-1.d
>> index 125612ff841f..d7756302b559 100644
>> --- a/gas/testsuite/gas/cfi-sframe/common-empty-1.d
>> +++ b/gas/testsuite/gas/cfi-sframe/common-empty-1.d
>> @@ -1,5 +1,5 @@
>>   #as: --gsframe
>> -#warning: skipping SFrame FDE due to DWARF CFI op 0xa
>> +#warning: skipping SFrame FDE due to DWARF CFI op 
>> DW_CFA_remember_state \(0xa\)
>>   #objdump: --sframe=.sframe
>>   #name: Uninteresting cfi directives generate an empty SFrame section
>>   #...
>> diff --git a/gas/testsuite/gas/cfi-sframe/common-empty-2.d 
>> b/gas/testsuite/gas/cfi-sframe/common-empty-2.d
>> index 59328fc1033f..20282c7854e8 100644
>> --- a/gas/testsuite/gas/cfi-sframe/common-empty-2.d
>> +++ b/gas/testsuite/gas/cfi-sframe/common-empty-2.d
>> @@ -1,5 +1,5 @@
>>   #as: --gsframe
>> -#warning: skipping SFrame FDE due to DWARF CFI op 0xe
>> +#warning: skipping SFrame FDE due to DWARF CFI op 
>> DW_CFA_def_cfa_offset \(0xe\)
>>   #objdump: --sframe=.sframe
>>   #name: SFrame supports only FP/SP based CFA
>>   #...

Regards,
Jens
diff mbox series

Patch

diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 75781fc8ccbd..d35baaac54b2 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -1197,6 +1197,46 @@  sframe_xlate_do_gnu_window_save (struct sframe_xlate_ctx *xlate_ctx,
   return SFRAME_XLATE_OK;
 }
 
+/* Returns the DWARF call frame instruction name or fake CFI name for the
+   specified CFI opcode, or NULL if the value is not recognized.  */
+
+static const char *
+sframe_get_cfi_name (int cfi_opc)
+{
+  const char *cfi_name;
+
+  switch (cfi_opc)
+    {
+      /* Fake CFI type; outside the byte range of any real CFI insn.  */
+      /* See gas/dw2gencfi.h.  */
+      case CFI_adjust_cfa_offset:
+	cfi_name = "CFI_adjust_cfa_offset";
+	break;
+      case CFI_return_column:
+	cfi_name = "CFI_return_column";
+	break;
+      case CFI_rel_offset:
+	cfi_name = "CFI_rel_offset";
+	break;
+      case CFI_escape:
+	cfi_name = "CFI_escape";
+	break;
+      case CFI_signal_frame:
+	cfi_name = "CFI_signal_frame";
+	break;
+      case CFI_val_encoded_addr:
+	cfi_name = "CFI_val_encoded_addr";
+	break;
+      case CFI_label:
+	cfi_name = "CFI_label";
+	break;
+      default:
+	cfi_name = get_DW_CFA_name (cfi_opc);
+    }
+
+  return cfi_name;
+}
+
 /* Process CFI_INSN and update the translation context with the FRE
    information.
 
@@ -1272,7 +1312,14 @@  sframe_do_cfi_insn (struct sframe_xlate_ctx *xlate_ctx,
   /* An error here will cause no SFrame FDE later.  Warn the user because this
      will affect the overall coverage and hence, asynchronicity.  */
   if (err)
-    as_warn (_("skipping SFrame FDE due to DWARF CFI op %#x"), op);
+    {
+      const char *cfi_name = sframe_get_cfi_name (op);
+
+      if (!cfi_name)
+	cfi_name = _("(unknown)");
+      as_warn (_("skipping SFrame FDE due to DWARF CFI op %s (%#x)"),
+	       cfi_name, op);
+    }
 
   return err;
 }
diff --git a/gas/testsuite/gas/cfi-sframe/common-empty-1.d b/gas/testsuite/gas/cfi-sframe/common-empty-1.d
index 125612ff841f..d7756302b559 100644
--- a/gas/testsuite/gas/cfi-sframe/common-empty-1.d
+++ b/gas/testsuite/gas/cfi-sframe/common-empty-1.d
@@ -1,5 +1,5 @@ 
 #as: --gsframe
-#warning: skipping SFrame FDE due to DWARF CFI op 0xa
+#warning: skipping SFrame FDE due to DWARF CFI op DW_CFA_remember_state \(0xa\)
 #objdump: --sframe=.sframe
 #name: Uninteresting cfi directives generate an empty SFrame section
 #...
diff --git a/gas/testsuite/gas/cfi-sframe/common-empty-2.d b/gas/testsuite/gas/cfi-sframe/common-empty-2.d
index 59328fc1033f..20282c7854e8 100644
--- a/gas/testsuite/gas/cfi-sframe/common-empty-2.d
+++ b/gas/testsuite/gas/cfi-sframe/common-empty-2.d
@@ -1,5 +1,5 @@ 
 #as: --gsframe
-#warning: skipping SFrame FDE due to DWARF CFI op 0xe
+#warning: skipping SFrame FDE due to DWARF CFI op DW_CFA_def_cfa_offset \(0xe\)
 #objdump: --sframe=.sframe
 #name: SFrame supports only FP/SP based CFA
 #...