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

[Eric Christopher via llvm-commits]

On Fri, Jan 19, 2018 at 5:11 PM Matthias Braun via Phabricator <
reviews at reviews.llvm.org> wrote:

> MatzeB accepted this revision.
> MatzeB added a comment.
>
> LGTM, I'm all for getting this into the LLVM tree, we can fine tune later
> as necessary.
>
> This is the first widely used machine module pass. Adding a module pass
> into the pipeline means we will have all the MI in memory at the same time
> rather than creating MI for 1 function at a time and freeing it after
> emitting each function. So it would be good to keep an eye on the compilers
> memory consumption... Maybe we can find a way to refactor the codegen
> pipeline later so that we can go back to 1 function at a time and still
> have a way to emit some new code afterwards...
>
>
>
FWIW it would possible by splitting it into an IR pass to create the
retpoline IR function if it notices a retpoline attributed function and a
later MI pass to fill them in. I didn't do this originally, but it should
be OK(tm).

-eric



>
> ================
> Comment at: llvm/include/llvm/CodeGen/TargetPassConfig.h:412-413
>
> +  /// This pass may be implemented by targets that want to run passes
> +  /// that emit MI directly and bypass all other machine passes.
> +  virtual void addEmitPass() {}
> ----------------
> Even though this documentation is mostly copied, how about changing it to:
> ```
> Targets may add passes immediately before machine code is emitted in this
> callback.
> This is called later than addPreEmitPass().
> ```
>
>
> ================
> Comment at: llvm/include/llvm/CodeGen/TargetPassConfig.h:414
> +  /// that emit MI directly and bypass all other machine passes.
> +  virtual void addEmitPass() {}
> +
> ----------------
> I find the name `addEmitPass()` misleading as it isn't adding the assembly
> emission passes. The best bad name I can think of right now is
> `addPreEmit2()`, in the spirit of the existing `addPreSched2()` callback...
>
>
> ================
> Comment at: llvm/lib/Target/X86/X86RetpolineThunks.cpp:83-84
> +  auto *TPC = getAnalysisIfAvailable<TargetPassConfig>();
> +  if (!TPC)
> +    return false;
> +
> ----------------
> There shouldn't be any way to use any Target/X86 without also having a
> TargetPassConfig in the pipeline. An assert should be enough.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D41723
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180120/f1277aa7/attachment.html>