[compiler-rt] Fix warnings in interception code

[etienneb]

Summary

This patch is re-introducing the code to fix the
dynamic hooking on windows and to fix a compiler
warning on Apple.

Related patches:

• https://reviews.llvm.org/D22641
• https://reviews.llvm.org/D22610
• https://reviews.llvm.org/rL276311
• https://reviews.llvm.org/rL276490

Both architecture are using different techniques to
hook on library functions (memchr, strcpy,...).

On Apple, the function is not dynamically hooked and
the symbol always points to a valid function
(i.e. can't be null). The REAL macro returns the
symbol.

On windows, the function is dynamically patch and the
REAL(...) function may or may not be null. It depend
on whether or not the function was hooked correctly.
Also, on windows memcpy and memmove are the same.

#if !defined(__APPLE__)
[...]
# define REAL(x) __interception::PTR_TO_REAL(x)
# define ASSIGN_REAL(dst, src) REAL(dst) = REAL(src)
[...]
#else  // __APPLE__
[...]
# define REAL(x) x
# define ASSIGN_REAL(x, y)
[...]
#endif  // __APPLE__

[hans]

I tried this out on my mac, and the tests passed fine.

[etienneb]

Tried on windows 32/64 (check-asan). Tests are fine too.

-- Testing: 532 tests, 16 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
Testing Time: 99.99s
  Expected Passes    : 366
  Passes With Retry  : 2
  Expected Failures  : 18
  Unsupported Tests  : 146

-- Testing: 534 tests, 16 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
Testing Time: 94.54s
  Expected Passes    : 370
  Expected Failures  : 18
  Unsupported Tests  : 146

[etienneb]

Tests are fine on linux too:

Testing Time: 84.96s
  Expected Passes    : 1369
  Expected Failures  : 12
  Unsupported Tests  : 415

[bruno]

Gonna test the patch now, get back to you soon!

[bruno]

The tests also pass fine here as well. I still see 3 warnings:

warning: address of function 'memchr' will always evaluate to 'true' [-Wpointer-bool-conversion]

But are you not intending to fix those in this patch, are you?

[etienneb]

In D22758#495090, @bruno wrote:

The tests also pass fine here as well. I still see 3 warnings:

warning: address of function 'memchr' will always evaluate to 'true' [-Wpointer-bool-conversion]

But are you not intending to fix those in this patch, are you?

Yes. It supposed to be fixed.

I think this is the mistake:

#ifdef SANITIZER_WINDOWS   -->  #if SANITIZER_WINDOWS

[etienneb]

fix ifdef

[bruno]

Testing...

[bruno]

WFM now!

[etienneb]

In D22758#495186, @bruno wrote:

WFM now!

Wouhou.
Thanks for testing.

[kcc]

please remember to *NOT* use #ifdef if if() is an alternative.
If you can only use #ifdef, think again.

[rnk]

In D22758#495682, @kcc wrote:

please remember to *NOT* use #ifdef if if() is an alternative.
If you can only use #ifdef, think again.

We either have to use ifdef in this case or some other contortion to avoid a warning about the address of a function never being null.

[etienneb]

In D22758#496945, @rnk wrote:

In D22758#495682, @kcc wrote:

please remember to *NOT* use #ifdef if if() is an alternative.
If you can only use #ifdef, think again.

We either have to use ifdef in this case or some other contortion to avoid a warning about the address of a function never being null.

On apple, that code is generating a warning:

if (REAL(memchr)) {

The condition is always true.

[rnk]

lgtm

Oops, I tried to stamp it on my last comment, but it didn't happen...

lib/asan/asan_interceptors.cc

 namespace __asan {
 void InitializeAsanInterceptors() {
   static bool was_called_once;
   CHECK(was_called_once == false);
   was_called_once = true;
   InitializeCommonInterceptors();
 
   // Intercept mem* functions.
-  ASAN_INTERCEPT_FUNC(memcpy);
+  ASAN_INTERCEPT_FUNC(memmove);
   ASAN_INTERCEPT_FUNC(memset);
   if (PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE) {
     // In asan, REAL(memmove) is not used, but it is used in msan.
-    ASAN_INTERCEPT_FUNC(memmove);
+    ASAN_INTERCEPT_FUNC(memcpy);
+  } else {
+    ASSIGN_REAL(memcpy, memmove);
   }
   CHECK(REAL(memcpy));
 
   // Intercept str* functions.
   ASAN_INTERCEPT_FUNC(strcat);  // NOLINT
   ASAN_INTERCEPT_FUNC(strcpy);  // NOLINT
   ASAN_INTERCEPT_FUNC(wcslen);
   ASAN_INTERCEPT_FUNC(strncat);

lib/asan/tests/asan_str_test.cc

TEST(AddressSanitizer, StrArgsOverlapTest) {
   size_t size = Ident(100);
   char *str = Ident((char*)malloc(size));
 
 // Do not check memcpy() on OS X 10.7 and later, where it actually aliases
 // memmove().
 #if !defined(__APPLE__) || !defined(MAC_OS_X_VERSION_10_7) || \
     (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
   // Check "memcpy". Use Ident() to avoid inlining.
+#if PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE
   memset(str, 'z', size);
   Ident(memcpy)(str + 1, str + 11, 10);
   Ident(memcpy)(str, str, 0);
   EXPECT_DEATH(Ident(memcpy)(str, str + 14, 15), OverlapErrorMessage("memcpy"));
   EXPECT_DEATH(Ident(memcpy)(str + 14, str, 15), OverlapErrorMessage("memcpy"));
 #endif
+#endif
 
   // We do not treat memcpy with to==from as a bug.
   // See http://llvm.org/bugs/show_bug.cgi?id=11763.
   // EXPECT_DEATH(Ident(memcpy)(str + 20, str + 20, 1),
   //              OverlapErrorMessage("memcpy"));
 
   // Check "strcpy".
   memset(str, 'z', size);

lib/interception/interception.h

 # define REAL(x) __interception::PTR_TO_REAL(x)
 # define FUNC_TYPE(x) x##_f
 
 # define DECLARE_REAL(ret_type, func, ...) \
     typedef ret_type (*FUNC_TYPE(func))(__VA_ARGS__); \
     namespace __interception { \
       extern FUNC_TYPE(func) PTR_TO_REAL(func); \
     }
+# define ASSIGN_REAL(dst, src) REAL(dst) = REAL(src)
 #else  // __APPLE__
 # define REAL(x) x
 # define DECLARE_REAL(ret_type, func, ...) \
     extern "C" ret_type func(__VA_ARGS__);
+# define ASSIGN_REAL(x, y)
 #endif  // __APPLE__
 
 #define DECLARE_REAL_AND_INTERCEPTOR(ret_type, func, ...) \
   DECLARE_REAL(ret_type, func, __VA_ARGS__) \
   extern "C" ret_type WRAP(func)(__VA_ARGS__);
 
 // Generally, you don't need to use DEFINE_REAL by itself, as INTERCEPTOR
 // macros does its job. In exceptional cases you may need to call REAL(foo)

lib/sanitizer_common/sanitizer_common_interceptors.inc

 #endif
 
 #if SANITIZER_INTERCEPT_MEMCHR
 INTERCEPTOR(void*, memchr, const void *s, int c, SIZE_T n) {
   if (COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED)
     return internal_memchr(s, c, n);
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, memchr, s, c, n);
+#if SANITIZER_WINDOWS
   void *res;
   if (REAL(memchr)) {
     res = REAL(memchr)(s, c, n);
   } else {
     res = internal_memchr(s, c, n);
   }
+#else
+  void *res = REAL(memchr)(s, c, n);
+#endif
   uptr len = res ? (char *)res - (const char *)s + 1 : n;
   COMMON_INTERCEPTOR_READ_RANGE(ctx, s, len);
   return res;
 }
 
 #define INIT_MEMCHR COMMON_INTERCEPT_FUNCTION(memchr)
 #else
 #define INIT_MEMCHR