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

Matt Morehouse via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 13 15:23:21 PDT 2020


morehouse added inline comments.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:14
+
+#if LIBFUZZER_LINUX
+
----------------
We should include FuzzerDefs.h to use this, not FuzzerBuiltins.h.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:25
+
+typedef unsigned long uptr;
+
----------------
Any reason not to use `uintptr_t` instead?


================
Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:41
+    // We don't want to intercept the wrapper and have it point to itself.
+    if ((uptr)addr == wrapper_addr)
+      addr = nullptr;
----------------
Nit:  I think we should prefer C++ constructs, so `reinterpret_cast<uintptr_t>` here.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:116
+
+extern "C++" ATTRIBUTE_INTERFACE char *strstr(char *s1, const char *s2) {
+  char *result = REAL(strstr)(s1, s2);
----------------
Why `extern "C++"`?  I don't think we want that here.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:123
+
+extern "C++" ATTRIBUTE_INTERFACE char *strcasestr(char *s1, const char *s2) {
+  char *result = REAL(strcasestr)(s1, s2);
----------------
Also why `extern "C++"` here?


================
Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:140
+  REAL(memcmp) =
+      reinterpret_cast<memcmp_type>(GetFuncAddr("memcmp", (uptr)&memcmp));
+  REAL(strncmp) =
----------------
Nit:  `reinterpret_cast<uintptr_t>()`


================
Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:159
+    *__local_fuzzer_preinit)(void) = __fuzzer_init;
+}
+
----------------
Nit: Please add this comment:
```
} // extern "C"
```


================
Comment at: compiler-rt/test/fuzzer/no-asan-strstr.test:5
+CHECK: BINGO
+
----------------
Since these "no-asan" cases are pretty small, can we just add them to the existing `*cmp` tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494





More information about the cfe-commits mailing list