[PATCH] D135411: Add generic KCFI operand bundle lowering

[Sami Tolvanen via Phabricator via llvm-commits]

samitolvanen added inline comments.


================
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.",
----------------
MaskRay wrote:
> 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.
> Try avoiding report_fatal_error. Use a proper error handling mechanism like `Ctx.diagnose(...)` with a `DiagnosticInfo` subclass.

I was looking at this, but I feel like adding a new class might be a bit of an overkill for this situation. Looking at the code in `lib/Transforms`, there are 70 uses of `report_fatal_error` and 17 uses of `Ctx.diagnose(...)`. The latter seem to all be more complex diagnostics than just printing out an error message. Do you think I should nevertheless use `Ctx.diagnose` for this message?

> Remove the trailing period for the message.

Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135411