[PATCH] D49383: [cfi-verify] Support cross-DSO

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: tools/llvm-cfi-verify/lib/FileAnalysis.cpp:539
+  TrapOnFailFunctions.insert("__cfi_slowpath");
+  TrapOnFailFunctions.insert("abort");
+  TrapOnFailFunctions.insert("__cfi_slowpath at plt");
----------------
jgalenson wrote:
> pcc wrote:
> > jgalenson wrote:
> > > eugenis wrote:
> > > > pcc wrote:
> > > > > Where does `abort` come from? In the diagnostic mode the failure path will end up calling `__ubsan_handle_cfi_check_fail` or `__ubsan_handle_cfi_check_fail_abort`.
> > > > I think it comes from a combination of -fsanitize-trap and -ftrap-function=abort.
> > > Yes, I figured it was a common function that might get used instead of a trap instruction.  I can remove it.
> > > 
> > > I didn't add the diagnostic ones because I couldn't easily generate a test file using them and I didn't want to add them without a test.
> > I'd remove it since we shouldn't be trapping directly in cross-DSO mode, so even if a trap function is specified we shouldn't be emitting a call to it in connection with a CFI check.
> You mean remove `__cfi_slowpath_diag` from this list?  I actually want to keep it since I see calls to it in real binaries.  It's also specified in https://clang.llvm.org/docs/ControlFlowIntegrityDesign.html#cfi-slowpath just next to `__cfi_slowpath`, which apparently calls `__cfi_slowpath_diag`.
Sorry, I meant `abort`.


https://reviews.llvm.org/D49383