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 04/15] readelf/objdump: Display SFrame fixed RA offset as 'f' in dump
Date: Thu, 18 Apr 2024 00:40:02 -0700 [thread overview]
Message-ID: <da5e45a9-9cdc-4050-9260-19d34696e9bb@oracle.com> (raw)
In-Reply-To: <20240412144718.4191286-5-jremus@linux.ibm.com>
On 4/12/24 07:47, Jens Remus wrote:
> For the SFrame FRE frame-pointer (FP) offset from CFA a 'u' is displayed
> if it is unavailable.
>
> For the SFrame FRE return-address (RA) offset from CFA a 'u' was
> displayed if the ABI uses a fixed RA offset from CFA. By chance a
> 'u' was also displayed if the RA offset is unavailable, as the string
> buffer was not initialized after formatting the FP offset. Note that it
> could not occur that the FP offset was erroneously displayed as RA
> offset, as the SFrame format cannot have a FRE with FP offset without
> RA offset.
>
> For the FRE RA offset display 'f' if the ABI uses a fixed RA offset
> from CFA. Display a 'u' if it is unavailable.
>
> libsframe/
> * sframe-dump.c: Display SFrame fixed RA offset as 'f' in dump.
>
> gas/testsuite/
> * gas/cfi-sframe/cfi-sframe-common-4.d: Test for RA displayed
> either as 'u' (if RA tracking) or as 'f' (fixed RA offset if no
> RA tracking).
> * gas/cfi-sframe/cfi-sframe-common-5.d: Likewise.
> * gas/cfi-sframe/cfi-sframe-common-6.d: Likewise.
> * gas/cfi-sframe/cfi-sframe-common-7.d: Likewise.
> * gas/cfi-sframe/cfi-sframe-common-8.d: Likewise.
> * gas/cfi-sframe/cfi-sframe-x86_64-1.d: Test for RA displayed
> as 'f' (fixed RA offset), as x86-64 does not use RA tracking.
> * gas/scfi/x86_64/scfi-cfi-sections-1.d: Likewise.
> * gas/scfi/x86_64/scfi-dyn-stack-1.d: Likewise.
>
> ld/testsuite/
> * ld-x86-64/sframe-plt-1.d: Test for RA displayed as 'f' (fixed
> RA offset), as x86-64 does not use RA tracking.
> * ld-x86-64/sframe-simple-1.d: Likewise.
>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
LGTM.
Thanks for your patch.
> ---
>
> Notes (jremus):
> Changes v2 -> v3:
> - New patch. If the change in display of fixed RA offset as 'f' is
> undesired the patch can be reduced to fixing the logic to display the
> FP offset.
>
> .../gas/cfi-sframe/cfi-sframe-common-4.d | 4 ++--
> .../gas/cfi-sframe/cfi-sframe-common-5.d | 4 ++--
> .../gas/cfi-sframe/cfi-sframe-common-6.d | 4 ++--
> .../gas/cfi-sframe/cfi-sframe-common-7.d | 4 ++--
> .../gas/cfi-sframe/cfi-sframe-common-8.d | 2 +-
> .../gas/cfi-sframe/cfi-sframe-x86_64-1.d | 8 ++++----
> .../gas/scfi/x86_64/scfi-cfi-sections-1.d | 10 +++++-----
> gas/testsuite/gas/scfi/x86_64/scfi-dyn-stack-1.d | 10 +++++-----
> ld/testsuite/ld-x86-64/sframe-plt-1.d | 8 ++++----
> ld/testsuite/ld-x86-64/sframe-simple-1.d | 16 ++++++++--------
> libsframe/sframe-dump.c | 8 +++++---
> 11 files changed, 40 insertions(+), 38 deletions(-)
>
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-4.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-4.d
> index ca559bd0a029..8632613f532f 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-4.d
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-4.d
> @@ -17,7 +17,7 @@ Contents of the SFrame section .sframe:
> func idx \[0\]: pc = 0x0, size = 12 bytes
> STARTPC + CFA + FP + RA +
> #...
> - 0+0004 +sp\+16 +u +u +
> - 0+0008 +sp\+32 +u +u +
> + 0+0004 +sp\+16 +u +[uf] +
> + 0+0008 +sp\+32 +u +[uf] +
>
> #pass
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-5.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-5.d
> index ee82053e13db..dd2c32d3d9fc 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-5.d
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-5.d
> @@ -17,7 +17,7 @@ Contents of the SFrame section .sframe:
> func idx \[0\]: pc = 0x0, size = 12 bytes
> STARTPC + CFA + FP + RA +
> #...
> - 0+0004 +sp\+16 +u +u +
> - 0+0008 +sp\+24 +u +u +
> + 0+0004 +sp\+16 +u +[uf] +
> + 0+0008 +sp\+24 +u +[uf] +
>
> #pass
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-6.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-6.d
> index 9d54b98552bf..34390c46a074 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-6.d
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-6.d
> @@ -17,7 +17,7 @@ Contents of the SFrame section .sframe:
> func idx \[0\]: pc = 0x0, size = 12 bytes
> STARTPC + CFA + FP + RA +
> #...
> - 0+0004 +sp\+8 +u +u +
> - 0+0008 +sp\+8 +u +u +
> + 0+0004 +sp\+8 +u +[uf] +
> + 0+0008 +sp\+8 +u +[uf] +
>
> #pass
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-7.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-7.d
> index 2b7fe3aec8f4..61efb9c4ed12 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-7.d
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-7.d
> @@ -17,7 +17,7 @@ Contents of the SFrame section .sframe:
> func idx \[0\]: pc = 0x0, size = 12 bytes
> STARTPC + CFA + FP + RA +
> #...
> - 0+0004 +sp\+8 +u +u +
> - 0+0008 +sp\+8 +u +u +
> + 0+0004 +sp\+8 +u +[uf] +
> + 0+0008 +sp\+8 +u +[uf] +
>
> #pass
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-8.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-8.d
> index d654e1d0bcd4..d77645636b36 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-8.d
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-8.d
> @@ -17,6 +17,6 @@ Contents of the SFrame section .sframe:
> func idx \[0\]: pc = 0x0, size = 8 bytes
> STARTPC + CFA + FP + RA +
> #...
> - 0+0004 +sp\+16 +u +u +
> + 0+0004 +sp\+16 +u +[uf] +
>
> #pass
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-1.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-1.d
> index c8b5e6adfea0..88b4cc63dbaa 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-1.d
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-1.d
> @@ -16,8 +16,8 @@ Contents of the SFrame section .sframe:
>
> func idx \[0\]: pc = 0x0, size = 25 bytes
> STARTPC +CFA +FP +RA +
> - 0+0000 +sp\+8 +u +u +
> - 0+0001 +sp\+16 +c\-16 +u +
> - 0+0004 +fp\+16 +c\-16 +u +
> - 0+0018 +sp\+8 +c\-16 +u +
> + 0+0000 +sp\+8 +u +f +
> + 0+0001 +sp\+16 +c\-16 +f +
> + 0+0004 +fp\+16 +c\-16 +f +
> + 0+0018 +sp\+8 +c\-16 +f +
> #pass
> diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-cfi-sections-1.d b/gas/testsuite/gas/scfi/x86_64/scfi-cfi-sections-1.d
> index c45933b72edc..7c247e33a6e8 100644
> --- a/gas/testsuite/gas/scfi/x86_64/scfi-cfi-sections-1.d
> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfi-sections-1.d
> @@ -16,10 +16,10 @@ Contents of the SFrame section .sframe:
>
> func idx \[0\]: pc = 0x0, size = 12 bytes
> STARTPC + CFA + FP + RA +
> - 0+0000 +sp\+8 +u +u +
> - 0+0001 +sp\+16 +c\-16 +u +
> - 0+0004 +fp\+16 +c-16 +u +
> - 0+000a +sp\+16 +c\-16 +u +
> - 0+000b +sp\+8 +u +u +
> + 0+0000 +sp\+8 +u +f +
> + 0+0001 +sp\+16 +c\-16 +f +
> + 0+0004 +fp\+16 +c-16 +f +
> + 0+000a +sp\+16 +c\-16 +f +
> + 0+000b +sp\+8 +u +f +
>
> #pass
> diff --git a/gas/testsuite/gas/scfi/x86_64/scfi-dyn-stack-1.d b/gas/testsuite/gas/scfi/x86_64/scfi-dyn-stack-1.d
> index 6cd0484d5793..c6a9b53f4e09 100644
> --- a/gas/testsuite/gas/scfi/x86_64/scfi-dyn-stack-1.d
> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-dyn-stack-1.d
> @@ -16,10 +16,10 @@ Contents of the SFrame section .sframe:
> Function Index :
>
> func idx \[0\]: pc = 0x0, size = 87 bytes
> - STARTPC + CFA + FP + RA
> - 0+0000 + sp\+8 + u + u
> - 0+0001 + sp\+16 + c-16 + u
> - 0+0004 + fp\+16 + c-16 + u
> - 0+0056 + sp\+8 + u + u
> + STARTPC + CFA + FP + RA +
> + 0+0000 + sp\+8 + u + f +
> + 0+0001 + sp\+16 + c-16 + f +
> + 0+0004 + fp\+16 + c-16 + f +
> + 0+0056 + sp\+8 + u + f +
>
> #pass
> diff --git a/ld/testsuite/ld-x86-64/sframe-plt-1.d b/ld/testsuite/ld-x86-64/sframe-plt-1.d
> index 9d123a73826d..617fb9950acc 100644
> --- a/ld/testsuite/ld-x86-64/sframe-plt-1.d
> +++ b/ld/testsuite/ld-x86-64/sframe-plt-1.d
> @@ -19,12 +19,12 @@ Contents of the SFrame section .sframe:
>
> func idx \[0\]: pc = 0x1000, size = 16 bytes
> STARTPC +CFA +FP +RA +
> - 0+1000 +sp\+16 +u +u +
> - 0+1006 +sp\+24 +u +u +
> + 0+1000 +sp\+16 +u +f +
> + 0+1006 +sp\+24 +u +f +
>
> func idx \[1\]: pc = 0x1010, size = 16 bytes
> STARTPC\[m\] +CFA +FP +RA +
> - 0+0000 +sp\+8 +u +u +
> - 0+000b +sp\+16 +u +u +
> + 0+0000 +sp\+8 +u +f +
> + 0+000b +sp\+16 +u +f +
>
> #...
> diff --git a/ld/testsuite/ld-x86-64/sframe-simple-1.d b/ld/testsuite/ld-x86-64/sframe-simple-1.d
> index ce5f94386ac2..18c863938d39 100644
> --- a/ld/testsuite/ld-x86-64/sframe-simple-1.d
> +++ b/ld/testsuite/ld-x86-64/sframe-simple-1.d
> @@ -23,14 +23,14 @@ Contents of the SFrame section .sframe:
>
> func idx \[2\]: pc = 0x1020, size = 53 bytes
> STARTPC +CFA +FP +RA +
> - 0+1020 +sp\+8 +u +u +
> - 0+1021 +sp\+16 +c-16 +u +
> - 0+1024 +fp\+16 +c-16 +u +
> - 0+1054 +sp\+8 +c-16 +u +
> + 0+1020 +sp\+8 +u +f +
> + 0+1021 +sp\+16 +c-16 +f +
> + 0+1024 +fp\+16 +c-16 +f +
> + 0+1054 +sp\+8 +c-16 +f +
>
> func idx \[3\]: pc = 0x1055, size = 37 bytes
> STARTPC +CFA +FP +RA +
> - 0+1055 +sp\+8 +u +u +
> - 0+1056 +sp\+16 +c-16 +u +
> - 0+1059 +fp\+16 +c-16 +u +
> - 0+1079 +sp\+8 +c-16 +u +
> + 0+1055 +sp\+8 +u +f +
> + 0+1056 +sp\+16 +c-16 +f +
> + 0+1059 +fp\+16 +c-16 +f +
> + 0+1079 +sp\+8 +c-16 +f +
> diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c
> index 493d052ce91f..69633d53a33a 100644
> --- a/libsframe/sframe-dump.c
> +++ b/libsframe/sframe-dump.c
> @@ -181,13 +181,15 @@ dump_sframe_func_with_fres (sframe_decoder_ctx *sfd_ctx,
> printf ("%-10s", temp);
>
> /* Dump RA info.
> - If an ABI does not track RA offset, e.g., AMD64, display a 'u',
> + If an ABI does not track RA offset, e.g., AMD64, display 'f',
> else display the offset d as 'c+-d'. */
> - if (sframe_decoder_get_fixed_ra_offset(sfd_ctx)
> + if (sframe_decoder_get_fixed_ra_offset (sfd_ctx)
> != SFRAME_CFA_FIXED_RA_INVALID)
> - strcpy (temp, "u");
> + strcpy (temp, "f");
> else if (err[2] == 0)
> sprintf (temp, "c%+d", ra_offset);
> + else
> + strcpy (temp, "u");
>
> /* Mark SFrame FRE's RA information with "[s]" if the RA is mangled
> with signature bits. */
next prev parent reply other threads:[~2024-04-18 7:40 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 [this message]
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
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=da5e45a9-9cdc-4050-9260-19d34696e9bb@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).