Add support for non-byte-aligned bitstrings

[pguyot]

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 — c8fd1c1ed (“Add support for non-byte-aligned bitstrings”)

Verdict

Changes requested. The representation and matching changes are generally coherent, but construction still truncates dynamically supplied bitstrings, partial-width little-endian integers use the wrong bit layout, and one JIT matching path still measures only complete bytes.

Findings

1. High — dynamic bitstring segments are accepted and then truncated to complete bytes

bs_append, bs_private_append, bs_put_binary, and bs_create_bin now accept term_is_bitstring(src), but continue to derive the source length from term_binary_size(src) and copy with memcpy (src/libAtomVM/opcodesswitch.h:3448-3477, src/libAtomVM/opcodesswitch.h:3596-3632, and src/libAtomVM/opcodesswitch.h:5626-5663, src/libAtomVM/opcodesswitch.h:5872-5910). term_binary_size intentionally excludes the trailing partial byte, so accepting the wider type silently loses data and places subsequent segments at the wrong offset.

The JIT duplicates this behavior: its all size calculation reads the complete-byte count (libs/jit/src/jit.erl:2811-2826), and jit_bitstring_copy_binary copies that many bytes with memcpy (src/libAtomVM/jit.c:1711-1730).

This was reproduced against the rebuilt VM:

copy(Bits) -> <<Bits/bitstring>>.
append(Bits) -> <<Bits/bitstring, 16#AB:8>>.

Bits = <<1:1>>,
{copy(Bits), append(Bits)}.

OTP returns {<<1:1>>, <<1:1, 16#AB:8>>}; AtomVM returns {<<>>, <<16#AB>>}.

The complete fix is not a one-line change: size segments in bits (term_bit_size(src) for all), validate explicit bit lengths against term_bit_size(src), copy the requested number of bits at the current destination bit offset, and make the interpreter/JIT paths identical. Until that is implemented, restoring binary-only validation on paths that can only copy bytes is safer than accepting and corrupting the value.

At minimum, add a regression that prevents this behavior from returning unnoticed:

diff --git a/tests/erlang_tests/test_bs.erl b/tests/erlang_tests/test_bs.erl
--- a/tests/erlang_tests/test_bs.erl
+++ b/tests/erlang_tests/test_bs.erl
@@
 test_create_with_unaligned_int_size() ->
@@
     true = is_bitstring(B),
+    Bits = id(<<1:1>>),
+    <<1:1>> = copy_bitstring(Bits),
+    <<1:1, 16#AB:8>> = append_to_bitstring(Bits),
     ok.
+
+copy_bitstring(Bits) -> <<Bits/bitstring>>.
+append_to_bitstring(Bits) -> <<Bits/bitstring, 16#AB:8>>.

2. High — non-byte-sized little-endian integer fields have the wrong layout

The commit enables non-byte-aligned construction, but the generic little-endian helpers byte-swap ceil(N / 8) whole bytes and then read/write only N bits (src/libAtomVM/bitstring.c:32-58, src/libAtomVM/bitstring.c:61-112). For widths that are not multiples of eight, Erlang instead lays out complete low-order bytes first and then the remaining high-order bits.

For a dynamically constructed <<16#ABC:12/little>>, OTP produces <<16#BC, 16#A:4>>. The rebuilt AtomVM produces a first byte of 16#C0 (and the current printer hides the final four bits). Extraction applies the corresponding incorrect whole-value byte swap, so matching is affected too; a pack/unpack round trip may conceal the paired error.

Preserve the existing byte-aligned fast path. For a partial width, process complete eight-bit chunks in low-byte-first order, then process the final N rem 8 high-order bits; extraction must perform the inverse mapping.

Add byte-level, OTP-compatible tests rather than only round trips:

diff --git a/tests/erlang_tests/test_bs.erl b/tests/erlang_tests/test_bs.erl
--- a/tests/erlang_tests/test_bs.erl
+++ b/tests/erlang_tests/test_bs.erl
@@
 test_create_with_int_little_endian() ->
     <<2, 1>> = create_int_binary_little_endian(16#0102, 16),
+    <<16#BC, 16#A:4>> = create_int_binary_little_endian(16#ABC, id(12)),

The test matrix should include widths 1, 4, 7, 9, 12, 15, and 63, both at bit offset zero and after an unaligned prefix.

3. High — JIT bs_match_string rejects literals that consume trailing bits

The interpreter correctly changed its capacity check to term_bit_size(bs_bin) (src/libAtomVM/opcodesswitch.h:4173), but the JIT primitive still checks term_binary_size(bs_bin) * 8 (src/libAtomVM/jit.c:1899-1906). A one-bit bitstring therefore has zero capacity according to the JIT and a valid literal match fails only when JIT-compiled.

diff --git a/src/libAtomVM/jit.c b/src/libAtomVM/jit.c
--- a/src/libAtomVM/jit.c
+++ b/src/libAtomVM/jit.c
@@ -1902,7 +1902,9 @@ static bool jit_bitstring_match_module_str(Context *ctx, JITState *jit_state, te
     size_t remaining = 0;
     const uint8_t *str = module_get_str(jit_state->module, str_id, &remaining);
 
-    if (term_binary_size(bs_bin) * 8 - bs_offset < MIN(remaining * 8, bits)) {
+    size_t capacity_bits = term_bit_size(bs_bin);
+    if (bs_offset > capacity_bits
+        || capacity_bits - bs_offset < MIN(remaining * 8, bits)) {
         return false;
     }

Add an interpreter-versus-JIT test that matches a literal such as <<1:1>> from the trailing partial byte.

4. Medium — ETF serialization preserves insignificant padding instead of canonicalizing it

BIT_BINARY_EXT decoding accepts non-zero insignificant low bits in the final byte, as OTP does (src/libAtomVM/external_term.c:842-869). Serialization then copies the entire backing byte unchanged (src/libAtomVM/external_term.c:395-406). Consequently, semantically equal bitstrings can have different external encodings depending on their origin.

For example, AtomVM accepts <<131,77,0,0,0,1,1,255>> as <<1:1>>, but term_to_binary/1 emits the dirty final byte 255; OTP emits the canonical final byte 128.

diff --git a/src/libAtomVM/external_term.c b/src/libAtomVM/external_term.c
--- a/src/libAtomVM/external_term.c
+++ b/src/libAtomVM/external_term.c
@@ -401,6 +401,9 @@ static int serialize_term(uint8_t *buf, term t, GlobalContext *glb)
                 WRITE_32_UNALIGNED(buf + 1, nbytes);
                 buf[5] = last_byte_bits;
                 memcpy(buf + 6, data, nbytes);
+                if (nbytes != 0) {
+                    buf[6 + nbytes - 1] &= (uint8_t) (0xFFU << (8 - last_byte_bits));
+                }
             }
             return 6 + nbytes;

Add a decode/re-encode test using a final byte of 255 with one significant bit and assert that the serialized final byte is 128.

5. Low — printing a bitstring omits its trailing partial field

term_funprint now enters the binary branch for every bitstring, but still iterates over only term_binary_size(t) complete bytes (src/libAtomVM/term.c:346-399). Thus <<1:1>> prints as <<>>, and <<255,5:7>> prints as <<255>>, which misrepresents values in diagnostics and crash output.

diff --git a/src/libAtomVM/term.c b/src/libAtomVM/term.c
--- a/src/libAtomVM/term.c
+++ b/src/libAtomVM/term.c
@@ -391,6 +391,17 @@ int term_funprint(PrinterFun *fun, term t, const GlobalContext *global)
                 ret += printed;
             }
         }
+        uint8_t trailing_bits = (uint8_t) (term_bit_size(t) % 8);
+        if (trailing_bits != 0) {
+            uint8_t value = binary_data[len] >> (8 - trailing_bits);
+            int printed = fun->print(fun, "%s%u:%u", len != 0 ? "," : "",
+                (unsigned) value, (unsigned) trailing_bits);
+            if (UNLIKELY(printed < 0)) {
+                return printed;
+            }
+            ret += printed;
+        }
         int printed = fun->print(fun, ">>");

Validation performed

• cmake --build build -j4 — passed.
• build/tests/test-erlang — passed.
• Rebuilt AtomVM execution of test_bs.beam and test_binary_to_term.beam — both returned 0.
• Focused runtime probes confirmed findings 1, 2, and 4.
• Static interpreter/JIT comparison confirmed finding 3.

[petermm]

Follow-up PR Review — d0cfee66e

This review verifies the fixes pushed after the review of c8fd1c1ed.

Verdict

Changes still requested. Four original findings are fixed. Dynamic bitstring construction is fixed for the modern bs_create_bin path but remains unsafe in legacy construction opcodes and JIT private-append reuse. The new bit-copy implementation also has a boundary overrun, and the new bs_get_binary2 interpreter implementation mishandles some negative sizes through unsigned multiplication wraparound.

Original finding status

1. Partially fixed — dynamic bitstring construction

The ordinary bs_create_bin path is fixed in both runtimes:

• The interpreter sizes all segments with term_bit_size, validates explicit sizes in bits, and uses bitstring_copy_bits at the current bit offset (src/libAtomVM/opcodesswitch.h:5628-5663, src/libAtomVM/opcodesswitch.h:5873-5912).
• The JIT computes source sizes with term_bit_size and copies bit counts rather than byte counts (libs/jit/src/jit.erl:2827-2883, src/libAtomVM/jit.c:1726-1743).
• The new runtime tests exercise dynamic partial-bit sources and mixed unaligned segments.

The original reproduction is now correct:

Bits = <<1:1>>,
{<<Bits/bitstring>>, <<Bits/bitstring, 16#AB:8>>}.

AtomVM now returns {<<1:1>>, <<213,1:1>>}, matching OTP.

However, the legacy opcodes still accept bitstrings and truncate them to complete bytes:

• bs_append uses term_binary_size, memcpy, and src_size * 8 (src/libAtomVM/opcodesswitch.h:3448-3477).
• bs_private_append does the same through term_reuse_binary (src/libAtomVM/opcodesswitch.h:3499-3519).
• bs_put_binary maps all to term_binary_size(src) (src/libAtomVM/opcodesswitch.h:3596-3632).

These paths are used by OTP 26–27 with no_bs_create_bin. Existing legacy tests use only byte-aligned sources, so they do not cover the failure. Until the legacy operations become bit-aware, the smallest safe change is to reject partial bitstrings rather than silently truncate them:

diff --git a/src/libAtomVM/opcodesswitch.h b/src/libAtomVM/opcodesswitch.h
--- a/src/libAtomVM/opcodesswitch.h
+++ b/src/libAtomVM/opcodesswitch.h
@@
-                VERIFY_IS_BITSTRING(src, "bs_append", 0);
+                if (UNLIKELY(!term_is_binary(src))) {
+                    RAISE_ERROR(BADARG_ATOM);
+                }
@@
-                VERIFY_IS_BITSTRING(src, "bs_private_append", 0);
+                if (UNLIKELY(!term_is_binary(src))) {
+                    RAISE_ERROR(BADARG_ATOM);
+                }
@@
-                VERIFY_IS_BITSTRING(src, "bs_put_binary", 0);
+                if (UNLIKELY(!term_is_binary(src))) {
+                    RAISE_ERROR(BADARG_ATOM);
+                }

The JIT also decides whether to reuse a private_append/all source from the final result alignment only (libs/jit/src/jit.erl:2384-2421). Its reuse path then derives the initial offset from term_binary_size(src) (libs/jit/src/jit.erl:2976-3005). A partial source followed by enough bits to make the final result byte-aligned can therefore still lose the source trailing bits. Match the interpreter by requiring the reused source itself to be byte-aligned, or fall back to allocation plus a bit copy.

2. Fixed — partial-width little-endian integers

Insertion now writes complete low-order bytes first and then the remaining high-order bits; extraction implements the inverse mapping (src/libAtomVM/bitstring.c:27-60, src/libAtomVM/bitstring.c:68-126).

The new tests compare byte layouts for widths 1, 4, 7, 9, 12, 15, and 63 at offset zero and after a three-bit prefix. A focused runtime probe now returns <<188,10:4>> for dynamic <<16#ABC:12/little>>, matching OTP.

3. Fixed — JIT bs_match_string trailing capacity

The JIT now measures capacity with term_bit_size and checks bs_offset before subtracting (src/libAtomVM/jit.c:1912-1921). Tests include successful, differing, and too-short matches that consume trailing bits.

4. Fixed — ETF padding canonicalization

Serialization now masks insignificant low bits from the final byte (src/libAtomVM/external_term.c:389-410). The added test decodes a one-bit value backed by 255 and requires re-encoding with final byte 128. A focused runtime probe confirms the canonical result.

5. Fixed, test suggested — bitstring printing

term_funprint now prints the trailing Value:Size field and correctly handles a sub-byte value without an empty quoted prefix (src/libAtomVM/term.c:340-409). Runtime output now renders the tested values correctly.

There is no direct regression test for printing. Add coverage for <<1:1>>, <<255,5:7>>, and a printable byte followed by trailing bits.

New findings introduced or exposed by the fixes

1. Critical — incomplete bit copies access one byte past their buffers

bitstring_copy_bits_incomplete_bytes eagerly reads the next destination and source bytes whenever their bit indices reach zero, even when the bit just copied was the final requested bit; it then unconditionally writes the current destination byte after the loop (src/libAtomVM/bitstring.c:338-375).

Examples:

• Copying seven bits at destination offset one ends exactly at the byte boundary. The loop writes the completed byte, advances dst, reads *dst past the logical destination, and writes it again after the loop.
• Copying eight bits to an unaligned destination consumes the last source bit and then immediately reads the byte after the source.
• A zero-bit copy on the incomplete path still reads both buffers.

This helper is now used by interpreter and JIT construction, so the new bit-granular fix exposes both paths to out-of-bounds reads/writes. Heap padding can hide the issue in ordinary tests; refc binaries are allocated to their requested byte size.

The smallest robust fix is a bounded bit loop:

diff --git a/src/libAtomVM/bitstring.c b/src/libAtomVM/bitstring.c
--- a/src/libAtomVM/bitstring.c
+++ b/src/libAtomVM/bitstring.c
@@
 void bitstring_copy_bits_incomplete_bytes(uint8_t *dst, size_t bits_offset, const uint8_t *src, size_t bits_count)
 {
-    /* current prefetch/rollover implementation */
+    for (size_t i = 0; i < bits_count; ++i) {
+        size_t dst_bit = bits_offset + i;
+        uint8_t mask = (uint8_t) (1U << (7 - (dst_bit % 8)));
+        if (src[i / 8] & (uint8_t) (1U << (7 - (i % 8)))) {
+            dst[dst_bit / 8] |= mask;
+        } else {
+            dst[dst_bit / 8] &= (uint8_t) ~mask;
+        }
+    }
 }

Add exact-boundary tests for source widths 0, 7, and 8 at destination offset one, and run them under ASan.

2. High — bs_get_binary2 can accept a huge negative size after multiplication wraps

The interpreter casts a negative size to size_t and then multiplies by unit, assuming the result will remain huge and fail the capacity check (src/libAtomVM/opcodesswitch.h:4463-4486). Unsigned multiplication can instead wrap to zero or another small value.

This was reproduced against the rebuilt VM:

dyn(N, B) ->
    case B of
        <<X:N/binary-unit:64, R/bits>> -> {X, R};
        _ -> nope
    end.

dyn(-(1 bsl 58), <<1>>).

AtomVM incorrectly returns {<<>>, <<1>>}. OTP returns nope. The JIT explicitly rejects a negative value before multiplication, so this is also interpreter/JIT semantic divergence.

diff --git a/src/libAtomVM/opcodesswitch.h b/src/libAtomVM/opcodesswitch.h
--- a/src/libAtomVM/opcodesswitch.h
+++ b/src/libAtomVM/opcodesswitch.h
@@
                 size_t size_bits;
                 if (term_is_integer(size)) {
-                    size_bits = (size_t) term_to_int(size) * unit;
+                    avm_int_t size_value = term_to_int(size);
+                    if (size_value < 0
+                        || (unit != 0 && (size_t) size_value > remaining_bits / unit)) {
+                        JUMP_TO_ADDRESS(mod->labels[fail]);
+                    }
+                    size_bits = (size_t) size_value * unit;

Also guard bs_offset > term_bit_size(bs_bin) before calculating remaining_bits to avoid unsigned subtraction underflow on an invalid match state.

3. Medium — JIT rejects valid all matches with non-power-of-two units

The interpreter checks remaining_bits % unit, but the JIT implements divisibility as remaining_bits & (unit - 1) and therefore explicitly raises unsupported for non-power-of-two units (libs/jit/src/jit.erl:1390-1423). The new unit:3 test covers only a variable integer size, not all, so it misses this parity gap.

Use a remainder operation or a small divisibility primitive for the JIT and add successful/failing all cases with binary-unit:3.

Validation performed

• Compared c8fd1c1ed..d0cfee66e and inspected interpreter/JIT call paths.
• Consulted Oracle for an independent fix verification and regression review.
• cmake --build build -j4 — passed.
• build/tests/test-erlang — passed.
• Rebuilt AtomVM execution of test_bs.beam and test_binary_to_term.beam — both returned 0.
• Focused runtime probe confirmed fixes 1–5 on the modern paths.
• Focused runtime probe reproduced the negative-size wraparound finding.