[PATCH] D27052: [compiler-rt][asan] Fix overlaping parameters for memmove/memcpy on windows.

[Filipe Cabecinhas via Phabricator via llvm-commits]

filcab added inline comments.


================
Comment at: lib/asan/asan_interceptors.cc:460
   ASAN_INTERCEPTOR_ENTER(ctx, memcpy);
-#if !SANITIZER_MAC
+#if !SANITIZER_MAC && PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE
   ASAN_MEMCPY_IMPL(ctx, to, from, size);
----------------
zaks.anna wrote:
> filcab wrote:
> > This part looks ok, of course.
> Why do we need "!SANITIZER_MAC"? Wouldn't just calling PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE work on the Mac? It's defined as :
> bool PlatformHasDifferentMemcpyAndMemmove() {
>   // On OS X 10.7 memcpy() and memmove() are both resolved
>   // into memmove$VARIANT$sse42.
>   // See also https://github.com/google/sanitizers/issues/34.
>   // TODO(glider): need to check dynamically that memcpy() and memmove() are
>   // actually the same function.
>   return GetMacosVersion() == MACOS_VERSION_SNOW_LEOPARD;
> }
> 
Indeed, a regular
```if (PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE)
  ASAN_MEMCPY_IMPL(ctx, to, from, size);
else
  // Keep the comments...
  ASAN_MEMMOVE_IMPL(ctx, to, from, size);
```
Should do it. Will be folded away if `PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE` is just the constant `true` or `false`.

Additionally, you can make `PlatformHasDifferentMemcpyAndMemmove()` cache the result of the test. If we cache it, having that extra test all the time on OS X is probably not a big deal.


https://reviews.llvm.org/D27052