Reject negative dynamic segment sizes in bit syntax matching

[pguyot]

A dynamic segment size comes from a register and can be negative. It was scaled by the segment unit before being validated, so the scaled value could wrap to a small size that passed the capacity check and matched, or move the match state offset before the start of the binary.

The JIT also untagged small integers with a logical shift, which turned any negative integer into a large positive one.

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]

PR Review: Reject negative dynamic segment sizes

Commit: b2de9a752325f6d30594d9e9b898bc82772de553
Recommendation: Request changes

The interpreter now rejects negative and over-capacity scaled sizes before changing the match offset. However, the equivalent JIT paths still multiply before establishing that the product is representable or fits in the source binary. A large positive size can therefore reproduce the same wraparound class this commit is intended to close. The typed JIT fast path also assumes that every value described by t_integer is an immediate small integer, so boxed negative sizes can bypass the intended check.

Findings

[P1] Validate JIT scaling before multiplying

Locations: libs/jit/src/jit.erl:1304-1323, 1343-1361, and 1472-1489

The new JIT check only proves that a register size is non-negative:

MSt5 = cond_jump_to_label({SizeReg, '<', 0}, Fail, MMod, MSt4),
MSt6 = MMod:mul(MSt5, SizeReg, Unit)

MMod:mul/3 is a native-width multiplication. The integer, float, and skip paths do not prove that SizeReg * Unit is representable or no larger than the remaining source capacity before performing it.

Concrete triggers include:

• On a 64-bit JIT, (1 bsl 58) * 64 wraps to zero. A bs_skip_bits2 segment can consequently succeed as a zero-length skip.
• On a 32-bit JIT, (1 bsl 26) * 64 wraps to zero for the same reason.
• On a 64-bit JIT, (1 bsl 26) * 64 is 2^32, but jit_bitstring_extract_integer/6 and jit_bitstring_extract_float/5 receive the bit count through an int parameter (src/libAtomVM/jit.c:1473-1481, 1517-1519). It can truncate to zero in the primitive while the JIT later advances the match offset by 2^32.

This can produce a false successful match or write an invalid match offset. Because the lowering is shared, this affects all JIT backends rather than only x86-64.

The fix should mirror bs_scaled_size: obtain the binary capacity and current offset first, reject an invalid offset, compare the unscaled size with (capacity - offset) / unit, and only then multiply. The extraction primitive's bit-count parameter should also use a non-truncating type (or the JIT must explicitly reject values beyond its supported range before calling it).

Suggested regression coverage:

diff --git a/tests/erlang_tests/test_bs.erl b/tests/erlang_tests/test_bs.erl
@@
 test_negative_dynamic_size() ->
     B = id(<<1>>),
@@
     nope = float_unit64(id(-1), id(<<1, 2, 3, 4, 5, 6, 7, 8>>)),
+    % Products that wrap at native width, or truncate through the JIT primitive ABI,
+    % must fail rather than matching an empty segment or corrupting the match offset.
+    nope = skip_unit64(id(1 bsl 26), B),
+    nope = skip_unit64(id(1 bsl 58), B),
+    nope = int_unit64(id(1 bsl 26), B),
+    nope = int_unit64(id(1 bsl 58), B),
     nope = bin_unit8(id(-1), B),

The 1 bsl 26 cases cover 32-bit multiplication overflow and the 64-bit int ABI truncation; the 1 bsl 58 cases cover 64-bit multiplication overflow. These tests need to run through JIT execution, not only the interpreter.

[P1] Do not untag boxed values in the typed t_integer fast path

Location: libs/jit/src/jit.erl:4503-4521

term_to_int/4 now uses an arithmetic shift, which correctly preserves the sign of a small integer. But its typed optimization applies that shift to every t_integer:

term_to_int({typed, Term, {t_integer, _Range}}, _FailLabel, MMod, MSt0) ->
    {MSt1, Reg} = MMod:move_to_native_register(MSt0, Term),
    {MSt2, IntReg} = MMod:shift_right_arith(MSt1, {free, Reg}, 4),
    {MSt2, IntReg};

BEAM type information proves that the value is an integer, but its range can include values outside AtomVM's immediate-small-integer bounds. For a boxed integer, this shifts the boxed term/pointer instead of extracting its numeric value. The subsequent negative check therefore examines a pointer-derived value and does not reliably send a boxed negative segment size to the opcode's fail label.

Restrict the optimization to ranges proven to fit the backend's small-integer representation; otherwise use the checked path (or a proper boxed-integer conversion primitive). A minimal correctness-oriented change would be:

diff --git a/libs/jit/src/jit.erl b/libs/jit/src/jit.erl
@@
-term_to_int({typed, Term, {t_integer, _Range}}, _FailLabel, MMod, MSt0) ->
-    {MSt1, Reg} = MMod:move_to_native_register(MSt0, Term),
-    {MSt2, IntReg} = MMod:shift_right_arith(MSt1, {free, Reg}, 4),
-    {MSt2, IntReg};
+term_to_int({typed, Term, {t_integer, {Min, Max}}}, FailLabel, MMod, MSt0) ->
+    {MinSmall, MaxSmall} = small_integer_bounds(MMod),
+    if
+        is_integer(Min), is_integer(Max), Min >= MinSmall, Max =< MaxSmall ->
+            {MSt1, Reg} = MMod:move_to_native_register(MSt0, Term),
+            MMod:shift_right_arith(MSt1, {free, Reg}, 4);
+        true ->
+            term_to_int(Term, FailLabel, MMod, MSt0)
+    end;

The fallback above preserves fail-label behavior for boxed values. If positive boxed segment sizes are intended to succeed when capacity permits, the fallback must instead call a conversion primitive capable of unboxing arbitrary integers.

Additional test gap

The added runtime tests cover negative sizes, while tests/libs/jit/jit_tests.erl only verifies that x86-64 emits sar rather than shr. Neither detects positive scaling overflow. Add the overflow cases above and ensure they execute on representative 32-bit and 64-bit JIT backends.

Verification performed

• Reviewed git diff HEAD^ HEAD and the surrounding interpreter/JIT conversion and primitive code.
• Consulted Oracle for an independent correctness pass; its merge recommendation and primary concerns agreed with the findings above.
• git diff --check HEAD^ HEAD passes.
• Rebuilt AtomVM and test_bs.beam; build/src/AtomVM build/tests/erlang_tests/test_bs.beam returns 0 on the interpreter path.
• JIT runtime reproduction was not run because the configured build has AVM_DISABLE_JIT=ON.

Non-blocking suggestion

The changelog entry has a grammar typo:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@
-- Fixed a bug where negative segment size were not rejected in binary matching
+- Fixed a bug where negative segment sizes were not rejected in binary matching