Skip to content

fix out-of-range float to int conversion in to_nonnegative_int#4802

Merged
vitaut merged 1 commit into
fmtlib:mainfrom
aizu-m:chrono-nonnegative-int-float-cast
Jun 17, 2026
Merged

fix out-of-range float to int conversion in to_nonnegative_int#4802
vitaut merged 1 commit into
fmtlib:mainfrom
aizu-m:chrono-nonnegative-int-float-cast

Conversation

@aizu-m

@aizu-m aizu-m commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

The floating-point overload of detail::to_nonnegative_int casts value to Int before range-checking it, so the cast itself runs on out-of-range values.

Repro (header-only, clang -std=c++17 -fsanitize=undefined):

fmt::format("{:%j}", std::chrono::duration<double>(1e300));
include/fmt/chrono.h:937:37: runtime error: 1.15741e+295 is outside the range of representable values of type 'int'

Path: on_day_of_year -> write(days(), ...) -> to_nonnegative_int(days(), INT_MAX). Observation: static_cast<Int>(value) is evaluated first and the range check only sees the already-converted value, so the behaviour is undefined before the throw can happen.

Fix: do the range check first, comparing against static_cast<T>(upper) + 1. The + 1 matters at the float boundary: static_cast<float>(INT_MAX) rounds up to 2^31, so value > static_cast<T>(upper) would let a value equal to 2^31 through and the cast would still overflow. With >= upper + 1 both the comparison and the conversion are well defined.

Found while running the chrono formatters under UBSan. Added a regression test to chrono_test.out_of_range; all chrono tests pass.

@aizu-m aizu-m requested a review from vitaut as a code owner June 10, 2026 13:08
@vitaut

vitaut commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Please don't submit AI slop or you will be banned.

@vitaut vitaut closed this Jun 10, 2026
@aizu-m

aizu-m commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

The description was my mistake — I pasted the writeup from a different patch entirely. I've fixed the PR body to describe the actual change.

The bug itself is real: fmt::format("{:%j}", std::chrono::duration<double>(1e300)) evaluates static_cast<int>(value) before the range check, and UBSan flags float-cast-overflow at chrono.h:937. Repro and trace are in the updated description. Happy to resubmit clean if you'd rather take it that way.

@vitaut vitaut reopened this Jun 11, 2026
@vitaut

vitaut commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Makes sense now, reopened.

@vitaut vitaut force-pushed the chrono-nonnegative-int-float-cast branch from 7bd6c1b to 4b3276c Compare June 17, 2026 05:53

@vitaut vitaut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vitaut vitaut merged commit de4c6c5 into fmtlib:main Jun 17, 2026
46 checks passed
@vitaut

vitaut commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants