types: protect values under stringification from callbacks

[darekhta]

The stringification code relies on its argument staying alive and unchanged while it runs, but a tostring callback executes arbitrary script code. When the callback triggers a collection, the mark phase stops at the container already marked for cycle detection, its children get freed while the formatter still iterates them and the sweep clears the mark, defeating the cycle detection for the rest of the traversal. When the callback unlinks the value from its parent and throws, the reference held by the call frame is dropped during unwinding and the formatter continues with freed memory. When it deletes the next key of an object being formatted, the iteration visits a freed hash table entry.

Push a reference to each array, object or resource being stringified onto the VM stack to keep it reachable and alive across callbacks. Mark containers constant for the duration of their traversal so that callbacks cannot add, remove or replace their entries, restoring the original constness afterwards, and register them in a chain of stack frames in uc_thread_context_t. The collector clears the cycle detection marks of the registered containers before its mark phase and restores them after the sweep, so cycle detection keeps using the mark bit at constant cost per value. Formatting without a VM registers nothing since no callbacks can run, and once exit() has been called no further
callbacks are invoked, since the stack reset performed on exit also discards the formatter's slot.

Add regressions for explicit and automatic collection, arrays and objects, reentrant formatting with cycles and shared values, rejected mutations of containers under stringification, converter exceptions, exit() in a converter, converters modifying their own value, formatting of constant values and collection of unreachable cycles in a callback.

Fixes ucode -p 'let a; a = [ proto({}, { tostring: () => { gc(); return "A"; } }), { b: 7 } ]; a' crashing.

[jow-]

That feels too heavy. The gc will only destroy unreachable islands, I think it might be enough to both push a ref to the container under stringification onto the stack (keeps it reachable) and mark it constant (avoids deletions/other mutations during stringification). Then, after tostring(), restore the original constness (value might've been constant already so do not unconditionally revert), then pop the value ref from stack & put it

[darekhta]

Tried your variant first: pushing the container onto the stack and constifying it works for the mutation cases, but the gc-in-tostring cases still crash. The culprit is the cycle detection mark: ucv_gc_mark() stops at the marked container and its not-yet-formatted children get swept, even though the container itself is reachable (in print(values) it already sits on the stack as the argument).

So I kept your approach and only added two small loops to the collector: clear the marks of the containers currently being stringified before the mark phase, restore them after the sweep. Containers are tracked through a chain of C stack frames hanging off uc_thread_context_t. I also tried detecting cycles via the constness instead of the mark bit to avoid touching the collector at all, but that needs an ancestor walk and goes quadratic on back references, so I dropped it.

One deliberate deviation: the container is made constant for the traversal of its children, not during its own tostring(), so a converter can still cache into this while modifying a container being traversed raises "value is immutable". That also covers deleting keys of the object being printed, which crashed before.

[jow-]

Ah right, because tostring's cycle detection and gc's reachability walk both share the sole mark bit. Let me deliberate for a while to figure out whether your proposal is the best possible solution - but thank you so far for trying my suggestion.

[jow-]

Counter-proposal in #488.

It fixes the same bug with a different mechanism: instead of threading a per-container frame through the C stack and hooking it into the collector, it gives the collector a dedicated gc_mark bit (lending the top bit of the refcount field) and makes the stringification path self-contained — rooting the value on the VM value stack for the traversal and const-ifying the container while it is walked.

Consequences:

• The collector stays completely unaware of the formatting state: no GC hook, no per-collection walk, and nothing to unwind at each formatting exit point (including an exit() in a converter).
• The value is kept alive for the entire traversal, including re-entrant formatting, not only across the direct converter dispatch.
• Cost is one bit of refcount headroom (26 → 25 bits; only a refcount over 33 million would ever be affected), and the header keeps type/mark/ext_flag in their exact positions on both endiannesses.

The reproducer from the description is fixed and the full custom suite passes, plus a new regression test (tests/custom/99_bugs/59_format_gc_live_values) covering explicit/automatic collection, re-entrant cycles, rejected mutation, converter exceptions, and exit() in a converter — all green under ASan.

Happy to discuss the trade-offs; the two approaches put the complexity in quite different places (GC-side bookkeeping vs. a bit + a self-contained formatter).

[darekhta]

Superseded by #488.