[PATCH] D7203: [asan] Set abort_on_error=1 by default on OS X

[Filipe Cabecinhas]

filcab added a comment.

It seems abort_on_error-linux.cc could be called abort_on_error-default (or posix, or whatever), but that's just a nit on my part (I'm ok with it being *-linux) :-)

Where are you filtering which tests get executed, though? It looks like both will always be executed, and one will fail.
For now, I guess a "REQUIRES: darwin", and a "UNSUPPORTED: darwin" on the tests should be good enough.


================
Comment at: lib/asan/asan_flags.inc:81
@@ -80,3 +80,3 @@
 ASAN_FLAG(
-    bool, abort_on_error, false,
+    bool, abort_on_error, (SANITIZER_MAC == 1),
     "If set, the tool calls abort() instead of _exit() after printing the "
----------------
Why not just `SANITIZER_MAC`? Possibly even `SANITIZER_MAC || SANITIZER_IOSSIM || SANITIZER_IOS`, depending on what behavior you want for iOS and the iOS simulator.
Those identifiers are always defined to 1 or 0.

================
Comment at: lib/asan/tests/asan_test_main.cc:18
@@ -17,3 +17,3 @@
 extern "C" const char* __asan_default_options() {
-  return "symbolize=false";
+  return "symbolize=false:abort_on_error=0";
 }
----------------
Why do you do this here, if lit.cfg already sets `ASAN_OPTIONS`?

================
Comment at: test/asan/TestCases/Darwin/abort_on_error-darwin.cc:2
@@ +1,3 @@
+// Check that with empty ASAN_OPTIONS, ASan reports on OS X actually crash
+// the process (abort_on_error=1). See also Linux/abort_on_error-linux.cc.
+
----------------
I don't think the "See also ..." part is useful, and will quickly become annoying as other platforms are added.


http://reviews.llvm.org/D7203