public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Indu Bhagat <indu.bhagat@oracle.com>
To: Jens Remus <jremus@linux.ibm.com>, binutils@sourceware.org
Cc: Andreas Krebbel <krebbel@linux.ibm.com>
Subject: Re: [PATCH v3 13/15] gas: Don't skip SFrame FDE if .cfi_register specifies SP register
Date: Tue, 23 Apr 2024 01:15:44 -0700	[thread overview]
Message-ID: <12ef16e4-c881-41ea-af27-de3254c2c2ee@oracle.com> (raw)
In-Reply-To: <1e518650-2d11-4a28-a3b7-de94a32582b1@linux.ibm.com>

On 4/19/24 06:13, Jens Remus wrote:
> Am 18.04.2024 um 22:37 schrieb Indu Bhagat:
>> 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.
>>
> 
> I did not find any explanation how unwinding using SFrame is exactly 
> expected to be performed. Following is how I assume it to work in 
> general and on s390x specifically. Details of how to restore the stack 
> pointer register value at the call site are probably architecture specific.
> 
> 1. Use the program counter (PC) or subsequently return address (RA)
>     as address to do a binary search through the SFrame FDEs to determine
>     the FDE for the current function. Binary search can be used, as the
>     FDEs are of fixed size.
> 
> 2. Note/print the function start address taken from the FDE or lookup
>     its symbol for the stack trace.
> 
> 3. Do a linear search through the SFrame FREs associated with the FDE
>     to determine the FRE for the address within the function. Linear
>     search must be used, since the FREs are of different size and
>     different numbers of offsets may follow each.
> 
> 3. Unwind the stack-pointer (SP), frame-pointer (FP), and return-
>     address (RA) register values at function entry:
> 
>     - SP: Since SFrame does not track the SP register, the mean
>       to restore its contents at entry to the function must be to
>       use the SFrame CFA trackig information from the current FRE
>       and initial FRE of the function/FDE to restore the SP value:
> 
>          CFA = <CFA-base-reg> + <current-CFA-offset>
>          SP = CFA - <initial-CFA-offset>
> 
>          NOTE: Above does not restore the SP value from the stack,
>                as SFrame does not know whether and where it would
>                be saved. Instead it computes the SP value.
> 
>       The use of both the current and initial CFA offsets is valid,
>       as the DWARF standard mandates the CFA to be fixed for a
>       function:
> 
>          "The algorithm to compute CFA changes as you progress
>          through the prologue and epilogue code. (__By definition,
>          the CFA value does not change.__)"
> 
>     - FP: Without SFrame FP tracking use the fixed FP offset as follows.
>       With FP tacking (currently the default), if available use the
>       FP offset to recover the FP value as follows, otherwise the
>       FP value is still in the FP register.
> 
>      CFA = <CFA-base-reg> + <current-CFA-offset>
>      FP = [CFA + FP-offset]
> 
>      NOTE: Above restores the FP value from the stack.
> 
>     - RA: Without SFrame RA tracking use the fixed RA offset as follows.
>       With RA tracking (depending on architecture), if available use the
>       RA offset to recover the RA value as follows, otherwise the
>       RA value is still in the RA register.
> 
>          CFA = <CFA-base-reg> + <current-CFA-offset>
>          RA = [CFA + RA-offset]
> 
>          NOTE: Same as for FP.
> 
> 4. Unwind the SP register value at the call site. This step be merged
>     with the previous one.
> 
>     On s390x the value of the stack pointer on entry to a function is
>     identical to the value of the stack pointer at the call site. Thus
>     nothing has to be done.
>     Note that on s390x the CFA is not defined to be the value of the
>     stack pointer at the call site, as it is for other architectures.
>     Instead it is usually defined as the upper bound (exclusive) of
>     the current stack frame.
>     This is valid as the DWARF standard does not mandate the CFA to
>     be defined as the value of the SP at the call site.
> 
>        "__Typically__, the CFA is defined to be the value of the stack
>        pointer at the call site in the previous frame (which may be
>        different from its value on entry to the current frame)."
> 
>     On x86-64 the SP at the call site can be determined using a fixed
>     offset, or SP = CFA can be used, if the CFA is defined as the SP
>     value at the call site on x86-64.
> 
> 5. Repeat at step 1 using the RA value as address to perform the next
>     FDE lookup.
> 
> 
> With that to your question why I assume it would be acceptable to treat 
> SP different from FP and RA when it comes to .cfi_offset, 
> .cfi_val_offset, and .cfi_register:
> 
> On s390x, and I assume this to apply to all other architectures, the SP
> value on function entry and from that also at the call site can always 
> be restored using above steps. It is irrelevant whether the SP value on 
> function entry is saved on the stack (.cfi_offset), in another register 
> (.cfi_register), or is defined as offset from CFA (.cfi_val_offset).
> 
> Actually the SP is inherently defined as an offset from the CFA that can 
> be deducted from the CFA tracking information (see above).
> An exception would be if a compiler would generate code that defines a 
> non-SP register as CFA base register on function entry. I have not 
> observed that on s390x.
> 
> Regarding the aforementioned CFI directives I observed the following on 
> s390x:
> 
> The SP register value is saved on the stack in the function prologue and 
> restored in the epilogue, when allocating stack space of fixed size for 
> local variables. The compiler generates respective .cfi_offset 
> <SP-regno>, ... and .cfi_restore <SP-regno> CFI directives. Note that 
> the compiler could alternatively use arithmetic instructions to restore 
> the initial SP value. Due to the availability of Load/Store Multiple 
> instructions those are generally preferred.
> An unwinder using .eh_frame information would most likely use the 
> .cfi_offset information for the SP register.
> An unwinder using .sframe information would instead need to perform 
> above mentioned computations, as SFrame does not track the SP register.
> 
> As for the .cfi_val_offset <SP-regno>, ... instances in glibc it is 
> similar.
> An unwinder using .eh_frame information would use the .cfi_val_offset 
> information.
> An unwinder using .sframe information would instead again need to 
> perform above mentioned computations.
> 
> Does that make sense? If I have missed something, then SFrame generation 
> would probably need to skip FDE when the SP register is specified in 
> .cfi_offset (new), .cfi_register, and .cfi_val_offset, no?
> 

I think it is dawning on me: If '.cfi_register SP, reg2' is seen, this 
does not alter the tracking to recover prev-SP (doing so will need an 
explicit .cfi_def_cfa which we track).

Whereas if .cfi_register FP, reg3 is seen, the FDE needs to be skipped 
because we cannot represent this information.  A later instruction may 
be sabotaging FP, and later recovering back FP from reg3 via a 
.cfi_register reg3, FP.

I think some SCFI related work was polluting my thoughts.

> 
> Btw., besides the RFC Linux Kernel perf patch series "[PATCH RFC 00/10] 
> perf: user space sframe unwinding" [1] is there any reference 
> implementation of a SFrame unwinder I could use as base for testing 
> SFrame unwinding on s390x? Maybe a GDB Python script to do unwinding 
> using SFrame (not sure how to access the .sframe section)?
> 
> [1] [PATCH RFC 00/10] perf: user space sframe unwinding,
> https://lore.kernel.org/all/cover.1699487758.git.jpoimboe@kernel.org/
> 

We did have an "SFrame unwinder library" patchset for binutils, which we 
used for testing SFrame for x86 and aarch64 support.  It wasn't 
upstreamed because a) it had open issues, b) in hindsight, it doesn't 
belong to binutils-gdp repo in the current shape.

I will push it in a personal branch upstream and share some notes on its 
status and open issues. At the least, it should be helpful in testing 
your s390 enhancements to the SFrame format.

>>> 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));
>>
> 
> Thanks and regards,
> Jens


  reply	other threads:[~2024-04-23  8:15 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-12 14:47 [PATCH v3 00/15] sframe: Enhancements to SFrame info generation Jens Remus
2024-04-12 14:47 ` [PATCH v3 01/15] x86: Remove unused SFrame CFI RA register variable Jens Remus
2024-04-12 14:47 ` [PATCH v3 02/15] gas: Enhance arch-specific SFrame configuration descriptions Jens Remus
2024-04-18  7:39   ` Indu Bhagat
2024-05-03 12:30     ` Jens Remus
2024-04-12 14:47 ` [PATCH v3 03/15] readelf/objdump: Dump SFrame CFA fixed FP and RA offsets Jens Remus
2024-04-18  7:39   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 04/15] readelf/objdump: Display SFrame fixed RA offset as 'f' in dump Jens Remus
2024-04-18  7:40   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 05/15] gas: Print DWARF call frame insn name in SFrame warning message Jens Remus
2024-04-18  7:40   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 06/15] gas: Skip SFrame FDE if CFI specifies non-FP/SP base register Jens Remus
2024-04-18  7:40   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 07/15] gas: Warn if SFrame FDE is skipped due to non-default return column Jens Remus
2024-04-18  7:40   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 08/15] gas: Refactor SFrame CFI opcode DW_CFA_register processing Jens Remus
2024-04-18  7:40   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 09/15] gas: User readable warnings if SFrame FDE is not generated Jens Remus
2024-04-18 20:33   ` Indu Bhagat
2024-05-03 12:30     ` Jens Remus
2024-05-03 23:41       ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 10/15] gas: Skip SFrame FDE if FP without RA on stack Jens Remus
2024-04-16 13:14   ` Jens Remus
2024-04-17 23:56     ` Indu Bhagat
2024-04-18 10:27       ` Jens Remus
2024-04-18 20:35   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 11/15] gas: Skip SFrame FDE if .cfi_window_save Jens Remus
2024-04-18 20:36   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 12/15] gas: Don't skip SFrame FDE if .cfi_register specifies RA w/o tracking Jens Remus
2024-04-18 20:36   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 13/15] gas: Don't skip SFrame FDE if .cfi_register specifies SP register Jens Remus
2024-04-18 20:37   ` Indu Bhagat
2024-04-19 13:13     ` Jens Remus
2024-04-23  8:15       ` Indu Bhagat [this message]
2024-04-25 22:22         ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 14/15] gas: Test predicate whether SFrame RA tracking is used Jens Remus
2024-04-18 20:37   ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 15/15] gas: Validate SFrame RA tracking and fixed RA offset Jens Remus
2024-04-18 20:38   ` Indu Bhagat
2024-05-03 16:40     ` Jens Remus
2024-05-04  0:22       ` Indu Bhagat
2024-05-06 11:41         ` Jens Remus
2024-05-06 14:39           ` Jens Remus
2024-05-16 20:45             ` Indu Bhagat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=12ef16e4-c881-41ea-af27-de3254c2c2ee@oracle.com \
    --to=indu.bhagat@oracle.com \
    --cc=binutils@sourceware.org \
    --cc=jremus@linux.ibm.com \
    --cc=krebbel@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).