-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[asan][NFCI] Rename asan_(malloc_)?linux.cpp to ...unix.cpp #132263
base: main
Are you sure you want to change the base?
Conversation
asan_(malloc_)?linux.cpp are misleadingly named because they cover many non-Linux OSes, such as BSDs, Fuchsia and Solaris. This is a footgun where changes may be made to these files without remembering it is not Linux-specific (e.g., I broke the Solaris build - see llvm#131975 (comment)). This patch mitigates the issue by renaming the file from ...linux to ...unix, which should hopefully give pause to anyone (me) when making Linux-specific changes.
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Thurston Dang (thurstond) Changesasan_(malloc_)?linux.cpp are misleadingly named because they cover many non-Linux OSes, such as BSDs, Fuchsia and Solaris. This is a footgun where changes may be made to these files without remembering it is not Linux-specific (e.g., I broke the Solaris build - see #131975 (comment)). This patch mitigates the issue by renaming the file from ...linux to ...unix, which should hopefully give pause to anyone (me) when making Linux-specific changes. Full diff: https://github.com/llvm/llvm-project/pull/132263.diff 4 Files Affected:
diff --git a/compiler-rt/lib/asan/CMakeLists.txt b/compiler-rt/lib/asan/CMakeLists.txt
index e2f39f224df9c..e47acbec3c6f0 100644
--- a/compiler-rt/lib/asan/CMakeLists.txt
+++ b/compiler-rt/lib/asan/CMakeLists.txt
@@ -13,10 +13,9 @@ set(ASAN_SOURCES
asan_globals_win.cpp
asan_interceptors.cpp
asan_interceptors_memintrinsics.cpp
- asan_linux.cpp
asan_mac.cpp
- asan_malloc_linux.cpp
asan_malloc_mac.cpp
+ asan_malloc_unix.cpp
asan_malloc_win.cpp
asan_memory_profile.cpp
asan_poisoning.cpp
@@ -29,6 +28,7 @@ set(ASAN_SOURCES
asan_stats.cpp
asan_suppressions.cpp
asan_thread.cpp
+ asan_unix.cpp
asan_win.cpp
)
diff --git a/compiler-rt/lib/asan/asan_malloc_linux.cpp b/compiler-rt/lib/asan/asan_malloc_unix.cpp
similarity index 98%
rename from compiler-rt/lib/asan/asan_malloc_linux.cpp
rename to compiler-rt/lib/asan/asan_malloc_unix.cpp
index 3d6b03fefab70..c9c48cb443b9b 100644
--- a/compiler-rt/lib/asan/asan_malloc_linux.cpp
+++ b/compiler-rt/lib/asan/asan_malloc_unix.cpp
@@ -1,4 +1,4 @@
-//===-- asan_malloc_linux.cpp ---------------------------------------------===//
+//===-- asan_malloc_unix.cpp ----------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -8,7 +8,7 @@
//
// This file is a part of AddressSanitizer, an address sanity checker.
//
-// Linux-specific malloc interception.
+// Unix-like-specific malloc interception.
// We simply define functions like malloc, free, realloc, etc.
// They will replace the corresponding libc functions automagically.
//===----------------------------------------------------------------------===//
diff --git a/compiler-rt/lib/asan/asan_linux.cpp b/compiler-rt/lib/asan/asan_unix.cpp
similarity index 99%
rename from compiler-rt/lib/asan/asan_linux.cpp
rename to compiler-rt/lib/asan/asan_unix.cpp
index 01cf46dc2fa7d..527bb7e92c2f1 100644
--- a/compiler-rt/lib/asan/asan_linux.cpp
+++ b/compiler-rt/lib/asan/asan_unix.cpp
@@ -1,4 +1,4 @@
-//===-- asan_linux.cpp ----------------------------------------------------===//
+//===-- asan_unix.cpp -----------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
diff --git a/llvm/utils/gn/secondary/compiler-rt/lib/asan/BUILD.gn b/llvm/utils/gn/secondary/compiler-rt/lib/asan/BUILD.gn
index 42467c21aa24c..c79251666f2e8 100644
--- a/llvm/utils/gn/secondary/compiler-rt/lib/asan/BUILD.gn
+++ b/llvm/utils/gn/secondary/compiler-rt/lib/asan/BUILD.gn
@@ -33,10 +33,9 @@ if (current_toolchain == host_toolchain) {
"asan_interface.inc",
"asan_interface_internal.h",
"asan_internal.h",
- "asan_linux.cpp",
"asan_mac.cpp",
- "asan_malloc_linux.cpp",
"asan_malloc_mac.cpp",
+ "asan_malloc_unix.cpp",
"asan_malloc_win.cpp",
"asan_mapping.h",
"asan_memory_profile.cpp",
@@ -58,6 +57,7 @@ if (current_toolchain == host_toolchain) {
"asan_suppressions.h",
"asan_thread.cpp",
"asan_thread.h",
+ "asan_unix.cpp",
"asan_win.cpp",
]
if (current_os != "mac" && current_os != "win") {
|
You can test this locally with the following command:git-clang-format --diff adb57757b9640768e5070e0e3f6b217c774f9205 889126709951d77f65aeb1ed843494bfd0222898 --extensions cpp -- compiler-rt/lib/asan/asan_malloc_unix.cpp compiler-rt/lib/asan/asan_unix.cpp View the diff from clang-format here.diff --git a/compiler-rt/lib/asan/asan_malloc_unix.cpp b/compiler-rt/lib/asan/asan_malloc_unix.cpp
index c9c48cb443..7386d79ef9 100644
--- a/compiler-rt/lib/asan/asan_malloc_unix.cpp
+++ b/compiler-rt/lib/asan/asan_malloc_unix.cpp
@@ -52,62 +52,62 @@ INTERCEPTOR(void, free, void *ptr) {
asan_free(ptr, &stack, FROM_MALLOC);
}
-#if SANITIZER_INTERCEPT_CFREE
+# if SANITIZER_INTERCEPT_CFREE
INTERCEPTOR(void, cfree, void *ptr) {
if (DlsymAlloc::PointerIsMine(ptr))
return DlsymAlloc::Free(ptr);
GET_STACK_TRACE_FREE;
asan_free(ptr, &stack, FROM_MALLOC);
}
-#endif // SANITIZER_INTERCEPT_CFREE
+# endif // SANITIZER_INTERCEPT_CFREE
-INTERCEPTOR(void*, malloc, uptr size) {
+INTERCEPTOR(void *, malloc, uptr size) {
if (DlsymAlloc::Use())
return DlsymAlloc::Allocate(size);
GET_STACK_TRACE_MALLOC;
return asan_malloc(size, &stack);
}
-INTERCEPTOR(void*, calloc, uptr nmemb, uptr size) {
+INTERCEPTOR(void *, calloc, uptr nmemb, uptr size) {
if (DlsymAlloc::Use())
return DlsymAlloc::Callocate(nmemb, size);
GET_STACK_TRACE_MALLOC;
return asan_calloc(nmemb, size, &stack);
}
-INTERCEPTOR(void*, realloc, void *ptr, uptr size) {
+INTERCEPTOR(void *, realloc, void *ptr, uptr size) {
if (DlsymAlloc::Use() || DlsymAlloc::PointerIsMine(ptr))
return DlsymAlloc::Realloc(ptr, size);
GET_STACK_TRACE_MALLOC;
return asan_realloc(ptr, size, &stack);
}
-#if SANITIZER_INTERCEPT_REALLOCARRAY
-INTERCEPTOR(void*, reallocarray, void *ptr, uptr nmemb, uptr size) {
+# if SANITIZER_INTERCEPT_REALLOCARRAY
+INTERCEPTOR(void *, reallocarray, void *ptr, uptr nmemb, uptr size) {
AsanInitFromRtl();
GET_STACK_TRACE_MALLOC;
return asan_reallocarray(ptr, nmemb, size, &stack);
}
-#endif // SANITIZER_INTERCEPT_REALLOCARRAY
+# endif // SANITIZER_INTERCEPT_REALLOCARRAY
-#if SANITIZER_INTERCEPT_MEMALIGN
-INTERCEPTOR(void*, memalign, uptr boundary, uptr size) {
+# if SANITIZER_INTERCEPT_MEMALIGN
+INTERCEPTOR(void *, memalign, uptr boundary, uptr size) {
GET_STACK_TRACE_MALLOC;
return asan_memalign(boundary, size, &stack, FROM_MALLOC);
}
-INTERCEPTOR(void*, __libc_memalign, uptr boundary, uptr size) {
+INTERCEPTOR(void *, __libc_memalign, uptr boundary, uptr size) {
GET_STACK_TRACE_MALLOC;
return asan_memalign(boundary, size, &stack, FROM_MALLOC);
}
-#endif // SANITIZER_INTERCEPT_MEMALIGN
+# endif // SANITIZER_INTERCEPT_MEMALIGN
-#if SANITIZER_INTERCEPT_ALIGNED_ALLOC
-INTERCEPTOR(void*, aligned_alloc, uptr boundary, uptr size) {
+# if SANITIZER_INTERCEPT_ALIGNED_ALLOC
+INTERCEPTOR(void *, aligned_alloc, uptr boundary, uptr size) {
GET_STACK_TRACE_MALLOC;
return asan_aligned_alloc(boundary, size, &stack);
}
-#endif // SANITIZER_INTERCEPT_ALIGNED_ALLOC
+# endif // SANITIZER_INTERCEPT_ALIGNED_ALLOC
INTERCEPTOR(uptr, malloc_usable_size, void *ptr) {
GET_CURRENT_PC_BP_SP;
@@ -115,7 +115,7 @@ INTERCEPTOR(uptr, malloc_usable_size, void *ptr) {
return asan_malloc_usable_size(ptr, pc, bp);
}
-#if SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO
+# if SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO
// We avoid including malloc.h for portability reasons.
// man mallinfo says the fields are "long", but the implementation uses int.
// It doesn't matter much -- we just need to make sure that the libc's mallinfo
@@ -130,33 +130,29 @@ INTERCEPTOR(struct fake_mallinfo, mallinfo, void) {
return res;
}
-INTERCEPTOR(int, mallopt, int cmd, int value) {
- return 0;
-}
-#endif // SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO
+INTERCEPTOR(int, mallopt, int cmd, int value) { return 0; }
+# endif // SANITIZER_INTERCEPT_MALLOPT_AND_MALLINFO
INTERCEPTOR(int, posix_memalign, void **memptr, uptr alignment, uptr size) {
GET_STACK_TRACE_MALLOC;
return asan_posix_memalign(memptr, alignment, size, &stack);
}
-INTERCEPTOR(void*, valloc, uptr size) {
+INTERCEPTOR(void *, valloc, uptr size) {
GET_STACK_TRACE_MALLOC;
return asan_valloc(size, &stack);
}
-#if SANITIZER_INTERCEPT_PVALLOC
-INTERCEPTOR(void*, pvalloc, uptr size) {
+# if SANITIZER_INTERCEPT_PVALLOC
+INTERCEPTOR(void *, pvalloc, uptr size) {
GET_STACK_TRACE_MALLOC;
return asan_pvalloc(size, &stack);
}
-#endif // SANITIZER_INTERCEPT_PVALLOC
+# endif // SANITIZER_INTERCEPT_PVALLOC
-INTERCEPTOR(void, malloc_stats, void) {
- __asan_print_accumulated_stats();
-}
+INTERCEPTOR(void, malloc_stats, void) { __asan_print_accumulated_stats(); }
-#if SANITIZER_ANDROID
+# if SANITIZER_ANDROID
// Format of __libc_malloc_dispatch has changed in Android L.
// While we are moving towards a solution that does not depend on bionic
// internals, here is something to support both K* and L releases.
@@ -177,9 +173,9 @@ struct MallocDebugL {
uptr (*malloc_usable_size)(void *mem);
void *(*memalign)(uptr alignment, uptr bytes);
int (*posix_memalign)(void **memptr, uptr alignment, uptr size);
- void* (*pvalloc)(uptr size);
+ void *(*pvalloc)(uptr size);
void *(*realloc)(void *oldMem, uptr bytes);
- void* (*valloc)(uptr size);
+ void *(*valloc)(uptr size);
};
alignas(32) const MallocDebugK asan_malloc_dispatch_k = {
@@ -187,9 +183,15 @@ alignas(32) const MallocDebugK asan_malloc_dispatch_k = {
WRAP(realloc), WRAP(memalign), WRAP(malloc_usable_size)};
alignas(32) const MallocDebugL asan_malloc_dispatch_l = {
- WRAP(calloc), WRAP(free), WRAP(mallinfo),
- WRAP(malloc), WRAP(malloc_usable_size), WRAP(memalign),
- WRAP(posix_memalign), WRAP(pvalloc), WRAP(realloc),
+ WRAP(calloc),
+ WRAP(free),
+ WRAP(mallinfo),
+ WRAP(malloc),
+ WRAP(malloc_usable_size),
+ WRAP(memalign),
+ WRAP(posix_memalign),
+ WRAP(pvalloc),
+ WRAP(realloc),
WRAP(valloc)};
namespace __asan {
@@ -208,13 +210,12 @@ void ReplaceSystemMalloc() {
}
} // namespace __asan
-#else // SANITIZER_ANDROID
+# else // SANITIZER_ANDROID
namespace __asan {
-void ReplaceSystemMalloc() {
-}
+void ReplaceSystemMalloc() {}
} // namespace __asan
-#endif // SANITIZER_ANDROID
+# endif // SANITIZER_ANDROID
#endif // SANITIZER_FREEBSD || SANITIZER_FUCHSIA || SANITIZER_LINUX ||
// SANITIZER_NETBSD || SANITIZER_SOLARIS
diff --git a/compiler-rt/lib/asan/asan_unix.cpp b/compiler-rt/lib/asan/asan_unix.cpp
index 527bb7e92c..9fc4a84bd1 100644
--- a/compiler-rt/lib/asan/asan_unix.cpp
+++ b/compiler-rt/lib/asan/asan_unix.cpp
@@ -112,7 +112,7 @@ void FlushUnneededASanShadowMemory(uptr p, uptr size) {
}
void ReExecWithoutASLR() {
-# if SANITIZER_LINUX
+# if SANITIZER_LINUX
// ASLR personality check.
// Caution: 'personality' is sometimes forbidden by sandboxes, so only call
// this function as a last resort (when the memory mapping is incompatible
@@ -141,7 +141,7 @@ void ReExecWithoutASLR() {
ReExec();
}
-# endif
+# endif
}
# if SANITIZER_ANDROID
|
If we nitpicking, Linux is not UNIX. So I would not recommend incremental fixes until we have clear guideline for naming. |
Agree with Vitaly. We should agree on a naming scheme and then consistently apply it. |
I'm happy to consider other names. Alternatives considered:
Sure, I will defer until consensus is reached on naming. |
asan_(malloc_)?linux.cpp are misleadingly named because they cover many non-Linux OSes, such as BSDs, Fuchsia and Solaris. This is a footgun where changes may be made to these files without remembering it is not Linux-specific (e.g., I broke the Solaris build - see #131975 (comment)).
This patch mitigates the issue by renaming the files from ...linux to ...unix, which should hopefully give pause to anyone (me) when making Linux-specific changes.