[libc-commits] [libc] a21fc4c - [libc] Fix printf handling of INT_MIN width (#101729)

[via libc-commits]

Author: Michael Jones
Date: 2024-08-02T13:45:54-07:00
New Revision: a21fc4c0a681000c23089301e8935f579517b96a

URL: https://github.com/llvm/llvm-project/commit/a21fc4c0a681000c23089301e8935f579517b96a
DIFF: https://github.com/llvm/llvm-project/commit/a21fc4c0a681000c23089301e8935f579517b96a.diff

LOG: [libc] Fix printf handling of INT_MIN width (#101729)

Prevously, if INT_MIN was passed as a wildcard width to a printf
conversion the parser would attempt to negate it to get the positive
width (and set the left justify flag), but it would underflow and the
width would be treated as 0. This patch corrects the issue by instead
treating a width of INT_MIN as identical to -INT_MAX.

Also includes docs changes to explain this behavior and adding b to the
list of int conversions.

Added: 
    

Modified: 
    libc/docs/dev/printf_behavior.rst
    libc/src/stdio/printf_core/parser.h
    libc/test/src/stdio/printf_core/parser_test.cpp
    libc/test/src/stdio/snprintf_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/docs/dev/printf_behavior.rst b/libc/docs/dev/printf_behavior.rst
index c8b8ad45e987d..40e07562f38ca 100644
--- a/libc/docs/dev/printf_behavior.rst
+++ b/libc/docs/dev/printf_behavior.rst
@@ -160,7 +160,7 @@ If a conversion specification is provided an invalid type modifier, that type
 modifier will be ignored, and the default type for that conversion will be used.
 In the case of the length modifier "L" and integer conversions, it will be
 treated as if it was "ll" (lowercase LL). For this purpose the list of integer
-conversions is d, i, u, o, x, X, n.
+conversions is d, i, u, o, x, X, b, B, n.
 
 If a conversion specification ending in % has any options that consume arguments
 (e.g. "%*.*%") those arguments will be consumed as normal, but their values will
@@ -169,9 +169,13 @@ be ignored.
 If a conversion specification ends in a null byte ('\0') then it shall be
 treated as an invalid conversion followed by a null byte.
 
-If a number passed as a min width or precision value is out of range for an int,
-then it will be treated as the largest or smallest value in the int range
-(e.g. "%-999999999999.999999999999s" is the same as "%-2147483648.2147483647s").
+If a number passed as a field width or precision value is out of range for an
+int, then it will be treated as the largest value in the int range
+(e.g. "%-999999999999.999999999999s" is the same as "%-2147483647.2147483647s").
+
+If the field width is set to INT_MIN by using the '*' form, 
+e.g. printf("%*d", INT_MIN, 1), it will be treated as INT_MAX, since -INT_MIN is
+not representable as an int.
 
 If a number passed as a bit width is less than or equal to zero, the conversion
 is considered invalid. If the provided bit width is larger than the width of

diff  --git a/libc/src/stdio/printf_core/parser.h b/libc/src/stdio/printf_core/parser.h
index 207affd83f65b..2bb4be0feaa2a 100644
--- a/libc/src/stdio/printf_core/parser.h
+++ b/libc/src/stdio/printf_core/parser.h
@@ -129,7 +129,8 @@ template <typename ArgProvider> class Parser {
         cur_pos = cur_pos + result.parsed_len;
       }
       if (section.min_width < 0) {
-        section.min_width = -section.min_width;
+        section.min_width =
+            (section.min_width == INT_MIN) ? INT_MAX : -section.min_width;
         section.flags = static_cast<FormatFlags>(section.flags |
                                                  FormatFlags::LEFT_JUSTIFIED);
       }

diff  --git a/libc/test/src/stdio/printf_core/parser_test.cpp b/libc/test/src/stdio/printf_core/parser_test.cpp
index 66d6dd0a86c42..c277b304faea9 100644
--- a/libc/test/src/stdio/printf_core/parser_test.cpp
+++ b/libc/test/src/stdio/printf_core/parser_test.cpp
@@ -316,6 +316,47 @@ TEST(LlvmLibcPrintfParserTest, EvalThreeArgs) {
   ASSERT_PFORMAT_EQ(expected2, format_arr[2]);
 }
 
+TEST(LlvmLibcPrintfParserTest, EvalOneArgWithOverflowingWidthAndPrecision) {
+  LIBC_NAMESPACE::printf_core::FormatSection format_arr[10];
+  const char *str = "%-999999999999.999999999999d";
+  int arg1 = 12345;
+  evaluate(format_arr, str, arg1);
+
+  LIBC_NAMESPACE::printf_core::FormatSection expected;
+  expected.has_conv = true;
+
+  expected.raw_string = {str, 28};
+  expected.flags = LIBC_NAMESPACE::printf_core::FormatFlags::LEFT_JUSTIFIED;
+  expected.min_width = INT_MAX;
+  expected.precision = INT_MAX;
+  expected.conv_val_raw = arg1;
+  expected.conv_name = 'd';
+
+  ASSERT_PFORMAT_EQ(expected, format_arr[0]);
+}
+
+TEST(LlvmLibcPrintfParserTest,
+     EvalOneArgWithOverflowingWidthAndPrecisionAsArgs) {
+  LIBC_NAMESPACE::printf_core::FormatSection format_arr[10];
+  const char *str = "%*.*d";
+  int arg1 = INT_MIN; // INT_MIN = -2147483648 if int is 32 bits.
+  int arg2 = INT_MIN;
+  int arg3 = 12345;
+  evaluate(format_arr, str, arg1, arg2, arg3);
+
+  LIBC_NAMESPACE::printf_core::FormatSection expected;
+  expected.has_conv = true;
+
+  expected.raw_string = {str, 5};
+  expected.flags = LIBC_NAMESPACE::printf_core::FormatFlags::LEFT_JUSTIFIED;
+  expected.min_width = INT_MAX;
+  expected.precision = arg2;
+  expected.conv_val_raw = arg3;
+  expected.conv_name = 'd';
+
+  ASSERT_PFORMAT_EQ(expected, format_arr[0]);
+}
+
 #ifndef LIBC_COPT_PRINTF_DISABLE_INDEX_MODE
 
 TEST(LlvmLibcPrintfParserTest, IndexModeOneArg) {

diff  --git a/libc/test/src/stdio/snprintf_test.cpp b/libc/test/src/stdio/snprintf_test.cpp
index d898f2b80a86f..baaa664cdc9ee 100644
--- a/libc/test/src/stdio/snprintf_test.cpp
+++ b/libc/test/src/stdio/snprintf_test.cpp
@@ -41,6 +41,9 @@ TEST(LlvmLibcSNPrintfTest, CutOff) {
   // passing null as the output pointer is allowed as long as buffsz is 0.
   written = LIBC_NAMESPACE::snprintf(nullptr, 0, "%s and more", "1234567890");
   EXPECT_EQ(written, 19);
+
+  written = LIBC_NAMESPACE::snprintf(nullptr, 0, "%*s", INT_MIN, "nothing");
+  EXPECT_EQ(written, INT_MAX);
 }
 
 TEST(LlvmLibcSNPrintfTest, NoCutOff) {