[PATCH] D140655: [LowerTypeTests] Add ENDBR to .cfi.jumptable for x86 Indirect Branch Tracking

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1231
+    if (Endbr)
+      AsmOS << ".balign 16, 0xcc\n";
+    else
----------------
tejohnson wrote:
> Why is this needed only in the endbr case?
The endbr case has a larger jump table entry size (16 bytes). We can add as many INT3 as needed, but `.balign 16, 0xcc` seems easier.


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1416
+  if (JumpTableArch == Triple::x86 || JumpTableArch == Triple::x86_64)
+    F->addFnAttr(Attribute::NoCfCheck);
   // Make sure we don't emit .eh_frame for this function.
----------------
tejohnson wrote:
> Add comment about why this attribute is needed. I see that the attribute is only checked for the x86 backends. But can be queried on the function for any arch - is there any possibility this feature will be implemented for non-x86 in the future? If so, I suggest adding the attribute unconditionally here.
> 
> Also, is this needed in the non-endbr case for x86? I.e. it is being added here for all x86 which presumably also changes the pre-existing non-endbr x86 handling.
This is similar to ` F->addFnAttr("branch-target-enforcement", "false");` for AArch64 BTI (D81251). Without this attribute, the function will have 2 consecutive endbr64 at the function start (one from the BTI instrumentation, the other from the first instruction of the inline asm instruction).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140655