powerpc: Use sysconf (_SC_SIGSTKSZ) to set SIGSTKSZ and MINSIGSTKSZ.

Message ID 20230516111140.2585699-1-mmatti@linux.ibm.com
State New
Headers
Series powerpc: Use sysconf (_SC_SIGSTKSZ) to set SIGSTKSZ and MINSIGSTKSZ. |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Manjunath Matti May 16, 2023, 11:11 a.m. UTC
  Add support in PowerPC to use sysconf (_SC_SIGSTKSZ) to set SIGSTKSZ
and MINSIGSTKSZ similar to x86. The minimum signal stack size derived
from AT_MINSIGSTKSZ, which is the minimum number of bytes of free stack
space required in order to gurantee successful, non-nested handling
of a single signal whose handler is an empty function.

The kernel commit 2896b2dff49d0377e4372f470dcddbcb26f2be59 implements
AT_MINSIGSTKSZ AUXV entry, allowing userspace to dynamically size
stack allocations.
---
 .../unix/sysv/linux/powerpc/bits/sigstksz.h   | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
  

Comments

Florian Weimer May 24, 2023, 7:08 p.m. UTC | #1
* Manjunath Matti via Libc-alpha:

> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h b/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
> new file mode 100644
> index 0000000000..399a49c1b0
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
> @@ -0,0 +1,33 @@

> +#ifndef _SIGNAL_H
> +# error "Never include <bits/sigstksz.h> directly; use <signal.h> instead."
> +#endif
> +
> +#if defined __USE_DYNAMIC_STACK_SIZE && __USE_DYNAMIC_STACK_SIZE
> +# include <unistd.h>
> +
> +/* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ).  */
> +# undef SIGSTKSZ
> +# define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
> +
> +/* Minimum stack size for a signal handler: SIGSTKSZ/4.  */
> +# undef MINSIGSTKSZ
> +# define MINSIGSTKSZ (SIGSTKSZ >> 2)
> +#endif

I'm not sure if it is a good idea to hard-code that constant 4 here.
Similarly, it's not great to encode the constant 1 in the default case.

This is a subtle portability hazard because with these changes,
MINSIGSTKSZ is not enough on POWER to do anything on the signal stack
(because it's exactly the kernel supplied value).  On other
architectures, it's at least possible to do some minor stuff and call
e.g. siglongjmp (at least if lazy binding is not required).

The factor 4 in the implementation is somewhat x86-64-specific because
it accounts for the fact that we do a signal-like context switch
(pushing XSAVE data on the stack) in the lazy binding trampoline.

Thanks,
Florian
  
Manjunath S Matti June 23, 2023, 12:01 p.m. UTC | #2
On 25/05/23 12:38 am, Florian Weimer wrote:
> * Manjunath Matti via Libc-alpha:
>
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h b/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
>> new file mode 100644
>> index 0000000000..399a49c1b0
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
>> @@ -0,0 +1,33 @@
>> +#ifndef _SIGNAL_H
>> +# error "Never include <bits/sigstksz.h> directly; use <signal.h> instead."
>> +#endif
>> +
>> +#if defined __USE_DYNAMIC_STACK_SIZE && __USE_DYNAMIC_STACK_SIZE
>> +# include <unistd.h>
>> +
>> +/* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ).  */
>> +# undef SIGSTKSZ
>> +# define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
>> +
>> +/* Minimum stack size for a signal handler: SIGSTKSZ/4.  */
>> +# undef MINSIGSTKSZ
>> +# define MINSIGSTKSZ (SIGSTKSZ >> 2)
>> +#endif
> I'm not sure if it is a good idea to hard-code that constant 4 here.
> Similarly, it's not great to encode the constant 1 in the default case.
>
> This is a subtle portability hazard because with these changes,
> MINSIGSTKSZ is not enough on POWER to do anything on the signal stack
> (because it's exactly the kernel supplied value).  On other
> architectures, it's at least possible to do some minor stuff and call
> e.g. siglongjmp (at least if lazy binding is not required).

After deliberate thinking, internal discussions and the below comment by 
H J Lu.

https://patchwork.sourceware.org/project/glibc/patch/20230424105208.301614-1-mmatti@linux.ibm.com/#144525

I think we can drop this patch.

> The factor 4 in the implementation is somewhat x86-64-specific because
> it accounts for the fact that we do a signal-like context switch
> (pushing XSAVE data on the stack) in the lazy binding trampoline.
>
> Thanks,
> Florian
>
Thank you Florian for your time and support.

Regards,

Manjunath S Matti.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h b/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
new file mode 100644
index 0000000000..399a49c1b0
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h
@@ -0,0 +1,33 @@ 
+/* Definition of MINSIGSTKSZ and SIGSTKSZ.  Linux/PowerPC version.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SIGNAL_H
+# error "Never include <bits/sigstksz.h> directly; use <signal.h> instead."
+#endif
+
+#if defined __USE_DYNAMIC_STACK_SIZE && __USE_DYNAMIC_STACK_SIZE
+# include <unistd.h>
+
+/* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ).  */
+# undef SIGSTKSZ
+# define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
+
+/* Minimum stack size for a signal handler: SIGSTKSZ/4.  */
+# undef MINSIGSTKSZ
+# define MINSIGSTKSZ (SIGSTKSZ >> 2)
+#endif