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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

I looked up x86 ENDBR and the adding of that instruction seems fine to me, but I have a couple questions on the related changes in the patch.



================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1231
+    if (Endbr)
+      AsmOS << ".balign 16, 0xcc\n";
+    else
----------------
Why is this needed only in the endbr case?


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140655