[PATCH] Disable -fsplit-stack support on non-glibc targets

[PATCH] Disable -fsplit-stack support on non-glibc targets

[soeren]

From: Sören Tempel <soeren+git@soeren-tempel.net>

The -fsplit-stack option requires the pthread_t TCB definition in the
libc to provide certain struct fields at specific hardcoded offsets. As
far as I know, only glibc provides these fields at the required offsets.
Most notably, musl libc does not have these fields. However, since gcc
accesses the fields using a fixed offset, this does not cause a
compile-time error, but instead results in a silent memory corruption at
run-time with musl libc. For example, on s390x libgcc's
__stack_split_initialize CTOR will overwrite the cancel field in the
pthread_t TCB on musl.

The -fsplit-stack option is used within the gcc code base itself by
gcc-go (if available). On musl-based systems with split-stack support
(i.e. s390x or x86) this causes Go programs compiled with gcc-go to
misbehave at run-time.

This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
since it is not supported on non-glibc targets anyhow. This is achieved
by checking if TARGET_GLIBC_MAJOR is defined to a non-zero value (it
defaults to zero on non-glibc systems). The check has been added for x86
and s390x, the rs6000 config already checks for TARGET_GLIBC_MAJOR.
Other architectures do not have split-stack support. With this patch
applied, the gcc-go configure script will detect that -fsplit-stack
support is not available and will not use it.

See https://www.openwall.com/lists/musl/2012/10/16/12

This patch has been tested on Alpine Linux Edge on the s390x and x86
architectures by bootstrapping Google's Go implementation with gcc-go.

Signed-off-by: Sören Tempel <soeren+git@soeren-tempel.net>

gcc/ChangeLog:

	* common/config/s390/s390-common.c (s390_supports_split_stack):
	Only support split-stack on glibc targets.
	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
	* config/i386/gnu.h (defined): Ditto.
---
 gcc/common/config/s390/s390-common.c | 9 ++++++++-
 gcc/config/i386/gnu-user-common.h    | 5 +++--
 gcc/config/i386/gnu.h                | 6 +++++-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
index b6bc8501742..afbd8d3fe66 100644
--- a/gcc/common/config/s390/s390-common.c
+++ b/gcc/common/config/s390/s390-common.c
@@ -116,13 +116,20 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
 
 /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
    We don't verify it, since earlier versions just have padding at
-   its place, which works just as well.  */
+   its place, which works just as well. For other libc implementations
+   we disable the feature entirely to avoid corrupting the TCB.  */
 
 static bool
 s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
 			   struct gcc_options *opts ATTRIBUTE_UNUSED)
 {
+#if TARGET_GLIBC_MAJOR
   return true;
+#else
+  if (report)
+    error("%<-fsplit-stack%> currently only supported on GNU/Linux");
+  return false;
+#endif
 }
 
 #undef TARGET_DEFAULT_TARGET_FLAGS
diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
index 00226f5a455..69f2d7415ad 100644
--- a/gcc/config/i386/gnu-user-common.h
+++ b/gcc/config/i386/gnu-user-common.h
@@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
 #define STACK_CHECK_STATIC_BUILTIN 1
 
 /* We only build the -fsplit-stack support in libgcc if the
-   assembler has full support for the CFI directives.  */
-#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
+   assembler has full support for the CFI directives and
+   targets glibc.  */
+#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && TARGET_GLIBC_MAJOR
 #define TARGET_CAN_SPLIT_STACK
 #endif
diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
index 25fbc07f58c..895a7369816 100644
--- a/gcc/config/i386/gnu.h
+++ b/gcc/config/i386/gnu.h
@@ -35,7 +35,11 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
    crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
 #endif
 
-#ifdef TARGET_LIBC_PROVIDES_SSP
+/* -fsplit-stack uses a field in the TCB at a fixed offset. This
+   field is only available for glibc. Disable -fsplit-stack for
+   other libc implementation to avoid silent TCB corruptions.  */
+
+#if defined (TARGET_LIBC_PROVIDES_SSP) && TARGET_GLIBC_MAJOR
 
 /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
 #define TARGET_THREAD_SSP_OFFSET        0x14

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Andrew Pinski]

On Sat, Dec 18, 2021 at 2:44 AM soeren--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Sören Tempel <soeren+git@soeren-tempel.net>
>
> The -fsplit-stack option requires the pthread_t TCB definition in the
> libc to provide certain struct fields at specific hardcoded offsets. As
> far as I know, only glibc provides these fields at the required offsets.
> Most notably, musl libc does not have these fields. However, since gcc
> accesses the fields using a fixed offset, this does not cause a
> compile-time error, but instead results in a silent memory corruption at
> run-time with musl libc. For example, on s390x libgcc's
> __stack_split_initialize CTOR will overwrite the cancel field in the
> pthread_t TCB on musl.
>
> The -fsplit-stack option is used within the gcc code base itself by
> gcc-go (if available). On musl-based systems with split-stack support
> (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
> misbehave at run-time.
>
> This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
> since it is not supported on non-glibc targets anyhow. This is achieved
> by checking if TARGET_GLIBC_MAJOR is defined to a non-zero value (it
> defaults to zero on non-glibc systems). The check has been added for x86
> and s390x, the rs6000 config already checks for TARGET_GLIBC_MAJOR.
> Other architectures do not have split-stack support. With this patch
> applied, the gcc-go configure script will detect that -fsplit-stack
> support is not available and will not use it.
>
> See https://www.openwall.com/lists/musl/2012/10/16/12
>
> This patch has been tested on Alpine Linux Edge on the s390x and x86
> architectures by bootstrapping Google's Go implementation with gcc-go.
>
> Signed-off-by: Sören Tempel <soeren+git@soeren-tempel.net>
>
> gcc/ChangeLog:
>
>         * common/config/s390/s390-common.c (s390_supports_split_stack):
>         Only support split-stack on glibc targets.
>         * config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
>         * config/i386/gnu.h (defined): Ditto.
> ---
>  gcc/common/config/s390/s390-common.c | 9 ++++++++-
>  gcc/config/i386/gnu-user-common.h    | 5 +++--
>  gcc/config/i386/gnu.h                | 6 +++++-
>  3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
> index b6bc8501742..afbd8d3fe66 100644
> --- a/gcc/common/config/s390/s390-common.c
> +++ b/gcc/common/config/s390/s390-common.c
> @@ -116,13 +116,20 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
>
>  /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
>     We don't verify it, since earlier versions just have padding at
> -   its place, which works just as well.  */
> +   its place, which works just as well. For other libc implementations
> +   we disable the feature entirely to avoid corrupting the TCB.  */
>
>  static bool
>  s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
>                            struct gcc_options *opts ATTRIBUTE_UNUSED)
>  {
> +#if TARGET_GLIBC_MAJOR
>    return true;
> +#else
> +  if (report)
> +    error("%<-fsplit-stack%> currently only supported on GNU/Linux");
> +  return false;
> +#endif
>  }

I think it should check OPTION_MUSL at runtime instead of
TARGET_GLIBC_MAJOR at compile time.
or rather opts->x_linux_libc == LIBC_MUSL
The others should be done similarly too.

Thanks,
Andrew


>
>  #undef TARGET_DEFAULT_TARGET_FLAGS
> diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
> index 00226f5a455..69f2d7415ad 100644
> --- a/gcc/config/i386/gnu-user-common.h
> +++ b/gcc/config/i386/gnu-user-common.h
> @@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
>  #define STACK_CHECK_STATIC_BUILTIN 1
>
>  /* We only build the -fsplit-stack support in libgcc if the
> -   assembler has full support for the CFI directives.  */
> -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
> +   assembler has full support for the CFI directives and
> +   targets glibc.  */
> +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && TARGET_GLIBC_MAJOR
>  #define TARGET_CAN_SPLIT_STACK
>  #endif
> diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
> index 25fbc07f58c..895a7369816 100644
> --- a/gcc/config/i386/gnu.h
> +++ b/gcc/config/i386/gnu.h
> @@ -35,7 +35,11 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
>     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
>  #endif
>
> -#ifdef TARGET_LIBC_PROVIDES_SSP
> +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> +   field is only available for glibc. Disable -fsplit-stack for
> +   other libc implementation to avoid silent TCB corruptions.  */
> +
> +#if defined (TARGET_LIBC_PROVIDES_SSP) && TARGET_GLIBC_MAJOR
>
>  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
>  #define TARGET_THREAD_SSP_OFFSET        0x14

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Sören Tempel]

Andrew Pinski <pinskia@gmail.com> wrote:
> I think it should check OPTION_MUSL at runtime instead of
> TARGET_GLIBC_MAJOR at compile time.
> or rather opts->x_linux_libc == LIBC_MUSL
> The others should be done similarly too.

Thanks for pointing this out, I wasn't aware of OPTION_MUSL and
OPTION_GLIBC. However, I am wondering if isn't more useful to whitelist
this feature on glibc instead of blacklisting it on musl? I was
operating on the assumption that other libc implementations (uclibc,
bionic, dietlibc, …) do also not support the required pthread_t fields,
i.e. that glibc is the only libc where this feature actually works.

Greetings,
Sören

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Andrew Pinski]

On Sat, Dec 18, 2021 at 3:13 AM Sören Tempel <soeren@soeren-tempel.net> wrote:
>
> Andrew Pinski <pinskia@gmail.com> wrote:
> > I think it should check OPTION_MUSL at runtime instead of
> > TARGET_GLIBC_MAJOR at compile time.
> > or rather opts->x_linux_libc == LIBC_MUSL
> > The others should be done similarly too.
>
> Thanks for pointing this out, I wasn't aware of OPTION_MUSL and
> OPTION_GLIBC. However, I am wondering if isn't more useful to whitelist
> this feature on glibc instead of blacklisting it on musl? I was
> operating on the assumption that other libc implementations (uclibc,
> bionic, dietlibc, …) do also not support the required pthread_t fields,
> i.e. that glibc is the only libc where this feature actually works.

yes checking OPTION_GLIBC is better really. Saying glibc only is
better than saying it does not work on musl in this case.
Though I think opts->x_linux_libc == LIBC_GLIBC is the more correct
fix for this function as we are passed down the opts struction and
all.

Thanks,
Andrew Pinski

>
> Greetings,
> Sören

[PATCH v2] Disable -fsplit-stack support on non-glibc targets

[soeren]

From: Sören Tempel <soeren@soeren-tempel.net>

The -fsplit-stack option requires the pthread_t TCB definition in the
libc to provide certain struct fields at specific hardcoded offsets. As
far as I know, only glibc provides these fields at the required offsets.
Most notably, musl libc does not have these fields. However, since gcc
accesses the fields using a fixed offset, this does not cause a
compile-time error, but instead results in a silent memory corruption at
run-time with musl libc. For example, on s390x libgcc's
__stack_split_initialize CTOR will overwrite the cancel field in the
pthread_t TCB on musl.

The -fsplit-stack option is used within the gcc code base itself by
gcc-go (if available). On musl-based systems with split-stack support
(i.e. s390x or x86) this causes Go programs compiled with gcc-go to
misbehave at run-time.

This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
since it is not supported on non-glibc targets anyhow. This is achieved
by checking if gcc targets a glibc-based system. This check has been
added for x86 and s390x, the rs6000 config already checks for
TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
support. With this patch applied, the gcc-go configure script will
detect that -fsplit-stack support is not available and will not use it.

See https://www.openwall.com/lists/musl/2012/10/16/12

This patch was written under the assumption that glibc is the only libc
implementation which supports the required fields at the required
offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
Edge on the s390x and x86 architectures by bootstrapping Google's Go
implementation with gcc-go.

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>

gcc/ChangeLog:

	* common/config/s390/s390-common.c (s390_supports_split_stack):
	Only support split-stack on glibc targets.
	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
	* config/i386/gnu.h (defined): Ditto.
---
This version of the patch addresses feedback by Andrew Pinski and uses
OPTION_GLIBC as well as opts->x_linux_libc == LIBC_GLIBC to detect glibc
targets (instead of relying on TARGET_GLIBC_MAJOR).

 gcc/common/config/s390/s390-common.c | 11 +++++++++--
 gcc/config/i386/gnu-user-common.h    |  5 +++--
 gcc/config/i386/gnu.h                |  6 +++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
index b6bc8501742..fc86e0bc5e7 100644
--- a/gcc/common/config/s390/s390-common.c
+++ b/gcc/common/config/s390/s390-common.c
@@ -116,13 +116,20 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
 
 /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
    We don't verify it, since earlier versions just have padding at
-   its place, which works just as well.  */
+   its place, which works just as well. For other libc implementations
+   we disable the feature entirely to avoid corrupting the TCB.  */
 
 static bool
 s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
 			   struct gcc_options *opts ATTRIBUTE_UNUSED)
 {
-  return true;
+  if (opts->x_linux_libc == LIBC_GLIBC) {
+    return true;
+  } else {
+    if (report)
+      error("%<-fsplit-stack%> currently only supported on GNU/Linux");
+    return false;
+  }
 }
 
 #undef TARGET_DEFAULT_TARGET_FLAGS
diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
index 00226f5a455..6e13315b5a3 100644
--- a/gcc/config/i386/gnu-user-common.h
+++ b/gcc/config/i386/gnu-user-common.h
@@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
 #define STACK_CHECK_STATIC_BUILTIN 1
 
 /* We only build the -fsplit-stack support in libgcc if the
-   assembler has full support for the CFI directives.  */
-#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
+   assembler has full support for the CFI directives and
+   targets glibc.  */
+#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
 #define TARGET_CAN_SPLIT_STACK
 #endif
diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
index 25fbc07f58c..adfe817201e 100644
--- a/gcc/config/i386/gnu.h
+++ b/gcc/config/i386/gnu.h
@@ -35,7 +35,11 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
    crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
 #endif
 
-#ifdef TARGET_LIBC_PROVIDES_SSP
+/* -fsplit-stack uses a field in the TCB at a fixed offset. This
+   field is only available for glibc. Disable -fsplit-stack for
+   other libc implementation to avoid silent TCB corruptions.  */
+
+#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
 
 /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
 #define TARGET_THREAD_SSP_OFFSET        0x14

Re: Re: [PATCH v2] Disable -fsplit-stack support on non-glibc targets

[Sören Tempel]

Ping.

Summary: Patch disable -fstack-split on non-glibc targets to prevent
corruptions of the TCB on libcs which do not support the required
fields in pthread_t. This is an important fix for having gccgo work on
musl by default.

See: https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587142.html

If the patch needs to be revised further please let me know.

Greetings,
Sören

Sören Tempel <soeren@soeren-tempel.net> wrote:
> The -fsplit-stack option requires the pthread_t TCB definition in the
> libc to provide certain struct fields at specific hardcoded offsets. As
> far as I know, only glibc provides these fields at the required offsets.
> Most notably, musl libc does not have these fields. However, since gcc
> accesses the fields using a fixed offset, this does not cause a
> compile-time error, but instead results in a silent memory corruption at
> run-time with musl libc. For example, on s390x libgcc's
> __stack_split_initialize CTOR will overwrite the cancel field in the
> pthread_t TCB on musl.
> 
> The -fsplit-stack option is used within the gcc code base itself by
> gcc-go (if available). On musl-based systems with split-stack support
> (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
> misbehave at run-time.
> 
> This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
> since it is not supported on non-glibc targets anyhow. This is achieved
> by checking if gcc targets a glibc-based system. This check has been
> added for x86 and s390x, the rs6000 config already checks for
> TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
> support. With this patch applied, the gcc-go configure script will
> detect that -fsplit-stack support is not available and will not use it.
> 
> See https://www.openwall.com/lists/musl/2012/10/16/12
> 
> This patch was written under the assumption that glibc is the only libc
> implementation which supports the required fields at the required
> offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
> Edge on the s390x and x86 architectures by bootstrapping Google's Go
> implementation with gcc-go.
> 
> Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
> 
> gcc/ChangeLog:
> 
> 	* common/config/s390/s390-common.c (s390_supports_split_stack):
> 	Only support split-stack on glibc targets.
> 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> 	* config/i386/gnu.h (defined): Ditto.
> ---
> This version of the patch addresses feedback by Andrew Pinski and uses
> OPTION_GLIBC as well as opts->x_linux_libc == LIBC_GLIBC to detect glibc
> targets (instead of relying on TARGET_GLIBC_MAJOR).
> 
>  gcc/common/config/s390/s390-common.c | 11 +++++++++--
>  gcc/config/i386/gnu-user-common.h    |  5 +++--
>  gcc/config/i386/gnu.h                |  6 +++++-
>  3 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
> index b6bc8501742..fc86e0bc5e7 100644
> --- a/gcc/common/config/s390/s390-common.c
> +++ b/gcc/common/config/s390/s390-common.c
> @@ -116,13 +116,20 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
>  
>  /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
>     We don't verify it, since earlier versions just have padding at
> -   its place, which works just as well.  */
> +   its place, which works just as well. For other libc implementations
> +   we disable the feature entirely to avoid corrupting the TCB.  */
>  
>  static bool
>  s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
>  			   struct gcc_options *opts ATTRIBUTE_UNUSED)
>  {
> -  return true;
> +  if (opts->x_linux_libc == LIBC_GLIBC) {
> +    return true;
> +  } else {
> +    if (report)
> +      error("%<-fsplit-stack%> currently only supported on GNU/Linux");
> +    return false;
> +  }
>  }
>  
>  #undef TARGET_DEFAULT_TARGET_FLAGS
> diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
> index 00226f5a455..6e13315b5a3 100644
> --- a/gcc/config/i386/gnu-user-common.h
> +++ b/gcc/config/i386/gnu-user-common.h
> @@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
>  #define STACK_CHECK_STATIC_BUILTIN 1
>  
>  /* We only build the -fsplit-stack support in libgcc if the
> -   assembler has full support for the CFI directives.  */
> -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
> +   assembler has full support for the CFI directives and
> +   targets glibc.  */
> +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
>  #define TARGET_CAN_SPLIT_STACK
>  #endif
> diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
> index 25fbc07f58c..adfe817201e 100644
> --- a/gcc/config/i386/gnu.h
> +++ b/gcc/config/i386/gnu.h
> @@ -35,7 +35,11 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
>     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
>  #endif
>  
> -#ifdef TARGET_LIBC_PROVIDES_SSP
> +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> +   field is only available for glibc. Disable -fsplit-stack for
> +   other libc implementation to avoid silent TCB corruptions.  */
> +
> +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
>  
>  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
>  #define TARGET_THREAD_SSP_OFFSET        0x14

Re: [PATCH v2] Disable -fsplit-stack support on non-glibc targets

[Richard Sandiford]

cc:ing the x86 and s390 maintainers

soeren--- via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> From: Sören Tempel <soeren@soeren-tempel.net>
>
> The -fsplit-stack option requires the pthread_t TCB definition in the
> libc to provide certain struct fields at specific hardcoded offsets. As
> far as I know, only glibc provides these fields at the required offsets.
> Most notably, musl libc does not have these fields. However, since gcc
> accesses the fields using a fixed offset, this does not cause a
> compile-time error, but instead results in a silent memory corruption at
> run-time with musl libc. For example, on s390x libgcc's
> __stack_split_initialize CTOR will overwrite the cancel field in the
> pthread_t TCB on musl.
>
> The -fsplit-stack option is used within the gcc code base itself by
> gcc-go (if available). On musl-based systems with split-stack support
> (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
> misbehave at run-time.
>
> This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
> since it is not supported on non-glibc targets anyhow. This is achieved
> by checking if gcc targets a glibc-based system. This check has been
> added for x86 and s390x, the rs6000 config already checks for
> TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
> support. With this patch applied, the gcc-go configure script will
> detect that -fsplit-stack support is not available and will not use it.
>
> See https://www.openwall.com/lists/musl/2012/10/16/12
>
> This patch was written under the assumption that glibc is the only libc
> implementation which supports the required fields at the required
> offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
> Edge on the s390x and x86 architectures by bootstrapping Google's Go
> implementation with gcc-go.
>
> Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
>
> gcc/ChangeLog:
>
> 	* common/config/s390/s390-common.c (s390_supports_split_stack):
> 	Only support split-stack on glibc targets.
> 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> 	* config/i386/gnu.h (defined): Ditto.
> ---
> This version of the patch addresses feedback by Andrew Pinski and uses
> OPTION_GLIBC as well as opts->x_linux_libc == LIBC_GLIBC to detect glibc
> targets (instead of relying on TARGET_GLIBC_MAJOR).
>
>  gcc/common/config/s390/s390-common.c | 11 +++++++++--
>  gcc/config/i386/gnu-user-common.h    |  5 +++--
>  gcc/config/i386/gnu.h                |  6 +++++-
>  3 files changed, 17 insertions(+), 5 deletions(-)

Sorry for the slow review.  The patch LGTM bar some minor formatting
nits below, but target maintainers should have the final say.

> diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
> index b6bc8501742..fc86e0bc5e7 100644
> --- a/gcc/common/config/s390/s390-common.c
> +++ b/gcc/common/config/s390/s390-common.c
> @@ -116,13 +116,20 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
>  
>  /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
>     We don't verify it, since earlier versions just have padding at
> -   its place, which works just as well.  */
> +   its place, which works just as well. For other libc implementations

GCC style is to use 2 spaces after a full stop.  Same for the x86 part.

> +   we disable the feature entirely to avoid corrupting the TCB.  */
>  
>  static bool
>  s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
>  			   struct gcc_options *opts ATTRIBUTE_UNUSED)

These parameters are no longer unused after the patch, so it'd be good
to remove the attributes.

>  {
> -  return true;
> +  if (opts->x_linux_libc == LIBC_GLIBC) {
> +    return true;
> +  } else {
> +    if (report)
> +      error("%<-fsplit-stack%> currently only supported on GNU/Linux");
> +    return false;
> +  }

Normal GCC formatting would be something like:

  if (opts->x_linux_libc == LIBC_GLIBC)
    return true;

  if (report)
    error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
  return false;

Sorry for the fussy rules.

Thanks,
Richard

>  }
>  
>  #undef TARGET_DEFAULT_TARGET_FLAGS
> diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
> index 00226f5a455..6e13315b5a3 100644
> --- a/gcc/config/i386/gnu-user-common.h
> +++ b/gcc/config/i386/gnu-user-common.h
> @@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
>  #define STACK_CHECK_STATIC_BUILTIN 1
>  
>  /* We only build the -fsplit-stack support in libgcc if the
> -   assembler has full support for the CFI directives.  */
> -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
> +   assembler has full support for the CFI directives and
> +   targets glibc.  */
> +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
>  #define TARGET_CAN_SPLIT_STACK
>  #endif
> diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
> index 25fbc07f58c..adfe817201e 100644
> --- a/gcc/config/i386/gnu.h
> +++ b/gcc/config/i386/gnu.h
> @@ -35,7 +35,11 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
>     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
>  #endif
>  
> -#ifdef TARGET_LIBC_PROVIDES_SSP
> +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> +   field is only available for glibc. Disable -fsplit-stack for
> +   other libc implementation to avoid silent TCB corruptions.  */
> +
> +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
>  
>  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
>  #define TARGET_THREAD_SSP_OFFSET        0x14

Re: [PATCH v2] Disable -fsplit-stack support on non-glibc targets

[Andreas Krebbel]

On 1/20/22 23:52, Richard Sandiford wrote:
> cc:ing the x86 and s390 maintainers
> 
> soeren--- via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> From: Sören Tempel <soeren@soeren-tempel.net>
>>
>> The -fsplit-stack option requires the pthread_t TCB definition in the
>> libc to provide certain struct fields at specific hardcoded offsets. As
>> far as I know, only glibc provides these fields at the required offsets.
>> Most notably, musl libc does not have these fields. However, since gcc
>> accesses the fields using a fixed offset, this does not cause a
>> compile-time error, but instead results in a silent memory corruption at
>> run-time with musl libc. For example, on s390x libgcc's
>> __stack_split_initialize CTOR will overwrite the cancel field in the
>> pthread_t TCB on musl.
>>
>> The -fsplit-stack option is used within the gcc code base itself by
>> gcc-go (if available). On musl-based systems with split-stack support
>> (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
>> misbehave at run-time.
>>
>> This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
>> since it is not supported on non-glibc targets anyhow. This is achieved
>> by checking if gcc targets a glibc-based system. This check has been
>> added for x86 and s390x, the rs6000 config already checks for
>> TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
>> support. With this patch applied, the gcc-go configure script will
>> detect that -fsplit-stack support is not available and will not use it.
>>
>> See https://www.openwall.com/lists/musl/2012/10/16/12
>>
>> This patch was written under the assumption that glibc is the only libc
>> implementation which supports the required fields at the required
>> offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
>> Edge on the s390x and x86 architectures by bootstrapping Google's Go
>> implementation with gcc-go.
>>
>> Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
>>
>> gcc/ChangeLog:
>>
>> 	* common/config/s390/s390-common.c (s390_supports_split_stack):
>> 	Only support split-stack on glibc targets.
>> 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
>> 	* config/i386/gnu.h (defined): Ditto.

s390 parts are ok.

Thanks!

Andreas

>> ---
>> This version of the patch addresses feedback by Andrew Pinski and uses
>> OPTION_GLIBC as well as opts->x_linux_libc == LIBC_GLIBC to detect glibc
>> targets (instead of relying on TARGET_GLIBC_MAJOR).
>>
>>  gcc/common/config/s390/s390-common.c | 11 +++++++++--
>>  gcc/config/i386/gnu-user-common.h    |  5 +++--
>>  gcc/config/i386/gnu.h                |  6 +++++-
>>  3 files changed, 17 insertions(+), 5 deletions(-)
> 
> Sorry for the slow review.  The patch LGTM bar some minor formatting
> nits below, but target maintainers should have the final say.
> 
>> diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
>> index b6bc8501742..fc86e0bc5e7 100644
>> --- a/gcc/common/config/s390/s390-common.c
>> +++ b/gcc/common/config/s390/s390-common.c
>> @@ -116,13 +116,20 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
>>  
>>  /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
>>     We don't verify it, since earlier versions just have padding at
>> -   its place, which works just as well.  */
>> +   its place, which works just as well. For other libc implementations
> 
> GCC style is to use 2 spaces after a full stop.  Same for the x86 part.
> 
>> +   we disable the feature entirely to avoid corrupting the TCB.  */
>>  
>>  static bool
>>  s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
>>  			   struct gcc_options *opts ATTRIBUTE_UNUSED)
> 
> These parameters are no longer unused after the patch, so it'd be good
> to remove the attributes.
> 
>>  {
>> -  return true;
>> +  if (opts->x_linux_libc == LIBC_GLIBC) {
>> +    return true;
>> +  } else {
>> +    if (report)
>> +      error("%<-fsplit-stack%> currently only supported on GNU/Linux");
>> +    return false;
>> +  }
> 
> Normal GCC formatting would be something like:
> 
>   if (opts->x_linux_libc == LIBC_GLIBC)
>     return true;
> 
>   if (report)
>     error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
>   return false;
> 
> Sorry for the fussy rules.
> 
> Thanks,
> Richard
> 
>>  }
>>  
>>  #undef TARGET_DEFAULT_TARGET_FLAGS
>> diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
>> index 00226f5a455..6e13315b5a3 100644
>> --- a/gcc/config/i386/gnu-user-common.h
>> +++ b/gcc/config/i386/gnu-user-common.h
>> @@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
>>  #define STACK_CHECK_STATIC_BUILTIN 1
>>  
>>  /* We only build the -fsplit-stack support in libgcc if the
>> -   assembler has full support for the CFI directives.  */
>> -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
>> +   assembler has full support for the CFI directives and
>> +   targets glibc.  */
>> +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
>>  #define TARGET_CAN_SPLIT_STACK
>>  #endif
>> diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
>> index 25fbc07f58c..adfe817201e 100644
>> --- a/gcc/config/i386/gnu.h
>> +++ b/gcc/config/i386/gnu.h
>> @@ -35,7 +35,11 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
>>     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
>>  #endif
>>  
>> -#ifdef TARGET_LIBC_PROVIDES_SSP
>> +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
>> +   field is only available for glibc. Disable -fsplit-stack for
>> +   other libc implementation to avoid silent TCB corruptions.  */
>> +
>> +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
>>  
>>  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
>>  #define TARGET_THREAD_SSP_OFFSET        0x14

Re: [PATCH v2] Disable -fsplit-stack support on non-glibc targets

[Uros Bizjak]

On Thu, Jan 20, 2022 at 11:52 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> cc:ing the x86 and s390 maintainers
>
> soeren--- via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > From: Sören Tempel <soeren@soeren-tempel.net>
> >
> > The -fsplit-stack option requires the pthread_t TCB definition in the
> > libc to provide certain struct fields at specific hardcoded offsets. As
> > far as I know, only glibc provides these fields at the required offsets.
> > Most notably, musl libc does not have these fields. However, since gcc
> > accesses the fields using a fixed offset, this does not cause a
> > compile-time error, but instead results in a silent memory corruption at
> > run-time with musl libc. For example, on s390x libgcc's
> > __stack_split_initialize CTOR will overwrite the cancel field in the
> > pthread_t TCB on musl.
> >
> > The -fsplit-stack option is used within the gcc code base itself by
> > gcc-go (if available). On musl-based systems with split-stack support
> > (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
> > misbehave at run-time.
> >
> > This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
> > since it is not supported on non-glibc targets anyhow. This is achieved
> > by checking if gcc targets a glibc-based system. This check has been
> > added for x86 and s390x, the rs6000 config already checks for
> > TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
> > support. With this patch applied, the gcc-go configure script will
> > detect that -fsplit-stack support is not available and will not use it.
> >
> > See https://www.openwall.com/lists/musl/2012/10/16/12
> >
> > This patch was written under the assumption that glibc is the only libc
> > implementation which supports the required fields at the required
> > offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
> > Edge on the s390x and x86 architectures by bootstrapping Google's Go
> > implementation with gcc-go.
> >
> > Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
> >
> > gcc/ChangeLog:
> >
> >       * common/config/s390/s390-common.c (s390_supports_split_stack):
> >       Only support split-stack on glibc targets.
> >       * config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> >       * config/i386/gnu.h (defined): Ditto.

LGTM for x86 parts.

Thanks,
Uros.

> > ---
> > This version of the patch addresses feedback by Andrew Pinski and uses
> > OPTION_GLIBC as well as opts->x_linux_libc == LIBC_GLIBC to detect glibc
> > targets (instead of relying on TARGET_GLIBC_MAJOR).
> >
> >  gcc/common/config/s390/s390-common.c | 11 +++++++++--
> >  gcc/config/i386/gnu-user-common.h    |  5 +++--
> >  gcc/config/i386/gnu.h                |  6 +++++-
> >  3 files changed, 17 insertions(+), 5 deletions(-)
>
> Sorry for the slow review.  The patch LGTM bar some minor formatting
> nits below, but target maintainers should have the final say.
>
> > diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
> > index b6bc8501742..fc86e0bc5e7 100644
> > --- a/gcc/common/config/s390/s390-common.c
> > +++ b/gcc/common/config/s390/s390-common.c
> > @@ -116,13 +116,20 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
> >
> >  /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
> >     We don't verify it, since earlier versions just have padding at
> > -   its place, which works just as well.  */
> > +   its place, which works just as well. For other libc implementations
>
> GCC style is to use 2 spaces after a full stop.  Same for the x86 part.
>
> > +   we disable the feature entirely to avoid corrupting the TCB.  */
> >
> >  static bool
> >  s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
> >                          struct gcc_options *opts ATTRIBUTE_UNUSED)
>
> These parameters are no longer unused after the patch, so it'd be good
> to remove the attributes.
>
> >  {
> > -  return true;
> > +  if (opts->x_linux_libc == LIBC_GLIBC) {
> > +    return true;
> > +  } else {
> > +    if (report)
> > +      error("%<-fsplit-stack%> currently only supported on GNU/Linux");
> > +    return false;
> > +  }
>
> Normal GCC formatting would be something like:
>
>   if (opts->x_linux_libc == LIBC_GLIBC)
>     return true;
>
>   if (report)
>     error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
>   return false;
>
> Sorry for the fussy rules.
>
> Thanks,
> Richard
>
> >  }
> >
> >  #undef TARGET_DEFAULT_TARGET_FLAGS
> > diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
> > index 00226f5a455..6e13315b5a3 100644
> > --- a/gcc/config/i386/gnu-user-common.h
> > +++ b/gcc/config/i386/gnu-user-common.h
> > @@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
> >  #define STACK_CHECK_STATIC_BUILTIN 1
> >
> >  /* We only build the -fsplit-stack support in libgcc if the
> > -   assembler has full support for the CFI directives.  */
> > -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
> > +   assembler has full support for the CFI directives and
> > +   targets glibc.  */
> > +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
> >  #define TARGET_CAN_SPLIT_STACK
> >  #endif
> > diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
> > index 25fbc07f58c..adfe817201e 100644
> > --- a/gcc/config/i386/gnu.h
> > +++ b/gcc/config/i386/gnu.h
> > @@ -35,7 +35,11 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
> >     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
> >  #endif
> >
> > -#ifdef TARGET_LIBC_PROVIDES_SSP
> > +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> > +   field is only available for glibc. Disable -fsplit-stack for
> > +   other libc implementation to avoid silent TCB corruptions.  */
> > +
> > +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
> >
> >  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
> >  #define TARGET_THREAD_SSP_OFFSET        0x14

[PATCH v3] Disable -fsplit-stack support on non-glibc targets

[soeren]

From: Sören Tempel <soeren@soeren-tempel.net>

The -fsplit-stack option requires the pthread_t TCB definition in the
libc to provide certain struct fields at specific hardcoded offsets. As
far as I know, only glibc provides these fields at the required offsets.
Most notably, musl libc does not have these fields. However, since gcc
accesses the fields using a fixed offset, this does not cause a
compile-time error, but instead results in a silent memory corruption at
run-time with musl libc. For example, on s390x libgcc's
__stack_split_initialize CTOR will overwrite the cancel field in the
pthread_t TCB on musl.

The -fsplit-stack option is used within the gcc code base itself by
gcc-go (if available). On musl-based systems with split-stack support
(i.e. s390x or x86) this causes Go programs compiled with gcc-go to
misbehave at run-time.

This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
since it is not supported on non-glibc targets anyhow. This is achieved
by checking if gcc targets a glibc-based system. This check has been
added for x86 and s390x, the rs6000 config already checks for
TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
support. With this patch applied, the gcc-go configure script will
detect that -fsplit-stack support is not available and will not use it.

See https://www.openwall.com/lists/musl/2012/10/16/12

This patch was written under the assumption that glibc is the only libc
implementation which supports the required fields at the required
offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
Edge on the s390x and x86 architectures by bootstrapping Google's Go
implementation with gcc-go.

Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>

gcc/ChangeLog:

	* common/config/s390/s390-common.c (s390_supports_split_stack):
	Only support split-stack on glibc targets.
	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
	* config/i386/gnu.h (defined): Ditto.
---
This version of the patch fixes a few codingstyle violations pointed out
to me by Richard Sandiford, it does not include any functional changes
compared to previous versions of this patch.

 gcc/common/config/s390/s390-common.cc | 14 ++++++++++----
 gcc/config/i386/gnu-user-common.h     |  5 +++--
 gcc/config/i386/gnu.h                 |  5 ++++-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/gcc/common/config/s390/s390-common.cc b/gcc/common/config/s390/s390-common.cc
index 6ed2f89f3d0..547b0826f93 100644
--- a/gcc/common/config/s390/s390-common.cc
+++ b/gcc/common/config/s390/s390-common.cc
@@ -116,13 +116,19 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
 
 /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
    We don't verify it, since earlier versions just have padding at
-   its place, which works just as well.  */
+   its place, which works just as well.  For other libc implementations
+   we disable the feature entirely to avoid corrupting the TCB.  */
 
 static bool
-s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
-			   struct gcc_options *opts ATTRIBUTE_UNUSED)
+s390_supports_split_stack (bool report,
+			   struct gcc_options *opts)
 {
-  return true;
+  if (opts->x_linux_libc == LIBC_GLIBC)
+    return true;
+
+  if (report)
+    error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
+  return false;
 }
 
 #undef TARGET_DEFAULT_TARGET_FLAGS
diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
index 23b54c5be52..7525f788a9c 100644
--- a/gcc/config/i386/gnu-user-common.h
+++ b/gcc/config/i386/gnu-user-common.h
@@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
 #define STACK_CHECK_STATIC_BUILTIN 1
 
 /* We only build the -fsplit-stack support in libgcc if the
-   assembler has full support for the CFI directives.  */
-#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
+   assembler has full support for the CFI directives and
+   targets glibc.  */
+#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
 #define TARGET_CAN_SPLIT_STACK
 #endif
diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
index 401e60c9a02..daa505a5d45 100644
--- a/gcc/config/i386/gnu.h
+++ b/gcc/config/i386/gnu.h
@@ -35,7 +35,10 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
    crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
 #endif
 
-#ifdef TARGET_LIBC_PROVIDES_SSP
+/* -fsplit-stack uses a field in the TCB at a fixed offset. This
+   field is only available for glibc.  Disable -fsplit-stack for
+   other libc implementations to avoid silent TCB corruptions.  */
+#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
 
 /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
 #define TARGET_THREAD_SSP_OFFSET        0x14

Re: [PATCH v3] Disable -fsplit-stack support on non-glibc targets

[Richard Sandiford]

soeren@soeren-tempel.net writes:
> From: Sören Tempel <soeren@soeren-tempel.net>
>
> The -fsplit-stack option requires the pthread_t TCB definition in the
> libc to provide certain struct fields at specific hardcoded offsets. As
> far as I know, only glibc provides these fields at the required offsets.
> Most notably, musl libc does not have these fields. However, since gcc
> accesses the fields using a fixed offset, this does not cause a
> compile-time error, but instead results in a silent memory corruption at
> run-time with musl libc. For example, on s390x libgcc's
> __stack_split_initialize CTOR will overwrite the cancel field in the
> pthread_t TCB on musl.
>
> The -fsplit-stack option is used within the gcc code base itself by
> gcc-go (if available). On musl-based systems with split-stack support
> (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
> misbehave at run-time.
>
> This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
> since it is not supported on non-glibc targets anyhow. This is achieved
> by checking if gcc targets a glibc-based system. This check has been
> added for x86 and s390x, the rs6000 config already checks for
> TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
> support. With this patch applied, the gcc-go configure script will
> detect that -fsplit-stack support is not available and will not use it.
>
> See https://www.openwall.com/lists/musl/2012/10/16/12
>
> This patch was written under the assumption that glibc is the only libc
> implementation which supports the required fields at the required
> offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
> Edge on the s390x and x86 architectures by bootstrapping Google's Go
> implementation with gcc-go.
>
> Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
>
> gcc/ChangeLog:
>
> 	* common/config/s390/s390-common.c (s390_supports_split_stack):
> 	Only support split-stack on glibc targets.
> 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> 	* config/i386/gnu.h (defined): Ditto.

Thanks, pushed to trunk.

Richard

> ---
> This version of the patch fixes a few codingstyle violations pointed out
> to me by Richard Sandiford, it does not include any functional changes
> compared to previous versions of this patch.
>
>  gcc/common/config/s390/s390-common.cc | 14 ++++++++++----
>  gcc/config/i386/gnu-user-common.h     |  5 +++--
>  gcc/config/i386/gnu.h                 |  5 ++++-
>  3 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/common/config/s390/s390-common.cc b/gcc/common/config/s390/s390-common.cc
> index 6ed2f89f3d0..547b0826f93 100644
> --- a/gcc/common/config/s390/s390-common.cc
> +++ b/gcc/common/config/s390/s390-common.cc
> @@ -116,13 +116,19 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
>  
>  /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
>     We don't verify it, since earlier versions just have padding at
> -   its place, which works just as well.  */
> +   its place, which works just as well.  For other libc implementations
> +   we disable the feature entirely to avoid corrupting the TCB.  */
>  
>  static bool
> -s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
> -			   struct gcc_options *opts ATTRIBUTE_UNUSED)
> +s390_supports_split_stack (bool report,
> +			   struct gcc_options *opts)
>  {
> -  return true;
> +  if (opts->x_linux_libc == LIBC_GLIBC)
> +    return true;
> +
> +  if (report)
> +    error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
> +  return false;
>  }
>  
>  #undef TARGET_DEFAULT_TARGET_FLAGS
> diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
> index 23b54c5be52..7525f788a9c 100644
> --- a/gcc/config/i386/gnu-user-common.h
> +++ b/gcc/config/i386/gnu-user-common.h
> @@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
>  #define STACK_CHECK_STATIC_BUILTIN 1
>  
>  /* We only build the -fsplit-stack support in libgcc if the
> -   assembler has full support for the CFI directives.  */
> -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
> +   assembler has full support for the CFI directives and
> +   targets glibc.  */
> +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
>  #define TARGET_CAN_SPLIT_STACK
>  #endif
> diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
> index 401e60c9a02..daa505a5d45 100644
> --- a/gcc/config/i386/gnu.h
> +++ b/gcc/config/i386/gnu.h
> @@ -35,7 +35,10 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
>     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
>  #endif
>  
> -#ifdef TARGET_LIBC_PROVIDES_SSP
> +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> +   field is only available for glibc.  Disable -fsplit-stack for
> +   other libc implementations to avoid silent TCB corruptions.  */
> +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
>  
>  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
>  #define TARGET_THREAD_SSP_OFFSET        0x14

Re: [PATCH v3] Disable -fsplit-stack support on non-glibc targets

[H.J. Lu]

On Fri, Jan 21, 2022 at 11:23 AM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> soeren@soeren-tempel.net writes:
> > From: Sören Tempel <soeren@soeren-tempel.net>
> >
> > The -fsplit-stack option requires the pthread_t TCB definition in the
> > libc to provide certain struct fields at specific hardcoded offsets. As
> > far as I know, only glibc provides these fields at the required offsets.
> > Most notably, musl libc does not have these fields. However, since gcc
> > accesses the fields using a fixed offset, this does not cause a
> > compile-time error, but instead results in a silent memory corruption at
> > run-time with musl libc. For example, on s390x libgcc's
> > __stack_split_initialize CTOR will overwrite the cancel field in the
> > pthread_t TCB on musl.
> >
> > The -fsplit-stack option is used within the gcc code base itself by
> > gcc-go (if available). On musl-based systems with split-stack support
> > (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
> > misbehave at run-time.
> >
> > This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
> > since it is not supported on non-glibc targets anyhow. This is achieved
> > by checking if gcc targets a glibc-based system. This check has been
> > added for x86 and s390x, the rs6000 config already checks for
> > TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
> > support. With this patch applied, the gcc-go configure script will
> > detect that -fsplit-stack support is not available and will not use it.
> >
> > See https://www.openwall.com/lists/musl/2012/10/16/12
> >
> > This patch was written under the assumption that glibc is the only libc
> > implementation which supports the required fields at the required
> > offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
> > Edge on the s390x and x86 architectures by bootstrapping Google's Go
> > implementation with gcc-go.
> >
> > Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
> >
> > gcc/ChangeLog:
> >
> >       * common/config/s390/s390-common.c (s390_supports_split_stack):
> >       Only support split-stack on glibc targets.
> >       * config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> >       * config/i386/gnu.h (defined): Ditto.
>
> Thanks, pushed to trunk.

This broke GCC bootstrap on Linux/i686:

https://gcc.gnu.org/pipermail/gcc-regression/2022-January/076271.html

> Richard
>
> > ---
> > This version of the patch fixes a few codingstyle violations pointed out
> > to me by Richard Sandiford, it does not include any functional changes
> > compared to previous versions of this patch.
> >
> >  gcc/common/config/s390/s390-common.cc | 14 ++++++++++----
> >  gcc/config/i386/gnu-user-common.h     |  5 +++--
> >  gcc/config/i386/gnu.h                 |  5 ++++-
> >  3 files changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/gcc/common/config/s390/s390-common.cc b/gcc/common/config/s390/s390-common.cc
> > index 6ed2f89f3d0..547b0826f93 100644
> > --- a/gcc/common/config/s390/s390-common.cc
> > +++ b/gcc/common/config/s390/s390-common.cc
> > @@ -116,13 +116,19 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
> >
> >  /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
> >     We don't verify it, since earlier versions just have padding at
> > -   its place, which works just as well.  */
> > +   its place, which works just as well.  For other libc implementations
> > +   we disable the feature entirely to avoid corrupting the TCB.  */
> >
> >  static bool
> > -s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
> > -                        struct gcc_options *opts ATTRIBUTE_UNUSED)
> > +s390_supports_split_stack (bool report,
> > +                        struct gcc_options *opts)
> >  {
> > -  return true;
> > +  if (opts->x_linux_libc == LIBC_GLIBC)
> > +    return true;
> > +
> > +  if (report)
> > +    error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
> > +  return false;
> >  }
> >
> >  #undef TARGET_DEFAULT_TARGET_FLAGS
> > diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
> > index 23b54c5be52..7525f788a9c 100644
> > --- a/gcc/config/i386/gnu-user-common.h
> > +++ b/gcc/config/i386/gnu-user-common.h
> > @@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
> >  #define STACK_CHECK_STATIC_BUILTIN 1
> >
> >  /* We only build the -fsplit-stack support in libgcc if the
> > -   assembler has full support for the CFI directives.  */
> > -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
> > +   assembler has full support for the CFI directives and
> > +   targets glibc.  */
> > +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
> >  #define TARGET_CAN_SPLIT_STACK
> >  #endif
> > diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
> > index 401e60c9a02..daa505a5d45 100644
> > --- a/gcc/config/i386/gnu.h
> > +++ b/gcc/config/i386/gnu.h
> > @@ -35,7 +35,10 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
> >     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
> >  #endif
> >
> > -#ifdef TARGET_LIBC_PROVIDES_SSP
> > +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> > +   field is only available for glibc.  Disable -fsplit-stack for
> > +   other libc implementations to avoid silent TCB corruptions.  */
> > +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
> >
> >  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
> >  #define TARGET_THREAD_SSP_OFFSET        0x14



-- 
H.J.

Re: [PATCH v2] Disable -fsplit-stack support on non-glibc targets

[H.J. Lu]

On Sat, Dec 18, 2021 at 4:20 AM soeren--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Sören Tempel <soeren@soeren-tempel.net>
>
> The -fsplit-stack option requires the pthread_t TCB definition in the
> libc to provide certain struct fields at specific hardcoded offsets. As
> far as I know, only glibc provides these fields at the required offsets.
> Most notably, musl libc does not have these fields. However, since gcc
> accesses the fields using a fixed offset, this does not cause a
> compile-time error, but instead results in a silent memory corruption at
> run-time with musl libc. For example, on s390x libgcc's
> __stack_split_initialize CTOR will overwrite the cancel field in the
> pthread_t TCB on musl.
>
> The -fsplit-stack option is used within the gcc code base itself by
> gcc-go (if available). On musl-based systems with split-stack support
> (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
> misbehave at run-time.
>
> This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
> since it is not supported on non-glibc targets anyhow. This is achieved
> by checking if gcc targets a glibc-based system. This check has been
> added for x86 and s390x, the rs6000 config already checks for
> TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
> support. With this patch applied, the gcc-go configure script will
> detect that -fsplit-stack support is not available and will not use it.
>
> See https://www.openwall.com/lists/musl/2012/10/16/12
>
> This patch was written under the assumption that glibc is the only libc
> implementation which supports the required fields at the required
> offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
> Edge on the s390x and x86 architectures by bootstrapping Google's Go
> implementation with gcc-go.
>
> Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
>
> gcc/ChangeLog:
>
>         * common/config/s390/s390-common.c (s390_supports_split_stack):
>         Only support split-stack on glibc targets.
>         * config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
>         * config/i386/gnu.h (defined): Ditto.
> ---
> This version of the patch addresses feedback by Andrew Pinski and uses
> OPTION_GLIBC as well as opts->x_linux_libc == LIBC_GLIBC to detect glibc
> targets (instead of relying on TARGET_GLIBC_MAJOR).
>
>  gcc/common/config/s390/s390-common.c | 11 +++++++++--
>  gcc/config/i386/gnu-user-common.h    |  5 +++--
>  gcc/config/i386/gnu.h                |  6 +++++-
>  3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/common/config/s390/s390-common.c b/gcc/common/config/s390/s390-common.c
> index b6bc8501742..fc86e0bc5e7 100644
> --- a/gcc/common/config/s390/s390-common.c
> +++ b/gcc/common/config/s390/s390-common.c
> @@ -116,13 +116,20 @@ s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED,
>
>  /* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
>     We don't verify it, since earlier versions just have padding at
> -   its place, which works just as well.  */
> +   its place, which works just as well. For other libc implementations
> +   we disable the feature entirely to avoid corrupting the TCB.  */
>
>  static bool
>  s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
>                            struct gcc_options *opts ATTRIBUTE_UNUSED)
>  {
> -  return true;
> +  if (opts->x_linux_libc == LIBC_GLIBC) {
> +    return true;
> +  } else {
> +    if (report)
> +      error("%<-fsplit-stack%> currently only supported on GNU/Linux");
> +    return false;
> +  }
>  }
>
>  #undef TARGET_DEFAULT_TARGET_FLAGS
> diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
> index 00226f5a455..6e13315b5a3 100644
> --- a/gcc/config/i386/gnu-user-common.h
> +++ b/gcc/config/i386/gnu-user-common.h
> @@ -66,7 +66,8 @@ along with GCC; see the file COPYING3.  If not see
>  #define STACK_CHECK_STATIC_BUILTIN 1
>
>  /* We only build the -fsplit-stack support in libgcc if the
> -   assembler has full support for the CFI directives.  */
> -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
> +   assembler has full support for the CFI directives and
> +   targets glibc.  */
> +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC

OPTION_GLIBC can't be used here since OPTION_GLIBC is
evaluated at run-time:

https://gcc.gnu.org/pipermail/gcc-regression/2022-January/076271.html

>  #define TARGET_CAN_SPLIT_STACK
>  #endif
> diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
> index 25fbc07f58c..adfe817201e 100644
> --- a/gcc/config/i386/gnu.h
> +++ b/gcc/config/i386/gnu.h
> @@ -35,7 +35,11 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
>     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
>  #endif
>
> -#ifdef TARGET_LIBC_PROVIDES_SSP
> +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> +   field is only available for glibc. Disable -fsplit-stack for
> +   other libc implementation to avoid silent TCB corruptions.  */
> +
> +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
>
>  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
>  #define TARGET_THREAD_SSP_OFFSET        0x14



-- 
H.J.

Re: [PATCH v3] Disable -fsplit-stack support on non-glibc targets

[H.J. Lu]

On Fri, Jan 21, 2022 at 11:47 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 21, 2022 at 11:23 AM Richard Sandiford via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > soeren@soeren-tempel.net writes:
> > > From: Sören Tempel <soeren@soeren-tempel.net>
> > >
> > > The -fsplit-stack option requires the pthread_t TCB definition in the
> > > libc to provide certain struct fields at specific hardcoded offsets. As
> > > far as I know, only glibc provides these fields at the required offsets.
> > > Most notably, musl libc does not have these fields. However, since gcc
> > > accesses the fields using a fixed offset, this does not cause a
> > > compile-time error, but instead results in a silent memory corruption at
> > > run-time with musl libc. For example, on s390x libgcc's
> > > __stack_split_initialize CTOR will overwrite the cancel field in the
> > > pthread_t TCB on musl.
> > >
> > > The -fsplit-stack option is used within the gcc code base itself by
> > > gcc-go (if available). On musl-based systems with split-stack support
> > > (i.e. s390x or x86) this causes Go programs compiled with gcc-go to
> > > misbehave at run-time.
> > >
> > > This patch fixes gcc-go on musl by disabling -fsplit-stack in gcc itself
> > > since it is not supported on non-glibc targets anyhow. This is achieved
> > > by checking if gcc targets a glibc-based system. This check has been
> > > added for x86 and s390x, the rs6000 config already checks for
> > > TARGET_GLIBC_MAJOR. Other architectures do not have split-stack
> > > support. With this patch applied, the gcc-go configure script will
> > > detect that -fsplit-stack support is not available and will not use it.
> > >
> > > See https://www.openwall.com/lists/musl/2012/10/16/12
> > >
> > > This patch was written under the assumption that glibc is the only libc
> > > implementation which supports the required fields at the required
> > > offsets in the pthread_t TCB. The patch has been tested on Alpine Linux
> > > Edge on the s390x and x86 architectures by bootstrapping Google's Go
> > > implementation with gcc-go.
> > >
> > > Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
> > >
> > > gcc/ChangeLog:
> > >
> > >       * common/config/s390/s390-common.c (s390_supports_split_stack):
> > >       Only support split-stack on glibc targets.
> > >       * config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> > >       * config/i386/gnu.h (defined): Ditto.
> >
> > Thanks, pushed to trunk.
>
> This broke GCC bootstrap on Linux/i686:
>
> https://gcc.gnu.org/pipermail/gcc-regression/2022-January/076271.html
>

I opened:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104170

H.J.

Re: [PATCH v3] Disable -fsplit-stack support on non-glibc targets

[Jakub Jelinek]

On Fri, Jan 21, 2022 at 08:16:11PM +0100, soeren--- via Gcc-patches wrote:
> gcc/ChangeLog:
> 
> 	* common/config/s390/s390-common.c (s390_supports_split_stack):
> 	Only support split-stack on glibc targets.
> 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> 	* config/i386/gnu.h (defined): Ditto.

Besides breaking bootstrap, this doesn't do what it talks about:

> --- a/gcc/config/i386/gnu.h
> +++ b/gcc/config/i386/gnu.h
> @@ -35,7 +35,10 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
>     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
>  #endif
>  
> -#ifdef TARGET_LIBC_PROVIDES_SSP
> +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> +   field is only available for glibc.  Disable -fsplit-stack for
> +   other libc implementations to avoid silent TCB corruptions.  */
> +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
>  
>  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
>  #define TARGET_THREAD_SSP_OFFSET        0x14

Because this doesn't disable just -fsplit-stack support, but also
-fstack-protector*.
Does that one work on musl?
I think common/config/i386/i386-common.c (ix86_supports_split_stack)
should have been changed instead of the config/i386/gnu*.h headers.

	Jakub

Re: [PATCH v2] Disable -fsplit-stack support on non-glibc targets

[Sören Tempel]

"H.J. Lu" <hjl.tools@gmail.com> wrote:
> OPTION_GLIBC can't be used here since OPTION_GLIBC is
> evaluated at run-time:
> 
> https://gcc.gnu.org/pipermail/gcc-regression/2022-January/076271.html

Oops, my bad, sorry! This accidentally broke in one of the two cleanup
commits. Originally I justed use TARGET_GLIBC_MAJOR in PATCH v1. Would
that be acceptable?

Greetings,
Sören

[PATCH] x86: Properly disable -fsplit-stack support on non-glibc targets

[H.J. Lu]

On Fri, Jan 21, 2022 at 09:18:41PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Jan 21, 2022 at 08:16:11PM +0100, soeren--- via Gcc-patches wrote:
> > gcc/ChangeLog:
> > 
> > 	* common/config/s390/s390-common.c (s390_supports_split_stack):
> > 	Only support split-stack on glibc targets.
> > 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> > 	* config/i386/gnu.h (defined): Ditto.
> 
> Besides breaking bootstrap, this doesn't do what it talks about:
> 
> > --- a/gcc/config/i386/gnu.h
> > +++ b/gcc/config/i386/gnu.h
> > @@ -35,7 +35,10 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
> >     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
> >  #endif
> >  
> > -#ifdef TARGET_LIBC_PROVIDES_SSP
> > +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> > +   field is only available for glibc.  Disable -fsplit-stack for
> > +   other libc implementations to avoid silent TCB corruptions.  */
> > +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
> >  
> >  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
> >  #define TARGET_THREAD_SSP_OFFSET        0x14
> 
> Because this doesn't disable just -fsplit-stack support, but also
> -fstack-protector*.
> Does that one work on musl?
> I think common/config/i386/i386-common.c (ix86_supports_split_stack)
> should have been changed instead of the config/i386/gnu*.h headers.
> 

Like this?


H.J.
---
Revert x86 changes in

commit c163647ffbc9a20c8feb6e079dbecccfe016c82e
Author: Soren Tempel <soeren@soeren-tempel.net>
Date:   Fri Jan 21 19:22:46 2022 +0000

    Disable -fsplit-stack support on non-glibc targets

and change ix86_supports_split_stack to return true only on glibc.

	PR bootstrap/104170
	* common/config/i386/i386-common.cc (ix86_supports_split_stack):
	Return true only on glibc.
	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN):
	Revert commit c163647ffbc.
	* config/i386/gnu.h (TARGET_LIBC_PROVIDES_SSP): Likewise.
---
 gcc/common/config/i386/i386-common.cc | 4 ++--
 gcc/config/i386/gnu-user-common.h     | 5 ++---
 gcc/config/i386/gnu.h                 | 5 +----
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/gcc/common/config/i386/i386-common.cc b/gcc/common/config/i386/i386-common.cc
index cae0b880d79..78e6ff730aa 100644
--- a/gcc/common/config/i386/i386-common.cc
+++ b/gcc/common/config/i386/i386-common.cc
@@ -1715,9 +1715,9 @@ ix86_option_init_struct (struct gcc_options *opts)
 
 static bool
 ix86_supports_split_stack (bool report ATTRIBUTE_UNUSED,
-			   struct gcc_options *opts ATTRIBUTE_UNUSED)
+			   struct gcc_options *opts)
 {
-  bool ret = true;
+  bool ret = opts->x_linux_libc == LIBC_GLIBC;
 
 #ifndef TARGET_THREAD_SPLIT_STACK_OFFSET
   if (report)
diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
index 7525f788a9c..23b54c5be52 100644
--- a/gcc/config/i386/gnu-user-common.h
+++ b/gcc/config/i386/gnu-user-common.h
@@ -66,8 +66,7 @@ along with GCC; see the file COPYING3.  If not see
 #define STACK_CHECK_STATIC_BUILTIN 1
 
 /* We only build the -fsplit-stack support in libgcc if the
-   assembler has full support for the CFI directives and
-   targets glibc.  */
-#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
+   assembler has full support for the CFI directives.  */
+#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
 #define TARGET_CAN_SPLIT_STACK
 #endif
diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
index daa505a5d45..401e60c9a02 100644
--- a/gcc/config/i386/gnu.h
+++ b/gcc/config/i386/gnu.h
@@ -35,10 +35,7 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
    crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
 #endif
 
-/* -fsplit-stack uses a field in the TCB at a fixed offset. This
-   field is only available for glibc.  Disable -fsplit-stack for
-   other libc implementations to avoid silent TCB corruptions.  */
-#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
+#ifdef TARGET_LIBC_PROVIDES_SSP
 
 /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
 #define TARGET_THREAD_SSP_OFFSET        0x14
-- 
2.34.1

Re: [PATCH] x86: Properly disable -fsplit-stack support on non-glibc targets

[Jakub Jelinek]

On Fri, Jan 21, 2022 at 01:31:32PM -0800, H.J. Lu wrote:
> On Fri, Jan 21, 2022 at 09:18:41PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > On Fri, Jan 21, 2022 at 08:16:11PM +0100, soeren--- via Gcc-patches wrote:
> > > gcc/ChangeLog:
> > > 
> > > 	* common/config/s390/s390-common.c (s390_supports_split_stack):
> > > 	Only support split-stack on glibc targets.
> > > 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> > > 	* config/i386/gnu.h (defined): Ditto.
> > 
> > Besides breaking bootstrap, this doesn't do what it talks about:
> > 
> > > --- a/gcc/config/i386/gnu.h
> > > +++ b/gcc/config/i386/gnu.h
> > > @@ -35,7 +35,10 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
> > >     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
> > >  #endif
> > >  
> > > -#ifdef TARGET_LIBC_PROVIDES_SSP
> > > +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> > > +   field is only available for glibc.  Disable -fsplit-stack for
> > > +   other libc implementations to avoid silent TCB corruptions.  */
> > > +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
> > >  
> > >  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
> > >  #define TARGET_THREAD_SSP_OFFSET        0x14
> > 
> > Because this doesn't disable just -fsplit-stack support, but also
> > -fstack-protector*.
> > Does that one work on musl?
> > I think common/config/i386/i386-common.c (ix86_supports_split_stack)
> > should have been changed instead of the config/i386/gnu*.h headers.
> > 
> 
> Like this?
> 
> 
> H.J.
> ---
> Revert x86 changes in
> 
> commit c163647ffbc9a20c8feb6e079dbecccfe016c82e
> Author: Soren Tempel <soeren@soeren-tempel.net>
> Date:   Fri Jan 21 19:22:46 2022 +0000
> 
>     Disable -fsplit-stack support on non-glibc targets
> 
> and change ix86_supports_split_stack to return true only on glibc.
> 
> 	PR bootstrap/104170
> 	* common/config/i386/i386-common.cc (ix86_supports_split_stack):
> 	Return true only on glibc.
> 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN):
> 	Revert commit c163647ffbc.
> 	* config/i386/gnu.h (TARGET_LIBC_PROVIDES_SSP): Likewise.
> ---
>  gcc/common/config/i386/i386-common.cc | 4 ++--
>  gcc/config/i386/gnu-user-common.h     | 5 ++---
>  gcc/config/i386/gnu.h                 | 5 +----
>  3 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/common/config/i386/i386-common.cc b/gcc/common/config/i386/i386-common.cc
> index cae0b880d79..78e6ff730aa 100644
> --- a/gcc/common/config/i386/i386-common.cc
> +++ b/gcc/common/config/i386/i386-common.cc
> @@ -1715,9 +1715,9 @@ ix86_option_init_struct (struct gcc_options *opts)
>  
>  static bool
>  ix86_supports_split_stack (bool report ATTRIBUTE_UNUSED,
> -			   struct gcc_options *opts ATTRIBUTE_UNUSED)
> +			   struct gcc_options *opts)
>  {
> -  bool ret = true;
> +  bool ret = opts->x_linux_libc == LIBC_GLIBC;
>  
>  #ifndef TARGET_THREAD_SPLIT_STACK_OFFSET
>    if (report)

Almost.
I'd think you should honor report and drop ATTRIBUTE_UNUSED from both args.

So instead:
  bool ret = true;

  if (opts->x_linux_libc != LIBC_GLIBC)
    {
      if (report)
	error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
      return false;
    }
#ifndef TARGET_THREAD_SPLIT_STACK_OFFSET
...

	Jakub

[PATCH v2] x86: Properly disable -fsplit-stack support on non-glibc targets

[H.J. Lu]

On Fri, Jan 21, 2022 at 10:42:03PM +0100, Jakub Jelinek wrote:
> On Fri, Jan 21, 2022 at 01:31:32PM -0800, H.J. Lu wrote:
> > On Fri, Jan 21, 2022 at 09:18:41PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > > On Fri, Jan 21, 2022 at 08:16:11PM +0100, soeren--- via Gcc-patches wrote:
> > > > gcc/ChangeLog:
> > > > 
> > > > 	* common/config/s390/s390-common.c (s390_supports_split_stack):
> > > > 	Only support split-stack on glibc targets.
> > > > 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN): Ditto.
> > > > 	* config/i386/gnu.h (defined): Ditto.
> > > 
> > > Besides breaking bootstrap, this doesn't do what it talks about:
> > > 
> > > > --- a/gcc/config/i386/gnu.h
> > > > +++ b/gcc/config/i386/gnu.h
> > > > @@ -35,7 +35,10 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
> > > >     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
> > > >  #endif
> > > >  
> > > > -#ifdef TARGET_LIBC_PROVIDES_SSP
> > > > +/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> > > > +   field is only available for glibc.  Disable -fsplit-stack for
> > > > +   other libc implementations to avoid silent TCB corruptions.  */
> > > > +#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
> > > >  
> > > >  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
> > > >  #define TARGET_THREAD_SSP_OFFSET        0x14
> > > 
> > > Because this doesn't disable just -fsplit-stack support, but also
> > > -fstack-protector*.
> > > Does that one work on musl?
> > > I think common/config/i386/i386-common.c (ix86_supports_split_stack)
> > > should have been changed instead of the config/i386/gnu*.h headers.
> > > 
> > 
> > Like this?
> > 
> > 
> > H.J.
> > ---
> > Revert x86 changes in
> > 
> > commit c163647ffbc9a20c8feb6e079dbecccfe016c82e
> > Author: Soren Tempel <soeren@soeren-tempel.net>
> > Date:   Fri Jan 21 19:22:46 2022 +0000
> > 
> >     Disable -fsplit-stack support on non-glibc targets
> > 
> > and change ix86_supports_split_stack to return true only on glibc.
> > 
> > 	PR bootstrap/104170
> > 	* common/config/i386/i386-common.cc (ix86_supports_split_stack):
> > 	Return true only on glibc.
> > 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN):
> > 	Revert commit c163647ffbc.
> > 	* config/i386/gnu.h (TARGET_LIBC_PROVIDES_SSP): Likewise.
> > ---
> >  gcc/common/config/i386/i386-common.cc | 4 ++--
> >  gcc/config/i386/gnu-user-common.h     | 5 ++---
> >  gcc/config/i386/gnu.h                 | 5 +----
> >  3 files changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/gcc/common/config/i386/i386-common.cc b/gcc/common/config/i386/i386-common.cc
> > index cae0b880d79..78e6ff730aa 100644
> > --- a/gcc/common/config/i386/i386-common.cc
> > +++ b/gcc/common/config/i386/i386-common.cc
> > @@ -1715,9 +1715,9 @@ ix86_option_init_struct (struct gcc_options *opts)
> >  
> >  static bool
> >  ix86_supports_split_stack (bool report ATTRIBUTE_UNUSED,
> > -			   struct gcc_options *opts ATTRIBUTE_UNUSED)
> > +			   struct gcc_options *opts)
> >  {
> > -  bool ret = true;
> > +  bool ret = opts->x_linux_libc == LIBC_GLIBC;
> >  
> >  #ifndef TARGET_THREAD_SPLIT_STACK_OFFSET
> >    if (report)
> 
> Almost.
> I'd think you should honor report and drop ATTRIBUTE_UNUSED from both args.
> 
> So instead:
>   bool ret = true;
> 
>   if (opts->x_linux_libc != LIBC_GLIBC)
>     {
>       if (report)
> 	error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
>       return false;
>     }
> #ifndef TARGET_THREAD_SPLIT_STACK_OFFSET
> ...
> 

Here is the v2 patch.


H.J.
---
Revert x86 changes in

commit c163647ffbc9a20c8feb6e079dbecccfe016c82e
Author: Soren Tempel <soeren@soeren-tempel.net>
Date:   Fri Jan 21 19:22:46 2022 +0000

    Disable -fsplit-stack support on non-glibc targets

and change ix86_supports_split_stack to return true only on glibc.

	PR bootstrap/104170
	* common/config/i386/i386-common.cc (ix86_supports_split_stack):
	Return true only on glibc.
	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN):
	Revert commit c163647ffbc.
	* config/i386/gnu.h (TARGET_LIBC_PROVIDES_SSP): Likewise.
---
 gcc/common/config/i386/i386-common.cc | 17 +++++++++++------
 gcc/config/i386/gnu-user-common.h     |  5 ++---
 gcc/config/i386/gnu.h                 |  5 +----
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/gcc/common/config/i386/i386-common.cc b/gcc/common/config/i386/i386-common.cc
index cae0b880d79..7496d179892 100644
--- a/gcc/common/config/i386/i386-common.cc
+++ b/gcc/common/config/i386/i386-common.cc
@@ -1714,16 +1714,21 @@ ix86_option_init_struct (struct gcc_options *opts)
    field in the TCB, so they cannot be used together.  */
 
 static bool
-ix86_supports_split_stack (bool report ATTRIBUTE_UNUSED,
+ix86_supports_split_stack (bool report,
 			   struct gcc_options *opts ATTRIBUTE_UNUSED)
 {
+#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
+  if (opts->x_linux_libc != LIBC_GLIBC)
+#endif
+    {
+      if (report)
+	error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
+      return false;
+    }
+
   bool ret = true;
 
-#ifndef TARGET_THREAD_SPLIT_STACK_OFFSET
-  if (report)
-    error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
-  ret = false;
-#else
+#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
   if (!HAVE_GAS_CFI_PERSONALITY_DIRECTIVE)
     {
       if (report)
diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
index 7525f788a9c..23b54c5be52 100644
--- a/gcc/config/i386/gnu-user-common.h
+++ b/gcc/config/i386/gnu-user-common.h
@@ -66,8 +66,7 @@ along with GCC; see the file COPYING3.  If not see
 #define STACK_CHECK_STATIC_BUILTIN 1
 
 /* We only build the -fsplit-stack support in libgcc if the
-   assembler has full support for the CFI directives and
-   targets glibc.  */
-#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
+   assembler has full support for the CFI directives.  */
+#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
 #define TARGET_CAN_SPLIT_STACK
 #endif
diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
index daa505a5d45..401e60c9a02 100644
--- a/gcc/config/i386/gnu.h
+++ b/gcc/config/i386/gnu.h
@@ -35,10 +35,7 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
    crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
 #endif
 
-/* -fsplit-stack uses a field in the TCB at a fixed offset. This
-   field is only available for glibc.  Disable -fsplit-stack for
-   other libc implementations to avoid silent TCB corruptions.  */
-#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
+#ifdef TARGET_LIBC_PROVIDES_SSP
 
 /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
 #define TARGET_THREAD_SSP_OFFSET        0x14
-- 
2.34.1

Re: [PATCH v2] x86: Properly disable -fsplit-stack support on non-glibc targets

[Jakub Jelinek]

On Fri, Jan 21, 2022 at 01:57:57PM -0800, H.J. Lu wrote:
> Revert x86 changes in
> 
> commit c163647ffbc9a20c8feb6e079dbecccfe016c82e
> Author: Soren Tempel <soeren@soeren-tempel.net>
> Date:   Fri Jan 21 19:22:46 2022 +0000
> 
>     Disable -fsplit-stack support on non-glibc targets
> 
> and change ix86_supports_split_stack to return true only on glibc.
> 
> 	PR bootstrap/104170
> 	* common/config/i386/i386-common.cc (ix86_supports_split_stack):
> 	Return true only on glibc.
> 	* config/i386/gnu-user-common.h (STACK_CHECK_STATIC_BUILTIN):
> 	Revert commit c163647ffbc.
> 	* config/i386/gnu.h (TARGET_LIBC_PROVIDES_SSP): Likewise.

Ok, thanks.

>  gcc/common/config/i386/i386-common.cc | 17 +++++++++++------
>  gcc/config/i386/gnu-user-common.h     |  5 ++---
>  gcc/config/i386/gnu.h                 |  5 +----
>  3 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/gcc/common/config/i386/i386-common.cc b/gcc/common/config/i386/i386-common.cc
> index cae0b880d79..7496d179892 100644
> --- a/gcc/common/config/i386/i386-common.cc
> +++ b/gcc/common/config/i386/i386-common.cc
> @@ -1714,16 +1714,21 @@ ix86_option_init_struct (struct gcc_options *opts)
>     field in the TCB, so they cannot be used together.  */
>  
>  static bool
> -ix86_supports_split_stack (bool report ATTRIBUTE_UNUSED,
> +ix86_supports_split_stack (bool report,
>  			   struct gcc_options *opts ATTRIBUTE_UNUSED)
>  {
> +#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
> +  if (opts->x_linux_libc != LIBC_GLIBC)
> +#endif
> +    {
> +      if (report)
> +	error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
> +      return false;
> +    }
> +
>    bool ret = true;
>  
> -#ifndef TARGET_THREAD_SPLIT_STACK_OFFSET
> -  if (report)
> -    error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
> -  ret = false;
> -#else
> +#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
>    if (!HAVE_GAS_CFI_PERSONALITY_DIRECTIVE)
>      {
>        if (report)
> diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
> index 7525f788a9c..23b54c5be52 100644
> --- a/gcc/config/i386/gnu-user-common.h
> +++ b/gcc/config/i386/gnu-user-common.h
> @@ -66,8 +66,7 @@ along with GCC; see the file COPYING3.  If not see
>  #define STACK_CHECK_STATIC_BUILTIN 1
>  
>  /* We only build the -fsplit-stack support in libgcc if the
> -   assembler has full support for the CFI directives and
> -   targets glibc.  */
> -#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE && OPTION_GLIBC
> +   assembler has full support for the CFI directives.  */
> +#if HAVE_GAS_CFI_PERSONALITY_DIRECTIVE
>  #define TARGET_CAN_SPLIT_STACK
>  #endif
> diff --git a/gcc/config/i386/gnu.h b/gcc/config/i386/gnu.h
> index daa505a5d45..401e60c9a02 100644
> --- a/gcc/config/i386/gnu.h
> +++ b/gcc/config/i386/gnu.h
> @@ -35,10 +35,7 @@ along with GCC.  If not, see <http://www.gnu.org/licenses/>.
>     crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s}"
>  #endif
>  
> -/* -fsplit-stack uses a field in the TCB at a fixed offset. This
> -   field is only available for glibc.  Disable -fsplit-stack for
> -   other libc implementations to avoid silent TCB corruptions.  */
> -#if defined (TARGET_LIBC_PROVIDES_SSP) && OPTION_GLIBC
> +#ifdef TARGET_LIBC_PROVIDES_SSP
>  
>  /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
>  #define TARGET_THREAD_SSP_OFFSET        0x14
> -- 
> 2.34.1

	Jakub

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Martin Liška]

Hello.

I've just noticed the patch broke a few cross compilers:

s390x-ibm-tpf:

/home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/s390/s390-common.cc: In function ‘bool s390_supports_split_stack(bool, gcc_options*)’:
/home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/s390/s390-common.cc:126:13: error: ‘struct gcc_options’ has no member named ‘x_linux_libc’
   126 |   if (opts->x_linux_libc == LIBC_GLIBC)
       |             ^~~~~~~~~~~~

i686-kopensolaris-gnu, i686-symbolics-gnu

/home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/i386/i386-common.cc: In function ‘bool ix86_supports_split_stack(bool, gcc_options*)’:
/home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/i386/i386-common.cc:1721:13: error: ‘struct gcc_options’ has no member named ‘x_linux_libc’
  1721 |   if (opts->x_linux_libc != LIBC_GLIBC)
       |             ^~~~~~~~~~~~
make[1]: *** [Makefile:2418: i386-common.o] Error 1

Can you please take a look? Btw. do you have a bugzilla account?

Cheers,
Martin

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Jakub Jelinek]

On Sat, Jan 22, 2022 at 10:32:21AM +0100, Martin Liška wrote:
> Hello.
> 
> I've just noticed the patch broke a few cross compilers:
> 
> s390x-ibm-tpf:
> 
> /home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/s390/s390-common.cc: In function ‘bool s390_supports_split_stack(bool, gcc_options*)’:
> /home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/s390/s390-common.cc:126:13: error: ‘struct gcc_options’ has no member named ‘x_linux_libc’
>   126 |   if (opts->x_linux_libc == LIBC_GLIBC)
>       |             ^~~~~~~~~~~~
> 
> i686-kopensolaris-gnu, i686-symbolics-gnu
> 
> /home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/i386/i386-common.cc: In function ‘bool ix86_supports_split_stack(bool, gcc_options*)’:
> /home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/i386/i386-common.cc:1721:13: error: ‘struct gcc_options’ has no member named ‘x_linux_libc’
>  1721 |   if (opts->x_linux_libc != LIBC_GLIBC)
>       |             ^~~~~~~~~~~~
> make[1]: *** [Makefile:2418: i386-common.o] Error 1
> 
> Can you please take a look? Btw. do you have a bugzilla account?

I bet instead of opts->x_linux_libc != LIBC_GLIBC it needs to use
#ifdef OPTION_GLIBC
  if (!OPTION_GLIBC)
#endif
or so.  I think the first committed patch actually used that
but used it in #if directive, which is wrong because it is something
that needs to be evaluated at runtime.

	Jakub

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Jakub Jelinek]

On Sat, Jan 22, 2022 at 10:32:21AM +0100, Martin Liška wrote:
> I've just noticed the patch broke a few cross compilers:
> 
> s390x-ibm-tpf:
> 
> /home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/s390/s390-common.cc: In function ‘bool s390_supports_split_stack(bool, gcc_options*)’:
> /home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/s390/s390-common.cc:126:13: error: ‘struct gcc_options’ has no member named ‘x_linux_libc’
>   126 |   if (opts->x_linux_libc == LIBC_GLIBC)
>       |             ^~~~~~~~~~~~
> 
> i686-kopensolaris-gnu, i686-symbolics-gnu
> 
> /home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/i386/i386-common.cc: In function ‘bool ix86_supports_split_stack(bool, gcc_options*)’:
> /home/marxin/buildworker/zen2-cross-compilers/build/gcc/common/config/i386/i386-common.cc:1721:13: error: ‘struct gcc_options’ has no member named ‘x_linux_libc’
>  1721 |   if (opts->x_linux_libc != LIBC_GLIBC)
>       |             ^~~~~~~~~~~~
> make[1]: *** [Makefile:2418: i386-common.o] Error 1
> 
> Can you please take a look? Btw. do you have a bugzilla account?

Actually, I suspect we either need something like following patch,
or need to change gcc/config/{linux,rs6000/linux{,64},alpha/linux}.h
so that next to those OPTION_GLIBC etc. macros it also defines versions
of those macros with opts argument.

Untested and I don't have cycles to test this right now.

2022-01-22  Jakub Jelinek  <jakub@redhat.com>

	* common/config/s390/s390-common.cc (s390_supports_split_stack): Re-add
	ATTRIBUTE_UNUSED to opts parameter.  If OPTION_GLIBC is defined and
	SINGLE_LIBC is defined too, use OPTION_GLIBC as condition, otherwise
	if OPTION_GLIBC is defined, use opts->x_linux_libc == LIBC_GLIBC as
	condition, otherwise assume if (false).
	* common/config/i386/i386-common.cc (ix86_supports_split_stack): If
	OPTION_GLIBC is defined and SINGLE_LIBC is defined too, use
	!OPTION_GLIBC as condition, otherwise if OPTION_GLIBC is defined,
	use opts->x_linux_libc != LIBC_GLIBC as condition, otherwise assume
	if (true).

--- gcc/common/config/s390/s390-common.cc.jj	2022-01-21 22:43:22.847719836 +0100
+++ gcc/common/config/s390/s390-common.cc	2022-01-22 13:05:31.205118265 +0100
@@ -121,10 +121,16 @@ s390_handle_option (struct gcc_options *
 
 static bool
 s390_supports_split_stack (bool report,
-			   struct gcc_options *opts)
+			   struct gcc_options *opts ATTRIBUTE_UNUSED)
 {
+#ifdef OPTION_GLIBC
+#ifdef SINGLE_LIBC
+  if (OPTION_GLIBC)
+#else
   if (opts->x_linux_libc == LIBC_GLIBC)
+#endif
     return true;
+#endif
 
   if (report)
     error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
--- gcc/common/config/i386/i386-common.cc.jj	2022-01-22 12:02:24.572563780 +0100
+++ gcc/common/config/i386/i386-common.cc	2022-01-22 13:07:12.223700957 +0100
@@ -1717,9 +1717,13 @@ static bool
 ix86_supports_split_stack (bool report,
 			   struct gcc_options *opts ATTRIBUTE_UNUSED)
 {
-#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
+#if defined(TARGET_THREAD_SPLIT_STACK_OFFSET) && defined(OPTION_GLIBC)
+#ifdef SINGLE_LIBC
+  if (!OPTION_GLIBC)
+#else
   if (opts->x_linux_libc != LIBC_GLIBC)
 #endif
+#endif
     {
       if (report)
 	error ("%<-fsplit-stack%> currently only supported on GNU/Linux");

> 
> Cheers,
> Martin

	Jakub

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Jakub Jelinek]

On Sat, Jan 22, 2022 at 01:16:38PM +0100, Jakub Jelinek via Gcc-patches wrote:
> Actually, I suspect we either need something like following patch,
> or need to change gcc/config/{linux,rs6000/linux{,64},alpha/linux}.h
> so that next to those OPTION_GLIBC etc. macros it also defines versions
> of those macros with opts argument.

And here is a larger but perhaps cleaner patch that matches how e.g.
options.h defines TARGET_WHATEVER_P(opts) options and then TARGET_WHATEVER
too.

Only compile tested on x86_64-linux so far.

2022-01-22  Jakub Jelinek  <jakub@redhat.com>

	* config/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
	using OPTION_*_P macros.
	* config/alpha/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
	using OPTION_*_P macros.
	* config/rs6000/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
	using OPTION_*_P macros.
	* config/rs6000/linux64.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
	using OPTION_*_P macros.
	* config/fuchsia.h (OPTION_MUSL_P): Redefine.
	* config/glibc-stdint.h (OPTION_MUSL_P): Define if not defined.
	* common/config/s390/s390-common.cc (s390_supports_split_stack): Re-add
	ATTRIBUTE_UNUSED to opts parameter.  If OPTION_GLIBC_P is defined, use
	OPTION_GLIBC_P (opts) as condition, otherwise assume if (false).
	* common/config/i386/i386-common.cc (ix86_supports_split_stack): If
	OPTION_GLIBC_P is defined use !OPTION_GLIBC_P (opts) as condition,
	otherwise assume if (true).

--- gcc/config/linux.h.jj	2022-01-18 11:58:59.160988086 +0100
+++ gcc/config/linux.h	2022-01-22 18:42:25.476235564 +0100
@@ -29,18 +29,23 @@ see the files COPYING3 and COPYING.RUNTI
 
 /* C libraries supported on Linux.  */
 #ifdef SINGLE_LIBC
-#define OPTION_GLIBC  (DEFAULT_LIBC == LIBC_GLIBC)
-#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC)
-#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC)
-#undef OPTION_MUSL
-#define OPTION_MUSL   (DEFAULT_LIBC == LIBC_MUSL)
+#define OPTION_GLIBC_P(opts)	(DEFAULT_LIBC == LIBC_GLIBC)
+#define OPTION_UCLIBC_P(opts)	(DEFAULT_LIBC == LIBC_UCLIBC)
+#define OPTION_BIONIC_P(opts)	(DEFAULT_LIBC == LIBC_BIONIC)
+#undef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts)	(DEFAULT_LIBC == LIBC_MUSL)
 #else
-#define OPTION_GLIBC  (linux_libc == LIBC_GLIBC)
-#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC)
-#define OPTION_BIONIC (linux_libc == LIBC_BIONIC)
-#undef OPTION_MUSL
-#define OPTION_MUSL   (linux_libc == LIBC_MUSL)
+#define OPTION_GLIBC_P(opts)	((opts)->x_linux_libc == LIBC_GLIBC)
+#define OPTION_UCLIBC_P(opts)	((opts)->x_linux_libc == LIBC_UCLIBC)
+#define OPTION_BIONIC_P(opts)	((opts)->x_linux_libc == LIBC_BIONIC)
+#undef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts)	((opts)->x_linux_libc == LIBC_MUSL)
 #endif
+#define OPTION_GLIBC		OPTION_GLIBC_P (&global_options)
+#define OPTION_UCLIBC		OPTION_UCLIBC_P (&global_options)
+#define OPTION_BIONIC		OPTION_BIONIC_P (&global_options)
+#undef OPTION_MUSL
+#define OPTION_MUSL		OPTION_MUSL_P (&global_options)
 
 #define GNU_USER_TARGET_OS_CPP_BUILTINS()			\
     do {							\
--- gcc/config/alpha/linux.h.jj	2022-01-11 23:11:21.692299963 +0100
+++ gcc/config/alpha/linux.h	2022-01-22 18:43:59.739923743 +0100
@@ -58,18 +58,23 @@ along with GCC; see the file COPYING3.
 #define WCHAR_TYPE "int"
 
 #ifdef SINGLE_LIBC
-#define OPTION_GLIBC  (DEFAULT_LIBC == LIBC_GLIBC)
-#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC)
-#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC)
-#undef OPTION_MUSL
-#define OPTION_MUSL   (DEFAULT_LIBC == LIBC_MUSL)
+#define OPTION_GLIBC_P(opts)	(DEFAULT_LIBC == LIBC_GLIBC)
+#define OPTION_UCLIBC_P(opts)	(DEFAULT_LIBC == LIBC_UCLIBC)
+#define OPTION_BIONIC_P(opts)	(DEFAULT_LIBC == LIBC_BIONIC)
+#undef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts)	(DEFAULT_LIBC == LIBC_MUSL)
 #else
-#define OPTION_GLIBC  (linux_libc == LIBC_GLIBC)
-#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC)
-#define OPTION_BIONIC (linux_libc == LIBC_BIONIC)
-#undef OPTION_MUSL
-#define OPTION_MUSL   (linux_libc == LIBC_MUSL)
+#define OPTION_GLIBC_P(opts)	((opts)->x_linux_libc == LIBC_GLIBC)
+#define OPTION_UCLIBC_P(opts)	((opts)->x_linux_libc == LIBC_UCLIBC)
+#define OPTION_BIONIC_P(opts)	((opts)->x_linux_libc == LIBC_BIONIC)
+#undef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts)	((opts)->x_linux_libc == LIBC_MUSL)
 #endif
+#define OPTION_GLIBC		OPTION_GLIBC_P (&global_options)
+#define OPTION_UCLIBC		OPTION_UCLIBC_P (&global_options)
+#define OPTION_BIONIC		OPTION_BIONIC_P (&global_options)
+#undef OPTION_MUSL
+#define OPTION_MUSL		OPTION_MUSL_P (&global_options)
 
 /* Determine what functions are present at the runtime;
    this includes full c99 runtime and sincos.  */
--- gcc/config/rs6000/linux.h.jj	2022-01-11 23:11:21.939296492 +0100
+++ gcc/config/rs6000/linux.h	2022-01-22 18:42:59.834757410 +0100
@@ -27,18 +27,23 @@
 #define NO_PROFILE_COUNTERS 1
 
 #ifdef SINGLE_LIBC
-#define OPTION_GLIBC  (DEFAULT_LIBC == LIBC_GLIBC)
-#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC)
-#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC)
-#undef OPTION_MUSL
-#define OPTION_MUSL   (DEFAULT_LIBC == LIBC_MUSL)
+#define OPTION_GLIBC_P(opts)	(DEFAULT_LIBC == LIBC_GLIBC)
+#define OPTION_UCLIBC_P(opts)	(DEFAULT_LIBC == LIBC_UCLIBC)
+#define OPTION_BIONIC_P(opts)	(DEFAULT_LIBC == LIBC_BIONIC)
+#undef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts)	(DEFAULT_LIBC == LIBC_MUSL)
 #else
-#define OPTION_GLIBC  (linux_libc == LIBC_GLIBC)
-#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC)
-#define OPTION_BIONIC (linux_libc == LIBC_BIONIC)
-#undef OPTION_MUSL
-#define OPTION_MUSL   (linux_libc == LIBC_MUSL)
+#define OPTION_GLIBC_P(opts)	((opts)->x_linux_libc == LIBC_GLIBC)
+#define OPTION_UCLIBC_P(opts)	((opts)->x_linux_libc == LIBC_UCLIBC)
+#define OPTION_BIONIC_P(opts)	((opts)->x_linux_libc == LIBC_BIONIC)
+#undef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts)	((opts)->x_linux_libc == LIBC_MUSL)
 #endif
+#define OPTION_GLIBC		OPTION_GLIBC_P (&global_options)
+#define OPTION_UCLIBC		OPTION_UCLIBC_P (&global_options)
+#define OPTION_BIONIC		OPTION_BIONIC_P (&global_options)
+#undef OPTION_MUSL
+#define OPTION_MUSL		OPTION_MUSL_P (&global_options)
 
 /* Determine what functions are present at the runtime;
    this includes full c99 runtime and sincos.  */
--- gcc/config/rs6000/linux64.h.jj	2022-01-11 23:11:21.939296492 +0100
+++ gcc/config/rs6000/linux64.h	2022-01-22 18:43:37.830228647 +0100
@@ -265,18 +265,23 @@ extern int dot_symbols;
 #define OS_MISSING_POWERPC64 !TARGET_64BIT
 
 #ifdef SINGLE_LIBC
-#define OPTION_GLIBC  (DEFAULT_LIBC == LIBC_GLIBC)
-#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC)
-#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC)
-#undef OPTION_MUSL
-#define OPTION_MUSL   (DEFAULT_LIBC == LIBC_MUSL)
+#define OPTION_GLIBC_P(opts)	(DEFAULT_LIBC == LIBC_GLIBC)
+#define OPTION_UCLIBC_P(opts)	(DEFAULT_LIBC == LIBC_UCLIBC)
+#define OPTION_BIONIC_P(opts)	(DEFAULT_LIBC == LIBC_BIONIC)
+#undef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts)	(DEFAULT_LIBC == LIBC_MUSL)
 #else
-#define OPTION_GLIBC  (linux_libc == LIBC_GLIBC)
-#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC)
-#define OPTION_BIONIC (linux_libc == LIBC_BIONIC)
-#undef OPTION_MUSL
-#define OPTION_MUSL   (linux_libc == LIBC_MUSL)
+#define OPTION_GLIBC_P(opts)	((opts)->x_linux_libc == LIBC_GLIBC)
+#define OPTION_UCLIBC_P(opts)	((opts)->x_linux_libc == LIBC_UCLIBC)
+#define OPTION_BIONIC_P(opts)	((opts)->x_linux_libc == LIBC_BIONIC)
+#undef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts)	((opts)->x_linux_libc == LIBC_MUSL)
 #endif
+#define OPTION_GLIBC		OPTION_GLIBC_P (&global_options)
+#define OPTION_UCLIBC		OPTION_UCLIBC_P (&global_options)
+#define OPTION_BIONIC		OPTION_BIONIC_P (&global_options)
+#undef OPTION_MUSL
+#define OPTION_MUSL		OPTION_MUSL_P (&global_options)
 
 /* Determine what functions are present at the runtime;
    this includes full c99 runtime and sincos.  */
--- gcc/config/fuchsia.h.jj	2022-01-11 23:11:21.750299147 +0100
+++ gcc/config/fuchsia.h	2022-01-22 18:49:53.927996890 +0100
@@ -52,6 +52,8 @@ along with GCC; see the file COPYING3.
 /* We are using MUSL as our libc.  */
 #undef  OPTION_MUSL
 #define OPTION_MUSL 1
+#undef  OPTION_MUSL_P
+#define OPTION_MUSL_P(opts) 1
 
 #ifndef TARGET_SUB_OS_CPP_BUILTINS
 #define TARGET_SUB_OS_CPP_BUILTINS()
--- gcc/config/glibc-stdint.h.jj	2022-01-11 23:11:21.753299105 +0100
+++ gcc/config/glibc-stdint.h	2022-01-22 18:49:53.928996876 +0100
@@ -27,6 +27,9 @@ see the files COPYING3 and COPYING.RUNTI
 #ifndef OPTION_MUSL
 #define OPTION_MUSL 0
 #endif
+#ifndef OPTION_MUSL_P
+#define OPTION_MUSL_P(opts) 0
+#endif
 
 #define SIG_ATOMIC_TYPE "int"
 
--- gcc/common/config/s390/s390-common.cc.jj	2022-01-22 18:37:18.701504795 +0100
+++ gcc/common/config/s390/s390-common.cc	2022-01-22 18:39:09.820958400 +0100
@@ -121,10 +121,12 @@ s390_handle_option (struct gcc_options *
 
 static bool
 s390_supports_split_stack (bool report,
-			   struct gcc_options *opts)
+			   struct gcc_options *opts ATTRIBUTE_UNUSED)
 {
-  if (opts->x_linux_libc == LIBC_GLIBC)
+#ifdef OPTION_GLIBC_P
+  if (OPTION_GLIBC_P (opts))
     return true;
+#endif
 
   if (report)
     error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
--- gcc/common/config/i386/i386-common.cc.jj	2022-01-22 18:37:18.700504809 +0100
+++ gcc/common/config/i386/i386-common.cc	2022-01-22 18:39:09.821958386 +0100
@@ -1717,8 +1717,8 @@ static bool
 ix86_supports_split_stack (bool report,
 			   struct gcc_options *opts ATTRIBUTE_UNUSED)
 {
-#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
-  if (opts->x_linux_libc != LIBC_GLIBC)
+#if defined(TARGET_THREAD_SPLIT_STACK_OFFSET) && defined(OPTION_GLIBC_P)
+  if (!OPTION_GLIBC_P (opts))
 #endif
     {
       if (report)


	Jakub

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Uros Bizjak]

On Sat, Jan 22, 2022 at 7:04 PM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Sat, Jan 22, 2022 at 01:16:38PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > Actually, I suspect we either need something like following patch,
> > or need to change gcc/config/{linux,rs6000/linux{,64},alpha/linux}.h
> > so that next to those OPTION_GLIBC etc. macros it also defines versions
> > of those macros with opts argument.
>
> And here is a larger but perhaps cleaner patch that matches how e.g.
> options.h defines TARGET_WHATEVER_P(opts) options and then TARGET_WHATEVER
> too.
>
> Only compile tested on x86_64-linux so far.
>
> 2022-01-22  Jakub Jelinek  <jakub@redhat.com>
>
>         * config/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
>         OPTION_BIONIC_P, OPTION_MUSL_P): Define.
>         (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
>         using OPTION_*_P macros.
>         * config/alpha/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
>         OPTION_BIONIC_P, OPTION_MUSL_P): Define.
>         (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
>         using OPTION_*_P macros.
>         * config/rs6000/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
>         OPTION_BIONIC_P, OPTION_MUSL_P): Define.
>         (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
>         using OPTION_*_P macros.
>         * config/rs6000/linux64.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
>         OPTION_BIONIC_P, OPTION_MUSL_P): Define.
>         (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
>         using OPTION_*_P macros.
>         * config/fuchsia.h (OPTION_MUSL_P): Redefine.
>         * config/glibc-stdint.h (OPTION_MUSL_P): Define if not defined.
>         * common/config/s390/s390-common.cc (s390_supports_split_stack): Re-add
>         ATTRIBUTE_UNUSED to opts parameter.  If OPTION_GLIBC_P is defined, use
>         OPTION_GLIBC_P (opts) as condition, otherwise assume if (false).
>         * common/config/i386/i386-common.cc (ix86_supports_split_stack): If
>         OPTION_GLIBC_P is defined use !OPTION_GLIBC_P (opts) as condition,
>         otherwise assume if (true).

I wonder why every target defines its own set of #defines. I'd expect
that they include toplevel gcc/config/linux.h and inherit these
defines from it.

Uros.

>
> --- gcc/config/linux.h.jj       2022-01-18 11:58:59.160988086 +0100
> +++ gcc/config/linux.h  2022-01-22 18:42:25.476235564 +0100
> @@ -29,18 +29,23 @@ see the files COPYING3 and COPYING.RUNTI
>
>  /* C libraries supported on Linux.  */
>  #ifdef SINGLE_LIBC
> -#define OPTION_GLIBC  (DEFAULT_LIBC == LIBC_GLIBC)
> -#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC)
> -#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC)
> -#undef OPTION_MUSL
> -#define OPTION_MUSL   (DEFAULT_LIBC == LIBC_MUSL)
> +#define OPTION_GLIBC_P(opts)   (DEFAULT_LIBC == LIBC_GLIBC)
> +#define OPTION_UCLIBC_P(opts)  (DEFAULT_LIBC == LIBC_UCLIBC)
> +#define OPTION_BIONIC_P(opts)  (DEFAULT_LIBC == LIBC_BIONIC)
> +#undef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts)    (DEFAULT_LIBC == LIBC_MUSL)
>  #else
> -#define OPTION_GLIBC  (linux_libc == LIBC_GLIBC)
> -#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC)
> -#define OPTION_BIONIC (linux_libc == LIBC_BIONIC)
> -#undef OPTION_MUSL
> -#define OPTION_MUSL   (linux_libc == LIBC_MUSL)
> +#define OPTION_GLIBC_P(opts)   ((opts)->x_linux_libc == LIBC_GLIBC)
> +#define OPTION_UCLIBC_P(opts)  ((opts)->x_linux_libc == LIBC_UCLIBC)
> +#define OPTION_BIONIC_P(opts)  ((opts)->x_linux_libc == LIBC_BIONIC)
> +#undef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts)    ((opts)->x_linux_libc == LIBC_MUSL)
>  #endif
> +#define OPTION_GLIBC           OPTION_GLIBC_P (&global_options)
> +#define OPTION_UCLIBC          OPTION_UCLIBC_P (&global_options)
> +#define OPTION_BIONIC          OPTION_BIONIC_P (&global_options)
> +#undef OPTION_MUSL
> +#define OPTION_MUSL            OPTION_MUSL_P (&global_options)
>
>  #define GNU_USER_TARGET_OS_CPP_BUILTINS()                      \
>      do {                                                       \
> --- gcc/config/alpha/linux.h.jj 2022-01-11 23:11:21.692299963 +0100
> +++ gcc/config/alpha/linux.h    2022-01-22 18:43:59.739923743 +0100
> @@ -58,18 +58,23 @@ along with GCC; see the file COPYING3.
>  #define WCHAR_TYPE "int"
>
>  #ifdef SINGLE_LIBC
> -#define OPTION_GLIBC  (DEFAULT_LIBC == LIBC_GLIBC)
> -#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC)
> -#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC)
> -#undef OPTION_MUSL
> -#define OPTION_MUSL   (DEFAULT_LIBC == LIBC_MUSL)
> +#define OPTION_GLIBC_P(opts)   (DEFAULT_LIBC == LIBC_GLIBC)
> +#define OPTION_UCLIBC_P(opts)  (DEFAULT_LIBC == LIBC_UCLIBC)
> +#define OPTION_BIONIC_P(opts)  (DEFAULT_LIBC == LIBC_BIONIC)
> +#undef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts)    (DEFAULT_LIBC == LIBC_MUSL)
>  #else
> -#define OPTION_GLIBC  (linux_libc == LIBC_GLIBC)
> -#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC)
> -#define OPTION_BIONIC (linux_libc == LIBC_BIONIC)
> -#undef OPTION_MUSL
> -#define OPTION_MUSL   (linux_libc == LIBC_MUSL)
> +#define OPTION_GLIBC_P(opts)   ((opts)->x_linux_libc == LIBC_GLIBC)
> +#define OPTION_UCLIBC_P(opts)  ((opts)->x_linux_libc == LIBC_UCLIBC)
> +#define OPTION_BIONIC_P(opts)  ((opts)->x_linux_libc == LIBC_BIONIC)
> +#undef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts)    ((opts)->x_linux_libc == LIBC_MUSL)
>  #endif
> +#define OPTION_GLIBC           OPTION_GLIBC_P (&global_options)
> +#define OPTION_UCLIBC          OPTION_UCLIBC_P (&global_options)
> +#define OPTION_BIONIC          OPTION_BIONIC_P (&global_options)
> +#undef OPTION_MUSL
> +#define OPTION_MUSL            OPTION_MUSL_P (&global_options)
>
>  /* Determine what functions are present at the runtime;
>     this includes full c99 runtime and sincos.  */
> --- gcc/config/rs6000/linux.h.jj        2022-01-11 23:11:21.939296492 +0100
> +++ gcc/config/rs6000/linux.h   2022-01-22 18:42:59.834757410 +0100
> @@ -27,18 +27,23 @@
>  #define NO_PROFILE_COUNTERS 1
>
>  #ifdef SINGLE_LIBC
> -#define OPTION_GLIBC  (DEFAULT_LIBC == LIBC_GLIBC)
> -#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC)
> -#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC)
> -#undef OPTION_MUSL
> -#define OPTION_MUSL   (DEFAULT_LIBC == LIBC_MUSL)
> +#define OPTION_GLIBC_P(opts)   (DEFAULT_LIBC == LIBC_GLIBC)
> +#define OPTION_UCLIBC_P(opts)  (DEFAULT_LIBC == LIBC_UCLIBC)
> +#define OPTION_BIONIC_P(opts)  (DEFAULT_LIBC == LIBC_BIONIC)
> +#undef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts)    (DEFAULT_LIBC == LIBC_MUSL)
>  #else
> -#define OPTION_GLIBC  (linux_libc == LIBC_GLIBC)
> -#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC)
> -#define OPTION_BIONIC (linux_libc == LIBC_BIONIC)
> -#undef OPTION_MUSL
> -#define OPTION_MUSL   (linux_libc == LIBC_MUSL)
> +#define OPTION_GLIBC_P(opts)   ((opts)->x_linux_libc == LIBC_GLIBC)
> +#define OPTION_UCLIBC_P(opts)  ((opts)->x_linux_libc == LIBC_UCLIBC)
> +#define OPTION_BIONIC_P(opts)  ((opts)->x_linux_libc == LIBC_BIONIC)
> +#undef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts)    ((opts)->x_linux_libc == LIBC_MUSL)
>  #endif
> +#define OPTION_GLIBC           OPTION_GLIBC_P (&global_options)
> +#define OPTION_UCLIBC          OPTION_UCLIBC_P (&global_options)
> +#define OPTION_BIONIC          OPTION_BIONIC_P (&global_options)
> +#undef OPTION_MUSL
> +#define OPTION_MUSL            OPTION_MUSL_P (&global_options)
>
>  /* Determine what functions are present at the runtime;
>     this includes full c99 runtime and sincos.  */
> --- gcc/config/rs6000/linux64.h.jj      2022-01-11 23:11:21.939296492 +0100
> +++ gcc/config/rs6000/linux64.h 2022-01-22 18:43:37.830228647 +0100
> @@ -265,18 +265,23 @@ extern int dot_symbols;
>  #define OS_MISSING_POWERPC64 !TARGET_64BIT
>
>  #ifdef SINGLE_LIBC
> -#define OPTION_GLIBC  (DEFAULT_LIBC == LIBC_GLIBC)
> -#define OPTION_UCLIBC (DEFAULT_LIBC == LIBC_UCLIBC)
> -#define OPTION_BIONIC (DEFAULT_LIBC == LIBC_BIONIC)
> -#undef OPTION_MUSL
> -#define OPTION_MUSL   (DEFAULT_LIBC == LIBC_MUSL)
> +#define OPTION_GLIBC_P(opts)   (DEFAULT_LIBC == LIBC_GLIBC)
> +#define OPTION_UCLIBC_P(opts)  (DEFAULT_LIBC == LIBC_UCLIBC)
> +#define OPTION_BIONIC_P(opts)  (DEFAULT_LIBC == LIBC_BIONIC)
> +#undef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts)    (DEFAULT_LIBC == LIBC_MUSL)
>  #else
> -#define OPTION_GLIBC  (linux_libc == LIBC_GLIBC)
> -#define OPTION_UCLIBC (linux_libc == LIBC_UCLIBC)
> -#define OPTION_BIONIC (linux_libc == LIBC_BIONIC)
> -#undef OPTION_MUSL
> -#define OPTION_MUSL   (linux_libc == LIBC_MUSL)
> +#define OPTION_GLIBC_P(opts)   ((opts)->x_linux_libc == LIBC_GLIBC)
> +#define OPTION_UCLIBC_P(opts)  ((opts)->x_linux_libc == LIBC_UCLIBC)
> +#define OPTION_BIONIC_P(opts)  ((opts)->x_linux_libc == LIBC_BIONIC)
> +#undef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts)    ((opts)->x_linux_libc == LIBC_MUSL)
>  #endif
> +#define OPTION_GLIBC           OPTION_GLIBC_P (&global_options)
> +#define OPTION_UCLIBC          OPTION_UCLIBC_P (&global_options)
> +#define OPTION_BIONIC          OPTION_BIONIC_P (&global_options)
> +#undef OPTION_MUSL
> +#define OPTION_MUSL            OPTION_MUSL_P (&global_options)
>
>  /* Determine what functions are present at the runtime;
>     this includes full c99 runtime and sincos.  */
> --- gcc/config/fuchsia.h.jj     2022-01-11 23:11:21.750299147 +0100
> +++ gcc/config/fuchsia.h        2022-01-22 18:49:53.927996890 +0100
> @@ -52,6 +52,8 @@ along with GCC; see the file COPYING3.
>  /* We are using MUSL as our libc.  */
>  #undef  OPTION_MUSL
>  #define OPTION_MUSL 1
> +#undef  OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts) 1
>
>  #ifndef TARGET_SUB_OS_CPP_BUILTINS
>  #define TARGET_SUB_OS_CPP_BUILTINS()
> --- gcc/config/glibc-stdint.h.jj        2022-01-11 23:11:21.753299105 +0100
> +++ gcc/config/glibc-stdint.h   2022-01-22 18:49:53.928996876 +0100
> @@ -27,6 +27,9 @@ see the files COPYING3 and COPYING.RUNTI
>  #ifndef OPTION_MUSL
>  #define OPTION_MUSL 0
>  #endif
> +#ifndef OPTION_MUSL_P
> +#define OPTION_MUSL_P(opts) 0
> +#endif
>
>  #define SIG_ATOMIC_TYPE "int"
>
> --- gcc/common/config/s390/s390-common.cc.jj    2022-01-22 18:37:18.701504795 +0100
> +++ gcc/common/config/s390/s390-common.cc       2022-01-22 18:39:09.820958400 +0100
> @@ -121,10 +121,12 @@ s390_handle_option (struct gcc_options *
>
>  static bool
>  s390_supports_split_stack (bool report,
> -                          struct gcc_options *opts)
> +                          struct gcc_options *opts ATTRIBUTE_UNUSED)
>  {
> -  if (opts->x_linux_libc == LIBC_GLIBC)
> +#ifdef OPTION_GLIBC_P
> +  if (OPTION_GLIBC_P (opts))
>      return true;
> +#endif
>
>    if (report)
>      error ("%<-fsplit-stack%> currently only supported on GNU/Linux");
> --- gcc/common/config/i386/i386-common.cc.jj    2022-01-22 18:37:18.700504809 +0100
> +++ gcc/common/config/i386/i386-common.cc       2022-01-22 18:39:09.821958386 +0100
> @@ -1717,8 +1717,8 @@ static bool
>  ix86_supports_split_stack (bool report,
>                            struct gcc_options *opts ATTRIBUTE_UNUSED)
>  {
> -#ifdef TARGET_THREAD_SPLIT_STACK_OFFSET
> -  if (opts->x_linux_libc != LIBC_GLIBC)
> +#if defined(TARGET_THREAD_SPLIT_STACK_OFFSET) && defined(OPTION_GLIBC_P)
> +  if (!OPTION_GLIBC_P (opts))
>  #endif
>      {
>        if (report)
>
>
>         Jakub
>

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Jakub Jelinek]

On Sun, Jan 23, 2022 at 10:06:24AM +0100, Uros Bizjak wrote:
> > 2022-01-22  Jakub Jelinek  <jakub@redhat.com>
> >
> >         * config/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> >         OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> >         (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> >         using OPTION_*_P macros.
> >         * config/alpha/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> >         OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> >         (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> >         using OPTION_*_P macros.
> >         * config/rs6000/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> >         OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> >         (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> >         using OPTION_*_P macros.
> >         * config/rs6000/linux64.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> >         OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> >         (OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> >         using OPTION_*_P macros.
> >         * config/fuchsia.h (OPTION_MUSL_P): Redefine.
> >         * config/glibc-stdint.h (OPTION_MUSL_P): Define if not defined.
> >         * common/config/s390/s390-common.cc (s390_supports_split_stack): Re-add
> >         ATTRIBUTE_UNUSED to opts parameter.  If OPTION_GLIBC_P is defined, use
> >         OPTION_GLIBC_P (opts) as condition, otherwise assume if (false).
> >         * common/config/i386/i386-common.cc (ix86_supports_split_stack): If
> >         OPTION_GLIBC_P is defined use !OPTION_GLIBC_P (opts) as condition,
> >         otherwise assume if (true).
> 
> I wonder why every target defines its own set of #defines. I'd expect
> that they include toplevel gcc/config/linux.h and inherit these
> defines from it.

Not every target, most of the Linux targets do use config/linux.h,
just ppc* and alpha haven't been converted to do so.
It would be nice to have that done at some point, but to me that
looks like stage1 task rather than stage4.

	Jakub

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Jakub Jelinek]

On Sat, Jan 22, 2022 at 07:03:48PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Sat, Jan 22, 2022 at 01:16:38PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > Actually, I suspect we either need something like following patch,
> > or need to change gcc/config/{linux,rs6000/linux{,64},alpha/linux}.h
> > so that next to those OPTION_GLIBC etc. macros it also defines versions
> > of those macros with opts argument.
> 
> And here is a larger but perhaps cleaner patch that matches how e.g.
> options.h defines TARGET_WHATEVER_P(opts) options and then TARGET_WHATEVER
> too.
> 
> Only compile tested on x86_64-linux so far.

Bootstrapped/regtested on x86_64-linux and i686-linux now, ok for trunk?

> 2022-01-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* config/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> 	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> 	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> 	using OPTION_*_P macros.
> 	* config/alpha/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> 	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> 	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> 	using OPTION_*_P macros.
> 	* config/rs6000/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> 	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> 	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> 	using OPTION_*_P macros.
> 	* config/rs6000/linux64.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> 	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> 	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> 	using OPTION_*_P macros.
> 	* config/fuchsia.h (OPTION_MUSL_P): Redefine.
> 	* config/glibc-stdint.h (OPTION_MUSL_P): Define if not defined.
> 	* common/config/s390/s390-common.cc (s390_supports_split_stack): Re-add
> 	ATTRIBUTE_UNUSED to opts parameter.  If OPTION_GLIBC_P is defined, use
> 	OPTION_GLIBC_P (opts) as condition, otherwise assume if (false).
> 	* common/config/i386/i386-common.cc (ix86_supports_split_stack): If
> 	OPTION_GLIBC_P is defined use !OPTION_GLIBC_P (opts) as condition,
> 	otherwise assume if (true).

	Jakub

Re: [PATCH] Disable -fsplit-stack support on non-glibc targets

[Richard Biener]

On Mon, 24 Jan 2022, Jakub Jelinek wrote:

> On Sat, Jan 22, 2022 at 07:03:48PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > On Sat, Jan 22, 2022 at 01:16:38PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > > Actually, I suspect we either need something like following patch,
> > > or need to change gcc/config/{linux,rs6000/linux{,64},alpha/linux}.h
> > > so that next to those OPTION_GLIBC etc. macros it also defines versions
> > > of those macros with opts argument.
> > 
> > And here is a larger but perhaps cleaner patch that matches how e.g.
> > options.h defines TARGET_WHATEVER_P(opts) options and then TARGET_WHATEVER
> > too.
> > 
> > Only compile tested on x86_64-linux so far.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux now, ok for trunk?

OK.

Thanks,
Richard.

> > 2022-01-22  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	* config/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> > 	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> > 	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> > 	using OPTION_*_P macros.
> > 	* config/alpha/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> > 	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> > 	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> > 	using OPTION_*_P macros.
> > 	* config/rs6000/linux.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> > 	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> > 	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> > 	using OPTION_*_P macros.
> > 	* config/rs6000/linux64.h (OPTION_GLIBC_P, OPTION_UCLIBC_P,
> > 	OPTION_BIONIC_P, OPTION_MUSL_P): Define.
> > 	(OPTION_GLIBC, OPTION_UCLIBC, OPTION_BIONIC, OPTION_MUSL): Redefine
> > 	using OPTION_*_P macros.
> > 	* config/fuchsia.h (OPTION_MUSL_P): Redefine.
> > 	* config/glibc-stdint.h (OPTION_MUSL_P): Define if not defined.
> > 	* common/config/s390/s390-common.cc (s390_supports_split_stack): Re-add
> > 	ATTRIBUTE_UNUSED to opts parameter.  If OPTION_GLIBC_P is defined, use
> > 	OPTION_GLIBC_P (opts) as condition, otherwise assume if (false).
> > 	* common/config/i386/i386-common.cc (ix86_supports_split_stack): If
> > 	OPTION_GLIBC_P is defined use !OPTION_GLIBC_P (opts) as condition,
> > 	otherwise assume if (true).
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)