diff mbox series

[v4,13/15] gas: Don't skip SFrame FDE if .cfi_register specifies SP register

Message ID 20240624142334.3283823-14-jremus@linux.ibm.com
State Committed
Headers
Series sframe: Enhancements to SFrame info generation |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jens Remus June 24, 2024, 2:23 p.m. UTC 
  Neither ".cfi_offset SP, <offset>", ".cfi_register SP, <regno>", nor
".cfi_val_offset SP, <offset>" alter the tracking information to recover
the stack pointer (SP). Doing so would need an explicit .cfi_def_cfa,
which SFrame tracks.

The stack-pointer (SP) register contents on entry can be reconstructed
from the SFrame CFA tracking information using information from the
current and initial SFrame FREs of the SFrame FDE:

1. Compute CFA from the current CFA base register (SP or FP) and CFA
   offset from the SFrame CFA tracking information from the SFrame FRE
   for the current instruction address:

     CFA = <current_base_reg> + <current_cfa_offset>

2. Compute SP from the current CFA and the CFA offset from the SFrame
   CFA tracking information from the initial SFrame FRE of the FDE:

     SP = CFA - <initial_cfa_offset>

While at it add comments to the processing of .cfi_offset and
.cfi_val_offset that the SP can be reconstructed from the CFA tracking
information.

gas/
	* gen-sframe.c (sframe_xlate_do_register): Do not skip SFrame
	FDE if .cfi_register specifies SP register.
	(sframe_xlate_do_offset,sframe_xlate_do_val_offset): Add comment
	that this is likewise.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes v3 -> v4:
    - Add comment to sframe_xlate_do_offset why SP register is ignored.
    - Reword commit message accordingly.
    
    Changes v2 -> v3:
    - New patch.

 gas/gen-sframe.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Indu Bhagat June 25, 2024, 11:59 p.m. UTC | #1 
On 6/24/24 07:23, Jens Remus wrote:
> Neither ".cfi_offset SP, <offset>", ".cfi_register SP, <regno>", nor
> ".cfi_val_offset SP, <offset>" alter the tracking information to recover
> the stack pointer (SP). Doing so would need an explicit .cfi_def_cfa,
> which SFrame tracks.
> 
> The stack-pointer (SP) register contents on entry can be reconstructed

Nit: stack pointer instead of stack-pointer.

> from the SFrame CFA tracking information using information from the
> current and initial SFrame FREs of the SFrame FDE:
> 
> 1. Compute CFA from the current CFA base register (SP or FP) and CFA
>     offset from the SFrame CFA tracking information from the SFrame FRE
>     for the current instruction address:
> 
>       CFA = <current_base_reg> + <current_cfa_offset>
> 
> 2. Compute SP from the current CFA and the CFA offset from the SFrame
>     CFA tracking information from the initial SFrame FRE of the FDE:
> 
>       SP = CFA - <initial_cfa_offset>
> 
> While at it add comments to the processing of .cfi_offset and
> .cfi_val_offset that the SP can be reconstructed from the CFA tracking
> information.
> 
> gas/
> 	* gen-sframe.c (sframe_xlate_do_register): Do not skip SFrame
> 	FDE if .cfi_register specifies SP register.
> 	(sframe_xlate_do_offset,sframe_xlate_do_val_offset): Add comment
> 	that this is likewise.
> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>

There is no need to post a V5 for the nit above.

LGTM

Thanks

> ---
> 
> Notes (jremus):
>      Changes v3 -> v4:
>      - Add comment to sframe_xlate_do_offset why SP register is ignored.
>      - Reword commit message accordingly.
>      
>      Changes v2 -> v3:
>      - New patch.
> 
>   gas/gen-sframe.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index 31f2e5118280..f4bdbb5944d9 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -1100,6 +1100,7 @@ sframe_xlate_do_offset (struct sframe_xlate_ctx *xlate_ctx,
>     gas_assert (cur_fre);
>     /* Change the rule for the register indicated by the register number to
>        be the specified offset.  */
> +  /* Ignore SP reg, as it can be recovered from the CFA tracking info.  */
>     if (cfi_insn->u.r == SFRAME_CFA_FP_REG)
>       {
>         gas_assert (!cur_fre->base_reg);
> @@ -1134,6 +1135,7 @@ sframe_xlate_do_val_offset (struct sframe_xlate_ctx *xlate_ctx ATTRIBUTE_UNUSED,
>   #ifdef SFRAME_FRE_RA_TRACKING
>         || (sframe_ra_tracking_p () && cfi_insn->u.r == SFRAME_CFA_RA_REG)
>   #endif
> +      /* Ignore SP reg, as it can be recovered from the CFA tracking info.  */
>         )
>       {
>         as_warn (_("skipping SFrame FDE; %s register %u in .cfi_val_offset"),
> @@ -1153,14 +1155,15 @@ sframe_xlate_do_register (struct sframe_xlate_ctx *xlate_ctx ATTRIBUTE_UNUSED,
>   			  struct cfi_insn_data *cfi_insn)
>   {
>     /* Previous value of register1 is register2.  However, if the specified
> -     register1 is not interesting (SP, FP, or RA reg), the current DW_CFA_register
> +     register1 is not interesting (FP or RA reg), the current DW_CFA_register
>        instruction can be safely skipped without sacrificing the asynchronicity of
>        stack trace information.  */
> -  if (cfi_insn->u.rr.reg1 == SFRAME_CFA_SP_REG
> +  if (cfi_insn->u.rr.reg1 == SFRAME_CFA_FP_REG
>   #ifdef SFRAME_FRE_RA_TRACKING
>         || (sframe_ra_tracking_p () && cfi_insn->u.rr.reg1 == SFRAME_CFA_RA_REG)
>   #endif
> -      || cfi_insn->u.rr.reg1 == SFRAME_CFA_FP_REG)
> +      /* Ignore SP reg, as it can be recovered from the CFA tracking info.  */
> +      )
>       {
>         as_warn (_("skipping SFrame FDE; %s register %u in .cfi_register"),
>   	       sframe_register_name (cfi_insn->u.rr.reg1), cfi_insn->u.rr.reg1);
diff mbox series

Patch

diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 31f2e5118280..f4bdbb5944d9 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -1100,6 +1100,7 @@  sframe_xlate_do_offset (struct sframe_xlate_ctx *xlate_ctx,
   gas_assert (cur_fre);
   /* Change the rule for the register indicated by the register number to
      be the specified offset.  */
+  /* Ignore SP reg, as it can be recovered from the CFA tracking info.  */
   if (cfi_insn->u.r == SFRAME_CFA_FP_REG)
     {
       gas_assert (!cur_fre->base_reg);
@@ -1134,6 +1135,7 @@  sframe_xlate_do_val_offset (struct sframe_xlate_ctx *xlate_ctx ATTRIBUTE_UNUSED,
 #ifdef SFRAME_FRE_RA_TRACKING
       || (sframe_ra_tracking_p () && cfi_insn->u.r == SFRAME_CFA_RA_REG)
 #endif
+      /* Ignore SP reg, as it can be recovered from the CFA tracking info.  */
       )
     {
       as_warn (_("skipping SFrame FDE; %s register %u in .cfi_val_offset"),
@@ -1153,14 +1155,15 @@  sframe_xlate_do_register (struct sframe_xlate_ctx *xlate_ctx ATTRIBUTE_UNUSED,
 			  struct cfi_insn_data *cfi_insn)
 {
   /* Previous value of register1 is register2.  However, if the specified
-     register1 is not interesting (SP, FP, or RA reg), the current DW_CFA_register
+     register1 is not interesting (FP or RA reg), the current DW_CFA_register
      instruction can be safely skipped without sacrificing the asynchronicity of
      stack trace information.  */
-  if (cfi_insn->u.rr.reg1 == SFRAME_CFA_SP_REG
+  if (cfi_insn->u.rr.reg1 == SFRAME_CFA_FP_REG
 #ifdef SFRAME_FRE_RA_TRACKING
       || (sframe_ra_tracking_p () && cfi_insn->u.rr.reg1 == SFRAME_CFA_RA_REG)
 #endif
-      || cfi_insn->u.rr.reg1 == SFRAME_CFA_FP_REG)
+      /* Ignore SP reg, as it can be recovered from the CFA tracking info.  */
+      )
     {
       as_warn (_("skipping SFrame FDE; %s register %u in .cfi_register"),
 	       sframe_register_name (cfi_insn->u.rr.reg1), cfi_insn->u.rr.reg1);