[libc] Fix printf handling of INT_MIN width #101729
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
@llvm/pr-subscribers-libc Author: Michael Jones (michaelrj-google) ChangesPrevously, if INT_MIN was passed as a wildcard width to a printf Also includes docs changes to explain this behavior and adding b to the Full diff: https://github.com/llvm/llvm-project/pull/101729.diff 4 Files Affected:
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) {
|
lntue
approved these changes
Aug 2, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/3144 Here is the relevant piece of the build log for the reference: |
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
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.
kstoimenov
pushed a commit
to kstoimenov/llvm-project
that referenced
this pull request
Aug 15, 2024
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.