Skip to content

Conversation

@vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Jul 12, 2024

Pointers print more leading zeroes for better alignment.

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Jul 12, 2024

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

Author: Vitaly Buka (vitalybuka)

Changes

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

12 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h (+2-2)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp (+2-1)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp (+2-2)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp (+2-2)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp (+2-2)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp (+4-3)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp (+1-1)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp (+2-2)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_netbsd_libcdep.cpp (+2-2)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp (+7-7)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_win.cpp (+4-4)
  • (modified) compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cpp (+1-1)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
index 6e73065d7f53c..cdc0e0921449b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
@@ -316,13 +316,13 @@ class SizeClassAllocator64 {
     Printf(
         "%s %02zd (%6zd): mapped: %6zdK allocs: %7zd frees: %7zd inuse: %6zd "
         "num_freed_chunks %7zd avail: %6zd rss: %6zdK releases: %6zd "
-        "last released: %6lldK region: 0x%zx\n",
+        "last released: %6lldK region: %p\n",
         region->exhausted ? "F" : " ", class_id, ClassIdToSize(class_id),
         region->mapped_user >> 10, region->stats.n_allocated,
         region->stats.n_freed, in_use, region->num_freed_chunks, avail_chunks,
         rss >> 10, region->rtoi.num_releases,
         region->rtoi.last_released_bytes >> 10,
-        SpaceBeg() + kRegionSize * class_id);
+        (void *)(SpaceBeg() + kRegionSize * class_id));
   }
 
   void PrintStats() {
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp
index ce4326967180d..506659a58c45e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp
@@ -75,7 +75,8 @@ static void SanitizerDumpCoverage(const uptr* unsorted_pcs, uptr len) {
     if (!pc) continue;
 
     if (!GetModuleAndOffsetForPc(pc, nullptr, 0, &pcs[i])) {
-      Printf("ERROR: unknown pc 0x%zx (may happen if dlclose is used)\n", pc);
+      Printf("ERROR: unknown pc %p (may happen if dlclose is used)\n",
+             (void*)pc);
       continue;
     }
     uptr module_base = pc - pcs[i];
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp
index b7fc9444cc66b..f0e1e3d69def5 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_libignore.cpp
@@ -105,8 +105,8 @@ void LibIgnore::OnLibraryLoaded(const char *name) {
           continue;
         if (IsPcInstrumented(range.beg) && IsPcInstrumented(range.end - 1))
           continue;
-        VReport(1, "Adding instrumented range 0x%zx-0x%zx from library '%s'\n",
-                range.beg, range.end, mod.full_name());
+        VReport(1, "Adding instrumented range %p-%p from library '%s'\n",
+                (void *)range.beg, (void *)range.end, mod.full_name());
         const uptr idx =
             atomic_load(&instrumented_ranges_count_, memory_order_relaxed);
         CHECK_LT(idx, ARRAY_SIZE(instrumented_code_ranges_));
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index cbdf3e95925bf..8ebe37d649415 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -1372,8 +1372,8 @@ void DumpProcessMap() {
   for (uptr i = 0; i < modules.size(); ++i) {
     char uuid_str[128];
     FormatUUID(uuid_str, sizeof(uuid_str), modules[i].uuid());
-    Printf("0x%zx-0x%zx %s (%s) %s\n", modules[i].base_address(),
-           modules[i].max_address(), modules[i].full_name(),
+    Printf("%p-%p %s (%s) %s\n", (void *)modules[i].base_address(),
+           (void *)modules[i].max_address(), modules[i].full_name(),
            ModuleArchToString(modules[i].arch()), uuid_str);
   }
   Printf("End of module map.\n");
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp
index 969327a7a0fbe..7d7d575431994 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp
@@ -130,8 +130,8 @@ static void *MmapFixedImpl(uptr fixed_addr, uptr size, bool tolerate_enomem,
     if (tolerate_enomem && reserrno == ENOMEM)
       return nullptr;
     char mem_type[40];
-    internal_snprintf(mem_type, sizeof(mem_type), "memory at address 0x%zx",
-                      fixed_addr);
+    internal_snprintf(mem_type, sizeof(mem_type), "memory at address %p",
+                      (void *)fixed_addr);
     ReportMmapFailureAndDie(size, mem_type, "allocate", reserrno);
   }
   IncreaseTotalMmap(size);
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
index ece2d7d63dd61..9ffb36f812c45 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
@@ -327,9 +327,10 @@ static bool MmapFixed(uptr fixed_addr, uptr size, int additional_flags,
                 MAP_PRIVATE | MAP_FIXED | additional_flags | MAP_ANON, name);
   int reserrno;
   if (internal_iserror(p, &reserrno)) {
-    Report("ERROR: %s failed to "
-           "allocate 0x%zx (%zd) bytes at address %zx (errno: %d)\n",
-           SanitizerToolName, size, size, fixed_addr, reserrno);
+    Report(
+        "ERROR: %s failed to "
+        "allocate 0x%zx (%zd) bytes at address %p (errno: %d)\n",
+        SanitizerToolName, size, size, (void *)fixed_addr, reserrno);
     return false;
   }
   IncreaseTotalMmap(size);
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
index b23796fccf6e3..dddae441d5de9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
@@ -192,7 +192,7 @@ void FormattedStackTracePrinter::RenderFrame(InternalScopedString *buffer,
       buffer->AppendF("%u", frame_no);
       break;
     case 'p':
-      buffer->AppendF("0x%zx", address);
+      buffer->AppendF("%p", (void *)address);
       break;
     case 'm':
       buffer->AppendF("%s", StripPathPrefix(info->module, strip_path_prefix));
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp
index 25c4af7085608..526a71c398260 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp
@@ -257,8 +257,8 @@ static void TracerThreadDieCallback() {
 static void TracerThreadSignalHandler(int signum, __sanitizer_siginfo *siginfo,
                                       void *uctx) {
   SignalContext ctx(siginfo, uctx);
-  Printf("Tracer caught signal %d: addr=0x%zx pc=0x%zx sp=0x%zx\n", signum,
-         ctx.addr, ctx.pc, ctx.sp);
+  Printf("Tracer caught signal %d: addr=%p pc=%p sp=%p\n", signum,
+         (void *)ctx.addr, (void *)ctx.pc, (void *)ctx.sp);
   ThreadSuspender *inst = thread_suspender_instance;
   if (inst) {
     if (signum == SIGABRT)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_netbsd_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_netbsd_libcdep.cpp
index 701db72619a3d..58a0cfdbf9d48 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_netbsd_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_netbsd_libcdep.cpp
@@ -158,8 +158,8 @@ static void TracerThreadDieCallback() {
 static void TracerThreadSignalHandler(int signum, __sanitizer_siginfo *siginfo,
                                       void *uctx) {
   SignalContext ctx(siginfo, uctx);
-  Printf("Tracer caught signal %d: addr=0x%zx pc=0x%zx sp=0x%zx\n", signum,
-         ctx.addr, ctx.pc, ctx.sp);
+  Printf("Tracer caught signal %d: addr=%p pc=%p sp=%p\n", signum,
+         (void *)ctx.addr, (void *)ctx.pc, (void *)ctx.sp);
   ThreadSuspender *inst = thread_suspender_instance;
   if (inst) {
     if (signum == SIGABRT)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
index 252979f1c2baa..7fc9edde34c3d 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
@@ -121,25 +121,25 @@ DTLS::DTV *DTLS_on_tls_get_addr(void *arg_void, void *res,
   uptr tls_size = 0;
   uptr tls_beg = reinterpret_cast<uptr>(res) - arg->offset - kDtvOffset;
   VReport(2,
-          "__tls_get_addr: %p {0x%zx,0x%zx} => %p; tls_beg: 0x%zx; sp: %p "
+          "__tls_get_addr: %p {0x%zx,0x%zx} => %p; tls_beg: %p; sp: %p "
           "num_live_dtls %zd\n",
-          (void *)arg, arg->dso_id, arg->offset, res, tls_beg, (void *)&tls_beg,
+          (void *)arg, arg->dso_id, arg->offset, res, (void *)tls_beg, &tls_beg,
           atomic_load(&number_of_live_dtls, memory_order_relaxed));
   if (dtls.last_memalign_ptr == tls_beg) {
     tls_size = dtls.last_memalign_size;
-    VReport(2, "__tls_get_addr: glibc <=2.24 suspected; tls={0x%zx,0x%zx}\n",
-            tls_beg, tls_size);
+    VReport(2, "__tls_get_addr: glibc <=2.24 suspected; tls={%p,0x%zx}\n",
+            (void *)tls_beg, tls_size);
   } else if (tls_beg >= static_tls_begin && tls_beg < static_tls_end) {
     // This is the static TLS block which was initialized / unpoisoned at thread
     // creation.
-    VReport(2, "__tls_get_addr: static tls: 0x%zx\n", tls_beg);
+    VReport(2, "__tls_get_addr: static tls: %p\n", (void *)tls_beg);
     tls_size = 0;
   } else if (const void *start =
                  __sanitizer_get_allocated_begin((void *)tls_beg)) {
     tls_beg = (uptr)start;
     tls_size = __sanitizer_get_allocated_size(start);
-    VReport(2, "__tls_get_addr: glibc >=2.25 suspected; tls={0x%zx,0x%zx}\n",
-            tls_beg, tls_size);
+    VReport(2, "__tls_get_addr: glibc >=2.25 suspected; tls={%p,0x%zx}\n",
+            (void *)tls_beg, tls_size);
   } else {
     VReport(2, "__tls_get_addr: Can't guess glibc version\n");
     // This may happen inside the DTOR of main thread, so just ignore it.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
index 0b198890fc798..995f00eddc38a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
@@ -276,8 +276,8 @@ void *MmapFixedOrDie(uptr fixed_addr, uptr size, const char *name) {
       MEM_COMMIT, PAGE_READWRITE);
   if (p == 0) {
     char mem_type[30];
-    internal_snprintf(mem_type, sizeof(mem_type), "memory at address 0x%zx",
-                      fixed_addr);
+    internal_snprintf(mem_type, sizeof(mem_type), "memory at address %p",
+                      (void *)fixed_addr);
     ReportMmapFailureAndDie(size, mem_type, "allocate", GetLastError());
   }
   return p;
@@ -308,8 +308,8 @@ void *MmapFixedOrDieOnFatalError(uptr fixed_addr, uptr size, const char *name) {
       MEM_COMMIT, PAGE_READWRITE);
   if (p == 0) {
     char mem_type[30];
-    internal_snprintf(mem_type, sizeof(mem_type), "memory at address 0x%zx",
-                      fixed_addr);
+    internal_snprintf(mem_type, sizeof(mem_type), "memory at address %p",
+                      (void *)fixed_addr);
     return ReturnNullptrOnOOMOrDie(size, mem_type, "allocate");
   }
   return p;
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cpp
index 769a9b9a9fa0a..11ca1fd7f0517 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cpp
@@ -284,7 +284,7 @@ TEST(GetCurrentPc, Basic) {
           StackTrace::GetCurrentPc(),
       };
       for (uptr i = 0; i < ARRAY_SIZE(pcs); i++)
-        Printf("pc%zu: 0x%zx\n", i, pcs[i]);
+        Printf("pc%zu: %p\n", i, (void *)(pcs[i]));
       for (uptr i = 1; i < ARRAY_SIZE(pcs); i++) {
         EXPECT_GT(pcs[i], pcs[0]);
         EXPECT_LT(pcs[i], pcs[0] + 1000);

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.nfcsanitizer_common-use-p-to-print-addresses to main July 12, 2024 05:41
Created using spr 1.3.4
@vitalybuka vitalybuka changed the title [NFC][sanitizer_common] Use %p to print addresses [sanitizer_common] Use %p to print addresses Jul 12, 2024
@vitalybuka vitalybuka merged commit bf4347b into main Jul 19, 2024
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcsanitizer_common-use-p-to-print-addresses branch July 19, 2024 01:14
@rorth
Copy link
Collaborator

rorth commented Jul 19, 2024

This patch broke the Solaris/amd64 buildbot: unlike e.g. glibc, Solaris printf doesn't emit a leading 0x with %p:

  • Solaris:
    0x%zx: p = 0x8050c2d
    %p   : p = 8050c2d
    
  • Linux:
    0x%zx: p = 0x8049176
    %p   : p = 0x8049176
    

@BertalanD
Copy link
Member

The added leading zeros also broke a test on Darwin which was comparing addresses:

// CHECK: WARNING: ThreadSanitizer: data race
// CHECK: Location is heap block of size 10 at {{.*}} allocated by thread T1
// CHECK: #0 [[ALLOC_FRAME_0]]
// CHECK: #1 [[ALLOC_FRAME_1]] in alloc_func
// CHECK: #2 [[ALLOC_FRAME_2]] in AllocThread

            16:  Location is heap block of size 10 at 0x720400000800 allocated by thread T1: 
 check:82'0                                                                                X~ error: no match found
 check:82'1                                                                                   with "ALLOC_FRAME_0" equal to "0x10afe151f"
            17:  #0 0x00010afe151f in malloc+0x6e (libclang_rt.tsan_osx_dynamic.dylib:x86_64+0x5d51e) 
 check:82'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 check:82'2      ?                                                                                     possible intended match

@vitalybuka
Copy link
Collaborator Author

vitalybuka commented Jul 19, 2024

Sorry for not mentioning 377e1eb here. It fixed Asan. I will update tsan as well.

vitalybuka added a commit that referenced this pull request Jul 19, 2024
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary: Pointers print more leading zeroes for better alignment.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251661
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary: Follow up to #98578

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251707
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.

6 participants