Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thurstond
Copy link
Contributor

@thurstond thurstond commented Mar 20, 2025

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.

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.
@llvmbot
Copy link
Member

llvmbot commented Mar 20, 2025

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

Author: Thurston Dang (thurstond)

Changes

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 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:

  • (modified) compiler-rt/lib/asan/CMakeLists.txt (+2-2)
  • (renamed) compiler-rt/lib/asan/asan_malloc_unix.cpp (+2-2)
  • (renamed) compiler-rt/lib/asan/asan_unix.cpp (+1-1)
  • (modified) llvm/utils/gn/secondary/compiler-rt/lib/asan/BUILD.gn (+2-2)
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") {

Copy link

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

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

@vitalybuka
Copy link
Collaborator

If we nitpicking, Linux is not UNIX.
And there are other places when linux includes some BSD oses.

So I would not recommend incremental fixes until we have clear guideline for naming.
As-is it's rather git log noise.

@fmayer
Copy link
Contributor

fmayer commented Mar 20, 2025

Agree with Vitaly. We should agree on a naming scheme and then consistently apply it.

@thurstond
Copy link
Contributor Author

thurstond commented Mar 20, 2025

If we nitpicking, Linux is not UNIX. And there are other places when linux includes some BSD oses.

I'm happy to consider other names.

Alternatives considered:

  • "UNIX-like": the problem is the filenames (asan_malloc_unix_like.cpp) could be misunderstood
  • UN*X, *NIX: can't put * in filenames

So I would not recommend incremental fixes until we have clear guideline for naming. As-is it's rather git log noise.

Sure, I will defer until consensus is reached on naming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants