Fix wrong iterator in InterpDropKeep ref tracking#2692
Open
sumleo wants to merge 1 commit intoWebAssembly:mainfrom
Open
Fix wrong iterator in InterpDropKeep ref tracking#2692sumleo wants to merge 1 commit intoWebAssembly:mainfrom
sumleo wants to merge 1 commit intoWebAssembly:mainfrom
Conversation
In the InterpDropKeep handler, the second loop ("find dropped refs")
checked *iter instead of *drop_iter in its break condition. Since
iter was not advanced in this loop, the condition was evaluated against
a stale value on every iteration, causing incorrect erasure of ref
tracking entries in the refs_ vector.
This could lead to reference-typed values losing their tracking in the
GC root set while still alive on the value stack, potentially allowing
premature collection during garbage collection.
Add a regression test that exercises drop=2/keep=1 with three
externref values on the stack.
548bcdb to
42fd0f4
Compare
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
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.
Summary
InterpDropKeephandler ininterp.cc, the second loop ("find dropped refs") checked*iterinstead of*drop_iterin its break condition, causing incorrect erasure of ref-tracking entries in therefs_vector.drop=2/keep=1with threeexternrefvalues on the stack.Details
The
InterpDropKeephandler has two loops overrefs_(reverse iterators):iter): shifts kept refs down bydroppositions.drop_iter): finds dropped refs to erase.The bug is in the second loop:
Since
iteris not advanced in this loop, the break condition is evaluated against a stale value on every iteration. This causes the loop to either break too early (erasing too few entries) or never break (erasing too many entries), depending on the stale*itervalue relative to the boundary.Fix: Change
*iterto*drop_iter.Test plan
InterpTest.DropKeepRefTrackingcreates a wasm module with 3 externref params, a block that pushes all three to the stack, and abrthat triggersdrop=2/keep=1