* [RFC PATCH 0/1] sframe: Represent FP without RA on stack (padding)
@ 2024-04-22 15:58 Jens Remus
2024-04-22 15:58 ` [RFC PATCH 1/1] sframe: Represent FP without RA on stack Jens Remus
0 siblings, 1 reply; 6+ messages in thread
From: Jens Remus @ 2024-04-22 15:58 UTC (permalink / raw)
To: binutils, Indu Bhagat; +Cc: Jens Remus, Andreas Krebbel
This patch series adds support in SFrame to represent the frame pointer
(FP) without the return address (RA) being saved on the stack (and/or on
s390x in another register).
This is the first of two proposed alternatives:
1. This patch series uses a dummy padding offset (invalid offset from
CFA value of zero) as RA offset to represent FP without RA on saved
on the stack.
2. The alternative patch series changes the SFrame FRE count field into
a bitmap, to convey which offsets follow the FRE.
Note that it currently applies on top of my v3 patch series series that
adds initial support to generate .sframe from CFI directives on s390x,
although it is independent of that.
The use of padding offsets has the benefit that it is a minor change
to the SFrame V2 format. The downside is that it adds some (but
apparently only minimal) bloat to the .sframe information. Also a value
of zero might not be an invalid offset from CFA on all architectures or
in all use cases (e.g. CFI in glibc longjmp() on some architectures
defines the jump buffer pointer register as CFA base for unwinders to
restore the jump target registers from (as if the return would be to the
jump target)).
A test build of glibc on s390x with this patch series applied shows the
following changes for libc.so:
The number of FDEs increases by 166 and the number of FREs increases by
861, while adding 337 dummy padding RA offsets. With a total of 28157
offsets the dummy padding offsets account for ~1.20 % of the offsets.
Thanks and regards,
Jens
Jens Remus (1):
sframe: Represent FP without RA on stack
gas/gen-sframe.c | 50 +++++++++++++++++++----------------------
include/sframe.h | 9 ++++++--
libsframe/sframe-dump.c | 4 ++++
3 files changed, 34 insertions(+), 29 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/1] sframe: Represent FP without RA on stack
2024-04-22 15:58 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (padding) Jens Remus
@ 2024-04-22 15:58 ` Jens Remus
2024-04-22 23:58 ` Indu Bhagat
0 siblings, 1 reply; 6+ messages in thread
From: Jens Remus @ 2024-04-22 15:58 UTC (permalink / raw)
To: binutils, Indu Bhagat; +Cc: Jens Remus, Andreas Krebbel
If an architecture uses both SFrame RA and FP tracking SFrame assumes
that the RA offset is the 2nd offset and the FP offset is the 3rd offset
following the SFrame FRE. An architecture does not need to store both on
the stack. SFrame cannot represent a FP without RA on stack, since it
cannot distinguish whether the 2nd offset is the RA or FP offset.
Use an invalid SFrame FRE RA offset value of zero as dummy padding to
represent the FP being saved on the stack when the RA is not saved on
the stack.
include/
* sframe.h (SFRAME_FRE_RA_OFFSET_INVALID): New macro defining
the invalid RA offset value used to represent a dummy padding
offset.
gas/
* gen-sframe.c (get_fre_num_offsets): Accommodate for dummy
padding RA offset if FP without RA on stack.
(sframe_get_fre_offset_size): Likewise.
(output_sframe_row_entry): Write a dummy padding RA offset
if FP without RA needs to be represented.
libsframe/
* sframe-dump.c (dump_sframe_func_with_fres): Treat invalid RA
offsets as if they were undefined. Display them as "u*" to
distinguish them.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
Notes (jremus):
This patch eliminates 497 occurrences of the warning "skipping SFrame
FDE due to FP without RA on stack" for a build of glibc on s390x. For
libc.so this increases the number of FDEs by 166 and the number of
FREs by 861, while adding 337 dummy padding RA offsets. With a total
of 28157 offsets the dummy padding offsets account for ~1.20 % of the
offsets.
SFrame statistics without patch:
VALUE TOTAL MIN MAX AVG
FDEs: 3478 - - -
FREs/FDE: 14441 1 15 4
Offsets/FDE: 28157 1 31 8
8-bit: 0 0 0 0
16-bit: 28157 1 31 8
32-bit: 0 0 0 0
Offsets/FRE: 28157 1 3 1
8-bit: - 0 0 0
16-bit: - 1 3 1
32-bit: - 0 0 0
SFrame statistics with patch applied:
VALUE TOTAL MIN MAX AVG
FDEs: 3644 - - -
FREs/FDE: 15302 1 20 4
Offsets/FDE: 29944 1 38 8
8-bit: 0 0 0 0
16-bit: 29944 1 38 8
32-bit: 0 0 0 0
Offsets/FRE: 29944 1 3 1
8-bit: - 0 0 0
16-bit: - 1 3 1
32-bit: - 0 0 0
O_Padd/FDE: 337 - - 0
8-bit: 0
16-bit: 337
32-bit: 0
Note that on s390x the offsets are at minimum 16-bits in size, due to
the mandatory CFA offset being at least 160.
gas/gen-sframe.c | 50 +++++++++++++++++++----------------------
include/sframe.h | 9 ++++++--
libsframe/sframe-dump.c | 4 ++++
3 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 4cc86eb6c815..990b08d87953 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -347,7 +347,9 @@ get_fre_num_offsets (struct sframe_row_entry *sframe_fre)
fre_num_offsets++;
#ifdef SFRAME_FRE_RA_TRACKING
if (sframe_ra_tracking_p ()
- && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+ && (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK
+ /* Accommodate for padding RA offset if FP without RA on stack. */
+ || sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK))
fre_num_offsets++;
#endif
return fre_num_offsets;
@@ -371,9 +373,14 @@ sframe_get_fre_offset_size (struct sframe_row_entry *sframe_fre)
if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
bp_offset_size = get_offset_size_in_bytes (sframe_fre->bp_offset);
#ifdef SFRAME_FRE_RA_TRACKING
- if (sframe_ra_tracking_p ()
- && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
- ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset);
+ if (sframe_ra_tracking_p ())
+ {
+ if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+ ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset);
+ /* Accommodate for padding RA offset if FP without RA on stack. */
+ else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
+ ra_offset_size = get_offset_size_in_bytes (SFRAME_FRE_RA_OFFSET_INVALID);
+ }
#endif
/* Get the maximum size needed to represent the offsets. */
@@ -537,11 +544,19 @@ output_sframe_row_entry (symbolS *fde_start_addr,
fre_write_offsets++;
#ifdef SFRAME_FRE_RA_TRACKING
- if (sframe_ra_tracking_p ()
- && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+ if (sframe_ra_tracking_p ())
{
- fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
- fre_write_offsets++;
+ if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+ {
+ fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
+ fre_write_offsets++;
+ }
+ /* Write padding RA offset if FP without RA on stack. */
+ else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
+ {
+ fre_offset_func_map[idx].out_func (SFRAME_FRE_RA_OFFSET_INVALID);
+ fre_write_offsets++;
+ }
}
#endif
if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
@@ -1497,25 +1512,6 @@ sframe_do_fde (struct sframe_xlate_ctx *xlate_ctx,
= get_dw_fde_end_addrS (xlate_ctx->dw_fde);
}
-#ifdef SFRAME_FRE_RA_TRACKING
- if (sframe_ra_tracking_p ())
- {
- struct sframe_row_entry *fre;
-
- /* Iterate over the scratchpad FREs and validate them. */
- for (fre = xlate_ctx->first_fre; fre; fre = fre->next)
- {
- /* SFrame format cannot represent FP on stack without RA on stack. */
- if (fre->ra_loc != SFRAME_FRE_ELEM_LOC_STACK
- && fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
- {
- as_warn (_("skipping SFrame FDE due to FP without RA on stack"));
- return SFRAME_XLATE_ERR_NOTREPRESENTED;
- }
- }
- }
-#endif /* SFRAME_FRE_RA_TRACKING */
-
return SFRAME_XLATE_OK;
}
diff --git a/include/sframe.h b/include/sframe.h
index 90bc92a32f84..d1a26875b3e2 100644
--- a/include/sframe.h
+++ b/include/sframe.h
@@ -237,6 +237,9 @@ typedef struct sframe_func_desc_entry
may or may not be tracked. */
#define SFRAME_FRE_FP_OFFSET_IDX 2
+/* Invalid RA offset. Used as padding to represent FP without RA on stack. */
+#define SFRAME_FRE_RA_OFFSET_INVALID 0
+
typedef struct sframe_fre_info
{
/* Information about
@@ -288,9 +291,11 @@ typedef struct sframe_fre_info
offset1 (interpreted as CFA = BASE_REG + offset1)
if RA is being tracked
- offset2 (interpreted as RA = CFA + offset2)
+ offset2 (interpreted as RA = CFA + offset2; an offset value of
+ SFRAME_FRE_RA_OFFSET_INVALID indicates a dummy padding RA offset
+ to represent FP without RA saved on stack)
if FP is being tracked
- offset3 (intrepreted as FP = CFA + offset2)
+ offset3 (intrepreted as FP = CFA + offset3)
fi
else
if FP is being tracked
diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c
index 40ea531314ba..3ea4bc327efd 100644
--- a/libsframe/sframe-dump.c
+++ b/libsframe/sframe-dump.c
@@ -199,6 +199,10 @@ dump_sframe_func_with_fres (sframe_decoder_ctx *sfd_ctx,
if (sframe_decoder_get_fixed_ra_offset (sfd_ctx)
!= SFRAME_CFA_FIXED_RA_INVALID)
strcpy (temp, "f");
+ /* If an ABI does track RA offset, e.g. AArch64 and S390, it can be a
+ dummy as padding to represent FP without RA being saved on stack. */
+ else if (err[2] == 0 && ra_offset == SFRAME_FRE_RA_OFFSET_INVALID)
+ sprintf (temp, "u*");
else if (err[2] == 0)
{
if (is_sframe_abi_arch_s390 (sfd_ctx) && (ra_offset & 1))
--
2.40.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/1] sframe: Represent FP without RA on stack
2024-04-22 15:58 ` [RFC PATCH 1/1] sframe: Represent FP without RA on stack Jens Remus
@ 2024-04-22 23:58 ` Indu Bhagat
2024-04-23 15:44 ` Jens Remus
0 siblings, 1 reply; 6+ messages in thread
From: Indu Bhagat @ 2024-04-22 23:58 UTC (permalink / raw)
To: Jens Remus, binutils; +Cc: Andreas Krebbel
On 4/22/24 08:58, Jens Remus wrote:
> If an architecture uses both SFrame RA and FP tracking SFrame assumes
> that the RA offset is the 2nd offset and the FP offset is the 3rd offset
> following the SFrame FRE. An architecture does not need to store both on
> the stack. SFrame cannot represent a FP without RA on stack, since it
> cannot distinguish whether the 2nd offset is the RA or FP offset.
>
> Use an invalid SFrame FRE RA offset value of zero as dummy padding to
> represent the FP being saved on the stack when the RA is not saved on
> the stack.
>
> include/
> * sframe.h (SFRAME_FRE_RA_OFFSET_INVALID): New macro defining
> the invalid RA offset value used to represent a dummy padding
> offset.
>
> gas/
> * gen-sframe.c (get_fre_num_offsets): Accommodate for dummy
> padding RA offset if FP without RA on stack.
> (sframe_get_fre_offset_size): Likewise.
> (output_sframe_row_entry): Write a dummy padding RA offset
> if FP without RA needs to be represented.
>
> libsframe/
> * sframe-dump.c (dump_sframe_func_with_fres): Treat invalid RA
> offsets as if they were undefined. Display them as "u*" to
> distinguish them.
>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>
> Notes (jremus):
> This patch eliminates 497 occurrences of the warning "skipping SFrame
> FDE due to FP without RA on stack" for a build of glibc on s390x. For
> libc.so this increases the number of FDEs by 166 and the number of
> FREs by 861, while adding 337 dummy padding RA offsets. With a total
> of 28157 offsets the dummy padding offsets account for ~1.20 % of the
> offsets.
While this increase seems small, it does look wasteful.
An orthogonal question below...
>
> SFrame statistics without patch:
>
> VALUE TOTAL MIN MAX AVG
> FDEs: 3478 - - -
> FREs/FDE: 14441 1 15 4
> Offsets/FDE: 28157 1 31 8
> 8-bit: 0 0 0 0
> 16-bit: 28157 1 31 8
> 32-bit: 0 0 0 0
> Offsets/FRE: 28157 1 3 1
> 8-bit: - 0 0 0
> 16-bit: - 1 3 1
> 32-bit: - 0 0 0
>
> SFrame statistics with patch applied:
>
> VALUE TOTAL MIN MAX AVG
> FDEs: 3644 - - -
> FREs/FDE: 15302 1 20 4
> Offsets/FDE: 29944 1 38 8
> 8-bit: 0 0 0 0
> 16-bit: 29944 1 38 8
> 32-bit: 0 0 0 0
> Offsets/FRE: 29944 1 3 1
> 8-bit: - 0 0 0
> 16-bit: - 1 3 1
> 32-bit: - 0 0 0
> O_Padd/FDE: 337 - - 0
> 8-bit: 0
> 16-bit: 337
> 32-bit: 0
>
> Note that on s390x the offsets are at minimum 16-bits in size, due to
> the mandatory CFA offset being at least 160.
>
IIUC, all stack layouts supported in the ABI use the offset 160. Is that
right ? I am wondering if adjusting the stored offsets in the SFrame
section (by decrementing 160 from it) will work ?
If yes, we could encode this constant in SFrame aux hdr bytes for s390x.
> gas/gen-sframe.c | 50 +++++++++++++++++++----------------------
> include/sframe.h | 9 ++++++--
> libsframe/sframe-dump.c | 4 ++++
> 3 files changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index 4cc86eb6c815..990b08d87953 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -347,7 +347,9 @@ get_fre_num_offsets (struct sframe_row_entry *sframe_fre)
> fre_num_offsets++;
> #ifdef SFRAME_FRE_RA_TRACKING
> if (sframe_ra_tracking_p ()
> - && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
> + && (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK
> + /* Accommodate for padding RA offset if FP without RA on stack. */
> + || sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK))
> fre_num_offsets++;
> #endif
> return fre_num_offsets;
> @@ -371,9 +373,14 @@ sframe_get_fre_offset_size (struct sframe_row_entry *sframe_fre)
> if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
> bp_offset_size = get_offset_size_in_bytes (sframe_fre->bp_offset);
> #ifdef SFRAME_FRE_RA_TRACKING
> - if (sframe_ra_tracking_p ()
> - && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
> - ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset);
> + if (sframe_ra_tracking_p ())
> + {
> + if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
> + ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset);
> + /* Accommodate for padding RA offset if FP without RA on stack. */
> + else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
> + ra_offset_size = get_offset_size_in_bytes (SFRAME_FRE_RA_OFFSET_INVALID);
> + }
> #endif
>
> /* Get the maximum size needed to represent the offsets. */
> @@ -537,11 +544,19 @@ output_sframe_row_entry (symbolS *fde_start_addr,
> fre_write_offsets++;
>
> #ifdef SFRAME_FRE_RA_TRACKING
> - if (sframe_ra_tracking_p ()
> - && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
> + if (sframe_ra_tracking_p ())
> {
> - fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
> - fre_write_offsets++;
> + if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
> + {
> + fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
> + fre_write_offsets++;
> + }
> + /* Write padding RA offset if FP without RA on stack. */
> + else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
> + {
> + fre_offset_func_map[idx].out_func (SFRAME_FRE_RA_OFFSET_INVALID);
> + fre_write_offsets++;
> + }
> }
> #endif
> if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
> @@ -1497,25 +1512,6 @@ sframe_do_fde (struct sframe_xlate_ctx *xlate_ctx,
> = get_dw_fde_end_addrS (xlate_ctx->dw_fde);
> }
>
> -#ifdef SFRAME_FRE_RA_TRACKING
> - if (sframe_ra_tracking_p ())
> - {
> - struct sframe_row_entry *fre;
> -
> - /* Iterate over the scratchpad FREs and validate them. */
> - for (fre = xlate_ctx->first_fre; fre; fre = fre->next)
> - {
> - /* SFrame format cannot represent FP on stack without RA on stack. */
> - if (fre->ra_loc != SFRAME_FRE_ELEM_LOC_STACK
> - && fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
> - {
> - as_warn (_("skipping SFrame FDE due to FP without RA on stack"));
> - return SFRAME_XLATE_ERR_NOTREPRESENTED;
> - }
> - }
> - }
> -#endif /* SFRAME_FRE_RA_TRACKING */
> -
> return SFRAME_XLATE_OK;
> }
>
> diff --git a/include/sframe.h b/include/sframe.h
> index 90bc92a32f84..d1a26875b3e2 100644
> --- a/include/sframe.h
> +++ b/include/sframe.h
> @@ -237,6 +237,9 @@ typedef struct sframe_func_desc_entry
> may or may not be tracked. */
> #define SFRAME_FRE_FP_OFFSET_IDX 2
>
> +/* Invalid RA offset. Used as padding to represent FP without RA on stack. */
> +#define SFRAME_FRE_RA_OFFSET_INVALID 0
> +
> typedef struct sframe_fre_info
> {
> /* Information about
> @@ -288,9 +291,11 @@ typedef struct sframe_fre_info
> offset1 (interpreted as CFA = BASE_REG + offset1)
>
> if RA is being tracked
> - offset2 (interpreted as RA = CFA + offset2)
> + offset2 (interpreted as RA = CFA + offset2; an offset value of
> + SFRAME_FRE_RA_OFFSET_INVALID indicates a dummy padding RA offset
> + to represent FP without RA saved on stack)
> if FP is being tracked
> - offset3 (intrepreted as FP = CFA + offset2)
> + offset3 (intrepreted as FP = CFA + offset3)
I too noticed this typo recently and have a patch fixing this.
> fi
> else
> if FP is being tracked
> diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c
> index 40ea531314ba..3ea4bc327efd 100644
> --- a/libsframe/sframe-dump.c
> +++ b/libsframe/sframe-dump.c
> @@ -199,6 +199,10 @@ dump_sframe_func_with_fres (sframe_decoder_ctx *sfd_ctx,
> if (sframe_decoder_get_fixed_ra_offset (sfd_ctx)
> != SFRAME_CFA_FIXED_RA_INVALID)
> strcpy (temp, "f");
> + /* If an ABI does track RA offset, e.g. AArch64 and S390, it can be a
> + dummy as padding to represent FP without RA being saved on stack. */
> + else if (err[2] == 0 && ra_offset == SFRAME_FRE_RA_OFFSET_INVALID)
> + sprintf (temp, "u*");
> else if (err[2] == 0)
> {
> if (is_sframe_abi_arch_s390 (sfd_ctx) && (ra_offset & 1))
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/1] sframe: Represent FP without RA on stack
2024-04-22 23:58 ` Indu Bhagat
@ 2024-04-23 15:44 ` Jens Remus
2024-04-25 6:59 ` Indu Bhagat
0 siblings, 1 reply; 6+ messages in thread
From: Jens Remus @ 2024-04-23 15:44 UTC (permalink / raw)
To: Indu Bhagat, binutils; +Cc: Andreas Krebbel
Am 23.04.2024 um 01:58 schrieb Indu Bhagat:
> On 4/22/24 08:58, Jens Remus wrote:
>> If an architecture uses both SFrame RA and FP tracking SFrame assumes
>> that the RA offset is the 2nd offset and the FP offset is the 3rd offset
>> following the SFrame FRE. An architecture does not need to store both on
>> the stack. SFrame cannot represent a FP without RA on stack, since it
>> cannot distinguish whether the 2nd offset is the RA or FP offset.
>>
>> Use an invalid SFrame FRE RA offset value of zero as dummy padding to
>> represent the FP being saved on the stack when the RA is not saved on
>> the stack.
>>
>> include/
>> * sframe.h (SFRAME_FRE_RA_OFFSET_INVALID): New macro defining
>> the invalid RA offset value used to represent a dummy padding
>> offset.
>>
>> gas/
>> * gen-sframe.c (get_fre_num_offsets): Accommodate for dummy
>> padding RA offset if FP without RA on stack.
>> (sframe_get_fre_offset_size): Likewise.
>> (output_sframe_row_entry): Write a dummy padding RA offset
>> if FP without RA needs to be represented.
>>
>> libsframe/
>> * sframe-dump.c (dump_sframe_func_with_fres): Treat invalid RA
>> offsets as if they were undefined. Display them as "u*" to
>> distinguish them.
>>
>> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
>> ---
>>
>> Notes (jremus):
>> This patch eliminates 497 occurrences of the warning "skipping
>> SFrame
>> FDE due to FP without RA on stack" for a build of glibc on s390x.
>> For
>> libc.so this increases the number of FDEs by 166 and the number of
>> FREs by 861, while adding 337 dummy padding RA offsets. With a total
>> of 28157 offsets the dummy padding offsets account for ~1.20 % of
>> the
>> offsets.
>
> While this increase seems small, it does look wasteful.
>
> An orthogonal question below...
>
>> SFrame statistics without patch:
>> VALUE TOTAL MIN MAX AVG
>> FDEs: 3478 - - -
>> FREs/FDE: 14441 1 15 4
>> Offsets/FDE: 28157 1 31 8
>> 8-bit: 0 0 0 0
>> 16-bit: 28157 1 31 8
>> 32-bit: 0 0 0 0
>> Offsets/FRE: 28157 1 3 1
>> 8-bit: - 0 0 0
>> 16-bit: - 1 3 1
>> 32-bit: - 0 0 0
>> SFrame statistics with patch applied:
>> VALUE TOTAL MIN MAX AVG
>> FDEs: 3644 - - -
>> FREs/FDE: 15302 1 20 4
>> Offsets/FDE: 29944 1 38 8
>> 8-bit: 0 0 0 0
>> 16-bit: 29944 1 38 8
>> 32-bit: 0 0 0 0
>> Offsets/FRE: 29944 1 3 1
>> 8-bit: - 0 0 0
>> 16-bit: - 1 3 1
>> 32-bit: - 0 0 0
>> O_Padd/FDE: 337 - - 0
>> 8-bit: 0
>> 16-bit: 337
>> 32-bit: 0
>> Note that on s390x the offsets are at minimum 16-bits in size,
>> due to
>> the mandatory CFA offset being at least 160.
>>
>
> IIUC, all stack layouts supported in the ABI use the offset 160. Is that
> right ? I am wondering if adjusting the stored offsets in the SFrame
> section (by decrementing 160 from it) will work ?
>
> If yes, we could encode this constant in SFrame aux hdr bytes for s390x.
Thank you for the hint! Using a constant adjustment of -160 on s390x for
the CFA offset from CFA base register should work to allow for 8-bit
offsets to be used. Aren't all tracked offsets (i.e. CFA, FP, and RA)
signed anyway? Thus applying a constant adjustment should work in any case?
Couldn't it simply be an architecture specific constant in the code to
begin with? For example a new macro, which is only applied when defined?
#define S390_SFRAME_CFA_OFFSET_ADJUSTMENT -160
#define SFRAME_CFA_OFFSET_ADJUSTMENT S390_SFRAME_CFA_OFFSET_ADJUSTMENT
Implementing this in the SFrame auxiliary header would of course allow
to implement this enhancement at a later stage and to change the
adjustment value in the future, as the linker can then either reject or
merge different adjustment values during link editing.
I wonder whether it would make sense to store the FP register number in
the SFrame auxiliary header for s390x as well. Register 11 is just a
convention of the compilers and not defined by the ABI. That would
enable us to choose a different register as frame pointer in the future.
>
>> gas/gen-sframe.c | 50 +++++++++++++++++++----------------------
>> include/sframe.h | 9 ++++++--
>> libsframe/sframe-dump.c | 4 ++++
>> 3 files changed, 34 insertions(+), 29 deletions(-)
>>
>> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
>> index 4cc86eb6c815..990b08d87953 100644
>> --- a/gas/gen-sframe.c
>> +++ b/gas/gen-sframe.c
>> @@ -347,7 +347,9 @@ get_fre_num_offsets (struct sframe_row_entry
>> *sframe_fre)
>> fre_num_offsets++;
>> #ifdef SFRAME_FRE_RA_TRACKING
>> if (sframe_ra_tracking_p ()
>> - && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> + && (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK
>> + /* Accommodate for padding RA offset if FP without RA on
>> stack. */
>> + || sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK))
>> fre_num_offsets++;
>> #endif
>> return fre_num_offsets;
>> @@ -371,9 +373,14 @@ sframe_get_fre_offset_size (struct
>> sframe_row_entry *sframe_fre)
>> if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> bp_offset_size = get_offset_size_in_bytes (sframe_fre->bp_offset);
>> #ifdef SFRAME_FRE_RA_TRACKING
>> - if (sframe_ra_tracking_p ()
>> - && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> - ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset);
>> + if (sframe_ra_tracking_p ())
>> + {
>> + if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> + ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset);
>> + /* Accommodate for padding RA offset if FP without RA on
>> stack. */
>> + else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> + ra_offset_size = get_offset_size_in_bytes
>> (SFRAME_FRE_RA_OFFSET_INVALID);
>> + }
>> #endif
>> /* Get the maximum size needed to represent the offsets. */
>> @@ -537,11 +544,19 @@ output_sframe_row_entry (symbolS *fde_start_addr,
>> fre_write_offsets++;
>> #ifdef SFRAME_FRE_RA_TRACKING
>> - if (sframe_ra_tracking_p ()
>> - && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> + if (sframe_ra_tracking_p ())
>> {
>> - fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
>> - fre_write_offsets++;
>> + if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> + {
>> + fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
>> + fre_write_offsets++;
>> + }
>> + /* Write padding RA offset if FP without RA on stack. */
>> + else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> + {
>> + fre_offset_func_map[idx].out_func (SFRAME_FRE_RA_OFFSET_INVALID);
>> + fre_write_offsets++;
>> + }
>> }
>> #endif
>> if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> @@ -1497,25 +1512,6 @@ sframe_do_fde (struct sframe_xlate_ctx *xlate_ctx,
>> = get_dw_fde_end_addrS (xlate_ctx->dw_fde);
>> }
>> -#ifdef SFRAME_FRE_RA_TRACKING
>> - if (sframe_ra_tracking_p ())
>> - {
>> - struct sframe_row_entry *fre;
>> -
>> - /* Iterate over the scratchpad FREs and validate them. */
>> - for (fre = xlate_ctx->first_fre; fre; fre = fre->next)
>> - {
>> - /* SFrame format cannot represent FP on stack without RA on
>> stack. */
>> - if (fre->ra_loc != SFRAME_FRE_ELEM_LOC_STACK
>> - && fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> - {
>> - as_warn (_("skipping SFrame FDE due to FP without RA on
>> stack"));
>> - return SFRAME_XLATE_ERR_NOTREPRESENTED;
>> - }
>> - }
>> - }
>> -#endif /* SFRAME_FRE_RA_TRACKING */
>> -
>> return SFRAME_XLATE_OK;
>> }
>> diff --git a/include/sframe.h b/include/sframe.h
>> index 90bc92a32f84..d1a26875b3e2 100644
>> --- a/include/sframe.h
>> +++ b/include/sframe.h
>> @@ -237,6 +237,9 @@ typedef struct sframe_func_desc_entry
>> may or may not be tracked. */
>> #define SFRAME_FRE_FP_OFFSET_IDX 2
>> +/* Invalid RA offset. Used as padding to represent FP without RA on
>> stack. */
>> +#define SFRAME_FRE_RA_OFFSET_INVALID 0
>> +
>> typedef struct sframe_fre_info
>> {
>> /* Information about
>> @@ -288,9 +291,11 @@ typedef struct sframe_fre_info
>> offset1 (interpreted as CFA = BASE_REG + offset1)
>> if RA is being tracked
>> - offset2 (interpreted as RA = CFA + offset2)
>> + offset2 (interpreted as RA = CFA + offset2; an offset value of
>> + SFRAME_FRE_RA_OFFSET_INVALID indicates a dummy padding RA
>> offset
>> + to represent FP without RA saved on stack)
>> if FP is being tracked
>> - offset3 (intrepreted as FP = CFA + offset2)
>> + offset3 (intrepreted as FP = CFA + offset3)
>
> I too noticed this typo recently and have a patch fixing this.
>
>> fi
>> else
>> if FP is being tracked
>> diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c
>> index 40ea531314ba..3ea4bc327efd 100644
>> --- a/libsframe/sframe-dump.c
>> +++ b/libsframe/sframe-dump.c
>> @@ -199,6 +199,10 @@ dump_sframe_func_with_fres (sframe_decoder_ctx
>> *sfd_ctx,
>> if (sframe_decoder_get_fixed_ra_offset (sfd_ctx)
>> != SFRAME_CFA_FIXED_RA_INVALID)
>> strcpy (temp, "f");
>> + /* If an ABI does track RA offset, e.g. AArch64 and S390, it
>> can be a
>> + dummy as padding to represent FP without RA being saved on
>> stack. */
>> + else if (err[2] == 0 && ra_offset == SFRAME_FRE_RA_OFFSET_INVALID)
>> + sprintf (temp, "u*");
>> else if (err[2] == 0)
>> {
>> if (is_sframe_abi_arch_s390 (sfd_ctx) && (ra_offset & 1))
>
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
jremus@de.ibm.com
IBM
IBM Deutschland Research & Development GmbH; Vorsitzender des
Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der
Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/1] sframe: Represent FP without RA on stack
2024-04-23 15:44 ` Jens Remus
@ 2024-04-25 6:59 ` Indu Bhagat
0 siblings, 0 replies; 6+ messages in thread
From: Indu Bhagat @ 2024-04-25 6:59 UTC (permalink / raw)
To: Jens Remus, binutils; +Cc: Andreas Krebbel
On 4/23/24 08:44, Jens Remus wrote:
> Am 23.04.2024 um 01:58 schrieb Indu Bhagat:
>> On 4/22/24 08:58, Jens Remus wrote:
>>> If an architecture uses both SFrame RA and FP tracking SFrame assumes
>>> that the RA offset is the 2nd offset and the FP offset is the 3rd offset
>>> following the SFrame FRE. An architecture does not need to store both on
>>> the stack. SFrame cannot represent a FP without RA on stack, since it
>>> cannot distinguish whether the 2nd offset is the RA or FP offset.
>>>
>>> Use an invalid SFrame FRE RA offset value of zero as dummy padding to
>>> represent the FP being saved on the stack when the RA is not saved on
>>> the stack.
>>>
>>> include/
>>> * sframe.h (SFRAME_FRE_RA_OFFSET_INVALID): New macro defining
>>> the invalid RA offset value used to represent a dummy padding
>>> offset.
>>>
>>> gas/
>>> * gen-sframe.c (get_fre_num_offsets): Accommodate for dummy
>>> padding RA offset if FP without RA on stack.
>>> (sframe_get_fre_offset_size): Likewise.
>>> (output_sframe_row_entry): Write a dummy padding RA offset
>>> if FP without RA needs to be represented.
>>>
>>> libsframe/
>>> * sframe-dump.c (dump_sframe_func_with_fres): Treat invalid RA
>>> offsets as if they were undefined. Display them as "u*" to
>>> distinguish them.
>>>
>>> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
>>> ---
>>>
>>> Notes (jremus):
>>> This patch eliminates 497 occurrences of the warning "skipping
>>> SFrame
>>> FDE due to FP without RA on stack" for a build of glibc on
>>> s390x. For
>>> libc.so this increases the number of FDEs by 166 and the number of
>>> FREs by 861, while adding 337 dummy padding RA offsets. With a
>>> total
>>> of 28157 offsets the dummy padding offsets account for ~1.20 %
>>> of the
>>> offsets.
>>
>> While this increase seems small, it does look wasteful.
>>
>> An orthogonal question below...
>>
>>> SFrame statistics without patch:
>>> VALUE TOTAL MIN MAX AVG
>>> FDEs: 3478 - - -
>>> FREs/FDE: 14441 1 15 4
>>> Offsets/FDE: 28157 1 31 8
>>> 8-bit: 0 0 0 0
>>> 16-bit: 28157 1 31 8
>>> 32-bit: 0 0 0 0
>>> Offsets/FRE: 28157 1 3 1
>>> 8-bit: - 0 0 0
>>> 16-bit: - 1 3 1
>>> 32-bit: - 0 0 0
>>> SFrame statistics with patch applied:
>>> VALUE TOTAL MIN MAX AVG
>>> FDEs: 3644 - - -
>>> FREs/FDE: 15302 1 20 4
>>> Offsets/FDE: 29944 1 38 8
>>> 8-bit: 0 0 0 0
>>> 16-bit: 29944 1 38 8
>>> 32-bit: 0 0 0 0
>>> Offsets/FRE: 29944 1 3 1
>>> 8-bit: - 0 0 0
>>> 16-bit: - 1 3 1
>>> 32-bit: - 0 0 0
>>> O_Padd/FDE: 337 - - 0
>>> 8-bit: 0
>>> 16-bit: 337
>>> 32-bit: 0
>>> Note that on s390x the offsets are at minimum 16-bits in size,
>>> due to
>>> the mandatory CFA offset being at least 160.
>>>
>>
>> IIUC, all stack layouts supported in the ABI use the offset 160. Is
>> that right ? I am wondering if adjusting the stored offsets in the
>> SFrame section (by decrementing 160 from it) will work ?
>>
>> If yes, we could encode this constant in SFrame aux hdr bytes for s390x.
>
> Thank you for the hint! Using a constant adjustment of -160 on s390x for
> the CFA offset from CFA base register should work to allow for 8-bit
> offsets to be used. Aren't all tracked offsets (i.e. CFA, FP, and RA)
> signed anyway? Thus applying a constant adjustment should work in any case?
>
Yes, these offsets are signed. Applying the constant should work for CFA.
> Couldn't it simply be an architecture specific constant in the code to
> begin with? For example a new macro, which is only applied when defined?
>
> #define S390_SFRAME_CFA_OFFSET_ADJUSTMENT -160
> #define SFRAME_CFA_OFFSET_ADJUSTMENT S390_SFRAME_CFA_OFFSET_ADJUSTMENT
>
I think a macro should also work in this case.
> Implementing this in the SFrame auxiliary header would of course allow
> to implement this enhancement at a later stage and to change the
> adjustment value in the future, as the linker can then either reject or
> merge different adjustment values during link editing.
>
> I wonder whether it would make sense to store the FP register number in
> the SFrame auxiliary header for s390x as well. Register 11 is just a
> convention of the compilers and not defined by the ABI. That would
> enable us to choose a different register as frame pointer in the future.
>
I think the problem will remain that there is ATM no way to communicate
this information to the assembler (that compiler used r11 as fp role).
And even if there was a way, I am not so sure. Since the ABI doesnt
mandate r11 as fp, the compiler may pick another register for say a
different stack layout etc, in the future ? IOW, even it picking
different fp register across functions is a possibility, no? So what is
expected of the compiler then...
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/1] sframe: Represent FP without RA on stack
2024-04-22 15:59 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (bitmap) Jens Remus
@ 2024-04-22 15:59 ` Jens Remus
0 siblings, 0 replies; 6+ messages in thread
From: Jens Remus @ 2024-04-22 15:59 UTC (permalink / raw)
To: binutils, Indu Bhagat; +Cc: Jens Remus, Andreas Krebbel
If an architecture uses both SFrame RA and FP tracking SFrame assumes
that the RA offset is the 2nd offset and the FP offset is the 3rd offset
following the SFrame FRE. An architecture does not need to store both on
the stack. SFrame cannot represent a FP without RA on stack.
Update the SFrame V2 FRE info word format to redefine the 4-bit offset
count field from the SFrame V1 FRE info word as 4-bit offset presence
flags field in the SFrame V2 FRE info word. This allows up to four
optional offsets to be tracked. While the mandatory CFA offset is
represented in the offset presence flags, it could be omitted to free
up one offset presence flag for an optional offset.
include/
* sframe.h: Updated SFrame FRE info word for V2. Redefine the
4-bit offset count field as offset presence bits.
(SFRAME_V2_FRE_INFO): New macro to build the SFrame V2 FRE info
word using offset presence bits instead of offset count.
(SFRAME_V2_FRE_OFFSET_FLAGS): New macro to extract the offset
presence flags from the SFrame V2 FRE info word.
gas/
* gen-sframe.c (sframe_v2_set_fre_info): New function to build
the SFrame V2 FRE info word using offset presence bits instead
of offset count.
(sframe_set_version): Use new sframe_v2_set_fre_info to set the
SFrame V2 FRE info word.
(sframe_set_fre_info): Redefine parameter num_offsets as
offset_count_or_flags.
(get_fre_offset_flags): New helper to compute the offset
presence flags required for a SFrame V2 FRE.
(output_sframe_row_entry): Use new get_fre_offset_flags helper
and pass the offset presence flags instead of offset count to
sframe_set_fre_info.
(sframe_do_fde): Revert commit "gas: Skip SFrame FDE if FP
without RA on stack".
libsframe/
* sframe.c (sframe_get_fre_offset_count): Compute offset count
from offset presence bits using a lookup table.
(sframe_get_fre_offset_index): New helper to compute the offset
index from the offset presence bits and the offset ID.
(sframe_fre_get_cfa_offset, sframe_fre_get_fp_offset,
sframe_fre_get_ra_offset): Use new sframe_get_fre_offset_index
helper to determine the CFA, FP, and RA offset index.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
Notes (jremus):
TODO:
- Change wording of SFRAME_FRE_{CFA,FP,RA}_OFFSET_IDX, as they no longer
represent an index, which the FP offset actually never really did.
Maybe use _ID instead of _IDX?
- Update SFrame V2 specification accordingly. Can this change still be
included in V2 or would it require a V3?
gas/gen-sframe.c | 66 ++++++++++++++++++++++++++++------------------
include/sframe.h | 13 ++++++---
libsframe/sframe.c | 51 +++++++++++++++++++++++++++--------
3 files changed, 90 insertions(+), 40 deletions(-)
diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 4cc86eb6c815..7f3b406c7882 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -251,7 +251,7 @@ fre_offset_func_map[SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_MAX+1] =
static struct sframe_version_ops sframe_ver_ops;
/* SFrame (SFRAME_VERSION_1) set FRE info. */
-
+/*
static unsigned char
sframe_v1_set_fre_info (unsigned int base_reg, unsigned int num_offsets,
unsigned int offset_size, bool mangled_ra_p)
@@ -261,6 +261,19 @@ sframe_v1_set_fre_info (unsigned int base_reg, unsigned int num_offsets,
fre_info = SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P (mangled_ra_p, fre_info);
return fre_info;
}
+*/
+
+/* SFrame (SFRAME_VERSION_2) set FRE info. */
+
+static unsigned char
+sframe_v2_set_fre_info (unsigned int base_reg, unsigned int offset_flags,
+ unsigned int offset_size, bool mangled_ra_p)
+{
+ unsigned char fre_info;
+ fre_info = SFRAME_V2_FRE_INFO (base_reg, offset_flags, offset_size);
+ fre_info = SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P (mangled_ra_p, fre_info);
+ return fre_info;
+}
/* SFrame (SFRAME_VERSION_1) set function info. */
static unsigned char
@@ -280,10 +293,10 @@ sframe_set_version (uint32_t sframe_version ATTRIBUTE_UNUSED)
{
sframe_ver_ops.format_version = SFRAME_VERSION_2;
- /* These operations remain the same for SFRAME_VERSION_2 as fre_info and
- func_info have not changed from SFRAME_VERSION_1. */
+ sframe_ver_ops.set_fre_info = sframe_v2_set_fre_info;
- sframe_ver_ops.set_fre_info = sframe_v1_set_fre_info;
+ /* This operation remains the same for SFRAME_VERSION_2 as func_info
+ has not changed from SFRAME_VERSION_1. */
sframe_ver_ops.set_func_info = sframe_v1_set_func_info;
}
@@ -291,10 +304,10 @@ sframe_set_version (uint32_t sframe_version ATTRIBUTE_UNUSED)
/* SFrame set FRE info. */
static unsigned char
-sframe_set_fre_info (unsigned int base_reg, unsigned int num_offsets,
+sframe_set_fre_info (unsigned int base_reg, unsigned int offset_count_or_flags,
unsigned int offset_size, bool mangled_ra_p)
{
- return sframe_ver_ops.set_fre_info (base_reg, num_offsets,
+ return sframe_ver_ops.set_fre_info (base_reg, offset_count_or_flags,
offset_size, mangled_ra_p);
}
@@ -353,6 +366,24 @@ get_fre_num_offsets (struct sframe_row_entry *sframe_fre)
return fre_num_offsets;
}
+/* Get offset flags necessary for the SFrame Frame Row Entry. */
+
+static unsigned int
+get_fre_offset_flags (struct sframe_row_entry *sframe_fre)
+{
+ /* CFA offset must always be present. */
+ unsigned int fre_offset_flags = 1 << SFRAME_FRE_CFA_OFFSET_IDX;
+
+ if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
+ fre_offset_flags |= 1 << SFRAME_FRE_FP_OFFSET_IDX;
+#ifdef SFRAME_FRE_RA_TRACKING
+ if (sframe_ra_tracking_p ()
+ && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
+ fre_offset_flags |= 1 << SFRAME_FRE_RA_OFFSET_IDX;
+#endif
+ return fre_offset_flags;
+}
+
/* Get the minimum necessary offset size (in bytes) for this
SFrame frame row entry. */
@@ -494,6 +525,7 @@ output_sframe_row_entry (symbolS *fde_start_addr,
{
unsigned char fre_info;
unsigned int fre_num_offsets;
+ unsigned int fre_offset_flags;
unsigned int fre_offset_size;
unsigned int fre_base_reg;
expressionS exp;
@@ -524,8 +556,9 @@ output_sframe_row_entry (symbolS *fde_start_addr,
size of offset in this frame row entry. */
fre_base_reg = get_fre_base_reg_id (sframe_fre);
fre_num_offsets = get_fre_num_offsets (sframe_fre);
+ fre_offset_flags = get_fre_offset_flags (sframe_fre);
fre_offset_size = sframe_get_fre_offset_size (sframe_fre);
- fre_info = sframe_set_fre_info (fre_base_reg, fre_num_offsets,
+ fre_info = sframe_set_fre_info (fre_base_reg, fre_offset_flags,
fre_offset_size, sframe_fre->mangled_ra_p);
out_one (fre_info);
@@ -1497,25 +1530,6 @@ sframe_do_fde (struct sframe_xlate_ctx *xlate_ctx,
= get_dw_fde_end_addrS (xlate_ctx->dw_fde);
}
-#ifdef SFRAME_FRE_RA_TRACKING
- if (sframe_ra_tracking_p ())
- {
- struct sframe_row_entry *fre;
-
- /* Iterate over the scratchpad FREs and validate them. */
- for (fre = xlate_ctx->first_fre; fre; fre = fre->next)
- {
- /* SFrame format cannot represent FP on stack without RA on stack. */
- if (fre->ra_loc != SFRAME_FRE_ELEM_LOC_STACK
- && fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
- {
- as_warn (_("skipping SFrame FDE due to FP without RA on stack"));
- return SFRAME_XLATE_ERR_NOTREPRESENTED;
- }
- }
- }
-#endif /* SFRAME_FRE_RA_TRACKING */
-
return SFRAME_XLATE_OK;
}
diff --git a/include/sframe.h b/include/sframe.h
index 90bc92a32f84..093dca6cda9a 100644
--- a/include/sframe.h
+++ b/include/sframe.h
@@ -241,14 +241,14 @@ typedef struct sframe_fre_info
{
/* Information about
- 1 bit: base reg for CFA
- - 4 bits: Number of offsets (N). A value of upto 3 is allowed to track
- all three of CFA, FP and RA (fixed implicit order).
+ - 4 bits: Flags for offsets. Upto 3 are allowed to track
+ all three of CFA, FP and RA (fixed order).
- 2 bits: information about size of the offsets (S) in bytes.
Valid values are SFRAME_FRE_OFFSET_1B, SFRAME_FRE_OFFSET_2B,
SFRAME_FRE_OFFSET_4B
- 1 bit: Mangled RA state bit (aarch64 only).
----------------------------------------------------------------------------------
- | Mangled-RA (aarch64) | Size of offsets | Number of offsets | base_reg |
+ | Mangled-RA (aarch64) | Size of offsets | Flags for offsets | base_reg |
| Unused (amd64, s390) | | | |
----------------------------------------------------------------------------------
8 7 5 1 0
@@ -264,6 +264,11 @@ typedef struct sframe_fre_info
(((0 & 0x1) << 7) | (((offset_size) & 0x3) << 5) | \
(((offset_num) & 0xf) << 1) | ((base_reg_id) & 0x1))
+/* Note: Set mangled_ra_p to zero by default. */
+#define SFRAME_V2_FRE_INFO(base_reg_id, offset_flags, offset_size) \
+ (((0 & 0x1) << 7) | (((offset_size) & 0x3) << 5) | \
+ (((offset_flags) & 0xf) << 1) | ((base_reg_id) & 0x1))
+
/* Set the mangled_ra_p bit as indicated. */
#define SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P(mangled_ra_p, fre_info) \
((((mangled_ra_p) & 0x1) << 7) | ((fre_info) & 0x7f))
@@ -273,6 +278,8 @@ typedef struct sframe_fre_info
#define SFRAME_V1_FRE_OFFSET_SIZE(data) (((data) >> 5) & 0x3)
#define SFRAME_V1_FRE_MANGLED_RA_P(data) (((data) >> 7) & 0x1)
+#define SFRAME_V2_FRE_OFFSET_FLAGS(data) (((data) >> 1) & 0xf)
+
/* SFrame Frame Row Entry definitions.
Used for AMD64, AARCH64, and S390.
diff --git a/libsframe/sframe.c b/libsframe/sframe.c
index 460face4168e..f43f9eaecfd1 100644
--- a/libsframe/sframe.c
+++ b/libsframe/sframe.c
@@ -115,7 +115,10 @@ sframe_get_hdr_size (sframe_header *sfh)
static uint8_t
sframe_fre_get_offset_count (uint8_t fre_info)
{
- return SFRAME_V1_FRE_OFFSET_COUNT (fre_info);
+ unsigned int fre_offset_flags = SFRAME_V2_FRE_OFFSET_FLAGS (fre_info);
+ const char flags_to_count[16] = { 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4 };
+
+ return flags_to_count[fre_offset_flags];
}
static uint8_t
@@ -130,6 +133,19 @@ sframe_get_fre_ra_mangled_p (uint8_t fre_info)
return SFRAME_V1_FRE_MANGLED_RA_P (fre_info);
}
+static int
+sframe_get_fre_offset_index (uint8_t fre_info, unsigned int tracked_offset)
+{
+ unsigned int fre_offset_flags = SFRAME_V2_FRE_OFFSET_FLAGS (fre_info);
+ const char flags_to_count[16] = { 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4 };
+ unsigned int mask = (1 << tracked_offset) - 1;
+
+ if ((fre_offset_flags & (1 << tracked_offset)) == 0)
+ return -1;
+
+ return flags_to_count[fre_offset_flags & mask];
+}
+
/* Access functions for info from function descriptor entry. */
static uint32_t
@@ -680,7 +696,13 @@ int32_t
sframe_fre_get_cfa_offset (sframe_decoder_ctx *dctx ATTRIBUTE_UNUSED,
sframe_frame_row_entry *fre, int *errp)
{
- return sframe_get_fre_offset (fre, SFRAME_FRE_CFA_OFFSET_IDX, errp);
+ int cfa_offset_idx = sframe_get_fre_offset_index (fre->fre_info,
+ SFRAME_FRE_CFA_OFFSET_IDX);
+ if (cfa_offset_idx == -1)
+ return sframe_set_errno (errp, SFRAME_ERR_FREOFFSET_NOPRESENT);
+ sframe_assert (cfa_offset_idx == 0);
+
+ return sframe_get_fre_offset (fre, cfa_offset_idx, errp);
}
/* Get the FP offset from the FRE. If the offset is invalid, sets errp. */
@@ -689,7 +711,7 @@ int32_t
sframe_fre_get_fp_offset (sframe_decoder_ctx *dctx,
sframe_frame_row_entry *fre, int *errp)
{
- uint32_t fp_offset_idx = 0;
+ int fp_offset_idx;
int8_t fp_offset = sframe_decoder_get_fixed_fp_offset (dctx);
/* If the FP offset is not being tracked, return the fixed FP offset
from the SFrame header. */
@@ -700,13 +722,13 @@ sframe_fre_get_fp_offset (sframe_decoder_ctx *dctx,
return fp_offset;
}
- /* In some ABIs, the stack offset to recover RA (using the CFA) from is
- fixed (like AMD64). In such cases, the stack offset to recover FP will
- appear at the second index. */
- fp_offset_idx = ((sframe_decoder_get_fixed_ra_offset (dctx)
- != SFRAME_CFA_FIXED_RA_INVALID)
- ? SFRAME_FRE_RA_OFFSET_IDX
- : SFRAME_FRE_FP_OFFSET_IDX);
+ /* Get the FP offset index and check that it is valid. */
+ fp_offset_idx = sframe_get_fre_offset_index (fre->fre_info,
+ SFRAME_FRE_FP_OFFSET_IDX);
+ if (fp_offset_idx == -1)
+ return sframe_set_errno (errp, SFRAME_ERR_FREOFFSET_NOPRESENT);
+
+ /* Otherwise, get the FP offset from the FRE. */
return sframe_get_fre_offset (fre, fp_offset_idx, errp);
}
@@ -716,6 +738,7 @@ int32_t
sframe_fre_get_ra_offset (sframe_decoder_ctx *dctx,
sframe_frame_row_entry *fre, int *errp)
{
+ int ra_offset_idx;
int8_t ra_offset = sframe_decoder_get_fixed_ra_offset (dctx);
/* If the RA offset was not being tracked, return the fixed RA offset
from the SFrame header. */
@@ -726,8 +749,14 @@ sframe_fre_get_ra_offset (sframe_decoder_ctx *dctx,
return ra_offset;
}
+ /* Get the RA offset index and check that it is valid. */
+ ra_offset_idx = sframe_get_fre_offset_index (fre->fre_info,
+ SFRAME_FRE_RA_OFFSET_IDX);
+ if (ra_offset_idx == -1)
+ return sframe_set_errno (errp, SFRAME_ERR_FREOFFSET_NOPRESENT);
+
/* Otherwise, get the RA offset from the FRE. */
- return sframe_get_fre_offset (fre, SFRAME_FRE_RA_OFFSET_IDX, errp);
+ return sframe_get_fre_offset (fre, ra_offset_idx, errp);
}
/* Get whether the RA is mangled. */
--
2.40.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-25 6:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-22 15:58 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (padding) Jens Remus
2024-04-22 15:58 ` [RFC PATCH 1/1] sframe: Represent FP without RA on stack Jens Remus
2024-04-22 23:58 ` Indu Bhagat
2024-04-23 15:44 ` Jens Remus
2024-04-25 6:59 ` Indu Bhagat
2024-04-22 15:59 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (bitmap) Jens Remus
2024-04-22 15:59 ` [RFC PATCH 1/1] sframe: Represent FP without RA on stack Jens Remus
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).