public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* Re: SIGSTKSZ is now a run-time variable
       [not found] <1841269.IEpri3ZHvQ@omega>
@ 2021-03-09 15:23 ` Eric Blake
  2021-03-09 15:26   ` Andreas Schwab
  2021-03-09 15:46   ` Zack Weinberg
  0 siblings, 2 replies; 14+ messages in thread
From: Eric Blake @ 2021-03-09 15:23 UTC (permalink / raw)
  To: Bruno Haible, Carol Bouchard, bug-m4, glibc list, austin-group-l

[adding glibc and Austin group lists]

On 3/6/21 12:50 PM, Bruno Haible wrote:
> Hi,
> 
> Carol Bouchard wrote in <https://lists.gnu.org/archive/html/bug-m4/2021-03/msg00000.html>:
>> A change that was introduced is the
>> #define SIGSTKSZ is no longer a statically defined variable.  It's value can
>> only be determined at run time.
>>
>> # define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
> 
> This is invalid. POSIX:2018 [1] defines two lists of macros:
> 
>   1) "The <signal.h> header shall define the following macros which shall
>       expand to integer constant expressions that need not be usable in
>       #if preprocessing directives:"
> 
>   2) "The <signal.h> header shall also define the following symbolic constants:"
> 
> SIGSTKSZ is in the second list. This implies that it must expand to a constant
> and that it must be usable in #if preprocessing directives.

The question becomes whether glibc is in violation of POSIX for having
made the change, or whether POSIX needs to be amended to allow SIGSTKSZ
to be non-preprocessor-safe and/or non-constant.

> 
> Besides being invalid, it is also not needed. The alternate signal stack
> needs to be dimensioned according to the CPU and ABI that is in use. For example,
> SPARC processors tend to use much more stack space than x86 per function
> invocation. Similarly, 64-bit execution on a bi-arch CPU tends to use more stack
> space than 32-bit execution, because return addresses and other pointers are
> 64-bit vs. 32-bit large. But once you have fixed the CPU and the ABI, there is
> no ambiguity any more.
> 
>> This affects m4 code since the code assumes a statically defined variable which
>> can be determined at preprocessor time.
> 
> POSIX guarantees this assumption.
> 
>> Please advise how I can get past this.
> 
> Fix your <signal.h>.

https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=6c57d320484988e87e446e2e60ce42816bf51d53
shows where glibc made the change, and I've now seen reports of several
projects failing to build when using glibc with this change included.

> 
> Bruno
> 
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 15:23 ` SIGSTKSZ is now a run-time variable Eric Blake
@ 2021-03-09 15:26   ` Andreas Schwab
  2021-03-09 15:48     ` Eric Blake
  2021-03-09 15:46   ` Zack Weinberg
  1 sibling, 1 reply; 14+ messages in thread
From: Andreas Schwab @ 2021-03-09 15:26 UTC (permalink / raw)
  To: Eric Blake via Libc-alpha
  Cc: Bruno Haible, Carol Bouchard, bug-m4, austin-group-l, Eric Blake

On Mär 09 2021, Eric Blake via Libc-alpha wrote:

> The question becomes whether glibc is in violation of POSIX for having
> made the change, or whether POSIX needs to be amended to allow SIGSTKSZ
> to be non-preprocessor-safe and/or non-constant.

POSIX already allows non-preprocessor-safe.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 15:23 ` SIGSTKSZ is now a run-time variable Eric Blake
  2021-03-09 15:26   ` Andreas Schwab
@ 2021-03-09 15:46   ` Zack Weinberg
  1 sibling, 0 replies; 14+ messages in thread
From: Zack Weinberg @ 2021-03-09 15:46 UTC (permalink / raw)
  To: glibc list, H.J. Lu, Florian Weimer
  Cc: Bruno Haible, Carol Bouchard, bug-m4, Eric Blake, austin-group-l

On Tue, Mar 9, 2021 at 10:23 AM Eric Blake via Libc-alpha
<libc-alpha@sourceware.org> wrote:
> [adding glibc and Austin group lists]
> On 3/6/21 12:50 PM, Bruno Haible wrote:
> > Besides being invalid, it is also not needed. The alternate signal stack
> > needs to be dimensioned according to the CPU and ABI that is in use. For example,
> > SPARC processors tend to use much more stack space than x86 per function
> > invocation. Similarly, 64-bit execution on a bi-arch CPU tends to use more stack
> > space than 32-bit execution, because return addresses and other pointers are
> > 64-bit vs. 32-bit large. But once you have fixed the CPU and the ABI, there is
> > no ambiguity any more.

The rationale for this change, from the glibc side, is that various
CPU architectures (notably x86 and ARM, as I understand it) have been
adding extensions that break the above premises.  The amount of space
required by the ucontext_t object (third argument to a SA_SIGINFO
signal handler) depends not only on the ISA and ABI but on the exact
set of ISA extensions that are available on the model of CPU that
happens to be running the program today.  Moreover, some of these
extensions require a great deal of space to store e.g. large SIMD
vector registers -- more than will fit in the fairly small MINSIGSTKSZ
we've historically provided (usually something like 2048 bytes).  And
there isn't anywhere else we / the kernel can save this state.

I was worried about exactly this kind of breakage when the patches
were originally being discussed, but _something_ has to give here and
I don't know what else it could be.   cc:ing H.J. and Florian, who
were more deeply involved.

zw

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 15:26   ` Andreas Schwab
@ 2021-03-09 15:48     ` Eric Blake
  2021-03-09 19:58       ` Bruno Haible
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Blake @ 2021-03-09 15:48 UTC (permalink / raw)
  To: Andreas Schwab, Eric Blake via Libc-alpha
  Cc: Carol Bouchard, Bruno Haible, austin-group-l, bug-m4

On 3/9/21 9:26 AM, Andreas Schwab wrote:
> On Mär 09 2021, Eric Blake via Libc-alpha wrote:
> 
>> The question becomes whether glibc is in violation of POSIX for having
>> made the change, or whether POSIX needs to be amended to allow SIGSTKSZ
>> to be non-preprocessor-safe and/or non-constant.
> 
> POSIX already allows non-preprocessor-safe.

True, but expanding 'SIGSTKSZ' to 'sysconf (_SC_SIGSTKSZ)' is not a
symbolic constant., as it is not "a compile-time constant expression
with an integer type', per definition 3.380.

Looks like this discussion is happening in parallel in:
https://sourceware.org/bugzilla/show_bug.cgi?id=20305

I can open a defect against POSIX if we decide that is needed, but want
some consensus first on whether it is glibc's change that went too far,
or POSIX's requirements that are too restrictive for what glibc wants to do.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 15:48     ` Eric Blake
@ 2021-03-09 19:58       ` Bruno Haible
  2021-03-09 21:05         ` H.J. Lu
       [not found]         ` <20210309211735.GW19922@dragon.sl.home>
  0 siblings, 2 replies; 14+ messages in thread
From: Bruno Haible @ 2021-03-09 19:58 UTC (permalink / raw)
  To: Eric Blake
  Cc: Andreas Schwab, libc-alpha, Carol Bouchard, austin-group-l, bug-m4

Eric Blake wrote:
> I can open a defect against POSIX if we decide that is needed, but want
> some consensus first on whether it is glibc's change that went too far,
> or POSIX's requirements that are too restrictive for what glibc wants to do.

Thanks for opening the discussion, Eric.

Here are a couple of questions, to understand the motivation and the possible
alternative solutions to the problem:

1) As far as I understand, the issue occurs with certain x86 or x86_64
   processors.

   1.1) What has been the value of MINSIGSTKSZ on x86 and x86_64 so far?
   1.2) What value of MINSIGSTKSZ is needed for AVX-512F support?
   1.3) Will the trend to larger MINSIGSTKSZ values continue for Intel
        processors?

2) Regarding the change of the macro MINSIGSTKSZ:
   Would it possible to just change the value of MINSIGSTKSZ to a larger
   constant?

   If there is a fear regarding ABI compatibility between a library and a
   program: How likely is it that a library offers an interface that takes
   a char[MINSIGSTKSZ] as argument, or that defines a variable of type
   char[MINSIGSTKSZ]?

3) Regarding the change of the macro SIGSTKSZ:
   Likewise, would it be possible to just change the value of SIGSTKSZ to a
   larger constant?

4) Since SIGSTKSZ has other uses than MINSIGSTKSZ, has it been considered
   to make MINSIGSTKSZ non-constant but keep SIGSTKSZ constant?

5) POSIX:2018 [1] defines SIGSTKSZ as the stack size for "the usual case".
   So, it should be composed of MINSIGSTKSZ for the initial stack frame,
   plus a certain amount of stack, depending on CPU, ABI, and compiler,
   for doing what a "usual" signal handler would do.

   What is the reason, then, for the computation
     SIGSTKSZ >= 4 * MINSIGSTKSZ
   in [2]? Shouldn't it be something like
     SIGSTKSZ >= MINSIGSTKSZ + (64 KB on SPARC and powerpc, 8 KB on other
                                processors)
   ?

Bruno

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaltstack.html
[2] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/sysconf-sigstksz.h


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 19:58       ` Bruno Haible
@ 2021-03-09 21:05         ` H.J. Lu
       [not found]         ` <20210309211735.GW19922@dragon.sl.home>
  1 sibling, 0 replies; 14+ messages in thread
From: H.J. Lu @ 2021-03-09 21:05 UTC (permalink / raw)
  To: Bruno Haible
  Cc: Eric Blake, GNU C Library, Carol Bouchard, Andreas Schwab,
	bug-m4, austin-group-l

On Tue, Mar 9, 2021 at 12:27 PM Bruno Haible <bruno@clisp.org> wrote:
>
> Eric Blake wrote:
> > I can open a defect against POSIX if we decide that is needed, but want
> > some consensus first on whether it is glibc's change that went too far,
> > or POSIX's requirements that are too restrictive for what glibc wants to do.
>
> Thanks for opening the discussion, Eric.
>
> Here are a couple of questions, to understand the motivation and the possible
> alternative solutions to the problem:
>
> 1) As far as I understand, the issue occurs with certain x86 or x86_64
>    processors.
>
>    1.1) What has been the value of MINSIGSTKSZ on x86 and x86_64 so far?

It is 2048 bytes in glibc 2.32.

>    1.2) What value of MINSIGSTKSZ is needed for AVX-512F support?

For i686, it is 3628 bytes.   For x86-64, it is 3212 bytes.

>    1.3) Will the trend to larger MINSIGSTKSZ values continue for Intel
>         processors?

With 8 1K Tile registers, we need another 8K.   I can't tell you how big
we will need in 10 years.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
       [not found]         ` <20210309211735.GW19922@dragon.sl.home>
@ 2021-03-16 19:46           ` Carol Bouchard
  2021-03-26 12:46             ` Carol Bouchard
  0 siblings, 1 reply; 14+ messages in thread
From: Carol Bouchard @ 2021-03-16 19:46 UTC (permalink / raw)
  To: Scott Lurndal
  Cc: Bruno Haible, Eric Blake, Andreas Schwab, libc-alpha,
	austin-group-l, bug-m4

Folks:

I misstated earlier that the latest Fedora is going in this direction.
This appears to be a glibc only issue as my build is pulling in glibc and
not playing nicely with m4.
According to https://pkgs.org/download/glibc-devel, fedora rawhide is
pulling in the latest version glibc-devel 2.33.
I'm wondering if there is a short-term work-around I can use.

Carol

On Tue, Mar 9, 2021 at 4:30 PM Scott Lurndal <slurndal@sonic.net> wrote:

> On Tue, Mar 09, 2021 at 08:58:38PM +0100, Bruno Haible via austin-group-l
> at The Open Group wrote:
> > Eric Blake wrote:
> > > I can open a defect against POSIX if we decide that is needed, but want
> > > some consensus first on whether it is glibc's change that went too far,
> > > or POSIX's requirements that are too restrictive for what glibc wants
> to do.
> >
> > Thanks for opening the discussion, Eric.
> >
> > Here are a couple of questions, to understand the motivation and the
> possible
> > alternative solutions to the problem:
> >
> > 1) As far as I understand, the issue occurs with certain x86 or x86_64
> >    processors.
> >
> >    1.1) What has been the value of MINSIGSTKSZ on x86 and x86_64 so far?
> >    1.2) What value of MINSIGSTKSZ is needed for AVX-512F support?
> >    1.3) Will the trend to larger MINSIGSTKSZ values continue for Intel
> >         processors?
>
> It's not just Intel processors.
>
> 64-bit ARM processors that support scalable vectors (SVE) support
> vectors of up to 2084 bits, and they have 32 vector registers which would
> require 8Kbytes for the SVE state alone if the implementation supports
> the full 2kbits.
>
> scott
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-16 19:46           ` Carol Bouchard
@ 2021-03-26 12:46             ` Carol Bouchard
  0 siblings, 0 replies; 14+ messages in thread
From: Carol Bouchard @ 2021-03-26 12:46 UTC (permalink / raw)
  To: Scott Lurndal
  Cc: Bruno Haible, Eric Blake, Andreas Schwab, libc-alpha,
	austin-group-l, bug-m4

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

M4 Experts:

Since this issue seems to be in a standstill, I've put together a patch
that I can use locally.
I'm seeking input for this patch I've produced for m4 package.  I'd
appreciate your thoughts.
Carol

On Tue, Mar 16, 2021 at 3:46 PM Carol Bouchard <cbouchar@redhat.com> wrote:

> Folks:
>
> I misstated earlier that the latest Fedora is going in this direction.
> This appears to be a glibc only issue as my build is pulling in glibc and
> not playing nicely with m4.
> According to https://pkgs.org/download/glibc-devel, fedora rawhide is
> pulling in the latest version glibc-devel 2.33.
> I'm wondering if there is a short-term work-around I can use.
>
> Carol
>
> On Tue, Mar 9, 2021 at 4:30 PM Scott Lurndal <slurndal@sonic.net> wrote:
>
>> On Tue, Mar 09, 2021 at 08:58:38PM +0100, Bruno Haible via austin-group-l
>> at The Open Group wrote:
>> > Eric Blake wrote:
>> > > I can open a defect against POSIX if we decide that is needed, but
>> want
>> > > some consensus first on whether it is glibc's change that went too
>> far,
>> > > or POSIX's requirements that are too restrictive for what glibc wants
>> to do.
>> >
>> > Thanks for opening the discussion, Eric.
>> >
>> > Here are a couple of questions, to understand the motivation and the
>> possible
>> > alternative solutions to the problem:
>> >
>> > 1) As far as I understand, the issue occurs with certain x86 or x86_64
>> >    processors.
>> >
>> >    1.1) What has been the value of MINSIGSTKSZ on x86 and x86_64 so far?
>> >    1.2) What value of MINSIGSTKSZ is needed for AVX-512F support?
>> >    1.3) Will the trend to larger MINSIGSTKSZ values continue for Intel
>> >         processors?
>>
>> It's not just Intel processors.
>>
>> 64-bit ARM processors that support scalable vectors (SVE) support
>> vectors of up to 2084 bits, and they have 32 vector registers which would
>> require 8Kbytes for the SVE state alone if the implementation supports
>> the full 2kbits.
>>
>> scott
>>
>>

[-- Attachment #2: m4_glibc_SIGSTKSZ_issue.patch --]
[-- Type: text/x-patch, Size: 2685 bytes --]

diff --git a/third-party/m4-1.4.18-glibc-sigstksz.patch b/third-party/m4-1.4.18-glibc-sigstksz.patch
new file mode 100644
index 0000000..c506603
--- /dev/null
+++ b/third-party/m4-1.4.18-glibc-sigstksz.patch
@@ -0,0 +1,65 @@
+diff --git a/lib/c-stack.c b/lib/c-stack.c
+index 5353c08..863f764 100644
+--- a/lib/c-stack.c
++++ b/lib/c-stack.c
+@@ -51,13 +51,14 @@
+ typedef struct sigaltstack stack_t;
+ #endif
+ #ifndef SIGSTKSZ
+-# define SIGSTKSZ 16384
+-#elif HAVE_LIBSIGSEGV && SIGSTKSZ < 16384
++#define get_sigstksz()  (16384)
++#elif HAVE_LIBSIGSEGV
+ /* libsigsegv 2.6 through 2.8 have a bug where some architectures use
+    more than the Linux default of an 8k alternate stack when deciding
+    if a fault was caused by stack overflow.  */
+-# undef SIGSTKSZ
+-# define SIGSTKSZ 16384
++#define get_sigstksz() ((SIGSTKSZ) < 16384 ? 16384 : (SIGSTKSZ))
++#else
++#define get_sigstksz() ((SIGSTKSZ))
+ #endif
+ 
+ #include <stdlib.h>
+@@ -131,7 +132,8 @@ die (int signo)
+ /* Storage for the alternate signal stack.  */
+ static union
+ {
+-  char buffer[SIGSTKSZ];
++  /* allocate buffer with size from get_sigstksz() */
++  char *buffer;
+ 
+   /* These other members are for proper alignment.  There's no
+      standard way to guarantee stack alignment, but this seems enough
+@@ -203,10 +205,11 @@ c_stack_action (void (*action) (int))
+   program_error_message = _("program error");
+   stack_overflow_message = _("stack overflow");
+ 
++  alternate_signal_stack.buffer = malloc(get_sigstksz());
+   /* Always install the overflow handler.  */
+   if (stackoverflow_install_handler (overflow_handler,
+                                      alternate_signal_stack.buffer,
+-                                     sizeof alternate_signal_stack.buffer))
++                                     get_sigstksz()))
+     {
+       errno = ENOTSUP;
+       return -1;
+@@ -279,14 +282,15 @@ c_stack_action (void (*action) (int))
+   stack_t st;
+   struct sigaction act;
+   st.ss_flags = 0;
++  alternate_signal_stack.buffer = malloc(get_sigstksz());
+ # if SIGALTSTACK_SS_REVERSED
+   /* Irix mistakenly treats ss_sp as the upper bound, rather than
+      lower bound, of the alternate stack.  */
+-  st.ss_sp = alternate_signal_stack.buffer + SIGSTKSZ - sizeof (void *);
+-  st.ss_size = sizeof alternate_signal_stack.buffer - sizeof (void *);
++  st.ss_sp = alternate_signal_stack.buffer + get_sigstksz() - sizeof (void *);
++  st.ss_size = get_sigstksz() - sizeof (void *);
+ # else
+   st.ss_sp = alternate_signal_stack.buffer;
+-  st.ss_size = sizeof alternate_signal_stack.buffer;
++  st.ss_size = get_sigstksz();
+ # endif
+   r = sigaltstack (&st, NULL);
+   if (r != 0)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 21:29 ` shwaresyst
@ 2021-03-09 23:58   ` H.J. Lu
  0 siblings, 0 replies; 14+ messages in thread
From: H.J. Lu @ 2021-03-09 23:58 UTC (permalink / raw)
  To: shwaresyst
  Cc: Eric Blake, Bruno Haible, Carol Bouchard, bug-m4, GNU C Library,
	austin-group-l

On Tue, Mar 9, 2021 at 1:53 PM shwaresyst via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
> Yes, it's not something an application would expect to need to keep increasing, just that's the part of <limits.h> I'd move it to. The definition could also be the max required by a processor family, with sysconf() reporting a possible lower value for a particular processor stepping. At least that way the application that doesn't use sysconf() won't be getting SIGSEGV faults.
>
> Additionally, I believe the definition can be calculated at compile time as a multiple of ( sizeof(ucontext_t)+sizeof(overhead_struct(s)) ), whatever other overhead applies, so I don't see any real need to use sysconf(). This may mean having to munge a <signal.in> by configure, based on config.guess, but that's not the standard's headache.
>

At compile time, we don't know what the minimum signal stack size is
at run-time, especially 10 years from now.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
       [not found] <1569476484.1894162.1615325397167.ref@mail.yahoo.com>
@ 2021-03-09 21:29 ` shwaresyst
  2021-03-09 23:58   ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: shwaresyst @ 2021-03-09 21:29 UTC (permalink / raw)
  To: eblake, bruno, cbouchar, bug-m4, libc-alpha, austin-group-l


Yes, it's not something an application would expect to need to keep increasing, just that's the part of <limits.h> I'd move it to. The definition could also be the max required by a processor family, with sysconf() reporting a possible lower value for a particular processor stepping. At least that way the application that doesn't use sysconf() won't be getting SIGSEGV faults.

Additionally, I believe the definition can be calculated at compile time as a multiple of ( sizeof(ucontext_t)+sizeof(overhead_struct(s)) ), whatever other overhead applies, so I don't see any real need to use sysconf(). This may mean having to munge a <signal.in> by configure, based on config.guess, but that's not the standard's headache.


The CS, SC, and PC constants are not in the XSH 2.2.2 table deliberately, from Issue 6 TC1, as adding any also requires a bump in POSIX_VERSION or POSIX2_VERSION, and often XSI_VERSION. This is so each usage of a constant doesn't need individual #ifdefs to test option group availability. The previous text was allowing if an implementation wasn't supporting an option group they could skip including the related constants in <unistd.h>. A simple check of VERSION at the top of a source C file suffices now to indicate those constants shall be available.
On Tuesday, March 9, 2021 Eric Blake <eblake@redhat.com> wrote:
On 3/9/21 10:14 AM, shwaresyst wrote:
> 
> To me that looks like a conformance violation and should be reverted. There is no _SC_SIGSTKSZ defined in <unistd.h> by the standard, to begin with, so that use of sysconf() is a non-portable extension on its own.

Portable apps can't use _SC_SIGSTKSZ, but the standard generally permits
implementations to define further constants.  Then again, re-reading XSH
2.2.2:

" Implementations may add symbols to the headers shown in the following
table, provided the identifiers for those symbols either:

    Begin with the corresponding reserved prefixes in the table, or
..."

but the table lacks a row for <unistd.h> with _CS_* and _SC_* constants.
 Looks like you found an independent defect.

> 
> I could see the definition of SIGSTKSZ being changed to the static minimum a particular processor requires, or is initially allocated as a 'safe' amount, rather than static "default size", and moving SIGSTKSZ to <limits.h>. This would contrast to MINSIGSTKSZ as the lowest value for a platform for all supported processors. Then an application could use sysconf() to query for the maximum size the configuration supports if it wants to use more than that, as a runtime increasable limit.

As I understand it, the concern in glibc is less about runtime
increasability, so much as ABI compatibility with applications compiled
against older headers at a time when the kernel had less state
information to store during a context switch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.          +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 19:34   ` Eric Blake
@ 2021-03-09 20:12     ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2021-03-09 20:12 UTC (permalink / raw)
  To: shwaresyst, bruno, cbouchar, bug-m4, libc-alpha, austin-group-l

On 3/9/21 1:34 PM, Eric Blake via austin-group-l at The Open Group wrote:
> On 3/9/21 10:14 AM, shwaresyst wrote:
>>
>> To me that looks like a conformance violation and should be reverted. There is no _SC_SIGSTKSZ defined in <unistd.h> by the standard, to begin with, so that use of sysconf() is a non-portable extension on its own.
> 
> Portable apps can't use _SC_SIGSTKSZ, but the standard generally permits
> implementations to define further constants.  Then again, re-reading XSH
> 2.2.2:
> 
> " Implementations may add symbols to the headers shown in the following
> table, provided the identifiers for those symbols either:
> 
>     Begin with the corresponding reserved prefixes in the table, or
> ..."
> 
> but the table lacks a row for <unistd.h> with _CS_* and _SC_* constants.
>  Looks like you found an independent defect.

Not quite, because later it states "The following identifiers are
reserved regardless of the inclusion of headers: 1. With the exception
of identifiers beginning with the prefix _POSIX_, all identifiers that
begin with an <underscore> and either an uppercase letter or another
<underscore> are always reserved for any use by the implementation.", so
an implementation can blindly add _SC_* constants at will without
violating the standard.

Still, I opened:
https://www.austingroupbugs.net/view.php?id=1456
to try and add some clarification.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 16:14 ` shwaresyst
  2021-03-09 19:33   ` Paul Eggert
@ 2021-03-09 19:34   ` Eric Blake
  2021-03-09 20:12     ` Eric Blake
  1 sibling, 1 reply; 14+ messages in thread
From: Eric Blake @ 2021-03-09 19:34 UTC (permalink / raw)
  To: shwaresyst, bruno, cbouchar, bug-m4, libc-alpha, austin-group-l

On 3/9/21 10:14 AM, shwaresyst wrote:
> 
> To me that looks like a conformance violation and should be reverted. There is no _SC_SIGSTKSZ defined in <unistd.h> by the standard, to begin with, so that use of sysconf() is a non-portable extension on its own.

Portable apps can't use _SC_SIGSTKSZ, but the standard generally permits
implementations to define further constants.  Then again, re-reading XSH
2.2.2:

" Implementations may add symbols to the headers shown in the following
table, provided the identifiers for those symbols either:

    Begin with the corresponding reserved prefixes in the table, or
..."

but the table lacks a row for <unistd.h> with _CS_* and _SC_* constants.
 Looks like you found an independent defect.

> 
> I could see the definition of SIGSTKSZ being changed to the static minimum a particular processor requires, or is initially allocated as a 'safe' amount, rather than static "default size", and moving SIGSTKSZ to <limits.h>. This would contrast to MINSIGSTKSZ as the lowest value for a platform for all supported processors. Then an application could use sysconf() to query for the maximum size the configuration supports if it wants to use more than that, as a runtime increasable limit.

As I understand it, the concern in glibc is less about runtime
increasability, so much as ABI compatibility with applications compiled
against older headers at a time when the kernel had less state
information to store during a context switch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
  2021-03-09 16:14 ` shwaresyst
@ 2021-03-09 19:33   ` Paul Eggert
  2021-03-09 19:34   ` Eric Blake
  1 sibling, 0 replies; 14+ messages in thread
From: Paul Eggert @ 2021-03-09 19:33 UTC (permalink / raw)
  To: shwaresyst, eblake, bruno, cbouchar, bug-m4, libc-alpha, austin-group-l

On 3/9/21 8:14 AM, shwaresyst via Libc-alpha wrote:
> The question becomes whether glibc is in violation of POSIX for having
> made the change,

I don't see how that would be. Apps must define _SC_SIGSTKSZ_SOURCE or 
_GNU_SOURCE to get the new API, which means the apps do not want strict 
POSIX conformance anyway.

> or whether POSIX needs to be amended to allow SIGSTKSZ
> to be non-preprocessor-safe and/or non-constant.

That would be a good idea, yes.

> I've now seen reports of several
> projects failing to build when using glibc with this change included.

Yes. I just now checked, and Emacs appears to have this problem so I 
installed a patch[1] to Emacs, which should fix it. I'm not surprised 
that other packages have similar issues, and would need similar patches. 
Gnulib was fixed to avoid this problem in October - before that, Gnulib 
didn't even conform to POSIX, because it used SIGSTKSZ in #if. The 
Gnulib patch[2] illustrates other portability messes in this area.

[1] 
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=f97e07ea807cc6d38774a3888a15091b20645ac6
[2] 
https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=f9e2b20a12a230efa30f1d479563ae07d276a94b

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: SIGSTKSZ is now a run-time variable
       [not found] <832918739.1734727.1615306471320.ref@mail.yahoo.com>
@ 2021-03-09 16:14 ` shwaresyst
  2021-03-09 19:33   ` Paul Eggert
  2021-03-09 19:34   ` Eric Blake
  0 siblings, 2 replies; 14+ messages in thread
From: shwaresyst @ 2021-03-09 16:14 UTC (permalink / raw)
  To: eblake, bruno, cbouchar, bug-m4, libc-alpha, austin-group-l


To me that looks like a conformance violation and should be reverted. There is no _SC_SIGSTKSZ defined in <unistd.h> by the standard, to begin with, so that use of sysconf() is a non-portable extension on its own.

I could see the definition of SIGSTKSZ being changed to the static minimum a particular processor requires, or is initially allocated as a 'safe' amount, rather than static "default size", and moving SIGSTKSZ to <limits.h>. This would contrast to MINSIGSTKSZ as the lowest value for a platform for all supported processors. Then an application could use sysconf() to query for the maximum size the configuration supports if it wants to use more than that, as a runtime increasable limit.
On Tuesday, March 9, 2021 Eric Blake via austin-group-l at The Open Group <eblake@redhat.com> wrote:
[adding glibc and Austin group lists]

On 3/6/21 12:50 PM, Bruno Haible wrote:
> Hi,
> 
> Carol Bouchard wrote in <https://lists.gnu.org/archive/html/bug-m4/2021-03/msg00000.html>:
>> A change that was introduced is the
>> #define SIGSTKSZ is no longer a statically defined variable.  It's value can
>> only be determined at run time.
>>
>> # define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
> 
> This is invalid. POSIX:2018 [1] defines two lists of macros:
> 
>  1) "The <signal.h> header shall define the following macros which shall
>      expand to integer constant expressions that need not be usable in
>      #if preprocessing directives:"
> 
>  2) "The <signal.h> header shall also define the following symbolic constants:"
> 
> SIGSTKSZ is in the second list. This implies that it must expand to a constant
> and that it must be usable in #if preprocessing directives.

The question becomes whether glibc is in violation of POSIX for having
made the change, or whether POSIX needs to be amended to allow SIGSTKSZ
to be non-preprocessor-safe and/or non-constant.

> 
> Besides being invalid, it is also not needed. The alternate signal stack
> needs to be dimensioned according to the CPU and ABI that is in use. For example,
> SPARC processors tend to use much more stack space than x86 per function
> invocation. Similarly, 64-bit execution on a bi-arch CPU tends to use more stack
> space than 32-bit execution, because return addresses and other pointers are
> 64-bit vs. 32-bit large. But once you have fixed the CPU and the ABI, there is
> no ambiguity any more.
> 
>> This affects m4 code since the code assumes a statically defined variable which
>> can be determined at preprocessor time.
> 
> POSIX guarantees this assumption.
> 
>> Please advise how I can get past this.
> 
> Fix your <signal.h>.

https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=6c57d320484988e87e446e2e60ce42816bf51d53
shows where glibc made the change, and I've now seen reports of several
projects failing to build when using glibc with this change included.

> 
> Bruno
> 
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.          +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-03-26 12:46 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1841269.IEpri3ZHvQ@omega>
2021-03-09 15:23 ` SIGSTKSZ is now a run-time variable Eric Blake
2021-03-09 15:26   ` Andreas Schwab
2021-03-09 15:48     ` Eric Blake
2021-03-09 19:58       ` Bruno Haible
2021-03-09 21:05         ` H.J. Lu
     [not found]         ` <20210309211735.GW19922@dragon.sl.home>
2021-03-16 19:46           ` Carol Bouchard
2021-03-26 12:46             ` Carol Bouchard
2021-03-09 15:46   ` Zack Weinberg
     [not found] <832918739.1734727.1615306471320.ref@mail.yahoo.com>
2021-03-09 16:14 ` shwaresyst
2021-03-09 19:33   ` Paul Eggert
2021-03-09 19:34   ` Eric Blake
2021-03-09 20:12     ` Eric Blake
     [not found] <1569476484.1894162.1615325397167.ref@mail.yahoo.com>
2021-03-09 21:29 ` shwaresyst
2021-03-09 23:58   ` H.J. Lu

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).