[PATCH] D41723: Introduce the "retpoline" x86 mitigation technique for variant #2 of the speculative execution vulnerabilities disclosed today, specifically identified by CVE-2017-5715, "Branch Target Injection", and is one of the two halves to Spectre..

Ahmed Bougacha via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 4 17:00:37 PST 2018

ab accepted this revision.
ab added a comment.

Clang/LLVM pieces LGTM.  Thanks for doing this, people.

Comment at: llvm/lib/CodeGen/IndirectBrExpandPass.cpp:56
Nit: Unnecessary 'private'?

Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:27120
+  for (const auto &MO : MI.operands()) {
+    if (MO.isReg() && MO.isUse())
+      for (unsigned &Reg : AvailableRegs)
I wonder: should this also check the CSR regmask?  There are a couple CCs that do preserve R11, but it probably doesn't matter for -mretpoline users. E.g.,

define void @t(void ()* %f) {
  call cc 17 void %f()
  ret void

Worth an assert?

Comment at: llvm/lib/Target/X86/X86RetpolineThunks.cpp:119
+    // .Lr11_call_target:
+    //   movq %r11, (%rsp)
'retq' at the end

Comment at: llvm/lib/Target/X86/X86RetpolineThunks.cpp:171
+  return true;
+Function *X86RetpolineThunks::createThunkFunction(Module &M, StringRef Name) {
Nit: newlines between functions

Comment at: llvm/lib/Target/X86/X86RetpolineThunks.cpp:183
+  B.addAttribute(llvm::Attribute::NoUnwind);
+  B.addAttribute(llvm::Attribute::NoInline);
+  B.addAttribute(llvm::Attribute::Naked);
'noinline' makes sense, but I'm curious: is it necessary?

Comment at: llvm/lib/Target/X86/X86Subtarget.h:344
+  /// Processor needs retpolin to avoid branch prediction errors.
+  bool UseRetpoline;
Nit: "Use retpoline to avoid ..."


More information about the llvm-commits mailing list