[PATCH] D61145: [NFC][Sanitizer] Change "return type" of INTERCEPT_FUNCTION to void

[Julian Lettner via Phabricator via llvm-commits]

yln marked 3 inline comments as done.
yln added inline comments.


================
Comment at: compiler-rt/lib/asan/asan_interceptors.h:127
+    bool same = (& (name) == & WRAP(name));                              \
+    if ((!same || !REAL(name)))                                          \
       VReport(1, "AddressSanitizer: failed to intercept '" #name "'\n"); \
----------------
vitalybuka wrote:
> yln wrote:
> > You said this protects against "double interception"?
> > If this works as expected, then should we push it up into INTERCEPT_FUNCTION?
> > Why do we only protect against "double interception" in ASan and MSan, but not the other sanitizers?
> > 
> > My best guess is that this has been obsolete for a while.
> Yes, I would like to put this into INTERCEPT_FUNCTION
> however maybe VReport is not available in all users 
Got it. I will try to make this nicer/more uniform in follow-ups.


================
Comment at: compiler-rt/lib/interception/interception_linux.h:37
+      (::__interception::uptr) & WRAP(func));                              \
+  } while (0)  // TODO(yln): temporarily make macro void.
 
----------------
vitalybuka wrote:
> why?
So the compiler tells me all the places where the return value is used. I want to commit it like this so that the bots compile it for all the configurations that I can't test locally.


================
Comment at: compiler-rt/lib/msan/msan_interceptors.cc:1247
+    INTERCEPT_FUNCTION(name);                                           \
+    bool same = (& (name) == & WRAP(name));                             \
+    if ((!same || !REAL(name)))                                         \
----------------
vitalybuka wrote:
> why you don't want to keep this comparison inside of INTERCEPT_FUNCTION_LINUX_OR_FREEBSD?
It prevents the cleanups in [tsan_interceptors.cc] (see https://reviews.llvm.org/D59504).
> // We can not use TSAN_INTERCEPT to get setjmp addr,
> // because it does &setjmp and setjmp is not present in some versions of libc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61145