pengfei updated this revision to Diff 431607.
pengfei marked 5 inline comments as done.
pengfei added a comment.
Address @nickdesaulniers 's comments. Thanks for the thorough review and
suggestions!
> So we pessimize tail calls. Please fix and add a test case for that. This
> might be an unintended side effect of using `isUnconditionalBranch`.
Done. The problem is tail calls have `isReturn = 1`, so we have to handle tail
call specailly.
> The first seems to have some interaction between -fcf-protection and
> __builtin_eh_return. Is that something we need to handle?
I think we should have covered the case since we do it for all JMP and RET.
However, the same option doesn't generate JMP at all on Clang. So I'm not sure
of that.
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/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 -verify-machineinstrs -mtriple=x86_64-unknown-unknown < %s | FileCheck %s -check-prefixes=CHECK,RET
+; RUN: llc -mattr=harden-sls-ind -verify-machineinstrs -mtriple=x86_64-unknown-unknown < %s | FileCheck %s -check-prefixes=CHECK,IND
+
+define dso_local i32 @double_return(i32 %a, i32 %b) local_unnamed_addr {
+; CHECK-LABEL: double_return:
+; CHECK: jle
+; CEHCK-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IND-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IND-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
+; IND-NEXT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IND-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IND-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
+; CEHCK-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IND-NOT: int3
+; CHECK: retq
+; RET-NEXT: int3
+; IND-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
+; IND-NEXT: int3
+; CEHCK-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
+; CEHCK-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,28 @@
}
}
+void X86AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) {
+ AsmPrinter::emitBasicBlockEnd(MBB);
+ if (Subtarget->hardenSlsRet() || Subtarget->hardenSlsInd()) {
+ auto I = MBB.getLastNonDebugInstr();
+ if (I != MBB.end()) {
+ const MCInstrDesc &Desc = I->getDesc();
+ auto IsIndirectTailCall = [I, &Desc]() {
+ return Desc.isCall() && Desc.isReturn() && Desc.isBarrier() &&
+ !I->getOperand(0).isGlobal();
+ };
+ if ((Subtarget->hardenSlsRet() && Desc.isReturn() && !Desc.isCall()) ||
+ (Subtarget->hardenSlsInd() &&
+ (Desc.isIndirectBranch() || IsIndirectTailCall()))) {
+ MCInst TmpInst;
+ TmpInst.setOpcode(X86::INT3);
+ EmitToStreamer(*OutStreamer, TmpInst);
+ }
+ }
+ }
+ 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 FeatureHardenSlsInd
+ : SubtargetFeature<
+ "harden-sls-ind", "HardenSlsInd", "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,16 @@
// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-crc32 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-CRC32 %s
// CRC32: "-target-feature" "+crc32"
// NO-CRC32: "-target-feature" "-crc32"
+
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=all %s -### -o %t.o 2>&1 | FileCheck -check-prefix=SLS-ALL %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=none %s -### -o %t.o 2>&1 | FileCheck -check-prefix=SLS-NONE %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=return %s -### -o %t.o 2>&1 | FileCheck -check-prefix=SLS-RET %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=indirect-branch %s -### -o %t.o 2>&1 | FileCheck -check-prefix=SLS-IND %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=none -mharden-sls=all %s -### -o %t.o 2>&1 | FileCheck -check-prefix=SLS-ALL %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=all -mharden-sls=none %s -### -o %t.o 2>&1 | FileCheck -check-prefix=SLS-NONE %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=return,indirect-branch %s -### -o %t.o 2>&1 | FileCheck -check-prefix=SLS-BAD %s
+// SLS-RET: "-target-feature" "+harden-sls-ret"
+// SLS-IND: "-target-feature" "+harden-sls-ind"
+// SLS-ALL: "-target-feature" "+harden-sls-ind" "-target-feature" "+harden-sls-ret"
+// SLS-NONE: "-target-feature" "-harden-sls-ind" "-target-feature" "-harden-sls-ret"
+// SLS-BAD: invalid sls hardening option '{{[^']+}}' in '-mharden-sls=
Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -242,4 +242,22 @@
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-ind");
+ Features.push_back("+harden-sls-ret");
+ } else if (Scope == "none") {
+ Features.push_back("-harden-sls-ind");
+ Features.push_back("-harden-sls-ret");
+ } else if (Scope == "return") {
+ Features.push_back("+harden-sls-ret");
+ } else if (Scope == "indirect-branch") {
+ Features.push_back("+harden-sls-ind");
+ } else {
+ D.Diag(diag::err_invalid_sls_hardening) << Scope << A->getAsString(Args);
+ }
+ }
}
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -368,6 +368,8 @@
X86 Support in Clang
--------------------
+- Support ``-mharden-sls=[none|all|return|indirect-branch]`` for X86.
+
DWARF Support in Clang
----------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
