Work around crashes in `__sanitizer_malloc_hook()` under Mac OSX.

[delcypher]

Summary

[LibFuzzer]
Work around crashes in __sanitizer_malloc_hook() under Mac OSX.

Under Mac OSX we intercept calls to malloc before thread local
storage is initialised leading to a crash when accessing
AllocTracer. To workaround this AllocTracer is only accessed
in the hook under Linux. For symmetry __sanitizer_free_hook()
is also modified in the same way.

To support this change a set of new macros
LIBFUZZER_LINUX and LIBFUZZER_APPLE has been defined which can be
used to check the target being compiled for.

[kcc]

This is very, very sad.
I intentionally made this thread local so that we don't use atomics in a very hot place (and malloc is often a very hot place).
Besides, I am only collecting counts from the main thread.
Need to find another solution, e.g. to not touch AllocTracer before we called Start() on it.

[delcypher]

@kcc

This is very, very sad.
I intentionally made this thread local so that we don't use atomics in a very hot place (and malloc is often a very hot place).

Okay. My implementation isn't very good then

Besides, I am only collecting counts from the main thread.

Okay, so that was intentional. So my implementation isn't just bad, it's also wrong. Is the reason you only want to collect counts from the main thread for the reason I gave in my earlier comment? Or is there another reason?

Need to find another solution, e.g. to not touch AllocTracer before we called Start() on it.

Well there's a really hacky way to do that. There could be a global bool guarding access to AllocTracer that we set to true once we call `Start()`. Not sure if that's ideal though as technically would could race on the global bool if there are multiple threads.

Just a side though. The LeakSanitizer currently doesn't work on Mac OSX. AFAICT it only works right now on Linux that is not Android on x86_64, aarch64 or MIPS (based on reading compiler-rt/lib/lsan/lsan_common.h where CAN_SANITIZE_LEAKS is set). Whether or not the CAN_SANITIZE_LEAKS macro was enabled (and consequently if leak sanitization can be performed) does not seem to be exposed in the Leak Sanitizer's interface. There is the __lsan_is_turned_off() function but that is not affected by the value of the CAN_SANITIZE_LEAKS macro. Ideally LibFuzzer shouldn't try to do anything related to leak checking if the leak sanitizer is not actually functional. If __lsan_is_turned_off()` was fixed to respect CAN_SANITIZE_LEAKS we could guard some of the code in LibFuzzer that supports running the LeakSanitizer.

[kcc]

Is the reason you only want to collect counts from the main thread for the reason I gave in my earlier comment? Or is there another reason?

Mostly performance. Note that this is not necessary a good heuristic -- in case we malloc in one thread and free() in another thread
we will get the counts wrong and call lsan too often (and then stop calling it at all).

But, 99% of targets we've tested with libFuzzer are single-threaded, so we simply don't have enough experience with multi-threaded targets.

Need to find another solution, e.g. to not touch AllocTracer before we called Start() on it.

Well there's a really hacky way to do that. There could be a global bool guarding access to AllocTracer that we set to true once we call `Start()`. Not sure if that's ideal though as technically would could race on the global bool if there are multiple threads.

Or we can have a global that we set just once at init time and never unset.
This will solve the problem, right? Then there will be no race.

Just a side though. The LeakSanitizer currently doesn't work on Mac OSX.

AFIACT -- yes.

AFAICT it only works right now on Linux that is not Android on x86_64, aarch64 or MIPS (based on reading compiler-rt/lib/lsan/lsan_common.h where CAN_SANITIZE_LEAKS is set). Whether or not the CAN_SANITIZE_LEAKS macro was enabled (and consequently if leak sanitization can be performed) does not seem to be exposed in the Leak Sanitizer's interface.

True.

There is the __lsan_is_turned_off() function but that is not affected by the value of the CAN_SANITIZE_LEAKS macro. Ideally LibFuzzer shouldn't try to do anything related to leak checking if the leak sanitizer is not actually functional. If __lsan_is_turned_off()` was fixed to respect CAN_SANITIZE_LEAKS we could guard some of the code in LibFuzzer that supports running the LeakSanitizer.

__lsan_is_turned_off is a different thing:

// The user may optionally provide this function to disallow leak checking
// for the program it is linked into (if the return value is non-zero). This
// function must be defined as returning a constant value; any behavior beyond
// that is unsupported.

Maybe for now just guard the code in sanitizer_malloc_hook with if (LIBFUZZER_LINUX)?

[delcypher]

@kcc
Is this new version of the patch acceptable? If so I'll commit it.

My other patches ( http://reviews.llvm.org/D20410 and http://reviews.llvm.org/D20409) will depend on the newly added platform macros in this patch so I will fix those once this patch is committed.

[aizatsky]

lib/Fuzzer/FuzzerLoop.cpp

446 ↗

(On Diff #57835)

Maybe if (!LIBFUZZER_APPLY) would be a better condition?

[kcc]

Yes, this looks ok...

lib/Fuzzer/FuzzerLoop.cpp

446 ↗

(On Diff #57835)

Yea, maybe..
if (!LIBFUZZER_APPLE)

Also, this code tends to not use {} for simple if statements. Same below.

[delcypher]

Make changes suggest by @kcc and @aizatsky.

Is this good enough now?

[kcc]

LGTM, thanks!

llvm/trunk/lib/Fuzzer/FuzzerInternal.h

 #include <string.h>
 #include <string>
 #include <unordered_set>
 #include <vector>
 
 #include "FuzzerInterface.h"
 #include "FuzzerTracePC.h"
 
+// Platform detection.
+#ifdef __linux__
+#define LIBFUZZER_LINUX 1
+#define LIBFUZZER_APPLE 0
+#elif __APPLE__
+#define LIBFUZZER_LINUX 0
+#define LIBFUZZER_APPLE 1
+#else
+#error "Support for your platform has not been implemented"
+#endif
+
 namespace fuzzer {
 
 typedef int (*UserCallback)(const uint8_t *Data, size_t Size);
 int FuzzerDriver(int argc, char **argv, UserCallback Callback);
 
 using namespace std::chrono;
 typedef std::vector<uint8_t> Unit;
 typedef std::vector<Unit> UnitVector;

llvm/trunk/lib/Fuzzer/FuzzerLoop.cpp

struct MallocFreeTracer {
   // Returns true if there were more mallocs than frees.
   bool Stop() { return Mallocs > Frees; }
   size_t Mallocs;
   size_t Frees;
 };
 
 static thread_local MallocFreeTracer AllocTracer;
 
+// FIXME: The hooks only count on Linux because
+// on Mac OSX calls to malloc are intercepted before
+// thread local storage is initialised leading to
+// crashes when accessing ``AllocTracer``.
 extern "C" {
-void __sanitizer_malloc_hook(void *ptr, size_t size) { AllocTracer.Mallocs++; }
-void __sanitizer_free_hook(void *ptr) { AllocTracer.Frees++; }
+void __sanitizer_malloc_hook(void *ptr, size_t size) {
+  if (!LIBFUZZER_APPLE)
+    AllocTracer.Mallocs++;
+}
+void __sanitizer_free_hook(void *ptr) {
+  if (!LIBFUZZER_APPLE)
+    AllocTracer.Frees++;
+}
 }  // extern "C"
 
 void Fuzzer::ExecuteCallback(const uint8_t *Data, size_t Size) {
   UnitStartTime = system_clock::now();
   // We copy the contents of Unit into a separate heap buffer
   // so that we reliably find buffer overflows in it.
   std::unique_ptr<uint8_t[]> DataCopy(new uint8_t[Size]);
   memcpy(DataCopy.get(), Data, Size);