[PATCH] D20577: [esan|wset] Intercept and chain signal handlers

[Filipe Cabecinhas via llvm-commits]

filcab added inline comments.

================
Comment at: lib/esan/working_set_posix.cpp:68
@@ +67,3 @@
+static void handleShadowFault(int SigNum, void *Info, void *Ctx) {
+  if (SigNum == SIGSEGV) {
+
----------------
But it handles any and all SEGV, right? Not just in the shadow memory.
A "shadow fault" doesn't really mean much by itself either (unless you're familiar with this code, of course).
Maybe we can agree on `handleMemoryFault`? I'm ok with not adding the POSIX-related names, since it's generic. But I'd rather not mention `Shadow` like that in a function that will be called for things outside the shadow memory.

================
Comment at: lib/esan/working_set_posix.cpp:85
@@ +84,3 @@
+void registerShadowFault() {
+  // We do not use an alternate signal stack, as doing so would require
+  // setting it up for each app thread.
----------------
Fair enough. `registerMemoryFaultHandler`?

================
Comment at: test/esan/TestCases/workingset-fault.cpp:32
@@ +31,3 @@
+  if (sigsetjmp(mark, 1) == 0)
+    *((volatile int *)(ssize_t)argc) = 42; // Raise SIGSEGV
+  fprintf(stderr, "Past longjmp for signal\n");
----------------
bruening wrote:
> filcab wrote:
> > Would `kill(SIGSEGV, getpid())` work?
> If POSIX guarantees synchronous delivery before kill returns (and platforms implement that), which I believe is true for a single-threaded app, yes, but a true fault just feels like a more authentic test.
Fair enough. No need casting argc twice, btw.
I cringe looking at that line, but couldn't get you an easier way to trigger the fault off the top of my head.


http://reviews.llvm.org/D20577