[ASan] Update meminstrinsics to use library memmove rather than internal

[DanBlackwell]

Currently memcpy and memset intrinsics map through to the library implementations if ASan has been inited, whereas memmove always calls internal_memmove.

This patch changes memmove to use the library implementation if ASan has been inited.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Dan Blackwell (DanBlackwell)

Changes

Currently memcpy and memset intrinsics map through to the library implementations if ASan has been inited, whereas memmove always calls internal_memmove.

This patch changes memmove to use the library implementation if ASan has been inited.

---

Full diff: https://github.com/llvm/llvm-project/pull/160740.diff

2 Files Affected:

• (modified) compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp (+3-1)
• (modified) compiler-rt/lib/asan/asan_interceptors_memintrinsics.h (+1)

diff --git a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
index bdf328f892063..f52ae9ae8d17c 100644
--- a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
@@ -55,8 +55,10 @@ using namespace __asan;
     if (LIKELY(replace_intrin_cached)) {       \
       ASAN_READ_RANGE(ctx, from, size);        \
       ASAN_WRITE_RANGE(ctx, to, size);         \
+    } else if (UNLIKELY(!AsanInited())) {      \
+      return internal_memmove(to, from, size); \
     }                                          \
-    return internal_memmove(to, from, size);   \
+    return REAL(memmove)(to, from, size);      \
   } while (0)
 
 void *__asan_memcpy(void *to, const void *from, uptr size) {
diff --git a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.h b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.h
index 14727a5d665ed..ec988cff51c59 100644
--- a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.h
+++ b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.h
@@ -20,6 +20,7 @@
 
 DECLARE_REAL(void *, memcpy, void *to, const void *from, SIZE_T size)
 DECLARE_REAL(void *, memset, void *block, int c, SIZE_T size)
+DECLARE_REAL(void *, memmove, void *to, const void *from, SIZE_T size)
 
 namespace __asan {
 

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions h,cpp -- compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp compiler-rt/lib/asan/asan_interceptors_memintrinsics.h

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.h b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.h
index ec988cff5..11e77e5ce 100644
--- a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.h
+++ b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.h
@@ -20,7 +20,7 @@
 
 DECLARE_REAL(void *, memcpy, void *to, const void *from, SIZE_T size)
 DECLARE_REAL(void *, memset, void *block, int c, SIZE_T size)
-DECLARE_REAL(void *, memmove, void *to, const void *from, SIZE_T size)
+DECLARE_REAL(void*, memmove, void* to, const void* from, SIZE_T size)
 
 namespace __asan {
 

[melver]

LGTM - do we need tests for this so it doesn't regress?

[DanBlackwell]

LGTM - do we need tests for this so it doesn't regress?

I can't see any tests for the memintrinsics stuff; do you think there's anything that we could be testing here @melver?

[melver]

LGTM - do we need tests for this so it doesn't regress?

I can't see any tests for the memintrinsics stuff; do you think there's anything that we could be testing here @melver?

I thought we should at least have tests they behave as expected when instrumented. But if they don't exist yet, perhaps nothing to do.
Did you encounter a particular issue that could be reproduced and used as a test?
In any case, feel free to submit.

[DanBlackwell]

Oh, looks like memmove is tested here:

llvm-project/compiler-rt/test/asan/TestCases/memset_test.cpp

Line 57 in f6fc5e8

#elif defined(TEST_MEMMOVE)

@melver what's your opinion on changing this test name to memintrinsics_test.cpp? It seems to cover the three mem* functions implemented in asan_interceptors_memintrinsics.cpp‎, rather than just memset as the name currently suggests.

Did you encounter a particular issue that could be reproduced and used as a test?

I came across this while looking at some profiling data (I was surprised to see a bunch of samples with internal_memmove being called from outside of the Asan library).

[melver]

Oh, looks like memmove is tested here:

llvm-project/compiler-rt/test/asan/TestCases/memset_test.cpp

Line 57 in f6fc5e8

#elif defined(TEST_MEMMOVE)

@melver what's your opinion on changing this test name to memintrinsics_test.cpp? It seems to cover the three mem* functions implemented in asan_interceptors_memintrinsics.cpp‎, rather than just memset as the name currently suggests.

That looks reasonable, although up to you - it's just a cosmetic change, but maybe will help to find its existence quicker in future.

Did you encounter a particular issue that could be reproduced and used as a test?

I came across this while looking at some profiling data (I was surprised to see a bunch of samples with internal_memmove being called from outside of the Asan library).

Thanks for clarifying.

[DanBlackwell]

Ok, I won't rename the test for now - will merge as is. Thanks for the review!