[NFC][Sanitizer] Clean up GetRealFunctionAddress

[yln]

Summary

bool GetRealFunctionAddress(const char *func_name, uptr *func_addr,
                            uptr real, uptr wrapper)

This function has evolved over the years. Comment about return value is
lying. Paramters real/wrapper don't make much sense and are not supplied
at any call site. Real return type is never used.

Refactored to be consistent with GetFuncAddrVer.

[yln]

*ping*

[delcypher]

compiler-rt/lib/interception/interception_linux.h

30

Is this right? The previous version didn't have an assignment.

[yln]

compiler-rt/lib/interception/interception_linux.h

30

The old version of GetRealFunctionAddress takes a pointer for it's second parameter, which is assigned:

*func_addr = (uptr)dlsym(RTLD_DEFAULT, func_name);

[vitalybuka]

Does not compile on linux

usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/interception/tests/interception_linux_test.cc:60:2: warning: embedding a directive within macro arguments has undefined behavior [-Wembedded-directive]
#endif  // SANITIZER_LINUX
 ^
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/interception/tests/interception_linux_test.cc:61:2: warning: embedding a directive within macro arguments has undefined behavior [-Wembedded-directive]
#endif  // #if !SANITIZER_DEBUG
 ^
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/interception/tests/interception_linux_test.cc:38:3: error: unterminated function-like macro invocation
  EXPECT_EQ(GetFuncAddr("dummy_doesnt_exist__", nullptr);
  ^
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1922:9: note: macro 'EXPECT_EQ' defined here
#define EXPECT_EQ(val1, val2) \
        ^
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/interception/tests/interception_linux_test.cc:61:32: error: expected '}'
#endif  // #if !SANITIZER_DEBUG
                               ^
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/interception/tests/interception_linux_test.cc:36:33: note: to match this '{'
TEST(Interception, GetFuncAddr) {
                                ^
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/interception/tests/interception_linux_test.cc:61:32: error: expected '}'
#endif  // #if !SANITIZER_DEBUG
                               ^
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/interception/tests/interception_linux_test.cc:34:26: note: to match this '{'
namespace __interception {

[vitalybuka]

This function has evolved over the years. Comment about return value is
lying. Paramters real/wrapper don't make much sense and are not supplied
at any call site. Real return type is never used.

I guess they are used

#define ASAN_INTERCEPT_FUNC(name)                                        \
  do {                                                                   \
    if ((!INTERCEPT_FUNCTION(name) || !REAL(name)))                      \
      VReport(1, "AddressSanitizer: failed to intercept '" #name "'\n"); \
  } while (0)

I understand that as weird protection against double intercept

Refactored to be consistent with GetFuncAddrVer.

compiler-rt/lib/interception/interception_linux.cc

40

It would be much easier to confirm that this is NFC if it was several patches

1. extract GetFuncAddr from GetRealFunctionAddress
2. Renames

...

[vitalybuka]

So it's not NFC, at least for VReport

[vitalybuka]

Fix compilation

[yln]

#define ASAN_INTERCEPT_FUNC(name)                                        \
  do {                                                                   \
    if ((!INTERCEPT_FUNCTION(name) || !REAL(name)))                      \
      VReport(1, "AddressSanitizer: failed to intercept '" #name "'\n"); \
  } while (0)

I understand that as weird protection against double intercept

Very weird indeed.

// returns true if a function with the given name was found.
bool GetRealFunctionAddress(const char *func_name, uptr *func_addr, uptr real, uptr wrapper) {
  // ...
  return real == wrapper;
}

Usage:

#define INTERCEPT_FUNCTION_LINUX_OR_FREEBSD(func)                          \
  ::__interception::GetRealFunctionAddress(                                \
      #func, (::__interception::uptr *)&__interception::PTR_TO_REAL(func), \
      (::__interception::uptr) & (func),                                   \
      (::__interception::uptr) & WRAP(func))

That it means it evaluates to & (func) == & WRAP(func).

I will do what you suggested and break it down into smaller parts.

compiler-rt/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 *GetFuncAddrVer(const char *func_name, const char *ver);
+void *GetFuncAddr(const char *name);
+void *GetFuncAddrVer(const char *name, const char *ver);
 }  // namespace __interception
 
-#define INTERCEPT_FUNCTION_LINUX_OR_FREEBSD(func)                          \
-  ::__interception::GetRealFunctionAddress(                                \
-      #func, (::__interception::uptr *)&__interception::PTR_TO_REAL(func), \
-      (::__interception::uptr) & (func),                                   \
-      (::__interception::uptr) & WRAP(func))
+#define INTERCEPT_FUNCTION_LINUX_OR_FREEBSD(func)                         \
+  (REAL(func) = (FUNC_TYPE(func)) ::__interception::GetFuncAddr(#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))
+#define INTERCEPT_FUNCTION_VER_LINUX_OR_FREEBSD(func, symver)             \
+  (REAL(func) =                                                           \
+      (FUNC_TYPE(func)) ::__interception::GetFuncAddrVer(#func, symver))
 #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

compiler-rt/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) {
+void *GetFuncAddr(const char *name) {
 #if SANITIZER_NETBSD
   // FIXME: Find a better way to handle renames
-  if (StrCmp(func_name, "sigaction"))
-    func_name = "__sigaction14";
+  if (StrCmp(name, "sigaction"))
+    name = "__sigaction14";
 #endif
-  *func_addr = (uptr)dlsym(RTLD_NEXT, func_name);
-  if (!*func_addr) {
+  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.
-    *func_addr = (uptr)dlsym(RTLD_DEFAULT, func_name);
+    addr = dlsym(RTLD_DEFAULT, name);
   }
-  return real == wrapper;
+  return addr;
 }
 
 // Android and Solaris do not have dlvsym
 #if !SANITIZER_ANDROID && !SANITIZER_SOLARIS && !SANITIZER_OPENBSD
-void *GetFuncAddrVer(const char *func_name, const char *ver) {
-  return dlvsym(RTLD_NEXT, func_name, ver);
+void *GetFuncAddrVer(const char *name, const char *ver) {
+  return dlvsym(RTLD_NEXT, name, ver);
 }
 #endif  // !SANITIZER_ANDROID
 
 }  // namespace __interception
 
 #endif  // SANITIZER_LINUX || SANITIZER_FREEBSD || SANITIZER_NETBSD ||
         // SANITIZER_OPENBSD || SANITIZER_SOLARIS

compiler-rt/lib/interception/tests/interception_linux_test.cc

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

compiler-rt/lib/tsan/rtl/tsan_interceptors.cc

 #define sigsetjmp_symname sigsetjmp
 #endif
 
 #define TSAN_INTERCEPTOR_SETJMP_(x) __interceptor_ ## x
 #define TSAN_INTERCEPTOR_SETJMP__(x) TSAN_INTERCEPTOR_SETJMP_(x)
 #define TSAN_INTERCEPTOR_SETJMP TSAN_INTERCEPTOR_SETJMP__(setjmp_symname)
 #define TSAN_INTERCEPTOR_SIGSETJMP TSAN_INTERCEPTOR_SETJMP__(sigsetjmp_symname)
 
-#define TSAN_STRING_SETJMP SANITIZER_STRINGIFY(setjmp_symname)
-#define TSAN_STRING_SIGSETJMP SANITIZER_STRINGIFY(sigsetjmp_symname)
-
 // Not called.  Merely to satisfy TSAN_INTERCEPT().
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 int TSAN_INTERCEPTOR_SETJMP(void *env);
 extern "C" int TSAN_INTERCEPTOR_SETJMP(void *env) {
   CHECK(0);
   return 0;
 }
 

 #endif
 
   new(interceptor_ctx()) InterceptorContext();
 
   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);
+  TSAN_INTERCEPT(setjmp_symname);
+  TSAN_INTERCEPT(_setjmp);
+  TSAN_INTERCEPT(sigsetjmp_symname);
 #if !SANITIZER_NETBSD
-  GetRealFunctionAddress("__sigsetjmp", (uptr*)&REAL(__sigsetjmp), 0, 0);
-#endif
+  TSAN_INTERCEPT(__sigsetjmp);
 #endif
 
   TSAN_INTERCEPT(longjmp_symname);
   TSAN_INTERCEPT(siglongjmp_symname);
 #if SANITIZER_NETBSD
   TSAN_INTERCEPT(_longjmp);
 #endif