Skip to content

Conversation

@dschuff
Copy link
Member

@dschuff dschuff commented Nov 9, 2024

The test uses numbers such as 0x8000000000000000ull but was converting to int64_t and overflowing,
leading to UB and different behavior when using saturating conversions.

Use uint64 for tests including values unrepresentable by i64
@dschuff dschuff requested a review from sbc100 November 9, 2024 00:19
sbc100
sbc100 approved these changes Nov 9, 2024
@dschuff dschuff enabled auto-merge (squash) November 9, 2024 00:35
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm, but what is the specific UB fixed here?

@dschuff
Copy link
Member Author

dschuff commented Nov 11, 2024

The test uses numbers such as 0x8000000000000000ull but was converting to int64_t and overflowing,
leading to UB and different behavior when using saturating conversions.
I updated the commit description

@dschuff dschuff disabled auto-merge November 11, 2024 18:12
@dschuff dschuff merged commit 9e0121c into emscripten-core:main Nov 11, 2024
26 of 28 checks passed
uysalibov pushed a commit to uysalibov/emscripten that referenced this pull request Nov 15, 2024
The test uses numbers such as 0x8000000000000000ull but was converting
to int64_t and overflowing,
leading to UB and different behavior when using saturating conversions.
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.

3 participants