[compiler-rt] 1e36156 - [scudo] Make the placeholder type specifier be consistent with C/C++

[Chia-hung Duan via llvm-commits]

Author: Chia-hung Duan
Date: 2023-04-04T19:13:15Z
New Revision: 1e36156ab1dc70f9f9b94a8d9820c819da4187be

URL: https://github.com/llvm/llvm-project/commit/1e36156ab1dc70f9f9b94a8d9820c819da4187be
DIFF: https://github.com/llvm/llvm-project/commit/1e36156ab1dc70f9f9b94a8d9820c819da4187be.diff

LOG: [scudo] Make the placeholder type specifier be consistent with C/C++

This avoids `-Wformat` complains the placeholder type specifier mismatch
on `lld`/`llu`(used for `s64`/`u64`) which have slightly different
interpretation in string_utils.cpp.

Also enable Timer build which was disabled because of the complaining of
`-Wformat`.

Differential Revision: https://reviews.llvm.org/D147496

Added: 
    

Modified: 
    compiler-rt/lib/scudo/standalone/CMakeLists.txt
    compiler-rt/lib/scudo/standalone/string_utils.cpp
    compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
    compiler-rt/lib/scudo/standalone/timing.h

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/scudo/standalone/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/CMakeLists.txt
index 86d4a47a391bc..6fcd4deddf716 100644
--- a/compiler-rt/lib/scudo/standalone/CMakeLists.txt
+++ b/compiler-rt/lib/scudo/standalone/CMakeLists.txt
@@ -85,9 +85,7 @@ set(SCUDO_HEADERS
   stack_depot.h
   stats.h
   string_utils.h
-  # TODO(chiahungduan): Temporarily disable the timer because of the print
-  # of u64 has incorrect placeholder.
-  # timing.h
+  timing.h
   tsd_exclusive.h
   tsd_shared.h
   tsd.h
@@ -110,9 +108,7 @@ set(SCUDO_SOURCES
   report.cpp
   rss_limit_checker.cpp
   string_utils.cpp
-  # TODO(chiahungduan): Temporarily disable the timer because of the print
-  # of u64 has incorrect placeholder.
-  # timing.cpp
+  timing.cpp
   )
 
 # Enable the necessary instruction set for scudo_crc32.cpp, if available.

diff  --git a/compiler-rt/lib/scudo/standalone/string_utils.cpp b/compiler-rt/lib/scudo/standalone/string_utils.cpp
index 13fdb9c6ca6cd..7e516f957abd6 100644
--- a/compiler-rt/lib/scudo/standalone/string_utils.cpp
+++ b/compiler-rt/lib/scudo/standalone/string_utils.cpp
@@ -195,6 +195,28 @@ static int formatString(char *Buffer, uptr BufferLength, const char *Format,
           appendChar(&Buffer, BufferEnd, static_cast<char>(va_arg(Args, int)));
       break;
     }
+    // In Scudo, `s64`/`u64` are supposed to use `lld` and `llu` respectively.
+    // However, `-Wformat` doesn't know we have a 
diff erent parser for those
+    // placeholders and it keeps complaining the type mismatch on 64-bit
+    // platform which uses `ld`/`lu` for `s64`/`u64`. Therefore, in order to
+    // silence the warning, we turn to use `PRId64`/`PRIu64` for printing
+    // `s64`/`u64` and handle the `ld`/`lu` here.
+    case 'l': {
+      ++Cur;
+      RAW_CHECK(*Cur == 'd' || *Cur == 'u');
+
+      if (*Cur == 'd') {
+        DVal = va_arg(Args, s64);
+        Res +=
+            appendSignedDecimal(&Buffer, BufferEnd, DVal, Width, PadWithZero);
+      } else {
+        UVal = va_arg(Args, u64);
+        Res += appendUnsigned(&Buffer, BufferEnd, UVal, 10, Width, PadWithZero,
+                              false);
+      }
+
+      break;
+    }
     case '%': {
       RAW_CHECK_MSG(!HaveFlags, PrintfFormatsHelp);
       Res += appendChar(&Buffer, BufferEnd, '%');

diff  --git a/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
index 3bb707d90a611..335e4b7dbd899 100644
--- a/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
+++ b/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
@@ -105,9 +105,7 @@ set(SCUDO_UNIT_TEST_SOURCES
   size_class_map_test.cpp
   stats_test.cpp
   strings_test.cpp
-  # TODO(chiahungduan): Temporarily disable the timer test because of the print
-  # of u64 has incorrect placeholder.
-  # timing_test.cpp
+  timing_test.cpp
   tsd_test.cpp
   vector_test.cpp
   scudo_unit_test_main.cpp

diff  --git a/compiler-rt/lib/scudo/standalone/timing.h b/compiler-rt/lib/scudo/standalone/timing.h
index c457e96214149..84caa79e5c3a2 100644
--- a/compiler-rt/lib/scudo/standalone/timing.h
+++ b/compiler-rt/lib/scudo/standalone/timing.h
@@ -14,6 +14,7 @@
 #include "string_utils.h"
 #include "thread_annotations.h"
 
+#include <inttypes.h>
 #include <string.h>
 
 namespace scudo {
@@ -178,11 +179,11 @@ class TimingManager {
         Occurrence == 0 ? 0
                         : ((AccumulatedTime % Occurrence) * 10) / Occurrence;
 
-    Str.append("%14lu.%lu(ns) %-11s", Integral, Fraction, " ");
+    Str.append("%14" PRId64 ".%" PRId64 "(ns) %-11s", Integral, Fraction, " ");
 
     for (u32 I = 0; I < ExtraIndent; ++I)
       Str.append("%s", "  ");
-    Str.append("%s (%lu)\n", Timers[HandleId].Name, Occurrence);
+    Str.append("%s (%" PRId64 ")\n", Timers[HandleId].Name, Occurrence);
 
     for (u32 I = 0; I < NumAllocatedTimers; ++I)
       if (Timers[I].Nesting == HandleId)