[libc-commits] [PATCH] D159458: [libc] Fix printf %p format

[Michael Jones via Phabricator via libc-commits]

michaelrj created this revision.
michaelrj added reviewers: sivachandra, lntue, jhuber6.
Herald added projects: libc-project, All.
Herald added a subscriber: libc-commits.
michaelrj requested review of this revision.
Herald added a subscriber: wangpc.

The %p format wasn't correctly passing along flags and modifiers to the
integer conversion behind the scenes. This patch fixes that behavior, as
well as changing the nullptr behavior to be a string conversion behind
the scenes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159458

Files:
  libc/src/stdio/printf_core/ptr_converter.h
  libc/test/src/stdio/sprintf_test.cpp


Index: libc/test/src/stdio/sprintf_test.cpp
===================================================================
--- libc/test/src/stdio/sprintf_test.cpp
+++ libc/test/src/stdio/sprintf_test.cpp
@@ -394,8 +394,54 @@
   EXPECT_EQ(written, 10);
   ASSERT_STREQ(buff, "0x1a2b3c4d");
 
+  if constexpr (sizeof(void *) > 4) {
+    written = __llvm_libc::sprintf(buff, "%p", 0x1a2b3c4d5e6f7081);
+    EXPECT_EQ(written, 18);
+    ASSERT_STREQ(buff, "0x1a2b3c4d5e6f7081");
+  }
+
   written = __llvm_libc::sprintf(buff, "%p", buff);
   EXPECT_GT(written, 0);
+
+  // Width tests:
+
+  written = __llvm_libc::sprintf(buff, "%20p", nullptr);
+  EXPECT_EQ(written, 20);
+  ASSERT_STREQ(buff, "           (nullptr)");
+
+  written = __llvm_libc::sprintf(buff, "%20p", 0x1a2b3c4d);
+  EXPECT_EQ(written, 20);
+  ASSERT_STREQ(buff, "          0x1a2b3c4d");
+
+  // Flag tests:
+
+  written = __llvm_libc::sprintf(buff, "%-20p", nullptr);
+  EXPECT_EQ(written, 20);
+  ASSERT_STREQ(buff, "(nullptr)           ");
+
+  written = __llvm_libc::sprintf(buff, "%-20p", 0x1a2b3c4d);
+  EXPECT_EQ(written, 20);
+  ASSERT_STREQ(buff, "0x1a2b3c4d          ");
+
+  // Using the 0 flag is technically undefined, but here we're following the
+  // convention of matching the behavior of %#x.
+  written = __llvm_libc::sprintf(buff, "%020p", 0x1a2b3c4d);
+  EXPECT_EQ(written, 20);
+  ASSERT_STREQ(buff, "0x00000000001a2b3c4d");
+
+  // Precision tests:
+  // These are all undefined behavior. The precision option is undefined for %p.
+
+  // Precision specifies the number of characters for a string conversion.
+  written = __llvm_libc::sprintf(buff, "%.5p", nullptr);
+  EXPECT_EQ(written, 5);
+  ASSERT_STREQ(buff, "(null");
+
+  // Precision specifies the number of digits to be written for %x conversions,
+  // and the "0x" doesn't count as part of the digits.
+  written = __llvm_libc::sprintf(buff, "%.20p", 0x1a2b3c4d);
+  EXPECT_EQ(written, 22);
+  ASSERT_STREQ(buff, "0x0000000000001a2b3c4d");
 }
 
 TEST(LlvmLibcSPrintfTest, OctConv) {
Index: libc/src/stdio/printf_core/ptr_converter.h
===================================================================
--- libc/src/stdio/printf_core/ptr_converter.h
+++ libc/src/stdio/printf_core/ptr_converter.h
@@ -14,19 +14,26 @@
 #include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/int_converter.h"
+#include "src/stdio/printf_core/string_converter.h"
 #include "src/stdio/printf_core/writer.h"
 
 namespace __llvm_libc {
 namespace printf_core {
 
 LIBC_INLINE int convert_pointer(Writer *writer, const FormatSection &to_conv) {
+
   if (to_conv.conv_val_ptr == (void *)(nullptr)) {
-    RET_IF_RESULT_NEGATIVE(writer->write("(nullptr)"));
+    constexpr char nullptr_str[] = "(nullptr)";
+    FormatSection str_conv = to_conv;
+    str_conv.conv_name = 's';
+    str_conv.conv_val_ptr = const_cast<char *>(nullptr_str);
+    return convert_string(writer, str_conv);
   } else {
-    FormatSection hex_conv;
-    hex_conv.has_conv = true;
+    FormatSection hex_conv = to_conv;
     hex_conv.conv_name = 'x';
-    hex_conv.flags = FormatFlags::ALTERNATE_FORM;
+    hex_conv.flags =
+        static_cast<FormatFlags>(to_conv.flags | FormatFlags::ALTERNATE_FORM);
+    hex_conv.length_modifier = LengthModifier::t;
     hex_conv.conv_val_raw = reinterpret_cast<uintptr_t>(to_conv.conv_val_ptr);
     return convert_int(writer, hex_conv);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D159458.555940.patch
Type: text/x-patch
Size: 3449 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230905/0fa2e76d/attachment-0001.bin>