[PATCH] D76900: Enable IBT(Indirect Branch Tracking) in JIT with CET(Control-flow Enforcement Technology)

[H.J Lu via Phabricator via llvm-commits]

hjl.tools added inline comments.


================
Comment at: llvm/lib/Target/X86/X86IndirectBranchTracking.cpp:128
   // unless nocf_check attribute is used.
-  if ((MF.getFunction().hasAddressTaken() ||
+  if ((isJITwithCET || MF.getFunction().hasAddressTaken() ||
        !MF.getFunction().hasLocalLinkage()) &&
----------------
xiangzhangllvm wrote:
> efriedma wrote:
> > xiangzhangllvm wrote:
> > > efriedma wrote:
> > > > This looks wrong; you can't get the address of a function with internal linkage, whether or not the JIT is involved.
> > > > 
> > > > That said, it's possible there something else going wrong here; the JIT defaults to the large code model, which is rarely used otherwise.
> > > The isInternalLinkage will be checked in line 129, for isInternalLinkage,  !MF.getFunction().hasLocalLinkage() will get true, and then we mainly check the indirect branch tracking requirement at line 130.
> > The logic without your patch is, if a function has its address taken, or the symbol is visible to other modules, emit ENDBR, because it could be the target of an indirect call.  Otherwise, we don't emit an ENDBR: all possible callers of the function call it directly, and direct calls will be lowered to a "call" instruction, which doesn't need ENDBR.
> > 
> > Your patch overrides that logic and says, if we're compiling for a JIT target, emit ENDBR anyway.  That doesn't make sense: the control flow rules are the same whether or not the compiled ELF file is written to disk.
> Make sense! here should not check isJIT, Thank you very much!
As

https://bugs.llvm.org/show_bug.cgi?id=45182

shows,  MF.getFunction().hasAddressTaken() isn't sufficient for IBT.  Assembly dump has
landing pad address taken, but MF.getFunction().hasAddressTaken()  returns false.  Without

  if ((isJITwithCET ||
       MF.getFunction().hasAddressTaken() ||
       !MF.getFunction().hasLocalLinkage()) &&
      !MF.getFunction().doesNoCfCheck()) {
    auto MBB = MF.begin();
    Changed |= addENDBR(*MBB, MBB->begin());
  }

VNC crashes.  A jitted function has to be referenced.  There are 3 ways to reference a function

1. Direct call.
2. Indirect call.
3. Take address.

2 and 3 are mostly likely in x86 jitted codes.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76900/new/

https://reviews.llvm.org/D76900