[GC] id from different slab #10397
Comments
vouillon
added
the
bug
|
Thanks for filing this bug! |
|
Minimized a bit further: (module
(type $func (func))
(type $array (array (mut i32)))
(type $struct (sub (struct (field $field (ref $func)))))
(elem func $nop)
(func $nop)
(func (export "")
(local $local_array (ref $array))
(local $local_struct (ref $struct))
(local.set $local_struct (struct.new $struct (ref.func $nop)))
(loop $outer
(local.set $local_array (array.new $array (i32.const 0) (i32.const 1)))
(loop $inner
(array.set $array (ref.cast (ref $array) (local.get $local_array))
(i32.const 0)
(i32.const 1))
(br_if $inner (i32.const 0))
)
(call_ref $func (struct.get $struct $field (local.get $local_struct)))
(br $outer)
)
)
)There are a couple interesting things that can't be removed from the test case while preserving the crash:
|
We were previously propagating the needs-inclusion-in-stack-maps bit from a variable to the values resulting from `builder.use_var(my_var)` and passed to `builder.def_var(my_var, my_val)`. However, that does not cover every `ir::Value` generated for a given `ir::Variable`: it is missing block parameters that the SSA builder inserted and which are *not* then used directly as the result of `use_var`, but instead passed as arguments another block, and that block's parameter is then directly or indirectly used as the result of `builder.use_var(...)`. This chaining can go arbitrarily deep, based on the program's control flow. By not marking these block parameters as needing inclusion in stack maps, our stack maps would be incomplete, which meant that the GC could too-eagerly collect objects, which meant that we would then get use-after-free bugs. The solution is for the `SSABuilder` to report all the block parameters that it inserted, and for which `ir::Variable` it inserted them. We already have a summary of the mutations that an `SSABuilder` performed, `SideEffects`, and which must be handled by the `FunctionBuilder` after, e.g., sealing a block, so we add this block-param-and-var information to it. Now, after the `FunctionBuilder` has the `SSABuilder` do its thing, it handles fixing up not only the modified blocks' status, which it was doing before, but also propagates the needs-inclusion-in-stack-maps metadata from variables to all those freshly-inserted block parameter values. After this change, we successfully include *all* of a needs-inclusion-in-stack-maps variable's values in stack maps, which avoids the over-eager object reclamation and associated GC heap corruption. Fixes bytecodealliance#10397
|
I have a fix for this over in #10456 Thanks again for the bug report! |
|
Thanks a lot! |
|
Yeah, we sandbox the GC heap, so when we have any kind of GC heap corruption bug, it tends to look like those panics rather than overwriting host memory or what have you. Occasionally it might look like a SIGILL at a Anyways, the point is, it is likely not the same bug if it still triggers after #10456, but just that all kinds of GC heap corruption tend to show the same symptoms. But as always, reduced test cases are super helpful, thanks for taking the time to make reproducers and all that! |
We were previously propagating the needs-inclusion-in-stack-maps bit from a variable to the values resulting from `builder.use_var(my_var)` and passed to `builder.def_var(my_var, my_val)`. However, that does not cover every `ir::Value` generated for a given `ir::Variable`: it is missing block parameters that the SSA builder inserted and which are *not* then used directly as the result of `use_var`, but instead passed as arguments another block, and that block's parameter is then directly or indirectly used as the result of `builder.use_var(...)`. This chaining can go arbitrarily deep, based on the program's control flow. By not marking these block parameters as needing inclusion in stack maps, our stack maps would be incomplete, which meant that the GC could too-eagerly collect objects, which meant that we would then get use-after-free bugs. The solution is for the `SSABuilder` to report all the block parameters that it inserted, and for which `ir::Variable` it inserted them. We already have a summary of the mutations that an `SSABuilder` performed, `SideEffects`, and which must be handled by the `FunctionBuilder` after, e.g., sealing a block, so we add this block-param-and-var information to it. Now, after the `FunctionBuilder` has the `SSABuilder` do its thing, it handles fixing up not only the modified blocks' status, which it was doing before, but also propagates the needs-inclusion-in-stack-maps metadata from variables to all those freshly-inserted block parameter values. After this change, we successfully include *all* of a needs-inclusion-in-stack-maps variable's values in stack maps, which avoids the over-eager object reclamation and associated GC heap corruption. Fixes #10397
Test Case
slab-mismatch.zip
Steps to Reproduce
Run the following command:
Expected Results
It should not crash.
Actual Results
We get a panic:
Versions and Environment
Wasmtime commit: a3381e4
Operating system: Linux
Architecture: x64
The text was updated successfully, but these errors were encountered: