xiangzhangllvm added inline comments.
================ Comment at: llvm/lib/Target/X86/X86IndirectBranchTracking.cpp:156-161 if (needsPrologueENDBR(MF, M)) { - auto MBB = MF.begin(); - Changed |= addENDBR(*MBB, MBB->begin()); + if (!ManualENDBR || MF.getFunction().doesCfCheck()) { + auto MBB = MF.begin(); + Changed |= addENDBR(*MBB, MBB->begin()); + } else { + // When -mmanual-endbr is in effect, the compiler does not ---------------- I am a little puzzle here. Let me copy your patch description here: ``` >When -mmanual-endbr is set, llvm refrains from automatically adding >ENDBR instructions to functions' prologues, which would have been >automatically added by -fcf-protection=branch. Although this works >correctly, missing ENDBR instructions where they are actually needed >could lead to broken binaries, which would fail only in running time. ``` I think the purpose of "-mmanual-endbr" should be "Supplementary Way" for the cases which the CET can't correctly insert endbr in automatic way. Here (in if (needsPrologueENDBR(MF, M)) ) the automatic way will insert the endbr. So I think the job for "-mmanual-endbr" should be done in parallel with the old condition. Some like: ``` if (ManualENDBR ){ do something }else { // automatic if (needsPrologueENDBR(MF, M)) {insert endbr} } } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118355/new/ https://reviews.llvm.org/D118355 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits