[libc-commits] [libc] [libc] Fix printf handling of INT_MIN width (PR #101729)
Michael Jones via libc-commits
libc-commits at lists.llvm.org
Fri Aug 2 11:24:25 PDT 2024
https://github.com/michaelrj-google created https://github.com/llvm/llvm-project/pull/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.
>From ee6950c6cc8b61ff7d4bbaebe80ccff8e9363294 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Fri, 2 Aug 2024 11:21:23 -0700
Subject: [PATCH] [libc] Fix printf handling of INT_MIN width
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.
---
libc/docs/dev/printf_behavior.rst | 12 ++++--
libc/src/stdio/printf_core/parser.h | 3 +-
.../src/stdio/printf_core/parser_test.cpp | 41 +++++++++++++++++++
libc/test/src/stdio/snprintf_test.cpp | 3 ++
4 files changed, 54 insertions(+), 5 deletions(-)
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) {
More information about the libc-commits
mailing list