Fix handling of self references in mark-compact GC, imrpove tests #2721
Conversation
This file contains 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
|
This PR does not affect the produced WebAssembly code. |
ulan
approved these changes
Aug 12, 2021
osa1
commented
Aug 12, 2021
| @@ -211,7 +211,7 @@ unsafe fn update_refs<SetHp: Fn(u32)>(set_hp: SetHp, heap_base: u32) { | |||
| /// Thread forwards pointers in object | |||
| unsafe fn thread_fwd_pointers(obj: *mut Obj, heap_base: u32) { | |||
| visit_pointer_fields(obj, obj.tag(), heap_base as usize, |field_addr| { | |||
| if (*field_addr).unskew() > field_addr as usize { | |||
| if (*field_addr).unskew() > obj as usize { | |||
There was a problem hiding this comment.
Refactored for consistency with the comparison above
crusso
approved these changes
Aug 12, 2021
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.
One line fix to thread self references during marking in mark-compact GC.
We had a test with self references but it accidentally passed. The problem was
none of the objects below the object with self reference were dead, so the
object was not moved, and as a result the field with the self reference had the
correct value even though we didn't update it.
The test is updated to add a dead object before the object with self ref. It
fails without the fix.
Test code is also updated to use a vector for the heap description, instead of
a map. This allows specifying object order in heap. With the previous HashMap
based implementation there is no way to reason about the object orders, so for
example the test with a backward pointer might or might not have a backward
pointer, depending on the HashMap (and hasher) implementation. With the vector
representation allocation is done in the same order as the test description,
allowing testing backward and forward pointers.
Thanks to @ulan for the catching the bug.