Re: [PATCH v3 13/15] gas: Don't skip SFrame FDE if .cfi_register specifies SP register

[Indu Bhagat <indu.bhagat@oracle.com>]

On 4/12/24 07:47, Jens Remus wrote:
> The stack-pointer (SP) register contents on entry can be reconstructed
> from the CFA base register tracking information from the current SFrame
> FRE and initial FRE from the FDE:
> 
> 1. Compute CFA from the current CFA base register (SP or FP) and CFA
>     offset from the SFrame CFA base register 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 a comment to the processing of .cfi_val_offset that the
> SP can be reconstructed from the CFA base register tracking information.
> 

Sorry, but I am not sure I follow.  Yes, SP can be reconstructed using 
the means you outline, but so can FP.  And also RA when applicable.

In other words, I dont follow why treat SP differently from FP / RA when 
it comes to .cfi_register (or even .cfi_val_offset) for reporting the 
warning.

> gas/
> 	* gen-sframe.c (sframe_xlate_do_register): Do not skip SFrame
> 	FDE if .cfi_register specifies SP register.
> 	(sframe_xlate_do_val_offset): Add comment that this is likewise.
> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
> 
> Notes (jremus):
>      Changes v2 -> v3:
>      - New patch.
> 
>   gas/gen-sframe.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index 61c846f214ee..12b523a8d59a 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -1136,6 +1136,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 due to .cfi_val_offset specifying %s register"),
> @@ -1155,14 +1156,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 due to .cfi_register specifying %s register"),
>   	       sframe_register_name (cfi_insn->u.rr.reg1));