[Sanitizer] Reland "Cleanup INTERCEPT_FUNCTION macro"

[yln]

Summary

On Linux both version of the INTERCEPT_FUNCTION macro now return true
when interception was successful. Adapt and cleanup some usages.

Also note that &(func) == &WRAP(func) is a link-time property, but we
do a runtime check.

Tested on Linux and macOS.

Previous attempt reverted by: 5642c3feb03d020dc06a62e3dc54f3206a97a391

This attempt to bring order to the interceptor macro goes the other
direction and aligns the Linux implementation with the way things are
done on Windows.

[vitalybuka]

Please don't submit without my review even if @rnk accept this.
I've noticed report that original patch breaks compiler-rt/test/tsan/Linux/user_malloc.cc on our internal llvm build.
I don't yet understand why it's there and not on our public bots or my local checkout. May be a problem with the patch or our miss-configuration. I'll debug this morning.

[vitalybuka]

In D61358#1485771, @vitalybuka wrote:

Please don't submit without my review even if @rnk accept this.
I've noticed report that original patch breaks compiler-rt/test/tsan/Linux/user_malloc.cc on our internal llvm build.
I don't yet understand why it's there and not on our public bots or my local checkout. May be a problem with the patch or our miss-configuration. I'll debug this morning.

Oh, the test is "// UNSUPPORTED: linux" and it didn't work before patches.

LGTM then

[yln]

Cool, thanks Vitaly! Waiting for sign-off from Reid.

[rnk]

lgtm, thanks

compiler-rt/lib/asan/asan_interceptors.h

125 ↗

(On Diff #197493)

This could be a change in behavior, but I think I'm OK with it. I see that the check is redundant on the Linux side, but it's not immediately clear that the Windows side interception maintains the invariant that it returns false if it can't fill in REAL(name). But, ultimately this condition only affects logging here, and it would help uncover bugs in the Windows side code, so I think this is a good change.

lib/asan/asan_interceptors.h

 DECLARE_REAL(SIZE_T, strlen, const char *s)
 DECLARE_REAL(char*, strncpy, char *to, const char *from, uptr size)
 DECLARE_REAL(uptr, strnlen, const char *s, uptr maxlen)
 DECLARE_REAL(char*, strstr, const char *s1, const char *s2)
 
 #if !SANITIZER_MAC
 #define ASAN_INTERCEPT_FUNC(name)                                        \
   do {                                                                   \
-    if ((!INTERCEPT_FUNCTION(name) || !REAL(name)))                      \
+    if (!INTERCEPT_FUNCTION(name))                                       \
       VReport(1, "AddressSanitizer: failed to intercept '" #name "'\n"); \
   } while (0)
 #define ASAN_INTERCEPT_FUNC_VER(name, ver)                                     \
   do {                                                                         \
-    if ((!INTERCEPT_FUNCTION_VER(name, ver) || !REAL(name)))                   \
+    if (!INTERCEPT_FUNCTION_VER(name, ver))                                    \
       VReport(                                                                 \
           1, "AddressSanitizer: failed to intercept '" #name "@@" #ver "'\n"); \
   } while (0)
 #else
 // OS X interceptors don't need to be initialized with INTERCEPT_FUNCTION.
 #define ASAN_INTERCEPT_FUNC(name)
 #endif  // SANITIZER_MAC
 
 #endif  // !SANITIZER_FUCHSIA
 
 #endif  // ASAN_INTERCEPTORS_H

lib/interception/interception_linux.h

 #if !defined(INCLUDED_FROM_INTERCEPTION_LIB)
 # error "interception_linux.h should be included from interception library only"
 #endif
 
 #ifndef INTERCEPTION_LINUX_H
 #define INTERCEPTION_LINUX_H
 
 namespace __interception {
-// returns true if a function with the given name was found.
-bool GetRealFunctionAddress(const char *func_name, uptr *func_addr,
-    uptr real, uptr wrapper);
-void *GetFuncAddr(const char *name);
-void *GetFuncAddrVer(const char *name, const char *ver);
+bool InterceptFunction(const char *name, uptr *ptr_to_real, uptr func,
+                       uptr wrapper);
+bool InterceptFunction(const char *name, const char *ver, uptr *ptr_to_real,
+                       uptr func, uptr wrapper);
 }  // namespace __interception
 
-#define INTERCEPT_FUNCTION_LINUX_OR_FREEBSD(func)                          \
-  ::__interception::GetRealFunctionAddress(                                \
-      #func, (::__interception::uptr *)&__interception::PTR_TO_REAL(func), \
-      (::__interception::uptr) & (func),                                   \
+#define INTERCEPT_FUNCTION_LINUX_OR_FREEBSD(func) \
+  ::__interception::InterceptFunction(            \
+      #func,                                      \
+      (::__interception::uptr *) & REAL(func),    \
+      (::__interception::uptr) & (func),          \
       (::__interception::uptr) & WRAP(func))
 
 // Android,  Solaris and OpenBSD do not have dlvsym
 #if !SANITIZER_ANDROID && !SANITIZER_SOLARIS && !SANITIZER_OPENBSD
 #define INTERCEPT_FUNCTION_VER_LINUX_OR_FREEBSD(func, symver) \
-  (::__interception::real_##func = (func##_type)(                \
-       unsigned long)::__interception::GetFuncAddrVer(#func, symver))
+  ::__interception::InterceptFunction(                        \
+      #func, symver,                                          \
+      (::__interception::uptr *) & REAL(func),                \
+      (::__interception::uptr) & (func),                      \
+      (::__interception::uptr) & WRAP(func))
 #else
 #define INTERCEPT_FUNCTION_VER_LINUX_OR_FREEBSD(func, symver) \
   INTERCEPT_FUNCTION_LINUX_OR_FREEBSD(func)
 #endif  // !SANITIZER_ANDROID && !SANITIZER_SOLARIS
 
 #endif  // INTERCEPTION_LINUX_H
 #endif  // SANITIZER_LINUX || SANITIZER_FREEBSD || SANITIZER_NETBSD ||
         // SANITIZER_OPENBSD || SANITIZER_SOLARIS

lib/interception/interception_linux.cc

while (true) {
     if (*s1 == 0)
       return true;
     s1++;
     s2++;
   }
 }
 #endif
 
-bool GetRealFunctionAddress(const char *func_name, uptr *func_addr,
-    uptr real, uptr wrapper) {
-  *func_addr = (uptr)GetFuncAddr(func_name);
-  return real == wrapper;
-}
-
-void *GetFuncAddr(const char *name) {
+static void *GetFuncAddr(const char *name) {
 #if SANITIZER_NETBSD
   // FIXME: Find a better way to handle renames
   if (StrCmp(name, "sigaction"))
     name = "__sigaction14";
 #endif
   void *addr = dlsym(RTLD_NEXT, name);
   if (!addr) {
     // If the lookup using RTLD_NEXT failed, the sanitizer runtime library is
     // later in the library search order than the DSO that we are trying to
     // intercept, which means that we cannot intercept this function. We still
     // want the address of the real definition, though, so look it up using
     // RTLD_DEFAULT.
     addr = dlsym(RTLD_DEFAULT, name);
   }
   return addr;
 }
 
+bool InterceptFunction(const char *name, uptr *ptr_to_real, uptr func,
+                       uptr wrapper) {
+  void *addr = GetFuncAddr(name);
+  *ptr_to_real = (uptr)addr;
+  return addr && (func == wrapper);
+}
+
 // Android and Solaris do not have dlvsym
 #if !SANITIZER_ANDROID && !SANITIZER_SOLARIS && !SANITIZER_OPENBSD
-void *GetFuncAddrVer(const char *name, const char *ver) {
+static void *GetFuncAddr(const char *name, const char *ver) {
   return dlvsym(RTLD_NEXT, name, ver);
 }
+
+bool InterceptFunction(const char *name, const char *ver, uptr *ptr_to_real,
+                       uptr func, uptr wrapper) {
+  void *addr = GetFuncAddr(name, ver);
+  *ptr_to_real = (uptr)addr;
+  return addr && (func == wrapper);
+}
 #endif  // !SANITIZER_ANDROID
 
 }  // namespace __interception
 
 #endif  // SANITIZER_LINUX || SANITIZER_FREEBSD || SANITIZER_NETBSD ||
         // SANITIZER_OPENBSD || SANITIZER_SOLARIS

lib/interception/tests/interception_linux_test.cc

 
 INTERCEPTOR(int, isdigit, int d) {
   ++InterceptorFunctionCalled;
   return d >= '0' && d <= '9';
 }
 
 namespace __interception {
 
-TEST(Interception, GetRealFunctionAddress) {
+TEST(Interception, InterceptFunction) {
   uptr malloc_address = 0;
-  EXPECT_TRUE(GetRealFunctionAddress("malloc", &malloc_address, 0, 0));
+  EXPECT_TRUE(InterceptFunction("malloc", &malloc_address, 0, 0));
   EXPECT_NE(0U, malloc_address);
+  EXPECT_FALSE(InterceptFunction("malloc", &malloc_address, 0, 1));
 
   uptr dummy_address = 0;
-  EXPECT_TRUE(
-      GetRealFunctionAddress("dummy_doesnt_exist__", &dummy_address, 0, 0));
+  EXPECT_FALSE(InterceptFunction("dummy_doesnt_exist__", &dummy_address, 0, 0));
   EXPECT_EQ(0U, dummy_address);
 }
 
-TEST(Interception, GetFuncAddr) {
-  EXPECT_NE(GetFuncAddr("malloc"), nullptr);
-  EXPECT_EQ(GetFuncAddr("does_not_exist"), nullptr);
-}
-
 TEST(Interception, Basic) {
-  INTERCEPT_FUNCTION(isdigit);
+  EXPECT_TRUE(INTERCEPT_FUNCTION(isdigit));
 
   // After interception, the counter should be incremented.
   InterceptorFunctionCalled = 0;
   EXPECT_NE(0, isdigit('1'));
   EXPECT_EQ(1, InterceptorFunctionCalled);
   EXPECT_EQ(0, isdigit('a'));
   EXPECT_EQ(2, InterceptorFunctionCalled);
 

lib/msan/msan_interceptors.cc

 #define CHECK_UNPOISONED_CTX(ctx, x, n)                         \
   do {                                                          \
     if (!((MSanInterceptorContext *)ctx)->in_interceptor_scope) \
       CHECK_UNPOISONED_0(x, n);                                 \
   } while (0)
 
 #define MSAN_INTERCEPT_FUNC(name)                                       \
   do {                                                                  \
-    if ((!INTERCEPT_FUNCTION(name) || !REAL(name)))                     \
+    if (!INTERCEPT_FUNCTION(name))                                      \
       VReport(1, "MemorySanitizer: failed to intercept '" #name "'\n"); \
   } while (0)
 
 #define MSAN_INTERCEPT_FUNC_VER(name, ver)                                    \
   do {                                                                        \
-    if ((!INTERCEPT_FUNCTION_VER(name, ver) || !REAL(name)))                  \
+    if (!INTERCEPT_FUNCTION_VER(name, ver))                                   \
       VReport(                                                                \
           1, "MemorySanitizer: failed to intercept '" #name "@@" #ver "'\n"); \
   } while (0)
 
 #define COMMON_INTERCEPT_FUNCTION(name) MSAN_INTERCEPT_FUNC(name)
 #define COMMON_INTERCEPT_FUNCTION_VER(name, ver)                          \
   MSAN_INTERCEPT_FUNC_VER(name, ver)
 #define COMMON_INTERCEPTOR_UNPOISON_PARAM(count)  \

lib/tsan/rtl/tsan_interceptors.cc

#endif
 
   InitializeCommonInterceptors();
   InitializeSignalInterceptors();
   InitializeLibdispatchInterceptors();
 
 #if !SANITIZER_MAC
   // We can not use TSAN_INTERCEPT to get setjmp addr,
   // because it does &setjmp and setjmp is not present in some versions of libc.
-  using __interception::GetRealFunctionAddress;
-  GetRealFunctionAddress(TSAN_STRING_SETJMP,
-                         (uptr*)&REAL(setjmp_symname), 0, 0);
-  GetRealFunctionAddress("_setjmp", (uptr*)&REAL(_setjmp), 0, 0);
-  GetRealFunctionAddress(TSAN_STRING_SIGSETJMP,
-                         (uptr*)&REAL(sigsetjmp_symname), 0, 0);
+  using __interception::InterceptFunction;
+  InterceptFunction(TSAN_STRING_SETJMP, (uptr*)&REAL(setjmp_symname), 0, 0);
+  InterceptFunction("_setjmp", (uptr*)&REAL(_setjmp), 0, 0);
+  InterceptFunction(TSAN_STRING_SIGSETJMP, (uptr*)&REAL(sigsetjmp_symname), 0,
+                    0);
 #if !SANITIZER_NETBSD
-  GetRealFunctionAddress("__sigsetjmp", (uptr*)&REAL(__sigsetjmp), 0, 0);
+  InterceptFunction("__sigsetjmp", (uptr*)&REAL(__sigsetjmp), 0, 0);
 #endif
 #endif
 
   TSAN_INTERCEPT(longjmp_symname);
   TSAN_INTERCEPT(siglongjmp_symname);
 #if SANITIZER_NETBSD
   TSAN_INTERCEPT(_longjmp);
 #endif