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

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

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
  

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
  

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.
  

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