Summary: | CET shouldn't be enabled in 32-bit run-time libraries by default | ||
---|---|---|---|
Product: | gcc | Reporter: | H.J. Lu <hjl.tools> |
Component: | target | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | fw, sherpya |
Priority: | P3 | ||
Version: | 8.0 | ||
Target Milestone: | 8.0 | ||
Host: | Target: | x86_64-*-*, i?86-*-* | |
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2018-02-12 00:00:00 | |
Bug Depends on: | |||
Bug Blocks: | 81652 | ||
Attachments: |
patch
patch #2 patch #3 |
Description
H.J. Lu
2018-01-31 13:21:18 UTC
How old are the CPUs which treat it as UD? Older than i686/Pentium Pro? Thanks. (In reply to Florian Weimer from comment #1) > How old are the CPUs which treat it as UD? Older than i686/Pentium Pro? > Thanks. They are NOPs since Pentium Pro. I get illegal instruction on a AMD GEODE (something like i586): processor : 0 vendor_id : AuthenticAMD cpu family : 5 model : 10 model name : Geode(TM) Integrated Processor by AMD PCS stepping : 2 cpu MHz : 498.044 cache size : 128 KB fdiv_bug : no f00f_bug : no coma_bug : no fpu : yes fpu_exception : yes cpuid level : 1 wp : yes flags : fpu de pse tsc msr cx8 sep pge cmov clflush mmx mmxext 3dnowext 3dnow cpuid 3dnowprefetch vmmcall bugs : sysret_ss_attrs spectre_v1 spectre_v2 bogomips : 996.08 clflush size : 32 cache_alignment : 32 address sizes : 32 bits physical, 32 bits virtual power management: simple testcase: __asm volatile("endbr32"); Created attachment 43400 [details]
patch
Beside the patch configure files have to be regenerated. (In reply to igor.v.tsimbalist from comment #4) > Created attachment 43400 [details] > patch 2 questions: 1. Should 32-bit multilib target libraries compiled on 64-bit host enable CET? 2. Should 64-bit multilib target libraries compiled with 32-bit compiler enable CET? (In reply to H.J. Lu from comment #6) > (In reply to igor.v.tsimbalist from comment #4) > > Created attachment 43400 [details] > > patch > > 2 questions: > > 1. Should 32-bit multilib target libraries compiled on 64-bit host > enable CET? > 2. Should 64-bit multilib target libraries compiled with 32-bit compiler > enable CET? Is there a configuration variable holding an indication of bit-ness for which the compilation of a library is done? I see there is some code in configure.ac in libgcc evaluating the host_address (32 or 64 bits) # Check 32bit or 64bit. In the case of MIPS, this really determines the # word size rather than the address size. cat > conftest.c <<EOF #if defined(__x86_64__) || (!defined(__i386__) && defined(__LP64__)) \ || defined(__mips64) host_address=64 #else host_address=32 #endif EOF eval `${CC-cc} -E conftest.c | grep host_address=` rm -f conftest.c If I use the 'host_address' in cet.m4 I got the expected results for libgcc: by default 64-bit library is CET enabled, 32-bit library is not. (In reply to igor.v.tsimbalist from comment #7) > (In reply to H.J. Lu from comment #6) > > (In reply to igor.v.tsimbalist from comment #4) > > > Created attachment 43400 [details] > > > patch > > > > 2 questions: > > > > 1. Should 32-bit multilib target libraries compiled on 64-bit host > > enable CET? > > 2. Should 64-bit multilib target libraries compiled with 32-bit compiler > > enable CET? > > Is there a configuration variable holding an indication of bit-ness for > which the compilation of a library is done? I see there is some code in > configure.ac in libgcc evaluating the host_address (32 or 64 bits) > > # Check 32bit or 64bit. In the case of MIPS, this really determines the > # word size rather than the address size. > cat > conftest.c <<EOF > #if defined(__x86_64__) || (!defined(__i386__) && defined(__LP64__)) \ > || defined(__mips64) > host_address=64 > #else > host_address=32 > #endif > EOF > eval `${CC-cc} -E conftest.c | grep host_address=` > rm -f conftest.c > > If I use the 'host_address' in cet.m4 I got the expected results for libgcc: > by default 64-bit library is CET enabled, 32-bit library is not. That would be wrong for x32. We need to enable CET for x86-64 by default, regardless address size. To enable CET, we only need to check if __SSE2__ is defined since all SSE2 processors support multi-byte NOPs. How can I check the fixes w/o old HW? I tried specifying --target=pentium to configure but the build has failed with the message
*** Configuration i586-pc-none not supported
make[1]: *** [configure-gcc] Error 1
make[1]: Leaving directory `/export/users/itsimbal/gcc_ws/build.cet'
make: *** [all] Error 2
Igor
> -----Original Message-----
> From: hjl.tools at gmail dot com [mailto:gcc-bugzilla@gcc.gnu.org]
> Sent: Tuesday, February 13, 2018 2:12 PM
> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Subject: [Bug target/84148] CET shouldn't be enabled in 32-bit run-time
> libraries by default
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84148
>
> --- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> ---
> (In reply to igor.v.tsimbalist from comment #7)
> > (In reply to H.J. Lu from comment #6)
> > > (In reply to igor.v.tsimbalist from comment #4)
> > > > Created attachment 43400 [details]
> > > > patch
> > >
> > > 2 questions:
> > >
> > > 1. Should 32-bit multilib target libraries compiled on 64-bit host
> > > enable CET?
> > > 2. Should 64-bit multilib target libraries compiled with 32-bit compiler
> > > enable CET?
> >
> > Is there a configuration variable holding an indication of bit-ness for
> > which the compilation of a library is done? I see there is some code in
> > configure.ac in libgcc evaluating the host_address (32 or 64 bits)
> >
> > # Check 32bit or 64bit. In the case of MIPS, this really determines the
> > # word size rather than the address size.
> > cat > conftest.c <<EOF
> > #if defined(__x86_64__) || (!defined(__i386__) && defined(__LP64__)) \
> > || defined(__mips64)
> > host_address=64
> > #else
> > host_address=32
> > #endif
> > EOF
> > eval `${CC-cc} -E conftest.c | grep host_address=`
> > rm -f conftest.c
> >
> > If I use the 'host_address' in cet.m4 I got the expected results for libgcc:
> > by default 64-bit library is CET enabled, 32-bit library is not.
>
> That would be wrong for x32. We need to enable CET for x86-64
> by default, regardless address size. To enable CET, we only
> need to check if __SSE2__ is defined since all SSE2 processors
> support multi-byte NOPs.
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
(In reply to igor.v.tsimbalist from comment #9) > How can I check the fixes w/o old HW? I tried specifying --target=pentium to > configure but the build has failed with the message > > *** Configuration i586-pc-none not supported > make[1]: *** [configure-gcc] Error 1 > make[1]: Leaving directory `/export/users/itsimbal/gcc_ws/build.cet' > make: *** [all] Error 2 To specify the default -march= for 32-bit target libraries, for 32-bit GCC, use --with-arch=cpu, for 64-bit GCC, use --with-arch_32=cpu to configure GCC. Created attachment 43408 [details]
patch #2
(In reply to H.J. Lu from comment #10) > (In reply to igor.v.tsimbalist from comment #9) > > How can I check the fixes w/o old HW? I tried specifying --target=pentium to > > configure but the build has failed with the message > > > > *** Configuration i586-pc-none not supported > > make[1]: *** [configure-gcc] Error 1 > > make[1]: Leaving directory `/export/users/itsimbal/gcc_ws/build.cet' > > make: *** [all] Error 2 > > To specify the default -march= for 32-bit target libraries, for 32-bit > GCC, use --with-arch=cpu, for 64-bit GCC, use --with-arch_32=cpu to > configure GCC. Thanks! This works as expected. New patch is attached. (In reply to igor.v.tsimbalist from comment #11) > Created attachment 43408 [details] > patch #2 case "$host" in - i[[34567]]86-*-linux* | x86_64-*-linux*) - case "$enable_cet" in - default) - # Check if assembler supports CET. - AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM( - [], - [asm ("setssbsy");])], - [enable_cet=yes], - [enable_cet=no]) - ;; - yes) - # Check if assembler supports CET. - AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM( +AC_MSG_CHECKING([for CET support]) + +# Adjust the default value. +if test x$enable_cet = xdefault; then + case "$host" in + i[[34567]]86-*-linux* | x86_64-*-linux*) + # Check if processor supports multi-byte NOPs. + AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM( + [ +#if !defined(__SSE2__) +#error compiler does not support multi-byte NOPs ^^^^^^^^ Change it to target. Please simply merge it with asm ("setssbsy"); Just do #if !defined(__SSE2__) #error target does not support multi-byte NOPs #else asm ("setssbsy"); #endif +#endif + ], + [])], + [enable_cet=yes], + [enable_cet=no]) + ;; + *) + enable_cet=no + ;; + esac +fi Created attachment 43410 [details]
patch #3
The simplified patch.
(In reply to igor.v.tsimbalist from comment #14) > Created attachment 43410 [details] > patch #3 > > The simplified patch. Looks good to me. Thanks. Author: itsimbal Date: Mon Feb 19 16:25:49 2018 New Revision: 257809 URL: https://gcc.gnu.org/viewcvs?rev=257809&root=gcc&view=rev Log: CET shouldn't be enabled in 32-bit run-time libraries by defualt ENDBR32 and RDSSPD are multi-byte NOPs on x86-64 processors and newer x86 processors, starting Pentium Pro. They are UD on older 32-bit processors. Detect this at configure time and adjust the default value for enable_cet. GCC will enable CET in 32-bit run-time libraries in any case if --enable-cet is used to configure GCC. PR target/84148 * config/cet.m4: Check if target support multi-byte NOPS (SSE). * libatomic/configure: Regenerate. * libbacktrace/configure: Likewise. * libgcc/configure: Likewise. * libgfortran/configure: Likewise. * libgomp/configure: Likewise. * libitm/configure: Likewise. * libmpx/configure: Likewise. * libobjc/configure: Likewise. * libquadmath/configure: Likewise. * libsanitizer/configure: Likewise. * libssp/configure: Likewise. * libstdc++-v3/configure: Likewise. * libvtv/configure: Likewise. Modified: trunk/config/ChangeLog trunk/config/cet.m4 trunk/libatomic/ChangeLog trunk/libatomic/configure trunk/libbacktrace/ChangeLog trunk/libbacktrace/configure trunk/libgcc/ChangeLog trunk/libgcc/configure trunk/libgfortran/ChangeLog trunk/libgfortran/configure trunk/libgomp/ChangeLog trunk/libgomp/configure trunk/libitm/ChangeLog trunk/libitm/configure trunk/libmpx/ChangeLog trunk/libmpx/configure trunk/libobjc/ChangeLog trunk/libobjc/configure trunk/libquadmath/ChangeLog trunk/libquadmath/configure trunk/libsanitizer/ChangeLog trunk/libsanitizer/configure trunk/libssp/ChangeLog trunk/libssp/configure trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/configure trunk/libvtv/ChangeLog trunk/libvtv/configure Fixed for GCC 8. |