samitolvanen added inline comments.
================ Comment at: llvm/lib/Target/X86/X86AsmPrinter.cpp:121 + if (N == Value) + return ~Value; + } ---------------- joaomoreira wrote: > Can we use another constant blinding scheme, such as a Value++ or anything > else? This way, we would prevent endbrs from being emitted in the indirect > branch guards too. > > Since we are using Value (prologue) and ~Value (caller/guard) for doing the > checks, we also need to check if ~ENDBR was picked as a KCFIType, otherwise > ENDBR will be emitted in the ibranch guards. > Can we use another constant blinding scheme, such as a Value++ or anything > else? This way, we would prevent endbrs from being emitted in the indirect > branch guards too. > > Since we are using Value (prologue) and ~Value (caller/guard) for doing the > checks, we also need to check if ~ENDBR was picked as a KCFIType, otherwise > ENDBR will be emitted in the ibranch guards. I don't mind changing this to `Value + 1`, but that actually doesn't change anything because we emit `-Value` in indirect call checks, not `~Value`. Therefore, using `~Value` works equally well here. Specifically, this code currently emits `~Value`in the preamble and `-(~Value) == Value + 1` in the indirect call check. Switching to `Value + 1` simply reverses the order; we'll emit `Value + 1` in the preamble and `-(Value + 1) == ~Value` in the indirect call check. However, you are right that we also need to avoid `-ENDBR` in this function. I'll fix that and clarify the comment. ================ Comment at: llvm/test/CodeGen/X86/kcfi.ll:91 + +;; Ensure we emit ~Type for unwanted values (e.g. endbr64 == 4196274163). +; ASM-LABEL: __cfi_f5: ---------------- joaomoreira wrote: > We need to also ensure/test that these are not emitted in the caller/indirect > branch guards. > > I assume that in the current scheme (blinding with ~Value) would be > unfeasible to do this, so maybe we need a different approach for masking (as > suggested above). Added a test for `-ENDBR` too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119296/new/ https://reviews.llvm.org/D119296 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
