pengfei updated this revision to Diff 432698.
pengfei marked 6 inline comments as done.
pengfei added a comment.
Address review comments. Thanks @MaskRay for the thorough review!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126137/new/
https://reviews.llvm.org/D126137
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Arch/X86.cpp
clang/test/Driver/x86-target-features.c
llvm/lib/Target/X86/X86.td
llvm/lib/Target/X86/X86AsmPrinter.cpp
llvm/lib/Target/X86/X86AsmPrinter.h
llvm/test/CodeGen/X86/speculation-hardening-sls.ll
Index: llvm/test/CodeGen/X86/speculation-hardening-sls.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/speculation-hardening-sls.ll
@@ -0,0 +1,97 @@
+; RUN: llc -mattr=harden-sls-ret -mtriple=x86_64-unknown-unknown < %s | FileCheck %s -check-prefixes=CHECK,RET
+; RUN: llc -mattr=harden-sls-ijmp -mtriple=x86_64-unknown-unknown < %s | FileCheck %s -check-prefixes=CHECK,IJMP
+
+define dso_local i32 @double_return(i32 %a, i32 %b) local_unnamed_addr {
+; CHECK-LABEL: double_return:
+; CHECK: jle
+; CHECK-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IJMP-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IJMP-NOT: int3
+entry:
+ %cmp = icmp sgt i32 %a, 0
+ br i1 %cmp, label %if.then, label %if.else
+
+if.then: ; preds = %entry
+ %div = sdiv i32 %a, %b
+ ret i32 %div
+
+if.else: ; preds = %entry
+ %div1 = sdiv i32 %b, %a
+ ret i32 %div1
+}
+
+@__const.indirect_branch.ptr = private unnamed_addr constant [2 x i8*] [i8* blockaddress(@indirect_branch, %return), i8* blockaddress(@indirect_branch, %l2)], align 8
+
+; Function Attrs: norecurse nounwind readnone
+define dso_local i32 @indirect_branch(i32 %a, i32 %b, i32 %i) {
+; CHECK-LABEL: indirect_branch:
+; CHECK: jmpq *
+; RET-NOT: int3
+; IJMP-NEXT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IJMP-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IJMP-NOT: int3
+entry:
+ %idxprom = sext i32 %i to i64
+ %arrayidx = getelementptr inbounds [2 x i8*], [2 x i8*]* @__const.indirect_branch.ptr, i64 0, i64 %idxprom
+ %0 = load i8*, i8** %arrayidx, align 8
+ indirectbr i8* %0, [label %return, label %l2]
+
+l2: ; preds = %entry
+ br label %return
+
+return: ; preds = %entry, %l2
+ %retval.0 = phi i32 [ 1, %l2 ], [ 0, %entry ]
+ ret i32 %retval.0
+}
+
+define i32 @asmgoto() {
+; CHECK-LABEL: asmgoto:
+; CHECK: # %bb.0: # %entry
+; CHECK: jmp .L
+; CHECK-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IJMP-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IJMP-NOT: int3
+entry:
+ callbr void asm sideeffect "jmp $0", "X"(i8* blockaddress(@asmgoto, %d))
+ to label %asm.fallthrough [label %d]
+ ; The asm goto above produces a direct branch:
+
+asm.fallthrough: ; preds = %entry
+ ret i32 0
+
+d: ; preds = %asm.fallthrough, %entry
+ ret i32 1
+}
+
+define void @bar(void ()* %0) {
+; CHECK-LABEL: bar:
+; CHECK: jmpq *
+; RET-NOT: int3
+; IJMP-NEXT: int3
+; CHECK-NOT: ret
+ tail call void %0()
+ ret void
+}
+
+declare dso_local void @foo()
+
+define dso_local void @bar2() {
+; CHECK-LABEL: bar2:
+; CHECK: jmp foo
+; CHECK-NOT: int3
+; CHECK-NOT: ret
+ tail call void @foo()
+ ret void
+}
Index: llvm/lib/Target/X86/X86AsmPrinter.h
===================================================================
--- llvm/lib/Target/X86/X86AsmPrinter.h
+++ llvm/lib/Target/X86/X86AsmPrinter.h
@@ -131,10 +131,7 @@
void emitInstruction(const MachineInstr *MI) override;
- void emitBasicBlockEnd(const MachineBasicBlock &MBB) override {
- AsmPrinter::emitBasicBlockEnd(MBB);
- SMShadowTracker.emitShadowPadding(*OutStreamer, getSubtargetInfo());
- }
+ void emitBasicBlockEnd(const MachineBasicBlock &MBB) override;
bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
const char *ExtraCode, raw_ostream &O) override;
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===================================================================
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -336,6 +336,37 @@
}
}
+static bool isSimpleReturn(const MachineInstr &MI) {
+ // We exclude all tail calls here which set both isReturn and isCall.
+ return MI.getDesc().isReturn() && !MI.getDesc().isCall();
+}
+
+static bool isIndirectBranchOrTailCall(const MachineInstr &MI) {
+ unsigned Opc = MI.getOpcode();
+ return MI.getDesc().isIndirectBranch() /*Make below code in a good shape*/ ||
+ Opc == X86::TAILJMPr || Opc == X86::TAILJMPm ||
+ Opc == X86::TAILJMPr64 || Opc == X86::TAILJMPm64 ||
+ Opc == X86::TCRETURNri || Opc == X86::TCRETURNmi ||
+ Opc == X86::TCRETURNri64 || Opc == X86::TCRETURNmi64 ||
+ Opc == X86::TAILJMPr64_REX || Opc == X86::TAILJMPm64_REX;
+}
+
+void X86AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) {
+ if (Subtarget->hardenSlsRet() || Subtarget->hardenSlsIJmp()) {
+ auto I = MBB.getLastNonDebugInstr();
+ if (I != MBB.end()) {
+ if ((Subtarget->hardenSlsRet() && isSimpleReturn(*I)) ||
+ (Subtarget->hardenSlsIJmp() && isIndirectBranchOrTailCall(*I))) {
+ MCInst TmpInst;
+ TmpInst.setOpcode(X86::INT3);
+ EmitToStreamer(*OutStreamer, TmpInst);
+ }
+ }
+ }
+ AsmPrinter::emitBasicBlockEnd(MBB);
+ SMShadowTracker.emitShadowPadding(*OutStreamer, getSubtargetInfo());
+}
+
void X86AsmPrinter::PrintMemReference(const MachineInstr *MI, unsigned OpNo,
raw_ostream &O, const char *Modifier) {
assert(isMem(*MI, OpNo) && "Invalid memory reference!");
Index: llvm/lib/Target/X86/X86.td
===================================================================
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -382,6 +382,17 @@
"Use an instruction sequence for taking the address of a global "
"that allows a memory tag in the upper address bits.">;
+// Control codegen mitigation against Straight Line Speculation vulnerability.
+def FeatureHardenSlsRet
+ : SubtargetFeature<
+ "harden-sls-ret", "HardenSlsRet", "true",
+ "Harden against straight line speculation across RET instructions.">;
+
+def FeatureHardenSlsIJmp
+ : SubtargetFeature<
+ "harden-sls-ijmp", "HardenSlsIJmp", "true",
+ "Harden against straight line speculation across indirect JMP instructions.">;
+
//===----------------------------------------------------------------------===//
// X86 Subtarget Tuning features
//===----------------------------------------------------------------------===//
Index: clang/test/Driver/x86-target-features.c
===================================================================
--- clang/test/Driver/x86-target-features.c
+++ clang/test/Driver/x86-target-features.c
@@ -304,3 +304,14 @@
// RUN: %clang --target=i386 -march=i386 -mno-crc32 %s -### 2>&1 | FileCheck -check-prefix=NO-CRC32 %s
// CRC32: "-target-feature" "+crc32"
// NO-CRC32: "-target-feature" "-crc32"
+
+// RUN: %clang --target=i386 -march=i386 -mharden-sls=return %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=SLS-RET,NO-SLS %s
+// RUN: %clang --target=i386 -march=i386 -mharden-sls=indirect-jmp %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=SLS-IJMP,NO-SLS %s
+// RUN: %clang --target=i386 -march=i386 -mharden-sls=none -mharden-sls=all %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=SLS-IJMP,SLS-RET %s
+// RUN: %clang --target=i386 -march=i386 -mharden-sls=all -mharden-sls=none %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-SLS %s
+// RUN: %clang --target=i386 -march=i386 -mharden-sls=return,indirect-jmp %s -### -o %t.o 2>&1 | FileCheck -check-prefix=BAD-SLS %s
+// NO-SLS-NOT: "+harden-sls-
+// SLS-RET-DAG: "-target-feature" "+harden-sls-ret"
+// SLS-IJMP-DAG: "-target-feature" "+harden-sls-ijmp"
+// NO-SLS-NOT: "+harden-sls-
+// BAD-SLS: unsupported argument '{{[^']+}}' to option '-mharden-sls='
Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -246,4 +246,20 @@
Name = Name.substr(3);
Features.push_back(Args.MakeArgString((IsNegative ? "-" : "+") + Name));
}
+
+ // Enable/disable straight line speculation hardening.
+ if (Arg *A = Args.getLastArg(options::OPT_mharden_sls_EQ)) {
+ StringRef Scope = A->getValue();
+ if (Scope == "all") {
+ Features.push_back("+harden-sls-ijmp");
+ Features.push_back("+harden-sls-ret");
+ } else if (Scope == "return") {
+ Features.push_back("+harden-sls-ret");
+ } else if (Scope == "indirect-jmp") {
+ Features.push_back("+harden-sls-ijmp");
+ } else if (Scope != "none") {
+ D.Diag(diag::err_drv_unsupported_option_argument)
+ << A->getOption().getName() << Scope;
+ }
+ }
}
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3521,7 +3521,10 @@
HelpText<"Enforce targets of indirect branches and function returns">;
def mharden_sls_EQ : Joined<["-"], "mharden-sls=">,
- HelpText<"Select straight-line speculation hardening scope">;
+ HelpText<"Select straight-line speculation hardening scope (ARM/AArch64/X86"
+ " only). <arg> must be: all, none, retbr(ARM/AArch64),"
+ " blr(ARM/AArch64), comdat(ARM/AArch64), nocomdat(ARM/AArch64),"
+ " return(X86), indirect-jmp(X86)">;
def msimd128 : Flag<["-"], "msimd128">, Group<m_wasm_Features_Group>;
def mno_simd128 : Flag<["-"], "mno-simd128">, Group<m_wasm_Features_Group>;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -449,6 +449,9 @@
X86 Support in Clang
--------------------
+- Support ``-mharden-sls=[none|all|return|indirect-jmp]`` for straight-line
+ speculation hardening.
+
DWARF Support in Clang
----------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
