From 6ec465ab8ff25f897aa61addb2daf424c7de5b6f Mon Sep 17 00:00:00 2001 From: Michael Jones Date: Tue, 14 Jun 2022 15:54:37 -0700 Subject: [PATCH] [libc] add printf oct conversion The oct converter handles the %o conversion. Reviewed By: lntue Differential Revision: https://reviews.llvm.org/D127985 --- libc/src/stdio/printf_core/CMakeLists.txt | 1 + libc/src/stdio/printf_core/converter.cpp | 2 +- libc/src/stdio/printf_core/converter_atlas.h | 1 + libc/src/stdio/printf_core/oct_converter.h | 111 ++++++++++++++++ .../src/stdio/printf_core/converter_test.cpp | 46 ++++--- libc/test/src/stdio/sprintf_test.cpp | 121 ++++++++++++++++++ 6 files changed, 266 insertions(+), 16 deletions(-) create mode 100644 libc/src/stdio/printf_core/oct_converter.h diff --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt index 1f1db368cb17..345ecc7e2eab 100644 --- a/libc/src/stdio/printf_core/CMakeLists.txt +++ b/libc/src/stdio/printf_core/CMakeLists.txt @@ -63,6 +63,7 @@ add_object_library( int_converter.h hex_converter.h ptr_converter.h + oct_converter.h DEPENDS .writer .core_structs diff --git a/libc/src/stdio/printf_core/converter.cpp b/libc/src/stdio/printf_core/converter.cpp index 609295195386..f809408ad077 100644 --- a/libc/src/stdio/printf_core/converter.cpp +++ b/libc/src/stdio/printf_core/converter.cpp @@ -40,7 +40,7 @@ int convert(Writer *writer, const FormatSection &to_conv) { case 'u': return convert_int(writer, to_conv); case 'o': - // return convert_oct(writer, to_conv); + return convert_oct(writer, to_conv); case 'x': case 'X': return convert_hex(writer, to_conv); diff --git a/libc/src/stdio/printf_core/converter_atlas.h b/libc/src/stdio/printf_core/converter_atlas.h index fe26060a7594..2c39255cdefe 100644 --- a/libc/src/stdio/printf_core/converter_atlas.h +++ b/libc/src/stdio/printf_core/converter_atlas.h @@ -23,6 +23,7 @@ #include "src/stdio/printf_core/int_converter.h" // defines convert_oct +#include "src/stdio/printf_core/oct_converter.h" // defines convert_hex #include "src/stdio/printf_core/hex_converter.h" diff --git a/libc/src/stdio/printf_core/oct_converter.h b/libc/src/stdio/printf_core/oct_converter.h new file mode 100644 index 000000000000..873464911ced --- /dev/null +++ b/libc/src/stdio/printf_core/oct_converter.h @@ -0,0 +1,111 @@ +//===-- Octal 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_OCT_CONVERTER_H +#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_OCT_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 +#include + +namespace __llvm_libc { +namespace printf_core { + +int inline convert_oct(Writer *writer, const FormatSection &to_conv) { + // This is the number of digits it takes to represent a octal value of a + // certain number of bits. Each oct digit represents 3 bits, so the value is + // ceil(number of bits / 3). + constexpr size_t BUFF_LEN = ((sizeof(uintmax_t) * 8) + 2) / 3; + uintmax_t num = to_conv.conv_val_raw; + char buffer[BUFF_LEN]; + + num = apply_length_modifier(num, to_conv.length_modifier); + + // Since the buffer is size to sized to be able fit the entire number, buf_cur + // can never reach 0. So, we do not need bounds checking on buf_cur. + size_t buff_cur = BUFF_LEN; + for (; num > 0 /* && buff_cur > 0 */; --buff_cur, num /= 8) + buffer[buff_cur - 1] = (num % 8) + '0'; + + size_t num_digits = BUFF_LEN - buff_cur; + + // These are signed to prevent underflow due to negative values. Negative + // values are treated the same as 0. + int zeroes; + int spaces; + + // 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. For example: ("%04o", 15) -> "0017" + zeroes = to_conv.min_width - num_digits; + spaces = 0; + } else if (num_digits < 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: ("%3o", 0) -> " 0" + zeroes = 1; + spaces = to_conv.min_width - zeroes; + } 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 - num_digits; + } + } else { + // If precision was specified, possibly write zeroes, and possibly write + // spaces. Example: ("%5.4o", 010000) -> "10000" + // If the check for if zeroes is negative was not there, spaces would be + // incorrectly evaluated as 1. + zeroes = to_conv.precision - num_digits; // a negative value means 0 + if (zeroes < 0) + zeroes = 0; + spaces = to_conv.min_width - zeroes - num_digits; + } + + // The alternate form prefix is "0", so it's handled by increasing the number + // of zeroes if necessary. + if (((to_conv.flags & FormatFlags::ALTERNATE_FORM) == + FormatFlags::ALTERNATE_FORM) && + zeroes < 1) { + zeroes = 1; + --spaces; + } + + if ((to_conv.flags & FormatFlags::LEFT_JUSTIFIED) == + FormatFlags::LEFT_JUSTIFIED) { + // If left justified the pattern is zeroes digits spaces + if (zeroes > 0) + RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes)); + if (num_digits > 0) + RET_IF_RESULT_NEGATIVE(writer->write(buffer + buff_cur, num_digits)); + if (spaces > 0) + RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces)); + } else { + // Else the pattern is spaces zeroes digits + if (spaces > 0) + RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces)); + if (zeroes > 0) + RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes)); + if (num_digits > 0) + RET_IF_RESULT_NEGATIVE(writer->write(buffer + buff_cur, num_digits)); + } + return 0; +} + +} // namespace printf_core +} // namespace __llvm_libc + +#endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_OCT_CONVERTER_H diff --git a/libc/test/src/stdio/printf_core/converter_test.cpp b/libc/test/src/stdio/printf_core/converter_test.cpp index d7c292394cb0..62a7356ec787 100644 --- a/libc/test/src/stdio/printf_core/converter_test.cpp +++ b/libc/test/src/stdio/printf_core/converter_test.cpp @@ -196,9 +196,6 @@ TEST_F(LlvmLibcPrintfConverterTest, IntConversionSimple) { 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); @@ -206,18 +203,17 @@ TEST(LlvmLibcPrintfConverterTest, HexConversion) { reinterpret_cast(&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); + __llvm_libc::printf_core::FormatSection section; + section.has_conv = true; + section.raw_string = "%#018x"; + section.raw_len = 6; + section.conv_name = 'x'; + section.flags = static_cast<__llvm_libc::printf_core::FormatFlags>( + __llvm_libc::printf_core::FormatFlags::ALTERNATE_FORM | + __llvm_libc::printf_core::FormatFlags::LEADING_ZEROES); + section.min_width = 18; + section.conv_val_raw = 0x123456ab; + __llvm_libc::printf_core::convert(&writer, section); str_writer.terminate(); ASSERT_STREQ(str, "0x00000000123456ab"); @@ -243,3 +239,23 @@ TEST(LlvmLibcPrintfConverterTest, PointerConversion) { ASSERT_STREQ(str, "0x123456ab"); ASSERT_EQ(writer.get_chars_written(), 10); } + +TEST(LlvmLibcPrintfConverterTest, OctConversion) { + char str[20]; + __llvm_libc::printf_core::StringWriter str_writer(str); + __llvm_libc::printf_core::Writer writer( + reinterpret_cast(&str_writer), + __llvm_libc::printf_core::write_to_string); + + __llvm_libc::printf_core::FormatSection section; + section.has_conv = true; + section.raw_string = "%o"; + section.raw_len = 2; + section.conv_name = 'o'; + section.conv_val_raw = 01234; + __llvm_libc::printf_core::convert(&writer, section); + + str_writer.terminate(); + ASSERT_STREQ(str, "1234"); + ASSERT_EQ(writer.get_chars_written(), 4); +} diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp index 4411216a1544..4c8e24e95329 100644 --- a/libc/test/src/stdio/sprintf_test.cpp +++ b/libc/test/src/stdio/sprintf_test.cpp @@ -362,6 +362,127 @@ TEST(LlvmLibcSPrintfTest, PointerConv) { EXPECT_GT(written, 0); } +TEST(LlvmLibcSPrintfTest, OctConv) { + char buff[64]; + int written; + + // Basic Tests. + + written = __llvm_libc::sprintf(buff, "%o", 01234); + EXPECT_EQ(written, 4); + ASSERT_STREQ(buff, "1234"); + + written = __llvm_libc::sprintf(buff, "%o", 04567); + EXPECT_EQ(written, 4); + ASSERT_STREQ(buff, "4567"); + + // Length Modifier Tests. + + written = __llvm_libc::sprintf(buff, "%hho", 0401); + EXPECT_EQ(written, 1); + ASSERT_STREQ(buff, "1"); + + written = __llvm_libc::sprintf(buff, "%llo", 01777777777777777777777ull); + EXPECT_EQ(written, 22); + ASSERT_STREQ(buff, "1777777777777777777777"); // ull max + + written = __llvm_libc::sprintf(buff, "%to", ~ptrdiff_t(0)); + if (sizeof(ptrdiff_t) == 8) { + EXPECT_EQ(written, 22); + ASSERT_STREQ(buff, "1777777777777777777777"); + } else if (sizeof(ptrdiff_t) == 4) { + EXPECT_EQ(written, 11); + ASSERT_STREQ(buff, "37777777777"); + } + + // Min Width Tests. + + written = __llvm_libc::sprintf(buff, "%4o", 0701); + EXPECT_EQ(written, 4); + ASSERT_STREQ(buff, " 701"); + + written = __llvm_libc::sprintf(buff, "%2o", 0107); + EXPECT_EQ(written, 3); + ASSERT_STREQ(buff, "107"); + + // Precision Tests. + + written = __llvm_libc::sprintf(buff, "%o", 0); + EXPECT_EQ(written, 1); + ASSERT_STREQ(buff, "0"); + + written = __llvm_libc::sprintf(buff, "%.0o", 0); + EXPECT_EQ(written, 0); + ASSERT_STREQ(buff, ""); + + written = __llvm_libc::sprintf(buff, "%.5o", 0153); + EXPECT_EQ(written, 5); + ASSERT_STREQ(buff, "00153"); + + written = __llvm_libc::sprintf(buff, "%.2o", 0135); + EXPECT_EQ(written, 3); + ASSERT_STREQ(buff, "135"); + + // Flag Tests. + + written = __llvm_libc::sprintf(buff, "%-5o", 0246); + EXPECT_EQ(written, 5); + ASSERT_STREQ(buff, "246 "); + + written = __llvm_libc::sprintf(buff, "%#o", 0234); + EXPECT_EQ(written, 4); + ASSERT_STREQ(buff, "0234"); + + written = __llvm_libc::sprintf(buff, "%05o", 0470); + EXPECT_EQ(written, 5); + ASSERT_STREQ(buff, "00470"); + + written = __llvm_libc::sprintf(buff, "%0#6o", 0753); + EXPECT_EQ(written, 6); + ASSERT_STREQ(buff, "000753"); + + written = __llvm_libc::sprintf(buff, "%-#6o", 0642); + EXPECT_EQ(written, 6); + ASSERT_STREQ(buff, "0642 "); + + // Combined Tests. + + written = __llvm_libc::sprintf(buff, "%#-07o", 0703); + EXPECT_EQ(written, 7); + ASSERT_STREQ(buff, "0703 "); + + written = __llvm_libc::sprintf(buff, "%7.5o", 0314); + EXPECT_EQ(written, 7); + ASSERT_STREQ(buff, " 00314"); + + written = __llvm_libc::sprintf(buff, "%#9.5o", 0234); + EXPECT_EQ(written, 9); + ASSERT_STREQ(buff, " 00234"); + + written = __llvm_libc::sprintf(buff, "%-7.5o", 0260); + EXPECT_EQ(written, 7); + ASSERT_STREQ(buff, "00260 "); + + written = __llvm_libc::sprintf(buff, "%5.4o", 010000); + EXPECT_EQ(written, 5); + ASSERT_STREQ(buff, "10000"); + + // Multiple Conversion Tests. + + written = __llvm_libc::sprintf(buff, "%10o %-#10o", 0456, 0123); + EXPECT_EQ(written, 21); + ASSERT_STREQ(buff, " 456 0123 "); + + written = __llvm_libc::sprintf(buff, "%-5.4o%#.4o", 075, 025); + EXPECT_EQ(written, 9); + ASSERT_STREQ(buff, "0075 0025"); + + written = __llvm_libc::sprintf(buff, "%04hho %#.5llo %-6.3zo", 256 + 077, + 01000000000000ll, size_t(2)); + EXPECT_EQ(written, 26); + ASSERT_STREQ(buff, "0077 01000000000000 002 "); +} + #ifndef LLVM_LIBC_PRINTF_DISABLE_INDEX_MODE TEST(LlvmLibcSPrintfTest, IndexModeParsing) { char buff[64];