Skip to content

Commit

Permalink
[libc] Fix printf %p format
Browse files Browse the repository at this point in the history
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.

Reviewed By: lntue, jhuber6

Differential Revision: https://reviews.llvm.org/D159458
  • Loading branch information
michaelrj-google committed Sep 7, 2023
1 parent 6678f60 commit dd51ae8
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 10 deletions.
24 changes: 14 additions & 10 deletions libc/src/stdio/printf_core/ptr_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,27 @@
#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)"));
} else {
FormatSection hex_conv;
hex_conv.has_conv = true;
hex_conv.conv_name = 'x';
hex_conv.flags = FormatFlags::ALTERNATE_FORM;
hex_conv.conv_val_raw = reinterpret_cast<uintptr_t>(to_conv.conv_val_ptr);
return convert_int(writer, hex_conv);
FormatSection new_conv = to_conv;

if (to_conv.conv_val_ptr == nullptr) {
constexpr char NULLPTR_STR[] = "(nullptr)";
new_conv.conv_name = 's';
new_conv.conv_val_ptr = const_cast<char *>(NULLPTR_STR);
return convert_string(writer, new_conv);
}
return WRITE_OK;
new_conv.conv_name = 'x';
new_conv.flags =
static_cast<FormatFlags>(to_conv.flags | FormatFlags::ALTERNATE_FORM);
new_conv.length_modifier = LengthModifier::t;
new_conv.conv_val_raw = reinterpret_cast<uintptr_t>(to_conv.conv_val_ptr);
return convert_int(writer, new_conv);
}

} // namespace printf_core
Expand Down
46 changes: 46 additions & 0 deletions libc/test/src/stdio/sprintf_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,54 @@ TEST(LlvmLibcSPrintfTest, PointerConv) {
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) {
Expand Down

0 comments on commit dd51ae8

Please sign in to comment.