r340515 - [x86/retpoline] Split the LLVM concept of retpolines into separate

Chandler Carruth via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 22 23:06:39 PDT 2018


Author: chandlerc
Date: Wed Aug 22 23:06:38 2018
New Revision: 340515

URL: http://llvm.org/viewvc/llvm-project?rev=340515&view=rev
Log:
[x86/retpoline] Split the LLVM concept of retpolines into separate
subtarget features for indirect calls and indirect branches.

This is in preparation for enabling *only* the call retpolines when
using speculative load hardening.

I've continued to use subtarget features for now as they continue to
seem the best fit given the lack of other retpoline like constructs so
far.

The LLVM side is pretty simple. I'd like to eventually get rid of the
old feature, but not sure what backwards compatibility issues that will
cause.

This does remove the "implies" from requesting an external thunk. This
always seemed somewhat questionable and is now clearly not desirable --
you specify a thunk the same way no matter which set of things are
getting retpolines.

I really want to keep this nicely isolated from end users and just an
LLVM implementation detail, so I've moved the `-mretpoline` flag in
Clang to no longer rely on a specific subtarget feature by that name and
instead to be directly handled. In some ways this is simpler, but in
order to preserve existing behavior I've had to add some fallback code
so that users who relied on merely passing -mretpoline-external-thunk
continue to get the same behavior. We should eventually remove this
I suspect (we have never tested that it works!) but I've not done that
in this patch.

Differential Revision: https://reviews.llvm.org/D51150

Modified:
    cfe/trunk/include/clang/Driver/Options.td
    cfe/trunk/lib/Basic/Targets/X86.cpp
    cfe/trunk/lib/Basic/Targets/X86.h
    cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
    cfe/trunk/test/Driver/x86-target-features.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=340515&r1=340514&r2=340515&view=diff
==============================================================================
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Aug 22 23:06:38 2018
@@ -1999,6 +1999,9 @@ def mno_rtd: Flag<["-"], "mno-rtd">, Gro
 def mno_soft_float : Flag<["-"], "mno-soft-float">, Group<m_Group>;
 def mno_stackrealign : Flag<["-"], "mno-stackrealign">, Group<m_Group>;
 
+def mretpoline : Flag<["-"], "mretpoline">, Group<m_Group>, Flags<[CoreOption,DriverOption]>;
+def mno_retpoline : Flag<["-"], "mno-retpoline">, Group<m_Group>, Flags<[CoreOption,DriverOption]>;
+
 def mrelax : Flag<["-"], "mrelax">, Group<m_riscv_Features_Group>,
   HelpText<"Enable linker relaxation">;
 def mno_relax : Flag<["-"], "mno-relax">, Group<m_riscv_Features_Group>,
@@ -2824,8 +2827,6 @@ def mxsaves : Flag<["-"], "mxsaves">, Gr
 def mno_xsaves : Flag<["-"], "mno-xsaves">, Group<m_x86_Features_Group>;
 def mshstk : Flag<["-"], "mshstk">, Group<m_x86_Features_Group>;
 def mno_shstk : Flag<["-"], "mno-shstk">, Group<m_x86_Features_Group>;
-def mretpoline : Flag<["-"], "mretpoline">, Group<m_x86_Features_Group>;
-def mno_retpoline : Flag<["-"], "mno-retpoline">, Group<m_x86_Features_Group>;
 def mretpoline_external_thunk : Flag<["-"], "mretpoline-external-thunk">, Group<m_x86_Features_Group>;
 def mno_retpoline_external_thunk : Flag<["-"], "mno-retpoline-external-thunk">, Group<m_x86_Features_Group>;
 

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=340515&r1=340514&r2=340515&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Wed Aug 22 23:06:38 2018
@@ -796,8 +796,6 @@ bool X86TargetInfo::handleTargetFeatures
       HasCLDEMOTE = true;
     } else if (Feature == "+rdpid") {
       HasRDPID = true;
-    } else if (Feature == "+retpoline") {
-      HasRetpoline = true;
     } else if (Feature == "+retpoline-external-thunk") {
       HasRetpolineExternalThunk = true;
     } else if (Feature == "+sahf") {
@@ -1397,7 +1395,6 @@ bool X86TargetInfo::hasFeature(StringRef
       .Case("rdpid", HasRDPID)
       .Case("rdrnd", HasRDRND)
       .Case("rdseed", HasRDSEED)
-      .Case("retpoline", HasRetpoline)
       .Case("retpoline-external-thunk", HasRetpolineExternalThunk)
       .Case("rtm", HasRTM)
       .Case("sahf", HasLAHFSAHF)

Modified: cfe/trunk/lib/Basic/Targets/X86.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.h?rev=340515&r1=340514&r2=340515&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Targets/X86.h (original)
+++ cfe/trunk/lib/Basic/Targets/X86.h Wed Aug 22 23:06:38 2018
@@ -98,7 +98,6 @@ class LLVM_LIBRARY_VISIBILITY X86TargetI
   bool HasMOVBE = false;
   bool HasPREFETCHWT1 = false;
   bool HasRDPID = false;
-  bool HasRetpoline = false;
   bool HasRetpolineExternalThunk = false;
   bool HasLAHFSAHF = false;
   bool HasWBNOINVD = false;

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp?rev=340515&r1=340514&r2=340515&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp Wed Aug 22 23:06:38 2018
@@ -144,6 +144,26 @@ void x86::getX86TargetFeatures(const Dri
       Features.push_back("+ssse3");
   }
 
+  // Translate the high level `-mretpoline` flag to the specific target feature
+  // flags. We also detect if the user asked for retpoline external thunks but
+  // failed to ask for retpolines themselves. This is a bit hacky but keeps
+  // existing usages working. We should consider deprecated this and instead
+  // warning if the user requests external retpoline thunks and *doesn't*
+  // request some form of retpolines.
+  if (Args.hasArgNoClaim(options::OPT_mretpoline, options::OPT_mno_retpoline)) {
+    if (Args.hasFlag(options::OPT_mretpoline, options::OPT_mno_retpoline,
+                     false)) {
+      Features.push_back("+retpoline-indirect-calls");
+      Features.push_back("+retpoline-indirect-branches");
+    }
+  } else if (Args.hasFlag(options::OPT_mretpoline_external_thunk,
+                          options::OPT_mno_retpoline_external_thunk, false)) {
+    // FIXME: Add a warning about failing to specify `-mretpoline` and
+    // eventually switch to an error here.
+    Features.push_back("+retpoline-indirect-calls");
+    Features.push_back("+retpoline-indirect-branches");
+  }
+
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.
   handleTargetFeaturesGroup(Args, Features, options::OPT_m_x86_Features_Group);

Modified: cfe/trunk/test/Driver/x86-target-features.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/x86-target-features.c?rev=340515&r1=340514&r2=340515&view=diff
==============================================================================
--- cfe/trunk/test/Driver/x86-target-features.c (original)
+++ cfe/trunk/test/Driver/x86-target-features.c Wed Aug 22 23:06:38 2018
@@ -132,8 +132,8 @@
 
 // RUN: %clang -target i386-linux-gnu -mretpoline %s -### -o %t.o 2>&1 | FileCheck -check-prefix=RETPOLINE %s
 // RUN: %clang -target i386-linux-gnu -mno-retpoline %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-RETPOLINE %s
-// RETPOLINE: "-target-feature" "+retpoline"
-// NO-RETPOLINE: "-target-feature" "-retpoline"
+// RETPOLINE: "-target-feature" "+retpoline-indirect-calls" "-target-feature" "+retpoline-indirect-branches"
+// NO-RETPOLINE-NOT: retpoline
 
 // RUN: %clang -target i386-linux-gnu -mretpoline -mretpoline-external-thunk %s -### -o %t.o 2>&1 | FileCheck -check-prefix=RETPOLINE-EXTERNAL-THUNK %s
 // RUN: %clang -target i386-linux-gnu -mretpoline -mno-retpoline-external-thunk %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-RETPOLINE-EXTERNAL-THUNK %s




More information about the cfe-commits mailing list