wasm-ctor-eval: Do not emit invalid code after hitting a GC cycle#8188
Conversation
| ;; A circular reference using a non-nullable (but mutable) field. Unlike cases | ||
| ;; above, we cannot break up such cycles, and must give up. We should at least | ||
| ;; not error. |
There was a problem hiding this comment.
Does this test show that continuing after the invalid state is cleared works as expected?
There was a problem hiding this comment.
Yes, that is what the global.set does. That change is not updated, because we throw all the state away.
There was a problem hiding this comment.
But I would have expected to see changes from precomputation that happens after the invalid state is cleared. Or am I misunderstanding whether that is expected to be possible?
There was a problem hiding this comment.
We don't continue after we hit the cycle. In general, when we hit something we can't fully precompute, we halt. (Though in theory, later code could undo a cycle... but we don't bet on such luck.)
There was a problem hiding this comment.
I see. "losing any evalling work thus far" in the description made me think that there might be some later evalling work would not be lost.
We break up cycles during serialization of GC data, but we can't always do
so: if the cycle uses immutable and non-nullable fields, we can't write a null
first and close the loop later. Before this PR, we would error inside the
topological sort utility; instead, catch an error from there, and stop our
work.
Unfortunately, it is complicated to detect this situation ahead of time: we
only hit a problem on the specific type of cycle mentioned before, and we
do so while already modifying the module state. Rather than do a large
refactoring to handle this, just give up on the state in memory, clearing it,
losing any evalling work thus far. If this becomes important enough, we
can consider handling it more gracefully later.