MaskRay added inline comments.
================
Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:47
+ // Find call instructions with KCFI operand bundles.
+ SmallVector<CallInst *, 8> KCFICalls;
+ for (Instruction &I : instructions(F)) {
----------------
You can omit `, 8` to use the default (also 8).
================
Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:61
+ if (F.hasFnAttribute("patchable-function-prefix"))
+ report_fatal_error("-fpatchable-function-entry=N,M, where M>0 is not "
+ "compatible with -fsanitize=kcfi on this target.",
----------------
Try avoiding report_fatal_error. Use a proper error handling mechanism like
`Ctx.diagnose(...)` with a `DiagnosticInfo` subclass. Remove the trailing
period for the message.
================
Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:72
+ // Get the expected hash value.
+ uint32_t ExpectedHash =
+
cast<ConstantInt>(CI->getOperandBundle(LLVMContext::OB_kcfi)->Inputs[0])
----------------
Add const if not modified
================
Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:97
+ Builder.CreateCall(Intrinsic::getDeclaration(&M, Intrinsic::trap));
+ NumKCFIChecks++;
+ }
----------------
pre-increment https://llvm.org/docs/CodingStandards.html#prefer-preincrement
================
Comment at: llvm/test/Transforms/KCFI/kcfi-supported.ll:4
+
+; If the back-end supports KCFI operand bundle lowering, KCFIPass should be a
no-op.
+
----------------
================
Comment at: llvm/test/Transforms/KCFI/kcfi.ll:3
+
+; CHECK-LABEL: define void @f1
+define void @f1(ptr noundef %x) {
----------------
Appending `(` to prevent match with another function with the same prefix (not
in this test, but this is a good general practice)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135411/new/
https://reviews.llvm.org/D135411
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
