Page MenuHomePhabricator

[NFC][Sanitizer] Clean up GetRealFunctionAddress
AbandonedPublic

Authored by yln on Mar 18 2019, 12:03 PM.

Details

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.

Event Timeline

yln created this revision.Mar 18 2019, 12:03 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 18 2019, 12:03 PM
Herald added subscribers: llvm-commits, Restricted Project, kubamracek. · View Herald Transcript
yln edited the summary of this revision. (Show Details)Mar 18 2019, 12:07 PM
delcypher added inline comments.Mar 21 2019, 3:40 PM
compiler-rt/lib/interception/interception_linux.h
30

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

yln marked 2 inline comments as done.Mar 21 2019, 7:21 PM
yln added inline comments.
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);
yln marked an inline comment as done.Mar 21 2019, 7:21 PM

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 {

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

...

So it's not NFC, at least for VReport

Fix compilation

yln added a comment.Apr 24 2019, 7:08 PM
#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.

yln abandoned this revision.Apr 24 2019, 7:11 PM