Skip to content

Commit

Permalink
[x86/retpoline] Split the LLVM concept of retpolines into separate
Browse files Browse the repository at this point in the history
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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@340515 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
chandlerc committed Aug 23, 2018
1 parent e88b17c commit 2d45136
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 8 deletions.
5 changes: 3 additions & 2 deletions include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -1999,6 +1999,9 @@ def mno_rtd: Flag<["-"], "mno-rtd">, Group<m_Group>;
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>,
Expand Down Expand Up @@ -2824,8 +2827,6 @@ def mxsaves : Flag<["-"], "mxsaves">, Group<m_x86_Features_Group>;
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>;

Expand Down
3 changes: 0 additions & 3 deletions lib/Basic/Targets/X86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -796,8 +796,6 @@ bool X86TargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
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") {
Expand Down Expand Up @@ -1397,7 +1395,6 @@ bool X86TargetInfo::hasFeature(StringRef Feature) const {
.Case("rdpid", HasRDPID)
.Case("rdrnd", HasRDRND)
.Case("rdseed", HasRDSEED)
.Case("retpoline", HasRetpoline)
.Case("retpoline-external-thunk", HasRetpolineExternalThunk)
.Case("rtm", HasRTM)
.Case("sahf", HasLAHFSAHF)
Expand Down
1 change: 0 additions & 1 deletion lib/Basic/Targets/X86.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public TargetInfo {
bool HasMOVBE = false;
bool HasPREFETCHWT1 = false;
bool HasRDPID = false;
bool HasRetpoline = false;
bool HasRetpolineExternalThunk = false;
bool HasLAHFSAHF = false;
bool HasWBNOINVD = false;
Expand Down
20 changes: 20 additions & 0 deletions lib/Driver/ToolChains/Arch/X86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,26 @@ void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple,
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);
Expand Down
4 changes: 2 additions & 2 deletions test/Driver/x86-target-features.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2d45136

Please sign in to comment.