detect uint64 overflow that wraps past min_safe in parse_int_string#393
Open
sahvx655-wq wants to merge 1 commit into
Open
detect uint64 overflow that wraps past min_safe in parse_int_string#393sahvx655-wq wants to merge 1 commit into
sahvx655-wq wants to merge 1 commit into
Conversation
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.
parse_int_stringonly confirms auint64_toverflow at themax_digits_u64(base)boundary by comparing the wrapped accumulator againstmin_safe_u64(base). I hit this while differentially fuzzing the integer path againststd::from_chars: in the default base 10, an input like30000000000000000000comes back as a successful parse holding a truncated11553255926290448384rather thanresult_out_of_range. Themin_safetest assumes the value wraps at most once, but a 20-digit base-10 number reaches about 5.4x 2^64, so the accumulator can wrap a full multiple of 2^64 and land back abovemin_safeand slip through. Every base whose max-length range exceeds 2^64 is affected (most non-power-of-two bases), and onlyuint64_t, since narrower types are still caught by the trailing range check; the practical risk is a caller seeingec == successand trusting a silently wrapped value.At
digit_count == max_digitsthe wrapped accumulator no longer holds enough information to decide, so the boundary is re-run over the parsed digits with a checked multiply-add and rejected as soon as it would pass 2^64. The cheapmin_safecomparison stays in front as a fast reject and the exact loop only fires at the digit-count boundary, so the hot path is unchanged. The regression test sits beside the existing out-of-range base cases intests/fast_int.cppand covers bases 3 to 36 (2, 4 and 16 already fit in a single 2^64 span); it fails before this change and passes after.