Skip to content

Conversation

tlively
Copy link
Member

@tlively tlively commented Jun 19, 2024

For 32-bit memories, the offset value must be in the u32 range. Update
the address.wast spec test to assert that a module with an overlarge
offset value is invalid rather than malformed.

@tlively tlively requested a review from kripken June 19, 2024 01:33
@tlively tlively changed the title [threads] Shared basic heap types (#6667) Validate memarg offsets Jun 19, 2024
@tlively tlively force-pushed the spec-test-offset64 branch from 7ad6eac to 10b03c8 Compare June 19, 2024 01:37
Memory* mem,
Expression* curr) {
shouldBeTrue(
mem->is64() || offset < (1ull << 32), curr, "offset must be u32");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mem->is64() || offset < (1ull << 32), curr, "offset must be u32");
mem->is64() || offset <= std::limits<uint32_t>::max(), curr, "offset must be u32");
Copy link
Member

Choose a reason for hiding this comment

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

We use that pattern elsewhere. But if it leads to annoying issues with compiler warnings on signed comparisons, feel free to ignore.

@tlively tlively force-pushed the spec-test-offset64 branch from c64bbe5 to e3bc30c Compare June 19, 2024 02:28
Base automatically changed from spec-test-utf8 to main June 19, 2024 03:08
@tlively tlively force-pushed the spec-test-offset64 branch from e3bc30c to baa8b95 Compare June 20, 2024 01:10
tlively added 3 commits June 19, 2024 22:14
This will hopefully fix the build on the coverage builder.
For 32-bit memories, the offset value must be in the u32 range. Update the address.wast spec test to assert that a module with an overlarge offset value is invalid rather than malformed.
@tlively tlively force-pushed the spec-test-offset64 branch from baa8b95 to 9119575 Compare June 20, 2024 05:23
@tlively tlively merged commit 45f6bdd into main Jun 20, 2024
@tlively tlively deleted the spec-test-offset64 branch June 20, 2024 16:58
@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants