[v3,02/15] gas: Enhance arch-specific SFrame configuration descriptions
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Testing passed
|
Commit Message
Explicitly mention "SFrame" in the descriptions for the architecture-
specific SFrame configuration macros, variables, and functions.
Use the term "frame pointer" (FP) instead of "base pointer". This aligns
with the terminology used in the SFrame specification. Additionally it
helps not to confuse "base-pointer register" with the term "BASE_REG"
used in the specification to denote either the SP or FP register.
Specify what the SFRAME_CFA_*_REG register numbers are used for:
- SP (stack pointer): CFA tracking
- FP (frame pointer): CFA and FP tracking
- RA (return address): RA tracking
Align the descriptions for definitions in the source files to the
declarations in the header files.
gas/
* config/tc-aarch64.h: Enhance architecture-specific SFrame
configuration descriptions.
* config/tc-aarch64.c: Likewise.
* config/tc-i386.h: Likewise.
* config/tc-i386.c: Likewise.
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
Notes (jremus):
Changes v2 -> v3:
- Add "SFrame" to architecture-specific SFrame macro, variable, and
function descriptions as suggested by Indu. Do so for all and not
only those previously touched.
- Reword further SFrame macro, variable, and function descriptions
to align with those previously touched.
- Align description of definition in source with declaration in header.
- Corrected formatting of ChangeLog in commit message.
- Changed commit subject prefix from "sframe" to "gas".
gas/config/tc-aarch64.c | 6 +++---
gas/config/tc-aarch64.h | 12 ++++++------
gas/config/tc-i386.c | 5 +++++
gas/config/tc-i386.h | 10 +++++-----
4 files changed, 19 insertions(+), 14 deletions(-)
Comments
On 4/12/24 07:47, Jens Remus wrote:
> Explicitly mention "SFrame" in the descriptions for the architecture-
> specific SFrame configuration macros, variables, and functions.
>
> Use the term "frame pointer" (FP) instead of "base pointer". This aligns
> with the terminology used in the SFrame specification. Additionally it
> helps not to confuse "base-pointer register" with the term "BASE_REG"
> used in the specification to denote either the SP or FP register.
>
> Specify what the SFRAME_CFA_*_REG register numbers are used for:
> - SP (stack pointer): CFA tracking
> - FP (frame pointer): CFA and FP tracking
> - RA (return address): RA tracking
>
> Align the descriptions for definitions in the source files to the
> declarations in the header files.
>
> gas/
> * config/tc-aarch64.h: Enhance architecture-specific SFrame
> configuration descriptions.
> * config/tc-aarch64.c: Likewise.
> * config/tc-i386.h: Likewise.
> * config/tc-i386.c: Likewise.
>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>
> Notes (jremus):
> Changes v2 -> v3:
> - Add "SFrame" to architecture-specific SFrame macro, variable, and
> function descriptions as suggested by Indu. Do so for all and not
> only those previously touched.
> - Reword further SFrame macro, variable, and function descriptions
> to align with those previously touched.
> - Align description of definition in source with declaration in header.
> - Corrected formatting of ChangeLog in commit message.
> - Changed commit subject prefix from "sframe" to "gas".
>
> gas/config/tc-aarch64.c | 6 +++---
> gas/config/tc-aarch64.h | 12 ++++++------
> gas/config/tc-i386.c | 5 +++++
> gas/config/tc-i386.h | 10 +++++-----
> 4 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
> index 6ad4fae8b0ec..077cbd485979 100644
> --- a/gas/config/tc-aarch64.c
> +++ b/gas/config/tc-aarch64.c
> @@ -8870,7 +8870,7 @@ aarch64_support_sframe_p (void)
> return (aarch64_abi == AARCH64_ABI_LP64);
> }
>
> -/* Specify if RA tracking is needed. */
> +/* Whether SFrame return-address tracking is needed. */
>
Nit. But then given the nature of this commit: I see that this commit
uses both "return-address" and "return address" keywords.
Also, the code already was already using "stack-pointer" and
"frame-pointer" (my bad), lets use the same style, and use
- "frame pointer"
- "stack pointer"
- "return address"
OK with that change.
Thanks for your patch.
> bool
> aarch64_sframe_ra_tracking_p (void)
> @@ -8878,8 +8878,8 @@ aarch64_sframe_ra_tracking_p (void)
> return true;
> }
>
> -/* Specify the fixed offset to recover RA from CFA.
> - (useful only when RA tracking is not needed). */
> +/* The fixed offset from CFA for SFrame to recover the return address.
> + (useful only when SFrame RA tracking is not needed). */
>
> offsetT
> aarch64_sframe_cfa_ra_offset (void)
> diff --git a/gas/config/tc-aarch64.h b/gas/config/tc-aarch64.h
> index 1b8badad9fdc..63acbd46a379 100644
> --- a/gas/config/tc-aarch64.h
> +++ b/gas/config/tc-aarch64.h
> @@ -267,24 +267,24 @@ extern void aarch64_after_parse_args (void);
> extern bool aarch64_support_sframe_p (void);
> #define support_sframe_p aarch64_support_sframe_p
>
> -/* The stack-pointer register number for SFrame stack trace info. */
> +/* The stack-pointer DWARF register number for SFrame CFA tracking. */
> extern unsigned int aarch64_sframe_cfa_sp_reg;
> #define SFRAME_CFA_SP_REG aarch64_sframe_cfa_sp_reg
>
> -/* The base-pointer register number for CFA stack trace info. */
> +/* The frame-pointer DWARF register number for SFrame CFA and FP tracking. */
> extern unsigned int aarch64_sframe_cfa_fp_reg;
> #define SFRAME_CFA_FP_REG aarch64_sframe_cfa_fp_reg
>
> -/* The return address register number for CFA stack trace info. */
> +/* The return-address DWARF register number for SFrame RA tracking. */
> extern unsigned int aarch64_sframe_cfa_ra_reg;
> #define SFRAME_CFA_RA_REG aarch64_sframe_cfa_ra_reg
>
> -/* Specify if RA tracking is needed. */
> +/* Whether SFrame return-address tracking is needed. */
> extern bool aarch64_sframe_ra_tracking_p (void);
> #define sframe_ra_tracking_p aarch64_sframe_ra_tracking_p
>
> -/* Specify the fixed offset to recover RA from CFA.
> - (useful only when RA tracking is not needed). */
> +/* The fixed offset from CFA for SFrame to recover the return address.
> + (useful only when SFrame RA tracking is not needed). */
> extern offsetT aarch64_sframe_cfa_ra_offset (void);
> #define sframe_cfa_ra_offset aarch64_sframe_cfa_ra_offset
>
> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> index cd20e0c9415e..08a8eb2aeb69 100644
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -11181,6 +11181,7 @@ x86_cleanup (void)
> subseg_set (seg, subseg);
> }
>
> +/* Whether SFrame stack trace info is supported. */
> bool
> x86_support_sframe_p (void)
> {
> @@ -11188,6 +11189,7 @@ x86_support_sframe_p (void)
> return (x86_elf_abi == X86_64_ABI);
> }
>
> +/* Whether SFrame return-address tracking is needed. */
> bool
> x86_sframe_ra_tracking_p (void)
> {
> @@ -11197,6 +11199,8 @@ x86_sframe_ra_tracking_p (void)
> return false;
> }
>
> +/* The fixed offset from CFA for SFrame to recover the return address.
> + (useful only when SFrame RA tracking is not needed). */
> offsetT
> x86_sframe_cfa_ra_offset (void)
> {
> @@ -11204,6 +11208,7 @@ x86_sframe_cfa_ra_offset (void)
> return (offsetT) -8;
> }
>
> +/* The abi/arch indentifier for SFrame. */
> unsigned char
> x86_sframe_get_abi_arch (void)
> {
> diff --git a/gas/config/tc-i386.h b/gas/config/tc-i386.h
> index 7aae7a33dc14..e98362111698 100644
> --- a/gas/config/tc-i386.h
> +++ b/gas/config/tc-i386.h
> @@ -441,20 +441,20 @@ extern bool x86_scfi_callee_saved_p (uint32_t dw2reg_num);
> extern bool x86_support_sframe_p (void);
> #define support_sframe_p x86_support_sframe_p
>
> -/* The stack-pointer register number for SFrame stack trace info. */
> +/* The stack-pointer DWARF register number for SFrame CFA tracking. */
> extern unsigned int x86_sframe_cfa_sp_reg;
> #define SFRAME_CFA_SP_REG x86_sframe_cfa_sp_reg
>
> -/* The frame-pointer register number for SFrame stack trace info. */
> +/* The frame-pointer DWARF register number for SFrame CFA and FP tracking. */
> extern unsigned int x86_sframe_cfa_fp_reg;
> #define SFRAME_CFA_FP_REG x86_sframe_cfa_fp_reg
>
> -/* Specify if RA tracking is needed. */
> +/* Whether SFrame return-address tracking is needed. */
> extern bool x86_sframe_ra_tracking_p (void);
> #define sframe_ra_tracking_p x86_sframe_ra_tracking_p
>
> -/* Specify the fixed offset to recover RA from CFA.
> - (useful only when RA tracking is not needed). */
> +/* The fixed offset from CFA for SFrame to recover the return address.
> + (useful only when SFrame RA tracking is not needed). */
> extern offsetT x86_sframe_cfa_ra_offset (void);
> #define sframe_cfa_ra_offset x86_sframe_cfa_ra_offset
>
Am 18.04.2024 um 09:39 schrieb Indu Bhagat:
> On 4/12/24 07:47, Jens Remus wrote:
>> Explicitly mention "SFrame" in the descriptions for the architecture-
>> specific SFrame configuration macros, variables, and functions.
>>
>> Use the term "frame pointer" (FP) instead of "base pointer". This aligns
>> with the terminology used in the SFrame specification. Additionally it
>> helps not to confuse "base-pointer register" with the term "BASE_REG"
>> used in the specification to denote either the SP or FP register.
>>
>> Specify what the SFRAME_CFA_*_REG register numbers are used for:
>> - SP (stack pointer): CFA tracking
>> - FP (frame pointer): CFA and FP tracking
>> - RA (return address): RA tracking
>>
>> Align the descriptions for definitions in the source files to the
>> declarations in the header files.
>>
>> gas/
>> * config/tc-aarch64.h: Enhance architecture-specific SFrame
>> configuration descriptions.
>> * config/tc-aarch64.c: Likewise.
>> * config/tc-i386.h: Likewise.
>> * config/tc-i386.c: Likewise.
>>
>> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
>> ---
>>
>> Notes (jremus):
>> Changes v2 -> v3:
>> - Add "SFrame" to architecture-specific SFrame macro, variable, and
>> function descriptions as suggested by Indu. Do so for all and not
>> only those previously touched.
>> - Reword further SFrame macro, variable, and function descriptions
>> to align with those previously touched.
>> - Align description of definition in source with declaration in
>> header.
>> - Corrected formatting of ChangeLog in commit message.
>> - Changed commit subject prefix from "sframe" to "gas".
>>
>> gas/config/tc-aarch64.c | 6 +++---
>> gas/config/tc-aarch64.h | 12 ++++++------
>> gas/config/tc-i386.c | 5 +++++
>> gas/config/tc-i386.h | 10 +++++-----
>> 4 files changed, 19 insertions(+), 14 deletions(-)
>>
>> diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
>> index 6ad4fae8b0ec..077cbd485979 100644
>> --- a/gas/config/tc-aarch64.c
>> +++ b/gas/config/tc-aarch64.c
>> @@ -8870,7 +8870,7 @@ aarch64_support_sframe_p (void)
>> return (aarch64_abi == AARCH64_ABI_LP64);
>> }
>> -/* Specify if RA tracking is needed. */
>> +/* Whether SFrame return-address tracking is needed. */
>
> Nit. But then given the nature of this commit: I see that this commit
> uses both "return-address" and "return address" keywords.
I wrongly assumed them to be compound adjectives [1], which usually
require a dash, when used in conjunction with e.g. "register". That is
why I differentiated between e.g. "stack pointer" (noun) and
"stack-pointer register" (falsely assumed "stack-pointer" to be a
compound adjective). I just learned that they are compound nouns [2] and
thus are always written without dash.
[1] https://www.grammarly.com/blog/compound-adjectives/
[2] https://www.grammarly.com/blog/compound-nouns/
>
> Also, the code already was already using "stack-pointer" and
> "frame-pointer" (my bad), lets use the same style, and use
> - "frame pointer"
> - "stack pointer"
> - "return address"
>
> OK with that change.
>
> Thanks for your patch.
Done.
>
>> bool
>> aarch64_sframe_ra_tracking_p (void)
>> @@ -8878,8 +8878,8 @@ aarch64_sframe_ra_tracking_p (void)
>> return true;
>> }
>> -/* Specify the fixed offset to recover RA from CFA.
>> - (useful only when RA tracking is not needed). */
>> +/* The fixed offset from CFA for SFrame to recover the return address.
>> + (useful only when SFrame RA tracking is not needed). */
>> offsetT
>> aarch64_sframe_cfa_ra_offset (void)
>> diff --git a/gas/config/tc-aarch64.h b/gas/config/tc-aarch64.h
>> index 1b8badad9fdc..63acbd46a379 100644
>> --- a/gas/config/tc-aarch64.h
>> +++ b/gas/config/tc-aarch64.h
>> @@ -267,24 +267,24 @@ extern void aarch64_after_parse_args (void);
>> extern bool aarch64_support_sframe_p (void);
>> #define support_sframe_p aarch64_support_sframe_p
>> -/* The stack-pointer register number for SFrame stack trace info. */
>> +/* The stack-pointer DWARF register number for SFrame CFA tracking. */
>> extern unsigned int aarch64_sframe_cfa_sp_reg;
>> #define SFRAME_CFA_SP_REG aarch64_sframe_cfa_sp_reg
>> -/* The base-pointer register number for CFA stack trace info. */
>> +/* The frame-pointer DWARF register number for SFrame CFA and FP
>> tracking. */
>> extern unsigned int aarch64_sframe_cfa_fp_reg;
>> #define SFRAME_CFA_FP_REG aarch64_sframe_cfa_fp_reg
>> -/* The return address register number for CFA stack trace info. */
>> +/* The return-address DWARF register number for SFrame RA tracking. */
>> extern unsigned int aarch64_sframe_cfa_ra_reg;
>> #define SFRAME_CFA_RA_REG aarch64_sframe_cfa_ra_reg
>> -/* Specify if RA tracking is needed. */
>> +/* Whether SFrame return-address tracking is needed. */
>> extern bool aarch64_sframe_ra_tracking_p (void);
>> #define sframe_ra_tracking_p aarch64_sframe_ra_tracking_p
>> -/* Specify the fixed offset to recover RA from CFA.
>> - (useful only when RA tracking is not needed). */
>> +/* The fixed offset from CFA for SFrame to recover the return address.
>> + (useful only when SFrame RA tracking is not needed). */
>> extern offsetT aarch64_sframe_cfa_ra_offset (void);
>> #define sframe_cfa_ra_offset aarch64_sframe_cfa_ra_offset
>> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
>> index cd20e0c9415e..08a8eb2aeb69 100644
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -11181,6 +11181,7 @@ x86_cleanup (void)
>> subseg_set (seg, subseg);
>> }
>> +/* Whether SFrame stack trace info is supported. */
>> bool
>> x86_support_sframe_p (void)
>> {
>> @@ -11188,6 +11189,7 @@ x86_support_sframe_p (void)
>> return (x86_elf_abi == X86_64_ABI);
>> }
>> +/* Whether SFrame return-address tracking is needed. */
>> bool
>> x86_sframe_ra_tracking_p (void)
>> {
>> @@ -11197,6 +11199,8 @@ x86_sframe_ra_tracking_p (void)
>> return false;
>> }
>> +/* The fixed offset from CFA for SFrame to recover the return address.
>> + (useful only when SFrame RA tracking is not needed). */
>> offsetT
>> x86_sframe_cfa_ra_offset (void)
>> {
>> @@ -11204,6 +11208,7 @@ x86_sframe_cfa_ra_offset (void)
>> return (offsetT) -8;
>> }
>> +/* The abi/arch indentifier for SFrame. */
>> unsigned char
>> x86_sframe_get_abi_arch (void)
>> {
>> diff --git a/gas/config/tc-i386.h b/gas/config/tc-i386.h
>> index 7aae7a33dc14..e98362111698 100644
>> --- a/gas/config/tc-i386.h
>> +++ b/gas/config/tc-i386.h
>> @@ -441,20 +441,20 @@ extern bool x86_scfi_callee_saved_p (uint32_t
>> dw2reg_num);
>> extern bool x86_support_sframe_p (void);
>> #define support_sframe_p x86_support_sframe_p
>> -/* The stack-pointer register number for SFrame stack trace info. */
>> +/* The stack-pointer DWARF register number for SFrame CFA tracking. */
>> extern unsigned int x86_sframe_cfa_sp_reg;
>> #define SFRAME_CFA_SP_REG x86_sframe_cfa_sp_reg
>> -/* The frame-pointer register number for SFrame stack trace info. */
>> +/* The frame-pointer DWARF register number for SFrame CFA and FP
>> tracking. */
>> extern unsigned int x86_sframe_cfa_fp_reg;
>> #define SFRAME_CFA_FP_REG x86_sframe_cfa_fp_reg
>> -/* Specify if RA tracking is needed. */
>> +/* Whether SFrame return-address tracking is needed. */
>> extern bool x86_sframe_ra_tracking_p (void);
>> #define sframe_ra_tracking_p x86_sframe_ra_tracking_p
>> -/* Specify the fixed offset to recover RA from CFA.
>> - (useful only when RA tracking is not needed). */
>> +/* The fixed offset from CFA for SFrame to recover the return address.
>> + (useful only when SFrame RA tracking is not needed). */
>> extern offsetT x86_sframe_cfa_ra_offset (void);
>> #define sframe_cfa_ra_offset x86_sframe_cfa_ra_offset
>
Regards,
Jens
@@ -8870,7 +8870,7 @@ aarch64_support_sframe_p (void)
return (aarch64_abi == AARCH64_ABI_LP64);
}
-/* Specify if RA tracking is needed. */
+/* Whether SFrame return-address tracking is needed. */
bool
aarch64_sframe_ra_tracking_p (void)
@@ -8878,8 +8878,8 @@ aarch64_sframe_ra_tracking_p (void)
return true;
}
-/* Specify the fixed offset to recover RA from CFA.
- (useful only when RA tracking is not needed). */
+/* The fixed offset from CFA for SFrame to recover the return address.
+ (useful only when SFrame RA tracking is not needed). */
offsetT
aarch64_sframe_cfa_ra_offset (void)
@@ -267,24 +267,24 @@ extern void aarch64_after_parse_args (void);
extern bool aarch64_support_sframe_p (void);
#define support_sframe_p aarch64_support_sframe_p
-/* The stack-pointer register number for SFrame stack trace info. */
+/* The stack-pointer DWARF register number for SFrame CFA tracking. */
extern unsigned int aarch64_sframe_cfa_sp_reg;
#define SFRAME_CFA_SP_REG aarch64_sframe_cfa_sp_reg
-/* The base-pointer register number for CFA stack trace info. */
+/* The frame-pointer DWARF register number for SFrame CFA and FP tracking. */
extern unsigned int aarch64_sframe_cfa_fp_reg;
#define SFRAME_CFA_FP_REG aarch64_sframe_cfa_fp_reg
-/* The return address register number for CFA stack trace info. */
+/* The return-address DWARF register number for SFrame RA tracking. */
extern unsigned int aarch64_sframe_cfa_ra_reg;
#define SFRAME_CFA_RA_REG aarch64_sframe_cfa_ra_reg
-/* Specify if RA tracking is needed. */
+/* Whether SFrame return-address tracking is needed. */
extern bool aarch64_sframe_ra_tracking_p (void);
#define sframe_ra_tracking_p aarch64_sframe_ra_tracking_p
-/* Specify the fixed offset to recover RA from CFA.
- (useful only when RA tracking is not needed). */
+/* The fixed offset from CFA for SFrame to recover the return address.
+ (useful only when SFrame RA tracking is not needed). */
extern offsetT aarch64_sframe_cfa_ra_offset (void);
#define sframe_cfa_ra_offset aarch64_sframe_cfa_ra_offset
@@ -11181,6 +11181,7 @@ x86_cleanup (void)
subseg_set (seg, subseg);
}
+/* Whether SFrame stack trace info is supported. */
bool
x86_support_sframe_p (void)
{
@@ -11188,6 +11189,7 @@ x86_support_sframe_p (void)
return (x86_elf_abi == X86_64_ABI);
}
+/* Whether SFrame return-address tracking is needed. */
bool
x86_sframe_ra_tracking_p (void)
{
@@ -11197,6 +11199,8 @@ x86_sframe_ra_tracking_p (void)
return false;
}
+/* The fixed offset from CFA for SFrame to recover the return address.
+ (useful only when SFrame RA tracking is not needed). */
offsetT
x86_sframe_cfa_ra_offset (void)
{
@@ -11204,6 +11208,7 @@ x86_sframe_cfa_ra_offset (void)
return (offsetT) -8;
}
+/* The abi/arch indentifier for SFrame. */
unsigned char
x86_sframe_get_abi_arch (void)
{
@@ -441,20 +441,20 @@ extern bool x86_scfi_callee_saved_p (uint32_t dw2reg_num);
extern bool x86_support_sframe_p (void);
#define support_sframe_p x86_support_sframe_p
-/* The stack-pointer register number for SFrame stack trace info. */
+/* The stack-pointer DWARF register number for SFrame CFA tracking. */
extern unsigned int x86_sframe_cfa_sp_reg;
#define SFRAME_CFA_SP_REG x86_sframe_cfa_sp_reg
-/* The frame-pointer register number for SFrame stack trace info. */
+/* The frame-pointer DWARF register number for SFrame CFA and FP tracking. */
extern unsigned int x86_sframe_cfa_fp_reg;
#define SFRAME_CFA_FP_REG x86_sframe_cfa_fp_reg
-/* Specify if RA tracking is needed. */
+/* Whether SFrame return-address tracking is needed. */
extern bool x86_sframe_ra_tracking_p (void);
#define sframe_ra_tracking_p x86_sframe_ra_tracking_p
-/* Specify the fixed offset to recover RA from CFA.
- (useful only when RA tracking is not needed). */
+/* The fixed offset from CFA for SFrame to recover the return address.
+ (useful only when SFrame RA tracking is not needed). */
extern offsetT x86_sframe_cfa_ra_offset (void);
#define sframe_cfa_ra_offset x86_sframe_cfa_ra_offset