Skip to content

wasm-reduce: Fix integer overflows#8665

Merged
kripken merged 7 commits intoWebAssembly:mainfrom
kripken:reduce.big
May 4, 2026
Merged

wasm-reduce: Fix integer overflows#8665
kripken merged 7 commits intoWebAssembly:mainfrom
kripken:reduce.big

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented May 1, 2026

Use a larger size than int, and be careful when multiplying by 9.

cc @tlively , I noticed that reducing a huge file - perhaps of a similar size as the one you were looking at - hit an overflow and immediately went to factor=1, which was extremely slow.

@kripken kripken requested a review from a team as a code owner May 1, 2026 00:20
@kripken kripken requested review from stevenfontanella and removed request for a team May 1, 2026 00:20
Comment thread src/tools/wasm-reduce/wasm-reduce.cpp Outdated
std::unique_ptr<Builder> builder;
Index funcsSeen;
int factor;
size_t factor;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we just make it uint64_t / int64_t if we're worried about overflowing a 32-bit int? I guess some systems may have 4 bytes for size_t?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, I guess we could. I feel size_t reads a bit better, and a 32-bit system couldn't load a wasm file this large anyhow?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I usually prefer explicit sizes to not have to worry about differences in architecture, e.g. when reading I might wonder if the code works for 64-bit size_t but not for 32-bit size_t. I don't have a strong preference though, LGTM.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok, I'm convinced. Changed to uint64_t.

@kripken kripken merged commit 2868a88 into WebAssembly:main May 4, 2026
16 checks passed
@kripken kripken deleted the reduce.big branch May 4, 2026 16:42
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