MaskRay 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.",
----------------
samitolvanen wrote:
> 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.
A lot of `report_fatal_error` uses are used similar to unreachable for both
assertion and non-assertion builds. The code is pretty much reachable (if a
user has a patchable_function_entry attribute and use -fsanitize=kcfi). It's
bad interface to surface a crash-style diagnostic.
You may add a `namespace { class ... : public DiagnosticInfo }` in this file.
It just needs to overload `print` which is not much boilerplate.
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
