[libc] add printf base 10 integer conversion

[michaelrj]

Summary

This patch adds support for d, i, and u conversions in printf, as well
as comprehensive unit tests.

[michaelrj]

add sprintf integer tests

[sivachandra]

libc/src/stdio/printf_core/int_converter.h

25

Seems like this one line function is used only once?

39

BUFF_LEN

40

Can this assignment ever lead to a truncation? May no because this is a convert_int which does not deal with integers wider than 64-bit?

60

Why are these masking operations required? Also, for max values, we have: https://github.com/llvm/llvm-project/blob/main/libc/src/__support/CPP/Limits.h

91

for (; num > 0; --buff_cur, num /= 10)
  buffer[buff_curr] = (num % 10) + '0';

[michaelrj]

address comments

[michaelrj]

libc/src/stdio/printf_core/int_converter.h

40

It can't, because uintmax_t is at least as large as the largest valid integer type for printf. The size of conv_val_raw may be larger than uintmax_t to fit long double values, but the value passed by the user can't be larger than uintmax_t.

60

This is to handle the fact that the length modifiers allow the user to specify the size of the integer to be converted. The type in num is always uintmax_t since that can fit any valid type, but the user can specify that they want it to be treated as a short, in which case we have to limit the value of num to what can fit in a short. The way this is done here is by creating a mask of just the bits in the specified type.

[sivachandra]

libc/src/stdio/printf_core/int_converter.h

60

Thanks for explaining. You can either use LLVM libc's own NumericLimits or directly use macros from the freestanding limits.h header. NumericLimits is probably more appropriate as you can get max values of size_t and ptrdiff_t is a straightforward way.

[michaelrj]

switch to using limits.h for type size masks, additionally adjust the buffer size to be more exact.

[sivachandra]

LGTM - But, if some of the changes to the tests are unrelated, then do it separately.

libc/src/stdio/printf_core/int_converter.h

19

Does this have to be in a header file? If yes, then we should mark it inline and also add header guards. You should probably do that with other <converter>.h files?

41

Nit: Do you need the static cast?

127

Nit: The first word starts with a captilized letter. Can we follow the same convention with all comments please?

libc/test/src/stdio/printf_core/converter_test.cpp

27

I am probably missing something - how are these changes to string conversion tests related to integer conversion? Also, do we really want a separate string converter and writer for every test? If not, they can be static globals.

Another point, which can be addressed separately - we should probably not be testing against internal API for a converter, writer etc. You can have a test for verifying that the plumbing works, but everything else should probably be tested using sprintf.

185

Should we put these integer conversion tests in a separate file?

[sivachandra]

libc/test/src/stdio/printf_core/converter_test.cpp

27

I forgot that StringWriter is stateful - ignore that part of the comment about needing a new string writer for every test.

[michaelrj]

move tests into sprintf_tests and split converter_test changes into a separate patch

libc/src/stdio/printf_core/int_converter.h

41

yes. The output of the arithmetic and is an int which doesn't have a default conversion to FormatFlags

libc/test/src/stdio/printf_core/converter_test.cpp

27

I think that having some tests specific to the writer, parser, and converter is helpful for determining which piece has the bug, although we can move primary functionality testing to sprintf for simplicity.

[sivachandra]

Feel free to submit after addressing the comments.

libc/src/stdio/printf_core/int_converter.h

41

Ah, I meant to ask if you can use FormatFlags(...) without the static_cast.

libc/src/stdio/printf_core/string_converter.h

8 ↗

(On Diff #435325)

Header guards?

[michaelrj]

address comments and add simple converter test

libc/src/stdio/printf_core/string_converter.h

8 ↗

(On Diff #435325)

yes, I meant to add those and forgot.

libc/src/__support/CPP/Limits.h

public:
   static constexpr long long max() { return LLONG_MAX; }
   static constexpr long long min() { return LLONG_MIN; }
 };
 template <> class NumericLimits<unsigned long long> {
 public:
   static constexpr unsigned long long max() { return ULLONG_MAX; }
   static constexpr unsigned long long min() { return 0; }
 };
+template <> class NumericLimits<short> {
+public:
+  static constexpr short max() { return SHRT_MAX; }
+  static constexpr short min() { return SHRT_MIN; }
+};
+template <> class NumericLimits<unsigned short> {
+public:
+  static constexpr unsigned short max() { return USHRT_MAX; }
+  static constexpr unsigned short min() { return 0; }
+};
+template <> class NumericLimits<char> {
+public:
+  static constexpr char max() { return CHAR_MAX; }
+  static constexpr char min() { return CHAR_MIN; }
+};
+template <> class NumericLimits<unsigned char> {
+public:
+  static constexpr unsigned char max() { return UCHAR_MAX; }
+  static constexpr unsigned char min() { return 0; }
+};
 #ifdef __SIZEOF_INT128__
 template <> class NumericLimits<__uint128_t> {
 public:
   static constexpr __uint128_t max() { return ~__uint128_t(0); }
   static constexpr __uint128_t min() { return 0; }
 };
 template <> class NumericLimits<__int128_t> {
 public:
   static constexpr __int128_t max() { return ~__uint128_t(0) >> 1; }
   static constexpr __int128_t min() { return __int128_t(1) << 127; }
 };
 #endif
 } // namespace cpp
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_SUPPORT_CPP_LIMITS_H

libc/src/stdio/printf_core/CMakeLists.txt

add_object_library(
   converter
   SRCS
     converter.cpp
   HDRS
     converter.h
     converter_atlas.h
     string_converter.h
     char_converter.h
+    int_converter.h
   DEPENDS
     .writer
     .core_structs
 )
 
 
 add_header_library(
   printf_main

libc/src/stdio/printf_core/converter.cpp

case 'c':
     convert_char(writer, to_conv);
     return;
   case 's':
     convert_string(writer, to_conv);
     return;
   case 'd':
   case 'i':
   case 'u':
-    // convert_int(writer, to_conv);
+    convert_int(writer, to_conv);
     return;
   case 'o':
     // convert_oct(writer, to_conv);
     return;
   case 'x':
   case 'X':
     // convert_hex(writer, to_conv);
     return;

libc/src/stdio/printf_core/converter_atlas.h

 
 // defines convert_string
 #include "src/stdio/printf_core/string_converter.h"
 
 // defines convert_char
 #include "src/stdio/printf_core/char_converter.h"
 
 // defines convert_int
+#include "src/stdio/printf_core/int_converter.h"
+
 // defines convert_oct
 // defines convert_hex
 
 // TODO(michaelrj): add a flag to disable float point values here
 // defines convert_float_decimal
 // defines convert_float_dec_exp
 // defines convert_float_hex_exp
 // defines convert_float_mixed
 
 // TODO(michaelrj): add a flag to disable writing an int here
 // defines convert_write_int
 
 // defines convert_pointer
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CONVERTER_ATLAS_H

libc/src/stdio/printf_core/int_converter.h

+//===-- Integer Converter for printf ----------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/CPP/Limits.h"
+#include "src/stdio/printf_core/core_structs.h"
+#include "src/stdio/printf_core/writer.h"
+
+#include <inttypes.h>
+#include <stddef.h>
+
+namespace __llvm_libc {
+namespace printf_core {
+
+void convert_int(Writer *writer, const FormatSection &to_conv) {
+  static constexpr size_t BITS_IN_BYTE = 8;
+  static constexpr size_t BITS_IN_NUM = sizeof(uintmax_t) * BITS_IN_BYTE;
+
+  // This approximates the number of digits it takes to represent an integer of
+  // a certain number of bits. The calculation is floor((bits * 5) / 16)
+  // 32 -> 10 (actually needs 10)
+  // 64 -> 20 (actually needs 20)
+  // 128 -> 40 (actually needs 39)
+  // This estimation grows slightly faster than the actual value, but is close
+  // enough.
+
+  static constexpr size_t BUFF_LEN =
+      ((sizeof(uintmax_t) * BITS_IN_BYTE * 5) / 16);
+  uintmax_t num = to_conv.conv_val_raw;
+  char buffer[BUFF_LEN];
+  bool is_negative = false;
+  FormatFlags flags = to_conv.flags;
+
+  if (to_conv.conv_name == 'u') {
+    // these flags are only for signed conversions, so this removes them if the
+    // conversion is unsigned.
+    flags = static_cast<FormatFlags>(
+        flags & ~(FormatFlags::FORCE_SIGN | FormatFlags::SPACE_PREFIX));
+  } else {
+    // Check if the number is negative by checking the high bit. This works even
+    // for smaller numbers because they're sign extended by default.
+    if ((num & (uintmax_t(1) << (BITS_IN_NUM - 1))) > 0) {
+      is_negative = true;
+      num = -num;
+    }
+  }
+
+  switch (to_conv.length_modifier) {
+  case LengthModifier::none:
+    num = num & cpp::NumericLimits<unsigned int>::max();
+    break;
+
+  case LengthModifier::l:
+    num = num & cpp::NumericLimits<unsigned long>::max();
+    break;
+  case LengthModifier::ll:
+  case LengthModifier::L:
+    num = num & cpp::NumericLimits<unsigned long long>::max();
+    break;
+  case LengthModifier::h:
+    num = num & cpp::NumericLimits<unsigned short>::max();
+    break;
+  case LengthModifier::hh:
+    num = num & cpp::NumericLimits<unsigned char>::max();
+    break;
+  case LengthModifier::z:
+    num = num & cpp::NumericLimits<size_t>::max();
+    break;
+  case LengthModifier::t:
+    // we don't have unsigned ptrdiff so uintptr_t is used, since we need an
+    // unsigned type and ptrdiff is usually the same size as a pointer.
+    static_assert(sizeof(ptrdiff_t) == sizeof(uintptr_t));
+    num = num & cpp::NumericLimits<uintptr_t>::max();
+    break;
+  case LengthModifier::j:
+    // j is intmax, so no mask is necessary.
+    break;
+  }
+
+  // buff_cur can never reach 0, since the buffer is sized to always be able to
+  // contain the whole integer. This means that bounds checking it should be
+  // unnecessary.
+  size_t buff_cur = BUFF_LEN;
+  for (; num > 0 /* && buff_cur > 0 */; --buff_cur, num /= 10)
+    buffer[buff_cur - 1] = (num % 10) + '0';
+
+  size_t digits_written = BUFF_LEN - buff_cur;
+
+  char sign_char = 0;
+
+  if (is_negative)
+    sign_char = '-';
+  else if ((flags & FormatFlags::FORCE_SIGN) == FormatFlags::FORCE_SIGN)
+    sign_char = '+'; // FORCE_SIGN has precedence over SPACE_PREFIX
+  else if ((flags & FormatFlags::SPACE_PREFIX) == FormatFlags::SPACE_PREFIX)
+    sign_char = ' ';
+
+  int sign_char_len = (sign_char == 0 ? 0 : 1);
+
+  // these are signed to prevent underflow due to negative values. The eventual
+  // values will always be non-negative.
+  int zeroes;
+  int spaces;
+
+  // negative precision indicates that it was not specified.
+  if (to_conv.precision < 0) {
+    if ((flags & (FormatFlags::LEADING_ZEROES | FormatFlags::LEFT_JUSTIFIED)) ==
+        FormatFlags::LEADING_ZEROES) {
+      // if this conv has flag 0 but not - and no specified precision, it's
+      // padded with 0's instead of spaces identically to if precision =
+      // min_width - (1 if sign_char). For example: ("%+04d", 1) -> "+001"
+      zeroes = to_conv.min_width - digits_written - sign_char_len;
+      if (zeroes < 0)
+        zeroes = 0;
+      spaces = 0;
+    } else if (digits_written < 1) {
+      // if no precision is specified, precision defaults to 1. This means that
+      // if the integer passed to the conversion is 0, a 0 will be printed.
+      // Example: ("%3d", 0) -> "  0"
+      zeroes = 1;
+      spaces = to_conv.min_width - zeroes - sign_char_len;
+    } else {
+      // If there are enough digits to pass over the precision, just write the
+      // number, padded by spaces.
+      zeroes = 0;
+      spaces = to_conv.min_width - digits_written - sign_char_len;
+    }
+  } else {
+    // if precision was specified, possibly write zeroes, and possibly write
+    // spaces. Example: ("%5.4d", 10000) -> "10000"
+    // If the check for if zeroes is negative was not there, spaces would be
+    // incorrectly evaluated as 1.
+    zeroes = to_conv.precision - digits_written; // a negative value means 0
+    if (zeroes < 0)
+      zeroes = 0;
+    spaces = to_conv.min_width - zeroes - digits_written - sign_char_len;
+  }
+  if (spaces < 0)
+    spaces = 0;
+
+  if ((flags & FormatFlags::LEFT_JUSTIFIED) == FormatFlags::LEFT_JUSTIFIED) {
+    // if left justified it goes sign zeroes digits spaces
+    if (sign_char != 0)
+      writer->write(&sign_char, 1);
+    if (zeroes > 0)
+      writer->write_chars('0', zeroes);
+    if (digits_written > 0)
+      writer->write(buffer + buff_cur, digits_written);
+    if (spaces > 0)
+      writer->write_chars(' ', spaces);
+  } else {
+    // else it goes spaces sign zeroes digits
+    if (spaces > 0)
+      writer->write_chars(' ', spaces);
+    if (sign_char != 0)
+      writer->write(&sign_char, 1);
+    if (zeroes > 0)
+      writer->write_chars('0', zeroes);
+    if (digits_written > 0)
+      writer->write(buffer + buff_cur, digits_written);
+  }
+}
+
+} // namespace printf_core
+} // namespace __llvm_libc

libc/test/src/stdio/printf_core/converter_test.cpp

 //===-- Unittests for the printf Converter --------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "src/stdio/printf_core/converter.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/string_writer.h"
 #include "src/stdio/printf_core/writer.h"
 
 #include "utils/UnitTest/Test.h"
 
-TEST(LlvmLibcPrintfConverterTest, SimpleRawConversion) {
-  char str[10];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer(
+class LlvmLibcPrintfConverterTest : public __llvm_libc::testing::Test {
+protected:
+  // void SetUp() override {}
+  // void TearDown() override {}
+
+  char str[60];
+  __llvm_libc::printf_core::StringWriter str_writer =
+      __llvm_libc::printf_core::StringWriter(str);
+  __llvm_libc::printf_core::Writer writer = __llvm_libc::printf_core::Writer(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
+};
 
+TEST_F(LlvmLibcPrintfConverterTest, SimpleRawConversion) {
   __llvm_libc::printf_core::FormatSection raw_section;
   raw_section.has_conv = false;
   raw_section.raw_string = "abc";
   raw_section.raw_len = 3;
 
   __llvm_libc::printf_core::convert(&writer, raw_section);
 
   str_writer.terminate();
 
   ASSERT_STREQ(str, "abc");
   ASSERT_EQ(writer.get_chars_written(), 3ull);
 }
 
-TEST(LlvmLibcPrintfConverterTest, PercentConversion) {
-  char str[20];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer(
-      reinterpret_cast<void *>(&str_writer),
-      __llvm_libc::printf_core::write_to_string);
-
+TEST_F(LlvmLibcPrintfConverterTest, PercentConversion) {
   __llvm_libc::printf_core::FormatSection simple_conv;
   simple_conv.has_conv = true;
-  simple_conv.raw_string = "abc123";
-  simple_conv.raw_len = 6;
+  simple_conv.raw_string = "%%";
   simple_conv.conv_name = '%';
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
   str[1] = '\0';
 
   ASSERT_STREQ(str, "%");
   ASSERT_EQ(writer.get_chars_written(), 1ull);
 }
 
-TEST(LlvmLibcPrintfConverterTest, CharConversion) {
-  char str[20];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer(
-      reinterpret_cast<void *>(&str_writer),
-      __llvm_libc::printf_core::write_to_string);
-
+TEST_F(LlvmLibcPrintfConverterTest, CharConversionSimple) {
   __llvm_libc::printf_core::FormatSection simple_conv;
   simple_conv.has_conv = true;
-  simple_conv.raw_string = "abc123";
-  simple_conv.raw_len = 6;
+  // If has_conv is true, the raw string is ignored. They are not being parsed
+  // and match the actual conversion taking place so that you can compare these
+  // tests with other implmentations. The raw strings are completely optional.
+  simple_conv.raw_string = "%c";
   simple_conv.conv_name = 'c';
   simple_conv.conv_val_raw = 'D';
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
-  str[1] = '\0';
+  str_writer.terminate();
 
   ASSERT_STREQ(str, "D");
   ASSERT_EQ(writer.get_chars_written(), 1ull);
+}
 
+TEST_F(LlvmLibcPrintfConverterTest, CharConversionRightJustified) {
   __llvm_libc::printf_core::FormatSection right_justified_conv;
   right_justified_conv.has_conv = true;
-  right_justified_conv.raw_string = "abc123";
-  right_justified_conv.raw_len = 6;
+  right_justified_conv.raw_string = "%4c";
   right_justified_conv.conv_name = 'c';
   right_justified_conv.min_width = 4;
   right_justified_conv.conv_val_raw = 'E';
   __llvm_libc::printf_core::convert(&writer, right_justified_conv);
 
-  str[5] = '\0';
+  str_writer.terminate();
 
-  ASSERT_STREQ(str, "D   E");
-  ASSERT_EQ(writer.get_chars_written(), 5ull);
+  ASSERT_STREQ(str, "   E");
+  ASSERT_EQ(writer.get_chars_written(), 4ull);
+}
 
+TEST_F(LlvmLibcPrintfConverterTest, CharConversionLeftJustified) {
   __llvm_libc::printf_core::FormatSection left_justified_conv;
   left_justified_conv.has_conv = true;
-  left_justified_conv.raw_string = "abc123";
-  left_justified_conv.raw_len = 6;
+  left_justified_conv.raw_string = "%-4c";
   left_justified_conv.conv_name = 'c';
   left_justified_conv.flags =
       __llvm_libc::printf_core::FormatFlags::LEFT_JUSTIFIED;
   left_justified_conv.min_width = 4;
   left_justified_conv.conv_val_raw = 'F';
   __llvm_libc::printf_core::convert(&writer, left_justified_conv);
 
-  str[9] = '\0';
+  str_writer.terminate();
 
-  ASSERT_STREQ(str, "D   EF   ");
-  ASSERT_EQ(writer.get_chars_written(), 9ull);
+  ASSERT_STREQ(str, "F   ");
+  ASSERT_EQ(writer.get_chars_written(), 4ull);
 }
 
-TEST(LlvmLibcPrintfConverterTest, StringConversion) {
-  char str[20];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer(
-      reinterpret_cast<void *>(&str_writer),
-      __llvm_libc::printf_core::write_to_string);
+TEST_F(LlvmLibcPrintfConverterTest, StringConversionSimple) {
 
   __llvm_libc::printf_core::FormatSection simple_conv;
   simple_conv.has_conv = true;
-  simple_conv.raw_string = "abc123";
-  simple_conv.raw_len = 6;
+  simple_conv.raw_string = "%s";
   simple_conv.conv_name = 's';
   simple_conv.conv_val_ptr = const_cast<char *>("DEF");
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
-  str[3] = '\0'; // this null terminator is just for checking after every step.
+  str_writer.terminate();
 
   ASSERT_STREQ(str, "DEF");
   ASSERT_EQ(writer.get_chars_written(), 3ull);
+}
 
-  // continuing to write to this str_writer will overwrite that null terminator.
-
+TEST_F(LlvmLibcPrintfConverterTest, StringConversionPrecisionHigh) {
   __llvm_libc::printf_core::FormatSection high_precision_conv;
   high_precision_conv.has_conv = true;
-  high_precision_conv.raw_string = "abc123";
-  high_precision_conv.raw_len = 6;
+  high_precision_conv.raw_string = "%4s";
   high_precision_conv.conv_name = 's';
   high_precision_conv.precision = 4;
   high_precision_conv.conv_val_ptr = const_cast<char *>("456");
   __llvm_libc::printf_core::convert(&writer, high_precision_conv);
 
-  str[6] = '\0';
+  str_writer.terminate();
 
-  ASSERT_STREQ(str, "DEF456");
-  ASSERT_EQ(writer.get_chars_written(), 6ull);
+  ASSERT_STREQ(str, "456");
+  ASSERT_EQ(writer.get_chars_written(), 3ull);
+}
 
+TEST_F(LlvmLibcPrintfConverterTest, StringConversionPrecisionLow) {
   __llvm_libc::printf_core::FormatSection low_precision_conv;
   low_precision_conv.has_conv = true;
-  low_precision_conv.raw_string = "abc123";
-  low_precision_conv.raw_len = 6;
+  low_precision_conv.raw_string = "%.2s";
   low_precision_conv.conv_name = 's';
   low_precision_conv.precision = 2;
   low_precision_conv.conv_val_ptr = const_cast<char *>("xyz");
   __llvm_libc::printf_core::convert(&writer, low_precision_conv);
 
-  str[8] = '\0';
+  str_writer.terminate();
 
-  ASSERT_STREQ(str, "DEF456xy");
-  ASSERT_EQ(writer.get_chars_written(), 8ull);
+  ASSERT_STREQ(str, "xy");
+  ASSERT_EQ(writer.get_chars_written(), 2ull);
+}
 
+TEST_F(LlvmLibcPrintfConverterTest, StringConversionRightJustified) {
   __llvm_libc::printf_core::FormatSection right_justified_conv;
   right_justified_conv.has_conv = true;
-  right_justified_conv.raw_string = "abc123";
-  right_justified_conv.raw_len = 6;
+  right_justified_conv.raw_string = "%4s";
   right_justified_conv.conv_name = 's';
   right_justified_conv.min_width = 4;
   right_justified_conv.conv_val_ptr = const_cast<char *>("789");
   __llvm_libc::printf_core::convert(&writer, right_justified_conv);
 
-  str[12] = '\0';
+  str_writer.terminate();
 
-  ASSERT_STREQ(str, "DEF456xy 789");
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  ASSERT_STREQ(str, " 789");
+  ASSERT_EQ(writer.get_chars_written(), 4ull);
+}
 
+TEST_F(LlvmLibcPrintfConverterTest, StringConversionLeftJustified) {
   __llvm_libc::printf_core::FormatSection left_justified_conv;
   left_justified_conv.has_conv = true;
-  left_justified_conv.raw_string = "abc123";
-  left_justified_conv.raw_len = 6;
+  left_justified_conv.raw_string = "%-4s";
   left_justified_conv.conv_name = 's';
   left_justified_conv.flags =
       __llvm_libc::printf_core::FormatFlags::LEFT_JUSTIFIED;
   left_justified_conv.min_width = 4;
   left_justified_conv.conv_val_ptr = const_cast<char *>("ghi");
   __llvm_libc::printf_core::convert(&writer, left_justified_conv);
 
-  str[16] = '\0';
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "ghi ");
+  ASSERT_EQ(writer.get_chars_written(), 4ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionSignedPositive) {
+
+  __llvm_libc::printf_core::FormatSection simple_conv;
+  simple_conv.has_conv = true;
+  simple_conv.raw_string = "%d";
+  simple_conv.conv_name = 'd';
+  simple_conv.conv_val_raw = 123;
+
+  __llvm_libc::printf_core::convert(&writer, simple_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "123");
+  ASSERT_EQ(writer.get_chars_written(), 3ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionSignedNegative) {
+
+  __llvm_libc::printf_core::FormatSection negative_conv;
+  negative_conv.has_conv = true;
+  negative_conv.raw_string = "%i";
+  negative_conv.conv_name = 'i';
+  negative_conv.conv_val_raw = -456;
+
+  __llvm_libc::printf_core::convert(&writer, negative_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "-456");
+  ASSERT_EQ(writer.get_chars_written(), 4ull);
+}
+
+// Length modifier tests.
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionUnsignedChar) {
+  __llvm_libc::printf_core::FormatSection unsigned_char_conv;
+  unsigned_char_conv.has_conv = true;
+  unsigned_char_conv.raw_string = "%hhu";
+  unsigned_char_conv.length_modifier =
+      __llvm_libc::printf_core::LengthModifier::hh;
+  unsigned_char_conv.conv_name = 'u';
+  unsigned_char_conv.conv_val_raw = 257; // 0x10001
+
+  __llvm_libc::printf_core::convert(&writer, unsigned_char_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "1");
+  ASSERT_EQ(writer.get_chars_written(), 1ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionUnsignedLongLong) {
+  __llvm_libc::printf_core::FormatSection unsigned_long_long_conv;
+  unsigned_long_long_conv.has_conv = true;
+  unsigned_long_long_conv.raw_string = "%llu";
+  unsigned_long_long_conv.length_modifier =
+      __llvm_libc::printf_core::LengthModifier::ll;
+  unsigned_long_long_conv.conv_name = 'u';
+  unsigned_long_long_conv.conv_val_raw = 18446744073709551615ull; // ull max.
+
+  __llvm_libc::printf_core::convert(&writer, unsigned_long_long_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "18446744073709551615");
+  ASSERT_EQ(writer.get_chars_written(), 20ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionUnsignedPtrdiff) {
+  __llvm_libc::printf_core::FormatSection unsigned_ptrdiff_conv;
+  unsigned_ptrdiff_conv.has_conv = true;
+  unsigned_ptrdiff_conv.raw_string = "%tu";
+  unsigned_ptrdiff_conv.length_modifier =
+      __llvm_libc::printf_core::LengthModifier::t;
+  unsigned_ptrdiff_conv.conv_name = 'u';
+  unsigned_ptrdiff_conv.conv_val_raw = ~ptrdiff_t(0);
+
+  __llvm_libc::printf_core::convert(&writer, unsigned_ptrdiff_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "18446744073709551615");
+  ASSERT_EQ(writer.get_chars_written(), 20ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionSignedLongLong) {
+  __llvm_libc::printf_core::FormatSection unsigned_long_long_conv;
+  unsigned_long_long_conv.has_conv = true;
+  unsigned_long_long_conv.raw_string = "%lld";
+  unsigned_long_long_conv.length_modifier =
+      __llvm_libc::printf_core::LengthModifier::ll;
+  unsigned_long_long_conv.conv_name = 'd';
+  unsigned_long_long_conv.conv_val_raw = -9223372036854775807ll - 1ll; // ll min
+  // ll min is split to suppress warnings about the constant being too large for
+  // any signed type.
+
+  __llvm_libc::printf_core::convert(&writer, unsigned_long_long_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "-9223372036854775808");
+  ASSERT_EQ(writer.get_chars_written(), 20ull);
+}
+
+// min width tests
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionMinWidthHigh) {
+  __llvm_libc::printf_core::FormatSection min_width_greater_conv;
+  min_width_greater_conv.has_conv = true;
+  min_width_greater_conv.raw_string = "%4d";
+  min_width_greater_conv.min_width = 4;
+  min_width_greater_conv.conv_name = 'd';
+  min_width_greater_conv.conv_val_raw = 789;
+
+  __llvm_libc::printf_core::convert(&writer, min_width_greater_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, " 789");
+  ASSERT_EQ(writer.get_chars_written(), 4ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionMinWidthLow) {
+
+  __llvm_libc::printf_core::FormatSection min_width_less_conv;
+  min_width_less_conv.has_conv = true;
+  min_width_less_conv.raw_string = "%2d";
+  min_width_less_conv.min_width = 2;
+  min_width_less_conv.conv_name = 'd';
+  min_width_less_conv.conv_val_raw = 987;
+
+  __llvm_libc::printf_core::convert(&writer, min_width_less_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "987");
+  ASSERT_EQ(writer.get_chars_written(), 3ull);
+}
+
+// precision tests
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionPrecisionDefault) {
+  __llvm_libc::printf_core::FormatSection default_precision_conv;
+  default_precision_conv.has_conv = true;
+  default_precision_conv.raw_string = "%d";
+  default_precision_conv.conv_name = 'd';
+  default_precision_conv.conv_val_raw = 0;
+
+  __llvm_libc::printf_core::convert(&writer, default_precision_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "0");
+  ASSERT_EQ(writer.get_chars_written(), 1ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionPrecisionZero) {
+  __llvm_libc::printf_core::FormatSection zero_precision_conv;
+  zero_precision_conv.has_conv = true;
+  zero_precision_conv.raw_string = "%d";
+  zero_precision_conv.precision = 0;
+  zero_precision_conv.conv_name = 'd';
+  zero_precision_conv.conv_val_raw = 0;
+
+  __llvm_libc::printf_core::convert(&writer, zero_precision_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "");
+  ASSERT_EQ(writer.get_chars_written(), 0ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionPrecisionHighPositive) {
+  __llvm_libc::printf_core::FormatSection precision_greater_conv;
+  precision_greater_conv.has_conv = true;
+  precision_greater_conv.raw_string = "%.5d";
+  precision_greater_conv.precision = 5;
+  precision_greater_conv.conv_name = 'd';
+  precision_greater_conv.conv_val_raw = 654;
+
+  __llvm_libc::printf_core::convert(&writer, precision_greater_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "00654");
+  ASSERT_EQ(writer.get_chars_written(), 5ull);
+}
 
-  ASSERT_STREQ(str, "DEF456xy 789ghi ");
-  ASSERT_EQ(writer.get_chars_written(), 16ull);
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionPrecisionHighNegative) {
+  __llvm_libc::printf_core::FormatSection precision_greater_negative_conv;
+  precision_greater_negative_conv.has_conv = true;
+  precision_greater_negative_conv.raw_string = "%.5d";
+  precision_greater_negative_conv.precision = 5;
+  precision_greater_negative_conv.conv_name = 'd';
+  precision_greater_negative_conv.conv_val_raw = -321;
+
+  __llvm_libc::printf_core::convert(&writer, precision_greater_negative_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "-00321");
+  ASSERT_EQ(writer.get_chars_written(), 6ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionPrecisionLow) {
+  __llvm_libc::printf_core::FormatSection precision_less_conv;
+  precision_less_conv.has_conv = true;
+  precision_less_conv.raw_string = "%.2d";
+  precision_less_conv.precision = 2;
+  precision_less_conv.conv_name = 'd';
+  precision_less_conv.conv_val_raw = 135;
+
+  __llvm_libc::printf_core::convert(&writer, precision_less_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "135");
+  ASSERT_EQ(writer.get_chars_written(), 3ull);
+}
+
+// Flag tests
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionLeftJustifiedPositive) {
+  __llvm_libc::printf_core::FormatSection left_justified_conv;
+  left_justified_conv.has_conv = true;
+  left_justified_conv.raw_string = "%-5d";
+  left_justified_conv.min_width = 5;
+  left_justified_conv.flags =
+      __llvm_libc::printf_core::FormatFlags::LEFT_JUSTIFIED;
+  left_justified_conv.conv_name = 'd';
+  left_justified_conv.conv_val_raw = 246;
+
+  __llvm_libc::printf_core::convert(&writer, left_justified_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "246  ");
+  ASSERT_EQ(writer.get_chars_written(), 5ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionLeftJustifiedNegative) {
+  __llvm_libc::printf_core::FormatSection left_justified_conv;
+  left_justified_conv.has_conv = true;
+  left_justified_conv.raw_string = "%-5d";
+  left_justified_conv.min_width = 5;
+  left_justified_conv.flags =
+      __llvm_libc::printf_core::FormatFlags::LEFT_JUSTIFIED;
+  left_justified_conv.conv_name = 'd';
+  left_justified_conv.conv_val_raw = -147;
+
+  __llvm_libc::printf_core::convert(&writer, left_justified_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "-147 ");
+  ASSERT_EQ(writer.get_chars_written(), 5ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionForceSign) {
+  __llvm_libc::printf_core::FormatSection force_sign_conv;
+  force_sign_conv.has_conv = true;
+  force_sign_conv.raw_string = "%+d";
+  force_sign_conv.flags = __llvm_libc::printf_core::FormatFlags::FORCE_SIGN;
+  force_sign_conv.conv_name = 'd';
+  force_sign_conv.conv_val_raw = 258;
+
+  __llvm_libc::printf_core::convert(&writer, force_sign_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "+258");
+  ASSERT_EQ(writer.get_chars_written(), 4ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionSpacePrefix) {
+  __llvm_libc::printf_core::FormatSection space_prefix_sign_conv;
+  space_prefix_sign_conv.has_conv = true;
+  space_prefix_sign_conv.raw_string = "%+d";
+  space_prefix_sign_conv.flags =
+      __llvm_libc::printf_core::FormatFlags::SPACE_PREFIX;
+  space_prefix_sign_conv.conv_name = 'd';
+  space_prefix_sign_conv.conv_val_raw = 369;
+
+  __llvm_libc::printf_core::convert(&writer, space_prefix_sign_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, " 369");
+  ASSERT_EQ(writer.get_chars_written(), 4ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionLeadingZeroesPositive) {
+  __llvm_libc::printf_core::FormatSection leading_zeroes_conv;
+  leading_zeroes_conv.has_conv = true;
+  leading_zeroes_conv.raw_string = "%05d";
+  leading_zeroes_conv.min_width = 5;
+  leading_zeroes_conv.flags =
+      __llvm_libc::printf_core::FormatFlags::LEADING_ZEROES;
+  leading_zeroes_conv.conv_name = 'd';
+  leading_zeroes_conv.conv_val_raw = 470;
+
+  __llvm_libc::printf_core::convert(&writer, leading_zeroes_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "00470");
+  ASSERT_EQ(writer.get_chars_written(), 5ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionLeadingZeroesNegative) {
+  __llvm_libc::printf_core::FormatSection leading_zeroes_conv;
+  leading_zeroes_conv.has_conv = true;
+  leading_zeroes_conv.raw_string = "%05d";
+  leading_zeroes_conv.min_width = 5;
+  leading_zeroes_conv.flags =
+      __llvm_libc::printf_core::FormatFlags::LEADING_ZEROES;
+  leading_zeroes_conv.conv_name = 'd';
+  leading_zeroes_conv.conv_val_raw = -581;
+
+  __llvm_libc::printf_core::convert(&writer, leading_zeroes_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "-0581");
+  ASSERT_EQ(writer.get_chars_written(), 5ull);
+}
+
+// combined tests
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionUnsignedWithSignedFlags) {
+  __llvm_libc::printf_core::FormatSection combined_conv;
+  combined_conv.has_conv = true;
+  combined_conv.raw_string = "%+ u";
+  combined_conv.flags = static_cast<__llvm_libc::printf_core::FormatFlags>(
+      __llvm_libc::printf_core::FormatFlags::FORCE_SIGN |
+      __llvm_libc::printf_core::FormatFlags::SPACE_PREFIX);
+  combined_conv.conv_name = 'u';
+  combined_conv.conv_val_raw = 692;
+
+  __llvm_libc::printf_core::convert(&writer, combined_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "692");
+  ASSERT_EQ(writer.get_chars_written(), 3ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionFlagPrecedence) {
+  __llvm_libc::printf_core::FormatSection combined_conv;
+  combined_conv.has_conv = true;
+  combined_conv.raw_string = "%+ -05d";
+  combined_conv.min_width = 5;
+  // FORCE_SIGN takes precedence over SPACE_PREFIX
+  // LEFT_JUSTIFIED takes precedence over LEADING_ZEROES
+  combined_conv.flags = static_cast<__llvm_libc::printf_core::FormatFlags>(
+      __llvm_libc::printf_core::FormatFlags::FORCE_SIGN |
+      __llvm_libc::printf_core::FormatFlags::SPACE_PREFIX |
+      __llvm_libc::printf_core::FormatFlags::LEADING_ZEROES |
+      __llvm_libc::printf_core::FormatFlags::LEFT_JUSTIFIED);
+  combined_conv.conv_name = 'd';
+  combined_conv.conv_val_raw = 703;
+
+  __llvm_libc::printf_core::convert(&writer, combined_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "+703 ");
+  ASSERT_EQ(writer.get_chars_written(), 5ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionPositivePrecisionAndWidth) {
+  __llvm_libc::printf_core::FormatSection combined_conv;
+  combined_conv.has_conv = true;
+  combined_conv.raw_string = "%7.5d";
+  combined_conv.min_width = 7;
+  combined_conv.precision = 5;
+  combined_conv.conv_name = 'd';
+  combined_conv.conv_val_raw = 814;
+
+  __llvm_libc::printf_core::convert(&writer, combined_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "  00814");
+  ASSERT_EQ(writer.get_chars_written(), 7ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionNegativePrecisionAndWidth) {
+  __llvm_libc::printf_core::FormatSection combined_conv;
+  combined_conv.has_conv = true;
+  combined_conv.raw_string = "%7.5d";
+  combined_conv.min_width = 7;
+  combined_conv.precision = 5;
+  combined_conv.conv_name = 'd';
+  combined_conv.conv_val_raw = -925;
+
+  __llvm_libc::printf_core::convert(&writer, combined_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, " -00925");
+  ASSERT_EQ(writer.get_chars_written(), 7ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionPrecisionOverLeadingZeroes) {
+  __llvm_libc::printf_core::FormatSection combined_conv;
+  combined_conv.has_conv = true;
+  combined_conv.raw_string = "%7.5d";
+  combined_conv.min_width = 7;
+  combined_conv.precision = 5;
+  combined_conv.flags = __llvm_libc::printf_core::FormatFlags::LEADING_ZEROES;
+  combined_conv.conv_name = 'd';
+  combined_conv.conv_val_raw = 159;
+
+  __llvm_libc::printf_core::convert(&writer, combined_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "  00159");
+  ASSERT_EQ(writer.get_chars_written(), 7ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionLeftJustifiedWithOptions) {
+  __llvm_libc::printf_core::FormatSection combined_conv;
+  combined_conv.has_conv = true;
+  combined_conv.raw_string = "% -7.5d";
+  combined_conv.min_width = 7;
+  combined_conv.precision = 5;
+  combined_conv.flags = static_cast<__llvm_libc::printf_core::FormatFlags>(
+      __llvm_libc::printf_core::FormatFlags::SPACE_PREFIX |
+      __llvm_libc::printf_core::FormatFlags::LEFT_JUSTIFIED);
+  combined_conv.conv_name = 'd';
+  combined_conv.conv_val_raw = 260;
+
+  __llvm_libc::printf_core::convert(&writer, combined_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, " 00260 ");
+  ASSERT_EQ(writer.get_chars_written(), 7ull);
+}
+
+TEST_F(LlvmLibcPrintfConverterTest, IntConversionLowPrecisionAndWidth) {
+  __llvm_libc::printf_core::FormatSection combined_conv;
+  combined_conv.has_conv = true;
+  combined_conv.raw_string = "%5.4d";
+  combined_conv.min_width = 5;
+  combined_conv.precision = 4;
+  combined_conv.conv_name = 'd';
+  combined_conv.conv_val_raw = 10000;
+
+  __llvm_libc::printf_core::convert(&writer, combined_conv);
+
+  str_writer.terminate();
+
+  ASSERT_STREQ(str, "10000");
+  ASSERT_EQ(writer.get_chars_written(), 5ull);
 }

libc/test/src/stdio/sprintf_test.cpp

TEST(LlvmLibcSPrintfTest, StringConv) {
   ASSERT_STREQ(buff, "word soup");
 
   written = __llvm_libc::sprintf(buff, "%*s %.*s %*.*s", 10, "beginning", 2,
                                  "isn't", 12, 10, "important. Ever.");
   EXPECT_EQ(written, 26);
   ASSERT_STREQ(buff, " beginning is   important.");
 }
 
+TEST(LlvmLibcSPrintfTest, IntConv) {
+  char buff[64];
+  int written;
+
+  // This tests all of the various integer conversion options, but for more
+  // thorough testing look in converter_test.cpp
+  written = __llvm_libc::sprintf(buff, "%d", 123);
+  EXPECT_EQ(written, 3);
+  ASSERT_STREQ(buff, "123");
+
+  written = __llvm_libc::sprintf(buff, "%10d %-10d", 456, -789);
+  EXPECT_EQ(written, 21);
+  ASSERT_STREQ(buff, "       456 -789      ");
+
+  written = __llvm_libc::sprintf(buff, "%-5.4d%+.4u", 75, 25);
+  EXPECT_EQ(written, 9);
+  ASSERT_STREQ(buff, "0075 0025");
+
+  written = __llvm_libc::sprintf(buff, "% 05hhi %+-0.5llu %-+ 06.3zd",
+                                 256 + 127, 68719476736ll, size_t(2));
+  EXPECT_EQ(written, 24);
+  ASSERT_STREQ(buff, " 0127 68719476736 +002  ");
+}
+
 #ifndef LLVM_LIBC_PRINTF_DISABLE_INDEX_MODE
 TEST(LlvmLibcSPrintfTest, IndexModeParsing) {
   char buff[64];
   int written;
 
   written = __llvm_libc::sprintf(buff, "%1$s", "abcDEF123");
   EXPECT_EQ(written, 9);
   ASSERT_STREQ(buff, "abcDEF123");