JIT: add missing memory_ensure_free in jit_bitstring_extract_float

[pguyot]

Following OP_BS_GET_FLOAT2 in opcodesswitch, reserve space for the boxed float before allocating it. Also fix OP_BS_GET_FLOAT2 to use live and memory_ensure_free_with_roots.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

[petermm]

AMP:

PR Review — JIT: add missing memory_ensure_free in jit_bitstring_extract_float

Commit: 79bbdc3282ebd1ca78118ad152e758efd63d8eea
Files: libs/jit/src/jit.erl, src/libAtomVM/jit.c, src/libAtomVM/jit.h, src/libAtomVM/opcodesswitch.h

Summary

The commit does two good things:

1. Interpreter (OP_BS_GET_FLOAT2): replaces the no-GC
   memory_ensure_free_opt(ctx, FLOAT_SIZE, MEMORY_NO_GC) with a proper
   TRIM_LIVE_REGS(live) + memory_ensure_free_with_roots(..., live, x_regs, MEMORY_CAN_SHRINK),
   so the boxed float allocation can trigger a GC that keeps live registers as roots.
   This is correct and matches the surrounding opcodes.

2. JIT (jit_bitstring_extract_float): the helper now receives the match-state
   pointer + live, advances the match-state offset itself, and reserves heap space
   for the boxed float before allocating it. Previously there was no heap reservation
   at all, so this fixes a real heap-overflow bug.

The interpreter side is solid. The JIT helper has one blocker and two minor issues.

---

🔴 Blocker — OOM is silently turned into a match failure (JIT path)

jit_bitstring_extract_float returns FALSE_ATOM for both a normal extraction
failure and a garbage-collection failure:

match_state_ptr[2] = (term) (offset + n);
if (UNLIKELY(memory_ensure_free_with_roots(ctx, FLOAT_SIZE, live, ctx->x, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
    return FALSE_ATOM;   // <-- OOM looks identical to "no match"
}
return term_from_float(value, &ctx->heap);

The JIT caller only distinguishes FALSE_ATOM and jumps to the fail label:

MSt8 = cond_jump_to_label({Result, '==', ?FALSE_ATOM}, Fail, MMod, MSt7),

So an out-of-memory condition does not raise OUT_OF_MEMORY_ATOM — it silently
jumps to the Fail label, which may match a different clause and produce a wrong
result instead of crashing. The interpreter, by contrast, raises:

if (UNLIKELY(memory_ensure_free_with_roots(...) != MEMORY_GC_OK)) {
    RAISE_ERROR(OUT_OF_MEMORY_ATOM);
}

This regresses behaviour relative to both the interpreter and the JIT integer
counterpart. jit_bitstring_extract_integer already signals OOM by returning the
invalid term 0, and its caller checks for it before the FALSE_ATOM check:

%% OP_BS_GET_INTEGER2
MSt10 = handle_error_if({Result, '==', 0}, MMod, MSt9),
MSt11 = cond_jump_to_label({Result, '==', ?FALSE_ATOM}, Fail, MMod, MSt10),

Suggested fix

Make the float path mirror the integer path: return an invalid term and stage the
exception on OOM, and have the caller run handle_error_if(Result == 0) first.

src/libAtomVM/jit.c:

     if (UNLIKELY(!status)) {
         return FALSE_ATOM;
     }
     match_state_ptr[2] = (term) (offset + n);
-    if (UNLIKELY(memory_ensure_free_with_roots(ctx, FLOAT_SIZE, live, ctx->x, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
-        return FALSE_ATOM;
+    TRIM_LIVE_REGS(live);
+    if (UNLIKELY(memory_ensure_free_with_roots(ctx, FLOAT_SIZE, live, ctx->x, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
+        context_set_exception_class(ctx, ERROR_ATOM);
+        ctx->exception_reason = OUT_OF_MEMORY_ATOM;
+        ctx->exception_stacktrace = term_invalid_term();
+        return term_invalid_term();
     }
     return term_from_float(value, &ctx->heap);

(This matches set_error(ctx, jit_state, 0, OUT_OF_MEMORY_ATOM); the helper has no
jit_state parameter, so the three fields are set inline exactly as set_error
does with offset == 0.)

libs/jit/src/jit.erl (OP_BS_GET_FLOAT2):

     {MSt7, Result} = MMod:call_primitive(MSt6, ?PRIM_BITSTRING_EXTRACT_FLOAT, [
         ctx, {free, MatchStateRegPtr}, {free, NumBits}, {free, FlagsValue}, Live
     ]),
-    MSt8 = cond_jump_to_label({Result, '==', ?FALSE_ATOM}, Fail, MMod, MSt7),
-    {MSt9, Dest, Rest7} = decode_dest(Rest6, MMod, MSt8),
+    MSt7b = handle_error_if({Result, '==', 0}, MMod, MSt7),
+    MSt8 = cond_jump_to_label({Result, '==', ?FALSE_ATOM}, Fail, MMod, MSt7b),
+    {MSt9, Dest, Rest7} = decode_dest(Rest6, MMod, MSt8),

---

🟡 Minor — missing TRIM_LIVE_REGS(live) before the GC in the helper

The interpreter clamps/trims live registers before the GC-capable allocation:

TRIM_LIVE_REGS(live);
memory_ensure_free_with_roots(ctx, FLOAT_SIZE, live, x_regs, MEMORY_CAN_SHRINK);

The JIT helper passes live straight to memory_ensure_free_with_roots without
trimming. If live > MAX_REG, live is used directly as the root count over
ctx->x (which holds MAX_REG entries), risking an out-of-bounds root scan, and
extended x-regs are not trimmed for consistency. TRIM_LIVE_REGS is already
available and used elsewhere in jit.c (e.g. around lines 657, 707, 715).

Fix is folded into the diff above (the added TRIM_LIVE_REGS(live); line).

---

🟡 Minor (pre-existing) — switch (n) dispatches on increment, not size

The helper dispatches the extractor on n, where the caller passes
NumBits = Size * Unit:

switch (n) {
    case 16: status = bitstring_extract_f16(match_state_ptr[1], offset, n, ...); break;
    case 32: ...
    case 64: ...
}

The interpreter instead dispatches on size_val (the field size) while passing
increment = size_val * unit as the bit count:

switch (size_val) {
    case 16: status = bitstring_extract_f16(bs_bin, bs_offset, increment, ...); break;
    ...
}

For the common case (unit == 1) n == size_val, so there is no divergence. They
only differ for unit != 1 float fields. This divergence predates this commit
(the JIT helper already switched on n beforehand), so it is out of scope here —
noting it only as a follow-up. If addressed, pass both size and increment to the
helper and dispatch on size.

---

🟢 OK — GC ordering / stale match_state_ptr

match_state_ptr is a raw pointer into the heap, which memory_ensure_free_with_roots
may move. The ordering here is safe: the helper reads the binary/offset and writes
the advanced offset (match_state_ptr[2] = offset + n) before the GC, and never
dereferences match_state_ptr afterwards. Keep this invariant — any future code that
needs the match state after the GC must reload it from a rooted term, not from this
raw pointer.

---

🟢 OK — signature / registration consistency

jit.h interface, the jit_bitstring_extract_float entry in the primitives table
(src/libAtomVM/jit.c), and ?PRIM_BITSTRING_EXTRACT_FLOAT (= 64) in
libs/jit/src/primitives.hrl are all consistent with the new signature.

---

Verdict

Interpreter change: ✅ correct.
JIT change: fix the OOM-as-match-failure blocker (and fold in TRIM_LIVE_REGS)
before merging. Add a JIT test that matches a float out of a bitstring under heap
pressure to exercise the GC path on the native backend.