[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

[Dokyung Song via Phabricator via cfe-commits]

dokyungs marked 4 inline comments as done.
dokyungs added inline comments.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:119
+    return internal_memcmp(s1, s2, n);
+  ensureFuzzerInited();
+  int result = REAL(memcmp)(s1, s2, n);
----------------
morehouse wrote:
> I think `ensureFuzzerInited` is no longer useful here.
Fixed also in `strncmp` and `strstr` interceptors.


================
Comment at: compiler-rt/test/fuzzer/custom-allocator.test:2
+UNSUPPORTED: freebsd
+RUN: %cpp_compiler -fno-sanitize=all -fno-builtin %S/CustomAllocator.cpp -fPIC %ld_flags_rpath_so1 -O0 -shared -o %dynamiclib1
+RUN: %cpp_compiler -fno-sanitize=address %S/CustomAllocatorTest.cpp %ld_flags_rpath_exe1 -o %t-NoAsanCustomAllocatorTest
----------------
morehouse wrote:
> Why do we need each of these flags?
With all the flags, I designed this test for the recent failure scenario in which tcmalloc calls strncmp (+memcmp/strstr) when the fuzzer interceptor library is linked into the libFuzzer executable.

As such, we need to turn off ASan (-fno-sanitize=address) when building the executable to let the fuzzer interceptor library be linked.

As to the flags used to build the allocator shared library, I wanted to disable ASan and Fuzzer (via `-fno-sanitize=all`) because allocator libraries are typically not instrumented for OOB/UAF errors or coverage. I also wanted to prevent the compiler from optimizing out our calls to strncmp(+memcmp/strstr) by giving `-fno-builtin`; calls to these functions must go to the fuzzer interceptor library to comply with the scenario.


================
Comment at: compiler-rt/test/fuzzer/memcmp.test:9
+RUN: %cpp_compiler -fno-sanitize=all -fno-builtin %S/CustomAllocator.cpp -fPIC %ld_flags_rpath_so1 -O0 -shared -o %dynamiclib1
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/MemcmpTest.cpp %ld_flags_rpath_exe1 -o %t-NoAsanCustomAllocatorMemcmpTest
+RUN: not %run %t-NoAsanCustomAllocatorMemcmpTest        -seed=1 -runs=10000000   2>&1 | FileCheck %s
----------------
morehouse wrote:
> Why is the custom allocator test here useful?
To make sure exercise the path where memcmp is called (i) in the calloc context, and (ii) then again in the LLVMFuzzerTestOneInput context. %t-NoAsanCustomAllocatorTest only tests (i), and %t-NoAsanMemcmpTest only tests (ii).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494