[compiler-rt] Avoid memintrinsic calls inserted by the compiler
D135716 introduced -ftrivial-auto-var-init=pattern where supported.
Unfortunately this introduces unwanted memset() for large stack arrays,
as shown by the new tests added for asan and msan (tsan already had this
test).
In general, the problem of compiler-inserted memintrinsic calls
(memset/memcpy/memmove) is not new to compiler-rt, and has been a
problem before.
To avoid introducing unwanted memintrinsic calls, we redefine
memintrinsics as __sanitizer_internal_mem* at the assembly level for
most source files automatically (where sanitizer_common_internal_defs.h
is included).
In few cases, redefining a symbol in this way causes issues for
interceptors, namely the memintrinsic interceptor themselves. For such
source files we have to selectively disable the redefinition.
Other alternatives have been considered, but simply do not work well in
the context of compiler-rt:
1. Linker --wrap: this does not work because --wrap only
applies to the final link, and would not apply when building
sanitizer static libraries.
2. Changing references to memset() via objcopy: this may work,
but due to the complexities of the build system, introducing
such a post-processing step for the right object files (in
particular object files defining memset cannot be touched)
seems infeasible.
The chosen solution works well (as shown by the tests). Other libraries
have chosen the same solution where nothing else works (see e.g. glibc's
"symbol-hacks.h").
v4:
- Add interface attribute to __sanitizer_internal_mem* declarations as
well, as otherwise some compilers (MSVC) will complain.
- Add SANITIZER_COMMON_NO_REDEFINE_BUILTINS to source files using
C++STL, since this could lead to ODR violations (see added comment).
v3:
- Don't use ALIAS() to alias internal_mem*() functions to
__sanitizer_internal_mem*() functions, but just define them as
ALWAYS_INLINE functions instead. This will work on darwin and windows.
v2:
- Fix ubsan_minimal build where compiler decides to insert
memset/memcpy: ubsan_minimal has work without RTSanitizerCommonLibc,
therefore do not redefine the builtins.
- Fix definition of internal_mem* functions with compilers that want the
aliased function to already be defined before.
- Fix definition of __sanitizer_internal_mem* functions with compilers
more pedantic about attribute placement around extern "C".
Reviewed By: vitalybuka, dvyukov
Differential Revision: https://reviews.llvm.org/D151152
GitOrigin-RevId: 0a71e25e2448ee471b1ebe74e910c5de9b9c82b4
diff --git a/lib/asan/asan_interceptors_memintrinsics.cpp b/lib/asan/asan_interceptors_memintrinsics.cpp
index 9d14524..4e4ea71 100644
--- a/lib/asan/asan_interceptors_memintrinsics.cpp
+++ b/lib/asan/asan_interceptors_memintrinsics.cpp
@@ -11,6 +11,8 @@
// ASan versions of memcpy, memmove, and memset.
//===---------------------------------------------------------------------===//
+#define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+
#include "asan_interceptors_memintrinsics.h"
#include "asan_interceptors.h"
diff --git a/lib/asan/tests/CMakeLists.txt b/lib/asan/tests/CMakeLists.txt
index a0c6d29..3c0d2d4 100644
--- a/lib/asan/tests/CMakeLists.txt
+++ b/lib/asan/tests/CMakeLists.txt
@@ -28,6 +28,7 @@
-I${COMPILER_RT_SOURCE_DIR}/lib
-I${COMPILER_RT_SOURCE_DIR}/lib/asan
-I${COMPILER_RT_SOURCE_DIR}/lib/sanitizer_common/tests
+ -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS
-fno-rtti
-O2
-Wno-format
diff --git a/lib/hwasan/hwasan_interceptors.cpp b/lib/hwasan/hwasan_interceptors.cpp
index 2610933..bffb4e0 100644
--- a/lib/hwasan/hwasan_interceptors.cpp
+++ b/lib/hwasan/hwasan_interceptors.cpp
@@ -14,6 +14,8 @@
// sanitizer_common/sanitizer_common_interceptors.h
//===----------------------------------------------------------------------===//
+#define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+
#include "hwasan.h"
#include "hwasan_allocator.h"
#include "hwasan_checks.h"
diff --git a/lib/interception/tests/CMakeLists.txt b/lib/interception/tests/CMakeLists.txt
index 688edc3..f6840e1 100644
--- a/lib/interception/tests/CMakeLists.txt
+++ b/lib/interception/tests/CMakeLists.txt
@@ -17,6 +17,7 @@
-I${COMPILER_RT_SOURCE_DIR}/include
-I${COMPILER_RT_SOURCE_DIR}/lib
-I${COMPILER_RT_SOURCE_DIR}/lib/interception
+ -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS
-fno-rtti
-O2
-Werror=sign-compare)
diff --git a/lib/memprof/memprof_interceptors_memintrinsics.cpp b/lib/memprof/memprof_interceptors_memintrinsics.cpp
index dae2ab5..56bd116 100644
--- a/lib/memprof/memprof_interceptors_memintrinsics.cpp
+++ b/lib/memprof/memprof_interceptors_memintrinsics.cpp
@@ -11,6 +11,8 @@
// MemProf versions of memcpy, memmove, and memset.
//===---------------------------------------------------------------------===//
+#define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+
#include "memprof_interceptors_memintrinsics.h"
#include "memprof_interceptors.h"
diff --git a/lib/memprof/tests/CMakeLists.txt b/lib/memprof/tests/CMakeLists.txt
index df74540..a5492f3 100644
--- a/lib/memprof/tests/CMakeLists.txt
+++ b/lib/memprof/tests/CMakeLists.txt
@@ -8,6 +8,7 @@
${COMPILER_RT_GMOCK_CFLAGS}
${SANITIZER_TEST_CXX_CFLAGS}
-I${COMPILER_RT_SOURCE_DIR}/lib/
+ -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS
-O2
-g
-fno-rtti
diff --git a/lib/msan/msan_interceptors.cpp b/lib/msan/msan_interceptors.cpp
index 96abc47..6f57c33 100644
--- a/lib/msan/msan_interceptors.cpp
+++ b/lib/msan/msan_interceptors.cpp
@@ -14,6 +14,8 @@
// sanitizer_common/sanitizer_common_interceptors.h
//===----------------------------------------------------------------------===//
+#define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+
#include "interception/interception.h"
#include "msan.h"
#include "msan_chained_origin_depot.h"
diff --git a/lib/sanitizer_common/CMakeLists.txt b/lib/sanitizer_common/CMakeLists.txt
index d4187a9..c14c66a 100644
--- a/lib/sanitizer_common/CMakeLists.txt
+++ b/lib/sanitizer_common/CMakeLists.txt
@@ -173,6 +173,7 @@
sanitizer_procmaps.h
sanitizer_ptrauth.h
sanitizer_quarantine.h
+ sanitizer_redefine_builtins.h
sanitizer_report_decorator.h
sanitizer_ring_buffer.h
sanitizer_signal_interceptors.inc
diff --git a/lib/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc b/lib/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc
index e6b967c..52e489d 100644
--- a/lib/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc
+++ b/lib/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc
@@ -9,6 +9,10 @@
// Memintrinsic function interceptors for tools like AddressSanitizer,
// ThreadSanitizer, MemorySanitizer, etc.
//
+// These interceptors are part of the common interceptors, but separated out so
+// that implementations may add them, if necessary, to a separate source file
+// that should define SANITIZER_COMMON_NO_REDEFINE_BUILTINS at the top.
+//
// This file should be included into the tool's memintrinsic interceptor file,
// which has to define its own macros:
// COMMON_INTERCEPTOR_ENTER
@@ -20,6 +24,10 @@
// COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED
//===----------------------------------------------------------------------===//
+#ifdef SANITIZER_REDEFINE_BUILTINS_H
+#error "Define SANITIZER_COMMON_NO_REDEFINE_BUILTINS in .cpp file"
+#endif
+
#include "interception/interception.h"
#include "sanitizer_platform_interceptors.h"
diff --git a/lib/sanitizer_common/sanitizer_common_interface.inc b/lib/sanitizer_common/sanitizer_common_interface.inc
index 37efb57..557207f 100644
--- a/lib/sanitizer_common/sanitizer_common_interface.inc
+++ b/lib/sanitizer_common/sanitizer_common_interface.inc
@@ -46,3 +46,7 @@
INTERFACE_FUNCTION(__sanitizer_print_memory_profile)
INTERFACE_WEAK_FUNCTION(__sanitizer_free_hook)
INTERFACE_WEAK_FUNCTION(__sanitizer_malloc_hook)
+// Memintrinsic functions.
+INTERFACE_FUNCTION(__sanitizer_internal_memcpy)
+INTERFACE_FUNCTION(__sanitizer_internal_memmove)
+INTERFACE_FUNCTION(__sanitizer_internal_memset)
diff --git a/lib/sanitizer_common/sanitizer_internal_defs.h b/lib/sanitizer_common/sanitizer_internal_defs.h
index 95f4760..e5dd65a 100644
--- a/lib/sanitizer_common/sanitizer_internal_defs.h
+++ b/lib/sanitizer_common/sanitizer_internal_defs.h
@@ -13,6 +13,7 @@
#define SANITIZER_DEFS_H
#include "sanitizer_platform.h"
+#include "sanitizer_redefine_builtins.h"
#ifndef SANITIZER_DEBUG
# define SANITIZER_DEBUG 0
diff --git a/lib/sanitizer_common/sanitizer_libc.cpp b/lib/sanitizer_common/sanitizer_libc.cpp
index d3076f0..4a6fa5e 100644
--- a/lib/sanitizer_common/sanitizer_libc.cpp
+++ b/lib/sanitizer_common/sanitizer_libc.cpp
@@ -10,6 +10,9 @@
// run-time libraries. See sanitizer_libc.h for details.
//===----------------------------------------------------------------------===//
+// Do not redefine builtins; this file is defining the builtin replacements.
+#define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+
#include "sanitizer_allocator_internal.h"
#include "sanitizer_common.h"
#include "sanitizer_libc.h"
@@ -46,7 +49,10 @@
return 0;
}
-void *internal_memcpy(void *dest, const void *src, uptr n) {
+extern "C" {
+SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memcpy(void *dest,
+ const void *src,
+ uptr n) {
char *d = (char*)dest;
const char *s = (const char *)src;
for (uptr i = 0; i < n; ++i)
@@ -54,7 +60,8 @@
return dest;
}
-void *internal_memmove(void *dest, const void *src, uptr n) {
+SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memmove(
+ void *dest, const void *src, uptr n) {
char *d = (char*)dest;
const char *s = (const char *)src;
sptr i, signed_n = (sptr)n;
@@ -72,7 +79,8 @@
return dest;
}
-void *internal_memset(void* s, int c, uptr n) {
+SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memset(void *s, int c,
+ uptr n) {
// Optimize for the most performance-critical case:
if ((reinterpret_cast<uptr>(s) % 16) == 0 && (n % 16) == 0) {
u64 *p = reinterpret_cast<u64*>(s);
@@ -95,6 +103,7 @@
}
return s;
}
+} // extern "C"
uptr internal_strcspn(const char *s, const char *reject) {
uptr i;
diff --git a/lib/sanitizer_common/sanitizer_libc.h b/lib/sanitizer_common/sanitizer_libc.h
index 39a2126..e881db2 100644
--- a/lib/sanitizer_common/sanitizer_libc.h
+++ b/lib/sanitizer_common/sanitizer_libc.h
@@ -24,15 +24,33 @@
// internal_X() is a custom implementation of X() for use in RTL.
+extern "C" {
+// These are used as builtin replacements; see sanitizer_redefine_builtins.h.
+// In normal runtime code, use the __sanitizer::internal_X() aliases instead.
+SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memcpy(void *dest,
+ const void *src,
+ uptr n);
+SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memmove(
+ void *dest, const void *src, uptr n);
+SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memset(void *s, int c,
+ uptr n);
+} // extern "C"
+
// String functions
s64 internal_atoll(const char *nptr);
void *internal_memchr(const void *s, int c, uptr n);
void *internal_memrchr(const void *s, int c, uptr n);
int internal_memcmp(const void* s1, const void* s2, uptr n);
-void *internal_memcpy(void *dest, const void *src, uptr n);
-void *internal_memmove(void *dest, const void *src, uptr n);
+ALWAYS_INLINE void *internal_memcpy(void *dest, const void *src, uptr n) {
+ return __sanitizer_internal_memcpy(dest, src, n);
+}
+ALWAYS_INLINE void *internal_memmove(void *dest, const void *src, uptr n) {
+ return __sanitizer_internal_memmove(dest, src, n);
+}
// Should not be used in performance-critical places.
-void *internal_memset(void *s, int c, uptr n);
+ALWAYS_INLINE void *internal_memset(void *s, int c, uptr n) {
+ return __sanitizer_internal_memset(s, c, n);
+}
char* internal_strchr(const char *s, int c);
char *internal_strchrnul(const char *s, int c);
int internal_strcmp(const char *s1, const char *s2);
diff --git a/lib/sanitizer_common/sanitizer_redefine_builtins.h b/lib/sanitizer_common/sanitizer_redefine_builtins.h
new file mode 100644
index 0000000..9910ae7
--- /dev/null
+++ b/lib/sanitizer_common/sanitizer_redefine_builtins.h
@@ -0,0 +1,52 @@
+//===-- sanitizer_redefine_builtins.h ---------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Redefine builtin functions to use internal versions. This is needed where
+// compiler optimizations end up producing unwanted libcalls!
+//
+//===----------------------------------------------------------------------===//
+#ifndef SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+#ifndef SANITIZER_REDEFINE_BUILTINS_H
+#define SANITIZER_REDEFINE_BUILTINS_H
+
+// The asm hack only works with GCC and Clang.
+#if !defined(_MSC_VER) || defined(__clang__)
+
+asm("memcpy = __sanitizer_internal_memcpy");
+asm("memmove = __sanitizer_internal_memmove");
+asm("memset = __sanitizer_internal_memset");
+
+// The builtins should not be redefined in source files that make use of C++
+// standard libraries, in particular where C++STL headers with inline functions
+// are used. The redefinition in such cases would lead to ODR violations.
+//
+// Try to break the build in common cases where builtins shouldn't be redefined.
+namespace std {
+class Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file {
+ Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file(
+ const Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file&) = delete;
+ Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file& operator=(
+ const Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file&) = delete;
+};
+using array = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using atomic = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using function = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using map = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using set = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using shared_ptr = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using string = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using unique_ptr = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using unordered_map = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using unordered_set = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+using vector = Define_SANITIZER_COMMON_NO_REDEFINE_BUILTINS_in_cpp_file;
+} // namespace std
+
+#endif // !_MSC_VER || __clang__
+
+#endif // SANITIZER_REDEFINE_BUILTINS_H
+#endif // SANITIZER_COMMON_NO_REDEFINE_BUILTINS
diff --git a/lib/sanitizer_common/tests/CMakeLists.txt b/lib/sanitizer_common/tests/CMakeLists.txt
index 4924a9e..dad90cd 100644
--- a/lib/sanitizer_common/tests/CMakeLists.txt
+++ b/lib/sanitizer_common/tests/CMakeLists.txt
@@ -70,6 +70,7 @@
-I${COMPILER_RT_SOURCE_DIR}/include
-I${COMPILER_RT_SOURCE_DIR}/lib
-I${COMPILER_RT_SOURCE_DIR}/lib/sanitizer_common
+ -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS
-fno-rtti
-O2
-Werror=sign-compare
diff --git a/lib/tsan/rtl/tsan_interceptors_memintrinsics.cpp b/lib/tsan/rtl/tsan_interceptors_memintrinsics.cpp
index 6a2a429..c8b6b2e 100644
--- a/lib/tsan/rtl/tsan_interceptors_memintrinsics.cpp
+++ b/lib/tsan/rtl/tsan_interceptors_memintrinsics.cpp
@@ -10,6 +10,8 @@
//
//===----------------------------------------------------------------------===//
+#define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+
#include "tsan_interceptors.h"
#include "tsan_interface.h"
diff --git a/lib/tsan/tests/CMakeLists.txt b/lib/tsan/tests/CMakeLists.txt
index 8afd217..c02c227 100644
--- a/lib/tsan/tests/CMakeLists.txt
+++ b/lib/tsan/tests/CMakeLists.txt
@@ -11,6 +11,7 @@
-I${COMPILER_RT_SOURCE_DIR}/include
-I${COMPILER_RT_SOURCE_DIR}/lib
-I${COMPILER_RT_SOURCE_DIR}/lib/tsan/rtl
+ -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS
-DGTEST_HAS_RTTI=0
-fno-rtti
)
diff --git a/lib/ubsan_minimal/CMakeLists.txt b/lib/ubsan_minimal/CMakeLists.txt
index 504dd3b..07e96a7 100644
--- a/lib/ubsan_minimal/CMakeLists.txt
+++ b/lib/ubsan_minimal/CMakeLists.txt
@@ -6,7 +6,9 @@
include_directories(..)
-set(UBSAN_CFLAGS ${SANITIZER_COMMON_CFLAGS})
+set(UBSAN_CFLAGS
+ ${SANITIZER_COMMON_CFLAGS}
+ -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS)
append_rtti_flag(OFF UBSAN_CFLAGS)
set(UBSAN_LINK_FLAGS ${SANITIZER_COMMON_LINK_FLAGS})
diff --git a/lib/xray/CMakeLists.txt b/lib/xray/CMakeLists.txt
index 731de2c..f81b0cb 100644
--- a/lib/xray/CMakeLists.txt
+++ b/lib/xray/CMakeLists.txt
@@ -141,7 +141,7 @@
set(XRAY_CFLAGS
${COMPILER_RT_COMMON_CFLAGS}
${COMPILER_RT_CXX_CFLAGS})
-set(XRAY_COMMON_DEFINITIONS XRAY_HAS_EXCEPTIONS=1)
+set(XRAY_COMMON_DEFINITIONS SANITIZER_COMMON_NO_REDEFINE_BUILTINS XRAY_HAS_EXCEPTIONS=1)
# Too many existing bugs, needs cleanup.
append_list_if(COMPILER_RT_HAS_WNO_FORMAT -Wno-format XRAY_CFLAGS)
diff --git a/test/asan/TestCases/Linux/check_memcpy.c b/test/asan/TestCases/Linux/check_memcpy.c
new file mode 100644
index 0000000..d5fee16
--- /dev/null
+++ b/test/asan/TestCases/Linux/check_memcpy.c
@@ -0,0 +1,8 @@
+// Verify runtime doesn't contain compiler-emitted memcpy/memmove calls.
+//
+// REQUIRES: shared_unwind, x86_64-target-arch
+
+// RUN: %clang_asan -O1 %s -o %t
+// RUN: llvm-objdump -d -l %t | FileCheck --implicit-check-not="{{(callq|jmpq) .*<(__interceptor_.*)?mem(cpy|set|move)>}}" %s
+
+int main() { return 0; }
diff --git a/test/msan/Linux/check_memcpy.c b/test/msan/Linux/check_memcpy.c
new file mode 100644
index 0000000..42af2d7
--- /dev/null
+++ b/test/msan/Linux/check_memcpy.c
@@ -0,0 +1,8 @@
+// Verify runtime doesn't contain compiler-emitted memcpy/memmove calls.
+//
+// REQUIRES: shared_unwind, x86_64-target-arch
+
+// RUN: %clang_msan -O1 %s -o %t
+// RUN: llvm-objdump -d -l %t | FileCheck --implicit-check-not="{{(callq|jmpq) .*<(__interceptor_.*)?mem(cpy|set|move)>}}" %s
+
+int main() { return 0; }
diff --git a/test/tsan/Linux/check_memcpy.c b/test/tsan/Linux/check_memcpy.c
index 26f9961..2148056 100644
--- a/test/tsan/Linux/check_memcpy.c
+++ b/test/tsan/Linux/check_memcpy.c
@@ -5,16 +5,9 @@
// This could fail if using a static libunwind because that static libunwind
// could be uninstrumented and contain memcpy/memmove calls not intercepted by
// tsan.
-// REQUIRES: shared_unwind
+// REQUIRES: shared_unwind, x86_64-target-arch
// RUN: %clang_tsan -O1 %s -o %t
-// RUN: llvm-objdump -d -l %t | FileCheck %s
+// RUN: llvm-objdump -d -l %t | FileCheck --implicit-check-not="{{(callq|jmpq) .*<(__interceptor_.*)?mem(cpy|set|move)>}}" %s
-int main() {
- return 0;
-}
-
-// CHECK-NOT: callq {{.*<(__interceptor_)?mem(cpy|set)>}}
-// tail calls:
-// CHECK-NOT: jmpq {{.*<(__interceptor_)?mem(cpy|set)>}}
-
+int main() { return 0; }
