Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add requireFunctionContext in necessary places #4388

Merged
merged 1 commit into from
Dec 14, 2021
Merged

Add requireFunctionContext in necessary places #4388

merged 1 commit into from
Dec 14, 2021

Conversation

kripken
Copy link
Member

@kripken kripken commented Dec 14, 2021

Fixes #4384

@kripken kripken mentioned this pull request Dec 14, 2021
Closed
tlively
tlively approved these changes Dec 14, 2021
View reviewed changes
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

What if we made requireFunctionContext into a mandatory getter for currFunction to make these bugs impossible in the future?

@kripken
Copy link
Member Author

kripken commented Dec 14, 2021

I think that might be a good idea. The overhead should not be too high.

@kripken kripken merged commit b1f6298 into main Dec 14, 2021
@kripken kripken deleted the fix-4384 branch December 14, 2021 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tlively tlively tlively approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Invalid memory address dereference in wasm::WasmBinaryBuilder::visitLet(wasm::Block*)
2 participants
@kripken @tlively