seq, printf: avoid integer overflow on extreme precision/exponent#13246
Open
sylvestre wants to merge 2 commits into
Open
seq, printf: avoid integer overflow on extreme precision/exponent#13246sylvestre wants to merge 2 commits into
sylvestre wants to merge 2 commits into
Conversation
Merging this PR will improve performance by 4.89%
Performance Changes
Tip Curious why this is faster? Comment Comparing 
Footnotes
|
|
GNU testsuite comparison: |
There was a problem hiding this comment.
Pull request overview
This PR hardens seq/printf numeric formatting against arithmetic overflows triggered by extreme user-controlled precision and decimal exponents, addressing the panic/wrap scenarios reported in #13221.
Changes:
- Centralize and apply a precision cap (
i32::MAX) via a newcheck_precisionhelper across spec parsing and float formatter construction. - Reject floating-point inputs whose decimal exponent/scale is outside
i32before performing exponent/precision math in the float formatters. - Prevent overflow in
seq -wpadding computation via saturating arithmetic, and add regression tests for the overflow cases.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/by-util/test_seq.rs | Adds regression tests for precision overflow rejection, exponent overflow rejection, and -w overflow avoidance. |
| tests/by-util/test_printf.rs | Adds regression tests ensuring extreme exponents fail cleanly across %a/%e/%g/%f. |
| src/uucore/src/lib/features/format/spec.rs | Replaces duplicated precision-limit checks with the shared check_precision helper. |
| src/uucore/src/lib/features/format/num_format.rs | Adds exponent-range rejection for floats and adjusts hex-float computations to avoid overflow. |
| src/uucore/src/lib/features/format/mod.rs | Introduces check_precision helper and unit tests for its behavior. |
| src/uu/seq/src/seq.rs | Uses saturating arithmetic in equal-width padding to avoid overflow on extreme digit counts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+153
to
+159
| pub(crate) fn check_precision(precision: usize) -> Result<(), FormatError> { | ||
| if precision as u64 > i32::MAX as u64 { | ||
| Err(FormatError::InvalidPrecision(precision.to_string())) | ||
| } else { | ||
| Ok(()) | ||
| } | ||
| } |
cakebaker
reviewed
Jul 3, 2026
Comment on lines
+62
to
+69
| fn test_equal_width_huge_exponent_does_not_overflow() { | ||
| // Equal-width padding adds the integral-digit count to the precision; a value | ||
| // with an astronomically large exponent must saturate instead of overflowing. | ||
| new_ucmd!() | ||
| .args(&["-w", "1e9223372036854775807", "1e-9223372036854775807", "1"]) | ||
| .succeeds() | ||
| .no_stdout(); | ||
| } |
Contributor
There was a problem hiding this comment.
I think this test is incorrect because GNU seq fails:
$ seq -w 1e9223372036854775807 1e-9223372036854775807 1
seq: invalid floating point argument: ‘1e9223372036854775807’
Try 'seq --help' for more information.
$ echo $?
1
cakebaker
reviewed
Jul 3, 2026
Comment on lines
+810
to
+814
| new_ucmd!() | ||
| .args(&[spec, "5e8123456789012345678"]) | ||
| .fails_with_code(1) | ||
| .no_stdout() | ||
| .stderr_contains("number too large to format"); |
Contributor
There was a problem hiding this comment.
GNU printf prints inf to stdout in all four cases.
cakebaker
reviewed
Jul 3, 2026
Comment on lines
+816
to
+820
| new_ucmd!() | ||
| .args(&[spec, "7E-8123456789012345678"]) | ||
| .fails_with_code(1) | ||
| .no_stdout() | ||
| .stderr_contains("number too large to format"); |
Contributor
There was a problem hiding this comment.
GNU printf prints 0 in different formats on stdout.
Cap seq's float precision at i32::MAX (as printf already does) and reject values whose decimal exponent does not fit in i32 before the float formatters do exponent math, preventing the panic/wrap reported in uutils#13221.
…havior The test asserted parsing succeeds for an exponent of exactly i64::MAX, but the parser (correctly, matching GNU seq) already rejects it -- the test's own expectation was wrong, not the parsing logic.
This file contains hidden or 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
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.
Cap seq's float precision at i32::MAX (as printf already does) and reject values whose decimal exponent does not fit in i32 before the float formatters do exponent math, preventing the panic/wrap reported in #13221.