diff mbox series

powerpc64le: The ABI requires stack frames to be 16-byte

Message ID 20241025172648.3634324-1-smonga@linux.ibm.com
State New
Headers show
Series powerpc64le: The ABI requires stack frames to be 16-byte | expand

Commit Message

Sachin Monga Oct. 25, 2024, 5:26 p.m. UTC
From: Sachin Monga <sachin.monga@ibm.com>

While creating stack frame for a call to memset,
the 16B padding alignment is missing.

Signed-off-by: Sachin Monga <sachin.monga@ibm.com>
---
 sysdeps/powerpc/powerpc64/le/power9/strncpy.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peter Bergner Oct. 25, 2024, 10:49 p.m. UTC | #1
On 10/25/24 12:26 PM, Sachin Monga wrote:
> From: Sachin Monga <sachin.monga@ibm.com>
> 
> While creating stack frame for a call to memset,
> the 16B padding alignment is missing.
> 
> Signed-off-by: Sachin Monga <sachin.monga@ibm.com>
> ---
>  sysdeps/powerpc/powerpc64/le/power9/strncpy.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
> index eccb2ffbb0..58139ad9e8 100644
> --- a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
> +++ b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
> @@ -43,7 +43,7 @@
>  # endif
>  #endif
>  
> -#define FRAMESIZE (FRAME_MIN_SIZE+8)
> +#define FRAMESIZE (FRAME_MIN_SIZE+16)
>  
>  /* Implements the function

This was built and regression tested, correct?  Proper patch submissions
should state where and how the patch was tested and that no regressions
were seen.

That said, the patch LGTM, but the subject line looks truncated as it
should say "aligned" at the end...and that would be better as the git
log entry.

I'll fix that up along with the Subject line and merge it once I've
heard from you that you did build and test the patch with no errors.


Reviewed-by: Peter Bergner <bergner@linux.ibm.com>

Peter
Sachin Monga Oct. 28, 2024, 6:20 a.m. UTC | #2
> This was built and regression tested, correct?  Proper patch submissions
> should state where and how the patch was tested and that no regressions
> were seen.

Yes. It was built and tested for regression with no errors. Generally, I 
send patch through "git send-email" so not sure if we use "--annotate" 
for this or simply edit the patch below commit message's ---

Will check.

> That said, the patch LGTM, but the subject line looks truncated as it
> should say "aligned" at the end...and that would be better as the git
> log entry.
>
> I'll fix that up along with the Subject line and merge it once I've
> heard from you that you did build and test the patch with no errors.
I am re-sharing it here as attachment.
From 2c8698feb66e947af7d326c1dde294f97e36ddf2 Mon Sep 17 00:00:00 2001
From: Sachin Monga <sachin.monga@ibm.com>
Date: Fri, 25 Oct 2024 01:44:46 -0500
Subject: [PATCH] powerpc64le: Adhering to stack alignment ABI requirement.

The ABI requires stack frames to be 16-byte aligned.

Signed-off-by: Sachin Monga <sachin.monga@ibm.com>
---
 sysdeps/powerpc/powerpc64/le/power9/strncpy.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
index eccb2ffbb0..58139ad9e8 100644
--- a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
+++ b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
@@ -43,7 +43,7 @@
 # endif
 #endif
 
-#define FRAMESIZE (FRAME_MIN_SIZE+8)
+#define FRAMESIZE (FRAME_MIN_SIZE+16)
 
 /* Implements the function
Florian Weimer Oct. 28, 2024, 8:52 a.m. UTC | #3
* Sachin Monga:

> From: Sachin Monga <sachin.monga@ibm.com>
>
> While creating stack frame for a call to memset,
> the 16B padding alignment is missing.

Is this really an ABI requirement for local functions with known stack
alignment requirements?  Is this something signal handlers rely on?

Thanks,
Florian
Peter Bergner Oct. 28, 2024, 1:39 p.m. UTC | #4
On 10/28/24 3:52 AM, Florian Weimer wrote:
> Is this really an ABI requirement for local functions with known stack
> alignment requirements?  Is this something signal handlers rely on?

From the ELFv2 ABI:

  The following general requirements apply to all stack frames:
    • The stack shall be quadword aligned.
    ...

So yes, all functions can rely on their stack frames being 16-byte
aligned, including signal handlers.

The stack frame being required to be 16-byte aligned is also true
for the ELFv1 and 32-bit ABIs as well.

Peter
Peter Bergner Oct. 28, 2024, 9:55 p.m. UTC | #5
On 10/25/24 5:49 PM, Peter Bergner wrote:
> That said, the patch LGTM, but the subject line looks truncated as it
> should say "aligned" at the end...and that would be better as the git
> log entry.
> 
> I'll fix that up along with the Subject line and merge it once I've
> heard from you that you did build and test the patch with no errors.

Pushed now with those updates.

Peter
diff mbox series

Patch

diff --git a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
index eccb2ffbb0..58139ad9e8 100644
--- a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
+++ b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
@@ -43,7 +43,7 @@ 
 # endif
 #endif
 
-#define FRAMESIZE (FRAME_MIN_SIZE+8)
+#define FRAMESIZE (FRAME_MIN_SIZE+16)
 
 /* Implements the function