[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..

[Rafael Avila de Espindola via llvm-commits]

Chandler Carruth <chandlerc at gmail.com> writes:

>>
>> > There is one other important source of indirect branches in x86 ELF
>> > binaries: the PLT. These patches also include support for LLD to
>> > generate PLT entries that perform a retpoline-style indirection.
>>
>> I see that we now generate similar code for -fno-plt. We should have a
>> test for that (nonlazybind at the llvm level).
>>
>
> ? The LLVM code generation for PLT-calls shouldn't be impacted by retpoline
> at all? It should just be the contents of the PLT section itself? Maybe I'm
> just misunderstanding...
>
> If you're wondering whether LLVM will correctly retpoline a -fno-plt call
> that it directly emits, I believe it uses the same instruction patterns as
> any other indirect call and so should be covered already.

Yes, that is what I was wondering. It is not obvious for someone not
current on the X86 backend if -fno-plt is implemented in the same code
path. So the request is just to add a llvm test for -fno-plt (i.e., a
test showing what nonlazybind codegens to now).

Hopefully all that you need is a version of
test/CodeGen/X86/fast-isel-noplt-pic.ll with

+attributes #0 = { "target-features"="+retpoline" }

>> > +Function *X86RetpolineThunks::createThunkFunction(Module &M, StringRef
>> Name) {
>> > +  LLVMContext &Ctx = M.getContext();
>> > +  auto Type = FunctionType::get(Type::getVoidTy(Ctx), false);
>> > +  Function *F =
>> > +      Function::Create(Type, GlobalValue::LinkOnceODRLinkage, Name, &M);
>> > +  F->setVisibility(GlobalValue::HiddenVisibility);
>>
>> We should probably put this in a comdat so the linker keeps only one.
>>
>
> Is LinkOnceODR not sufficient? We don't need a comdat *group* here. I
> thought you didn't need explicit comdats unless you're grouping multiple
> globals....

It is not sufficient. LLVM used to create implicit comdats, but we don't
do that anymore.

Given

define linkonce_odr void @foo() {
  ret void
}

llc puts it in .text or

.section        .text.foo,"ax", at progbits

if -function-sections is used.

Given

$foo = comdat any
define linkonce_odr void @foo() comdat {
  ret void
}

We always produce

.section        .text.foo,"axG", at progbits,foo,comdat


>> This pass converts each indirectbr to a switch. One feature we had in
>> the old clang expansion of computed gotos is that each function would
>> have a single switch. This would avoid code explosion in direct threaded
>> interpreters where after each opcode there is a jump that can go any
>> other opcode.
>>
>
> I'll see if this is easy to implement, but if it's going to take some time
> I might suggest we defer this to a follow-up patch.
>
>
> I think Rui got the rest of your comments, but give a shout if I've missed
> anything and thanks for review!

A FIXME or bug report for future improvement is fine.

Cheers,
Rafael