-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restrict offsets to index type's range in validation #79
Conversation
Yet again I have no idea what is wrong with the PDF build. |
Well, the error says it all: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good modulo <
Can you please add tests for this? Otherwise it's easy for engine to overlook this restriction and be like "we pass all tests, hence we fully implement memory64". |
Yes, I have a personal todo item to add spec tests for all the recent spec changes. I'll create a GitHub issue today to make that more visible for others though. Thanks for the reminder. |
Is this ready to land? |
Should be, assuming you're comfortable with tests coming later. |
This PR adds validation of
memarg.offset
to ensure that loads and stores for a 32-bit memory cannot have a 64-bit offset. Execution semantics are unaffected.Resolves #76.