{{ message }}
Fix UBSan error in parseReadableSize for values near UInt64 max#101097
Merged
Conversation
Move `std::ceil` before the overflow range check so that values rounded up beyond `std::numeric_limits<UInt64>::max()` are caught and throw an exception instead of causing undefined behavior in the cast. CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=101042&sha=6bce0d39d7fb522ace5a37cb78c047754d7f0a09&name_0=PR&name_1=Unit%20tests%20%28asan_ubsan%29 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Also fix the overflow check to use a direct comparison against 2^64 as a Float64 literal, using >= instead of >. The previous > comparison against `std::numeric_limits<UInt64>::max()` was ineffective because UInt64 max (2^64-1) rounds up to 2^64 when implicitly converted to Float64, making `2^64 > 2^64` always false. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
18.4 EB = 18,400,000,000,000,000,000 which is less than UInt64 max
(18,446,744,073,709,551,615), so `parseReadableSize('18.4 EB')` should
succeed. Changed to test 18.5 EB for the overflow case instead.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nickitat
approved these changes
Mar 30, 2026
Member
Author
|
The Stress test (arm_msan) failure is fixed by #101239, which should be merged first. After it is merged, please update the branch to include the fix. |
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
Member
Author
|
The failures of "Flaky check" in "functions_bad_arguments" will be fixed by #101994. |
1 task
Contributor
|
Hi, can we merge it once the PR is green? |
Contributor
LLVM Coverage ReportChanged lines: 100.00% (8/8) · Uncovered code |
robot-clickhouse-ci-2
added
the
pr-synced-to-cloud
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
4 participants
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.
The
FunctionsStressunit test triggers a UBSan error inparseReadableSize:The overflow check compared
num_bytes_with_decimalsagainstUInt64max, butstd::ceilwas applied after the check. Due to floating-point precision, a value could pass the check but thenstd::ceilwould round it up beyondUInt64max, causing undefined behavior in the cast.Fix: apply
std::ceilbefore the range check.CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=101042&sha=6bce0d39d7fb522ace5a37cb78c047754d7f0a09&name_0=PR&name_1=Unit%20tests%20%28asan_ubsan%29
#101042
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
...
Documentation entry for user-facing changes
Version info
26.4.1.999