wasm: fix GC scanning of allocas #3230
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Scanning of allocas was entirely broken on WebAssembly. The code intended to do this was never run. There were also no tests. Looking into this further, I found that it is actually not really necessary to do that: the C stack can be scanned conservatively and in fact this was already done for goroutine stacks (because they live on the heap and are always referenced). It wasn't done for the system stack however. With these fixes, I believe code should be both faster *and* more correct. I found this in my work to get opaque pointers supported in LLVM 15, because the code that was never reached now finally got run and was actually quite buggy.
Merged
Member
|
I'll review this Monday. |
Member
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Scanning of allocas was entirely broken on WebAssembly. The code intended to do this was never run. There were also no tests.
Looking into this further, I found that it is actually not really necessary to do that: the C stack can be scanned conservatively and in fact this was already done for goroutine stacks (because they live on the heap and are always referenced). It wasn't done for the system stack however.
With these fixes, I believe code should be both faster and more correct.
I found this in my work to get opaque pointers supported in LLVM 15, because the code that was never reached now finally got run and was actually quite buggy.