Message ID | YesmNERXN3olQRPU@gmail.com |
---|---|
State | New |
Headers | show |
Series | x86: Properly disable -fsplit-stack support on non-glibc targets | expand |
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
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