[libc] add printf hex conversion

[michaelrj]

Summary

The hex converter handles the %x and %X conversions.

[michaelrj]

This patch doesn't have comprehensive unit tests yet, but should be good to go once I add those.

[michaelrj]

add tests and header guards

[michaelrj]

rebase on new changes and move length modifier handling to a shared function.

libc/src/stdio/printf_core/CMakeLists.txt

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

libc/src/stdio/printf_core/char_converter.h

 //===-- String 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CHAR_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CHAR_CONVERTER_H
 
+#include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/writer.h"
 
 namespace __llvm_libc {
 namespace printf_core {
 
 int inline convert_char(Writer *writer, const FormatSection &to_conv) {
   char c = to_conv.conv_val_raw;

libc/src/stdio/printf_core/converter.cpp

int convert(Writer *writer, const FormatSection &to_conv) {
   case 'd':
   case 'i':
   case 'u':
     return convert_int(writer, to_conv);
   case 'o':
     // return convert_oct(writer, to_conv);
   case 'x':
   case 'X':
-    // return convert_hex(writer, to_conv);
+    return convert_hex(writer, to_conv);
   // TODO(michaelrj): add a flag to disable float point values here
   case 'f':
   case 'F':
     // return convert_float_decimal(writer, to_conv);
   case 'e':
   case 'E':
     // return convert_float_dec_exp(writer, to_conv);
   case 'a':

libc/src/stdio/printf_core/converter_atlas.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
+#include "src/stdio/printf_core/hex_converter.h"
 
 // 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/converter_utils.h

+//===-- Shared Converter Utilities 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CONVERTER_UTILS_H
+#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CONVERTER_UTILS_H
+
+#include "src/__support/CPP/Limits.h"
+#include "src/stdio/printf_core/core_structs.h"
+
+#include <inttypes.h>
+#include <stddef.h>
+
+namespace __llvm_libc {
+namespace printf_core {
+
+inline uintmax_t apply_length_modifier(uintmax_t num, LengthModifier lm) {
+  switch (lm) {
+  case LengthModifier::none:
+    return num & cpp::NumericLimits<unsigned int>::max();
+  case LengthModifier::l:
+    return num & cpp::NumericLimits<unsigned long>::max();
+  case LengthModifier::ll:
+  case LengthModifier::L:
+    return num & cpp::NumericLimits<unsigned long long>::max();
+  case LengthModifier::h:
+    return num & cpp::NumericLimits<unsigned short>::max();
+  case LengthModifier::hh:
+    return num & cpp::NumericLimits<unsigned char>::max();
+  case LengthModifier::z:
+    return num & cpp::NumericLimits<size_t>::max();
+  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));
+    return num & cpp::NumericLimits<uintptr_t>::max();
+  case LengthModifier::j:
+    return num; // j is intmax, so no mask is necessary.
+  }
+}
+
+#define RET_IF_RESULT_NEGATIVE(func)                                           \
+  {                                                                            \
+    int result = (func);                                                       \
+    if (result < 0)                                                            \
+      return result;                                                           \
+  }
+
+} // namespace printf_core
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CONVERTER_UTILS_H

libc/src/stdio/printf_core/core_structs.h

if (has_conv) {
       if (conv_name == 'p' || conv_name == 'n' || conv_name == 's')
         return (conv_val_ptr == other.conv_val_ptr);
       else if (conv_name != '%')
         return (conv_val_raw == other.conv_val_raw);
     }
     return true;
   }
 };
-
-#define RET_IF_RESULT_NEGATIVE(func)                                           \
-  {                                                                            \
-    int result = (func);                                                       \
-    if (result < 0)                                                            \
-      return result;                                                           \
-  }
-
 } // namespace printf_core
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CORE_STRUCTS_H

libc/src/stdio/printf_core/hex_converter.h

+//===-- Hexadecimal 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_HEX_CONVERTER_H
+#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_HEX_CONVERTER_H
+
+#include "src/stdio/printf_core/converter_utils.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 {
+
+int convert_hex(Writer *writer, const FormatSection &to_conv) {
+  // This approximates the number of digits it takes to represent a hexadecimal
+  // value of a certain number of bits. Each hex digit represents 4 bits, so the
+  // exact value is the number of bytes multiplied by 2.
+  static constexpr size_t BUFF_LEN = sizeof(uintmax_t) * 2;
+  uintmax_t num = to_conv.conv_val_raw;
+  char buffer[BUFF_LEN];
+
+  // All of the characters will be defined relative to variable a, which will be
+  // the appropriate case based on the name of the conversion.
+  char a;
+  if (to_conv.conv_name == 'x')
+    a = 'a';
+  else
+    a = 'A';
+
+  num = apply_length_modifier(num, to_conv.length_modifier);
+
+  // 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 /= 16)
+    buffer[buff_cur - 1] =
+        ((num % 16) > 9) ? ((num % 16) - 10 + a) : ((num % 16) + '0');
+
+  size_t digits_written = BUFF_LEN - buff_cur;
+
+  // these are signed to prevent underflow due to negative values. The eventual
+  // values will always be non-negative.
+  int zeroes;
+  int spaces;
+
+  // prefix is "0x"
+  int prefix_len;
+  char prefix[2];
+  if ((to_conv.flags & FormatFlags::ALTERNATE_FORM) ==
+      FormatFlags::ALTERNATE_FORM) {
+    prefix_len = 2;
+    prefix[0] = '0';
+    prefix[1] = a + ('x' - 'a');
+  } else {
+    prefix_len = 0;
+    prefix[0] = 0;
+  }
+
+  // negative precision indicates that it was not specified.
+  if (to_conv.precision < 0) {
+    if ((to_conv.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 - (2 if prefix). For example: ("%#04x", 15) -> "0x0f"
+      zeroes = to_conv.min_width - digits_written - prefix_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: ("%3x", 0) -> "  0"
+      zeroes = 1;
+      spaces = to_conv.min_width - zeroes - prefix_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 - prefix_len;
+    }
+  } else {
+    // if precision was specified, possibly write zeroes, and possibly write
+    // spaces. Example: ("%5.4x", 0x10000) -> "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 - prefix_len;
+  }
+  if (spaces < 0)
+    spaces = 0;
+
+  if ((to_conv.flags & FormatFlags::LEFT_JUSTIFIED) ==
+      FormatFlags::LEFT_JUSTIFIED) {
+    // if left justified it goes prefix zeroes digits spaces
+    if (prefix[0] != 0)
+      RET_IF_RESULT_NEGATIVE(writer->write(prefix, 2));
+    if (zeroes > 0)
+      RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes));
+    if (digits_written > 0)
+      RET_IF_RESULT_NEGATIVE(writer->write(buffer + buff_cur, digits_written));
+    if (spaces > 0)
+      RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces));
+  } else {
+    // else it goes spaces prefix zeroes digits
+    if (spaces > 0)
+      RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces));
+    if (prefix[0] != 0)
+      RET_IF_RESULT_NEGATIVE(writer->write(prefix, 2));
+    if (zeroes > 0)
+      RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes));
+    if (digits_written > 0)
+      RET_IF_RESULT_NEGATIVE(writer->write(buffer + buff_cur, digits_written));
+  }
+  return 0;
+}
+
+} // namespace printf_core
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_HEX_CONVERTER_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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_INT_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_INT_CONVERTER_H
 
-#include "src/__support/CPP/Limits.h"
+#include "src/stdio/printf_core/converter_utils.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 {
if (to_conv.conv_name == 'u') {
     // 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;
-  }
+  num = apply_length_modifier(num, to_conv.length_modifier);
 
   // 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';
 

libc/src/stdio/printf_core/string_converter.h

 //===-- String 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_STRING_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_STRING_CONVERTER_H
 
+#include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/writer.h"
 
 #include <stddef.h>
 
 namespace __llvm_libc {
 namespace printf_core {
 

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

TEST_F(LlvmLibcPrintfConverterTest, IntConversionSimple) {
   section.conv_val_raw = 12345;
   __llvm_libc::printf_core::convert(&writer, section);
 
   str_writer.terminate();
 
   ASSERT_STREQ(str, "12345");
   ASSERT_EQ(writer.get_chars_written(), 5);
 }
+
+// This needs to be switched to the new testing layout, but that's still in the
+// int patch so I need to land that first. This is what I get for not keeping my
+// patches small and focused.
+TEST(LlvmLibcPrintfConverterTest, HexConversion) {
+  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);
+
+  __llvm_libc::printf_core::FormatSection left_justified_conv;
+  left_justified_conv.has_conv = true;
+  left_justified_conv.raw_string = "%#018x";
+  left_justified_conv.raw_len = 6;
+  left_justified_conv.conv_name = 'x';
+  left_justified_conv.flags =
+      static_cast<__llvm_libc::printf_core::FormatFlags>(
+          __llvm_libc::printf_core::FormatFlags::ALTERNATE_FORM |
+          __llvm_libc::printf_core::FormatFlags::LEADING_ZEROES);
+  left_justified_conv.min_width = 18;
+  left_justified_conv.conv_val_raw = 0x123456ab;
+  __llvm_libc::printf_core::convert(&writer, left_justified_conv);
+
+  str_writer.terminate();
+  ASSERT_STREQ(str, "0x00000000123456ab");
+  ASSERT_EQ(writer.get_chars_written(), 18);
+}

libc/test/src/stdio/sprintf_test.cpp

TEST(LlvmLibcSPrintfTest, IntConv) {
   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  ");
 }
 
+TEST(LlvmLibcSPrintfTest, HexConv) {
+  char buff[64];
+  int written;
+
+  // Basic Tests.
+
+  written = __llvm_libc::sprintf(buff, "%x", 0x123a);
+  EXPECT_EQ(written, 4);
+  ASSERT_STREQ(buff, "123a");
+
+  written = __llvm_libc::sprintf(buff, "%X", 0x456b);
+  EXPECT_EQ(written, 4);
+  ASSERT_STREQ(buff, "456B");
+
+  // Length Modifier Tests.
+
+  written = __llvm_libc::sprintf(buff, "%hhx", 0x10001);
+  EXPECT_EQ(written, 1);
+  ASSERT_STREQ(buff, "1");
+
+  written = __llvm_libc::sprintf(buff, "%llx", 0xffffffffffffffffull);
+  EXPECT_EQ(written, 16);
+  ASSERT_STREQ(buff, "ffffffffffffffff"); // ull max
+
+  written = __llvm_libc::sprintf(buff, "%tX", ~ptrdiff_t(0));
+  if (sizeof(ptrdiff_t) == 8) {
+    EXPECT_EQ(written, 16);
+    ASSERT_STREQ(buff, "FFFFFFFFFFFFFFFF");
+  } else if (sizeof(ptrdiff_t) == 4) {
+    EXPECT_EQ(written, 8);
+    ASSERT_STREQ(buff, "FFFFFFFF");
+  }
+
+  // Min Width Tests.
+
+  written = __llvm_libc::sprintf(buff, "%4x", 0x789);
+  EXPECT_EQ(written, 4);
+  ASSERT_STREQ(buff, " 789");
+
+  written = __llvm_libc::sprintf(buff, "%2X", 0x987);
+  EXPECT_EQ(written, 3);
+  ASSERT_STREQ(buff, "987");
+
+  // Precision Tests.
+
+  written = __llvm_libc::sprintf(buff, "%x", 0);
+  EXPECT_EQ(written, 1);
+  ASSERT_STREQ(buff, "0");
+
+  written = __llvm_libc::sprintf(buff, "%.0x", 0);
+  EXPECT_EQ(written, 0);
+  ASSERT_STREQ(buff, "");
+
+  written = __llvm_libc::sprintf(buff, "%.5x", 0x1F3);
+  EXPECT_EQ(written, 5);
+  ASSERT_STREQ(buff, "001f3");
+
+  written = __llvm_libc::sprintf(buff, "%.2x", 0x135);
+  EXPECT_EQ(written, 3);
+  ASSERT_STREQ(buff, "135");
+
+  // Flag Tests.
+
+  written = __llvm_libc::sprintf(buff, "%-5x", 0x246);
+  EXPECT_EQ(written, 5);
+  ASSERT_STREQ(buff, "246  ");
+
+  written = __llvm_libc::sprintf(buff, "%#x", 0xd3f);
+  EXPECT_EQ(written, 5);
+  ASSERT_STREQ(buff, "0xd3f");
+
+  written = __llvm_libc::sprintf(buff, "%#X", 0xE40);
+  EXPECT_EQ(written, 5);
+  ASSERT_STREQ(buff, "0XE40");
+
+  written = __llvm_libc::sprintf(buff, "%05x", 0x470);
+  EXPECT_EQ(written, 5);
+  ASSERT_STREQ(buff, "00470");
+
+  written = __llvm_libc::sprintf(buff, "%0#6x", 0x8c3);
+  EXPECT_EQ(written, 6);
+  ASSERT_STREQ(buff, "0x08c3");
+
+  written = __llvm_libc::sprintf(buff, "%-#6x", 0x5f0);
+  EXPECT_EQ(written, 6);
+  ASSERT_STREQ(buff, "0x5f0 ");
+
+  // Combined Tests.
+
+  written = __llvm_libc::sprintf(buff, "%#-07x", 0x703);
+  EXPECT_EQ(written, 7);
+  ASSERT_STREQ(buff, "0x703  ");
+
+  written = __llvm_libc::sprintf(buff, "%7.5x", 0x814);
+  EXPECT_EQ(written, 7);
+  ASSERT_STREQ(buff, "  00814");
+
+  written = __llvm_libc::sprintf(buff, "%#9.5X", 0x9d4);
+  EXPECT_EQ(written, 9);
+  ASSERT_STREQ(buff, "  0X009D4");
+
+  written = __llvm_libc::sprintf(buff, "%-7.5x", 0x260);
+  EXPECT_EQ(written, 7);
+  ASSERT_STREQ(buff, "00260  ");
+
+  written = __llvm_libc::sprintf(buff, "%5.4x", 0x10000);
+  EXPECT_EQ(written, 5);
+  ASSERT_STREQ(buff, "10000");
+
+  // Multiple Conversion Tests.
+
+  written = __llvm_libc::sprintf(buff, "%10X %-#10x", 0x45b, 0x789);
+  EXPECT_EQ(written, 21);
+  ASSERT_STREQ(buff, "       45B 0x789     ");
+
+  written = __llvm_libc::sprintf(buff, "%-5.4x%#.4x", 0x75, 0x25);
+  EXPECT_EQ(written, 11);
+  ASSERT_STREQ(buff, "0075 0x0025");
+
+  written = __llvm_libc::sprintf(buff, "%04hhX %#.5llx %-6.3zX", 256 + 0x7f,
+                                 0x1000000000ll, size_t(2));
+  EXPECT_EQ(written, 24);
+  ASSERT_STREQ(buff, "007F 0x1000000000 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");