[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