Bug 84148

Summary: CET shouldn't be enabled in 32-bit run-time libraries by default
Product: gcc Reporter: H.J. Lu <hjl.tools>
Component: targetAssignee: 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
ENDBR32 and RDSSPD are multi-byte NOPs on x86-64 processors and
newer x86 processors.  They are UD on older 32-bit processors.
We should enable CET in 32-bit run-time libraries only if --enable-cet
is used to configure GCC.
Comment 1 Florian Weimer 2018-01-31 13:29:10 UTC
How old are the CPUs which treat it as UD?  Older than i686/Pentium Pro?  Thanks.
Comment 2 H.J. Lu 2018-01-31 13:38:08 UTC
(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.
Comment 3 Gianluigi Tiesi 2018-02-11 04:45:23 UTC
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");
Comment 4 igor.v.tsimbalist 2018-02-12 22:13:31 UTC
Created attachment 43400 [details]
patch
Comment 5 igor.v.tsimbalist 2018-02-12 22:15:45 UTC
Beside the patch configure files have to be regenerated.
Comment 6 H.J. Lu 2018-02-12 23:24:30 UTC
(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?
Comment 7 igor.v.tsimbalist 2018-02-13 12:22:18 UTC
(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.
Comment 8 H.J. Lu 2018-02-13 13:12:27 UTC
(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.
Comment 9 igor.v.tsimbalist 2018-02-13 15:52:16 UTC
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.
Comment 10 H.J. Lu 2018-02-13 15:57:43 UTC
(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.
Comment 11 igor.v.tsimbalist 2018-02-13 16:56:33 UTC
Created attachment 43408 [details]
patch #2
Comment 12 igor.v.tsimbalist 2018-02-13 16:57:15 UTC
(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.
Comment 13 H.J. Lu 2018-02-13 17:02:37 UTC
(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
Comment 14 igor.v.tsimbalist 2018-02-13 20:07:03 UTC
Created attachment 43410 [details]
patch #3

The simplified patch.
Comment 15 H.J. Lu 2018-02-13 20:08:39 UTC
(In reply to igor.v.tsimbalist from comment #14)
> Created attachment 43410 [details]
> patch #3
> 
> The simplified patch.

Looks good to me.  Thanks.
Comment 16 itsimbal 2018-02-19 16:26:21 UTC
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
Comment 17 H.J. Lu 2018-02-27 13:11:54 UTC
Fixed for GCC 8.