[PATCH] D62929: [GWP-ASan] Integration with Scudo [5].

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: compiler-rt/test/gwp_asan/invalid_free_left.cpp:3
+// RUN: %clangxx_gwp_asan %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
----------------
hctim wrote:
> vlad.tsyrklevich wrote:
> > This uses not while the others above use %expect_crash, you probably want %expect_crash here too and in the other places you use not?
> So at the moment, invalid free/double free are detected internally, and terminate through `exit(EXIT_FAILURE)`. The tests with `%expect_crash` are the ones which terminate through a segv signal (`$? == 127`).
> 
> This is mirroring the behaviour of scudo, where they `exit(EXIT_FAILURE)` on checksum failure or other error conditions. What might be better though, is when we detect a failure condition internally, we raise a signal. This would allow platforms like Android to still have their special death handlers run.
> 
> @morehouse - WDYT? I would say that I'd prefer to raise a signal internally-detected error. I'm not sure that raising SEGV is appropriate here, and it would also increase the complexity of the logic (we'd have to reinstate the previous SEGV handler first). Maybe SIGTRAP or SIGABRT?
FWIW, we trigger a SEGV in Google3 by touching the freed page.  Not sure if it's the best approach, but it simplified the error handling a bit.  Allows everything to go through SEGV handler and then forward to the previous handers, rather than having to implement multiple flows for different errors.  Also gives the address of the double-free to previous handlers for them to inspect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62929