fix(runtime): preserve stored partial bitstrings

CHANGELOG.md

@@ -2,6 +2,21 @@
 
 All notable milestones for the nera project are documented in this file.
 
+## Stored bitstrings keep their bytes and exact bit length
+
+- ETS and `persistent_term` now own copies of non-byte-aligned bitstrings,
+  just as they already do for whole-byte binaries. Small inline values and
+  large reference-counted values remain equal after the storing process exits
+  or collects garbage, as on BEAM.
+- Copying reads preserve the final partial byte's logical bit length and remain
+  valid after the table is deleted or the persistent term is erased.
+- Bitstring keys and bag duplicate checks compare significant bits and exact
+  length, not descriptor addresses or unused trailing bits, so copies remain
+  interchangeable as keys, including inside lists and tuples.
+- The `ets_stored_bitstring` and `ets_bitstring_keys` regressions exercise
+  writer lifetimes, copying reads, boxed updates and copied-key operations
+  unchanged natively and on BEAM.
+
 ## Receives nested in after bodies resume at their own wait
 
 - A `receive` directly inside another receive's `after` body now waits for

crates/nera-runtime/src/ets.rs

@@ -221,13 +221,20 @@ impl TermKey {
             out.extend_from_slice(&term.0.to_le_bytes());
             return;
         }
-        // Binary — serialize content bytes
-        if term.is_binary() {
+        // Bitstring identity is its exact length and significant bits, not
+        // the descriptor address or unused bits in the final backing byte.
+        if term.is_bitstring() {
             out.push(b'B');
             if let Some((ptr, len)) = unsafe { crate::ffi::binary_data_pub(term) } {
                 let slice = unsafe { std::slice::from_raw_parts(ptr, len) };
-                out.extend_from_slice(&(len as u64).to_le_bytes());
-                out.extend_from_slice(slice);
+                let bits = unsafe { term.binary_bit_size().unwrap() };
+                out.extend_from_slice(&(bits as u64).to_le_bytes());
+                let whole_bytes = bits / 8;
+                out.extend_from_slice(&slice[..whole_bytes]);
+                let trailing_bits = bits % 8;
+                if trailing_bits != 0 {
+                    out.push(slice[whole_bytes] & (u8::MAX << (8 - trailing_bits)));
+                }
             }
             return;
         }
@@ -402,7 +409,7 @@ unsafe fn object_heap_words(term: ErlTerm, pending: &mut WorkStack<ErlTerm>) ->
         queue(cursor); // tail
         return count;
     }
-    if term.is_binary() {
+    if term.is_bitstring() {
         if let Some((_ptr, len)) = unsafe { crate::ffi::binary_data_pub(term) } {
             if len <= REFC_BINARY_THRESHOLD {
                 return 1 + len.div_ceil(8); // header + data words
@@ -517,7 +524,7 @@ trait CopyTarget {
     /// Allocate `n` uninitialized words that stay in place.
     fn alloc_words(&mut self, n: usize) -> *mut u64;
 
-    /// Copy a boxed term that holds no other terms (a binary, float, bigint
+    /// Copy a boxed term that holds no other terms (a bitstring, float, bigint
     /// or reference); any other term is kept as it is.
     /// # Safety
     /// As for `deep_copy_into_compact`.
@@ -536,16 +543,17 @@ impl CopyTarget for CompactHeap {
                           REFC_BINARY_OFFSET_WORD_INDEX, REFERENCE_TOTAL_WORDS,
                           REFERENCE_ID_WORD_INDEX};
 
-        if term.is_binary() {
+        if term.is_bitstring() {
             if let Some((ptr, len)) = unsafe { crate::ffi::binary_data_pub(term) } {
                 let bytes = unsafe { std::slice::from_raw_parts(ptr, len) };
+                let bit_len = unsafe { term.binary_bit_size().unwrap() } as u64;
                 if len <= REFC_BINARY_THRESHOLD {
                     // Inline heap binary: [header | data words...]
                     let data_words = len.div_ceil(8);
                     let total_words = 1 + data_words;
                     let words = self.alloc_words(total_words);
                     unsafe {
-                        *words = BoxedHeader::new(crate::term::BoxedSubtype::HeapBinary, (len * 8) as u64).0;
+                        *words = BoxedHeader::new(crate::term::BoxedSubtype::HeapBinary, bit_len).0;
                         std::ptr::copy_nonoverlapping(
                             bytes.as_ptr(),
                             (words.add(1)) as *mut u8,
@@ -559,7 +567,7 @@ impl CopyTarget for CompactHeap {
                     let raw_arc_ptr = Arc::as_ptr(&data) as u64;
                     let words = self.alloc_words(REFC_BINARY_TOTAL_WORDS);
                     unsafe {
-                        *words = BoxedHeader::new(crate::term::BoxedSubtype::RefcBinary, (len * 8) as u64).0;
+                        *words = BoxedHeader::new(crate::term::BoxedSubtype::RefcBinary, bit_len).0;
                         *words.add(REFC_BINARY_PTR_WORD_INDEX) = raw_arc_ptr;
                         *words.add(REFC_BINARY_OFFSET_WORD_INDEX) = 0;
                     }
@@ -613,10 +621,16 @@ impl CopyTarget for ProcessHeap {
     }
 
     unsafe fn copy_leaf(&mut self, term: ErlTerm) -> ErlTerm {
-        if term.is_binary() {
+        if term.is_bitstring() {
             if let Some((ptr, len)) = unsafe { crate::ffi::binary_data_pub(term) } {
                 let bytes = unsafe { std::slice::from_raw_parts(ptr, len) };
-                return self.alloc_binary_auto(bytes);
+                let bit_len = unsafe { term.binary_bit_size().unwrap() } as u64;
+                let copy = self.alloc_binary_auto(bytes);
+                // Allocation rounds up to bytes; preserve the logical bit
+                // length rather than turning the trailing bits into a byte.
+                let header = copy.as_boxed_ptr().unwrap().cast_mut();
+                unsafe { *header = BoxedHeader(*header).with_arity(bit_len).0; }
+                return copy;
             }
             return term;
         }
@@ -690,7 +704,7 @@ impl<'a, T: CopyTarget> EtsCopier<'a, T> {
         if term.is_cons() {
             return unsafe { self.copy_list(term) };
         }
-        if !unsafe { term.is_binary() } {
+        if !unsafe { term.is_bitstring() } {
             if let Some(ptr) = term.as_boxed_ptr() {
                 let header = BoxedHeader(unsafe { *ptr });
                 let (subtype, raw, slots) = match header.subtype() {
@@ -3781,3 +3795,7 @@ mod tests {
         }
     }
 }
+
+#[cfg(test)]
+#[path = "ets_bitstring_tests.rs"]
+mod bitstring_tests;

crates/nera-runtime/src/ets_bitstring_tests.rs

@@ -0,0 +1,116 @@
+//! Focused invariants for the bitstring storage regressions. The Erlang
+//! fixtures own observable lifetime/key behavior; these tests deterministically
+//! check descriptor independence and insignificant backing-byte padding.
+
+use super::*;
+use crate::binary::BinarySlice;
+use crate::term::BoxedSubtype;
+
+#[test]
+fn bitstring_copies_own_descriptors_in_both_directions() {
+    // The reported values, plus a sub-byte value and the inline/refc boundary.
+    let mut large = vec![b'x'; 200];
+    large.push(0xc0); // <<3:2>>
+    let mut boundary = vec![b'b'; crate::heap::REFC_BINARY_THRESHOLD];
+    boundary.push(0x80); // <<1:1>>
+    let cases = [
+        (b"abcdefgh\x50".to_vec(), 4),
+        (large, 6),
+        (vec![0xa0], 5),
+        (boundary, 7),
+    ];
+
+    for (bytes, unused_bits) in cases {
+        let bits = bytes.len() * 8 - unused_bits;
+        for refc_window in [false, true] {
+            let mut source = ProcessHeap::new();
+            let original = if refc_window {
+                // A byte-offset window must not accidentally copy its prefix.
+                let prefix = b"prefix";
+                let backing = [prefix.as_slice(), &bytes, b"suffix"].concat();
+                let window =
+                    BinarySlice::new_bits(Arc::new(BinaryData::new(backing)), prefix.len(), bits)
+                        .unwrap();
+                source.alloc_binary_slice(window)
+            } else {
+                let term = source.alloc_binary(&bytes);
+                // SAFETY: the test exclusively owns the new descriptor and
+                // shortens its bit length within its initialized byte storage.
+                unsafe {
+                    *term.as_boxed_ptr().unwrap().cast_mut() =
+                        BoxedHeader::new(BoxedSubtype::HeapBinary, bits as u64).0;
+                }
+                term
+            };
+
+            // SAFETY: the source is live, immutable and cannot collect here.
+            let words = unsafe { count_heap_words(original) };
+            let mut pool = EtsRegionPool::new();
+            let mut stored = CompactHeap::from_pool(Some(pool.alloc(words)));
+            let in_storage = unsafe { deep_copy_into_compact(original, &mut stored) };
+            assert_ne!(
+                in_storage.as_boxed_ptr(),
+                original.as_boxed_ptr(),
+                "storage must own the bitstring descriptor"
+            );
+            drop(source);
+
+            let mut reader = ProcessHeap::new();
+            // SAFETY: stored keeps every source descriptor/backing byte live.
+            let read = unsafe { deep_copy_into_heap(in_storage, &mut reader) };
+            assert_ne!(
+                read.as_boxed_ptr(),
+                in_storage.as_boxed_ptr(),
+                "copying reads must own the bitstring descriptor"
+            );
+            drop(stored);
+            drop(pool);
+
+            let mut roots = [read];
+            // SAFETY: the sole root belongs to reader; no raw borrows survive.
+            unsafe { reader.collect_full(&mut roots) };
+            // SAFETY: the root was updated by GC and reader owns its bytes.
+            unsafe {
+                assert_eq!(roots[0].binary_bit_size(), Some(bits));
+                assert_eq!(roots[0].binary_bytes_any(), Some(bytes.as_slice()));
+            }
+        }
+    }
+}
+
+// Storage copies must not turn equal bitstring keys into
+// address-based keys. A real byte window can retain bits beyond its logical end.
+#[test]
+fn bitstring_keys_ignore_padding_but_preserve_significant_bits_and_length() {
+    let mut heap = ProcessHeap::new();
+    let bytes = b"abcdefghP"; // Last byte's high nibble is <<5:4>>.
+    let bits = bytes.len() * 8 - 4;
+    let make_heap_bits = |heap: &mut ProcessHeap, data: &[u8], bits: usize| {
+        let term = heap.alloc_binary(data);
+        // SAFETY: the test exclusively owns the descriptor and only shortens
+        // its bit window within initialized bytes.
+        unsafe {
+            *term.as_boxed_ptr().unwrap().cast_mut() =
+                BoxedHeader::new(BoxedSubtype::HeapBinary, bits as u64).0;
+        }
+        term
+    };
+    let original = make_heap_bits(&mut heap, bytes, bits);
+    let prefix = b"prefix";
+    let framed = [prefix.as_slice(), b"abcdefghZ", b"suffix"].concat();
+    let window =
+        BinarySlice::new_bits(Arc::new(BinaryData::new(framed)), prefix.len(), bits).unwrap();
+    let same_bits = heap.alloc_binary_slice(window);
+    let different_bits = make_heap_bits(&mut heap, b"abcdefgh@", bits);
+    let different_length = make_heap_bits(&mut heap, bytes, bits + 1);
+    let whole_bytes = heap.alloc_binary(bytes);
+
+    // SAFETY: heap keeps all immutable input descriptors and bytes live.
+    unsafe {
+        let key = TermKey::from_term(original);
+        assert!(key == TermKey::from_term(same_bits));
+        assert!(key != TermKey::from_term(different_bits));
+        assert!(key != TermKey::from_term(different_length));
+        assert!(key != TermKey::from_term(whole_bytes));
+    }
+}

tests/README.md

@@ -462,6 +462,8 @@ These are automatically skipped by the runner and show up as SKIP in output.
 | `ets_basic.erl`  | ETS new/insert/lookup/delete/tab2list | ~15        |
 | `ets_keypos.erl` | ETS keypos and named tables           | ~8         |
 | `ets_stored_fun.erl` | A fun stored in ETS (at the top of a row, nested in a list, tuple and map, after `update_element/3`) or `persistent_term` by a process that then exits or collects garbage keeps its captured list, binaries, tuple, map, float, bignum and inner fun when looked up and called; a fun read by `tab2list/1` or `persistent_term:get/1` still works after the table or term is deleted; completion marker; also run on BEAM | ~16 |
+| `ets_stored_bitstring.erl` | Non-byte-aligned heap/refc bitstrings, sub-byte values and extracted tails stored in ETS or `persistent_term` survive writer exit/GC; boxed `update_element/3` replacements and copying reads remain intact after table deletion or term erasure; completion marker; same fixture on BEAM | 4 lifetime scenarios |
+| `ets_bitstring_keys.erl` | Partial-bitstring bag duplicate suppression; copied direct/list/tuple keys in hash-backed tables for lookup, membership, insert_new, boxed updates, counters, deletion and forward/backward traversal; copied values as persistent_term keys; significant bits and exact lengths remain distinct; completion marker; same fixture on BEAM | Table-driven |
 | `ets_named_table_return.erl` | ets:new/2 with `named_table` returns the name (Cowboy's `?MODULE = ets:new(?MODULE, [named_table, ...])`): every table type, apply, variable module and fun calls; a table without `named_table` does not take the name, and creating or deleting one leaves a named table with the same atom reachable; the name is released by ets:delete/1; a table created in another process is reached by its name; also run on BEAM | Table-driven |
 
 ### Stdlib

tests/ets_bitstring_keys.erl

@@ -0,0 +1,95 @@
+%% EXPECT: ets_bitstring_keys_ok
+%% Regression for storage copies exposing pointer-based bitstring keys.
+%% Returned/copied keys and bag values must still compare by significant bits,
+%% including bitstrings nested in lists and tuples. Run unchanged on BEAM.
+-module(ets_bitstring_keys).
+-export([test/0]).
+
+test() ->
+    bag_duplicates(),
+    [table_keys(Type) || Type <- [set, bag, duplicate_bag]],
+    persistent_keys(),
+    io:format("ets_bitstring_keys_ok~n"),
+    ok.
+
+keys() ->
+    Small = <<"abcdefgh", (id(5)):4>>,
+    Bytes = list_to_binary(lists:duplicate(id(200), $x)),
+    Large = <<Bytes/binary, (id(3)):2>>,
+    [Small, Large, [Small], {wrapped, [Large, Small]},
+     <<(id(5)):3>>, <<(id(20)):5>>, <<(id(4)):3>>].
+
+id(X) -> X.
+
+bag_duplicates() ->
+    [Small, Large | _] = keys(),
+    Value = {Small, [Large, Small]},
+    T = ets:new(bitstring_bag, [bag]),
+    Row = {key, Value},
+    true = ets:insert(T, Row),
+    true = ets:insert(T, Row),
+    [Row] = ets:lookup(T, key),
+    true = ets:delete(T),
+    ok.
+
+table_keys(Type) ->
+    T = ets:new(bitstring_keys, [Type]),
+    Keys = keys(),
+    [true = ets:insert(T, {K, 0, unchanged}) || K <- Keys],
+    CopiedRows = ets:tab2list(T),
+    Count = length(Keys),
+    Count = length(CopiedRows),
+    [check_copied_key(Type, T, Row) || Row <- CopiedRows],
+    Sorted = lists:sort(Keys),
+    Sorted = lists:sort(walk(T, ets:first(T), next)),
+    Sorted = lists:sort(walk(T, ets:last(T), prev)),
+    %% Keys from traversal also work for lookup and membership.
+    true = ets:member(T, ets:first(T)),
+    true = ets:member(T, ets:last(T)),
+    [begin
+         true = ets:delete(T, K),
+         false = ets:member(T, K),
+         [] = ets:lookup(T, K)
+     end || {K, _, _} <- CopiedRows],
+    [] = ets:tab2list(T),
+    true = ets:delete(T),
+    ok.
+
+check_copied_key(Type, T, {K, 0, unchanged}) ->
+    true = ets:member(T, K),
+    [{K, 0, unchanged}] = ets:lookup(T, K),
+    false = ets:insert_new(T, {K, 99, replaced}),
+    case Type of
+        set ->
+            true = ets:update_element(T, K, {3, [updated]}),
+            1 = ets:update_counter(T, K, {2, 1}),
+            [{K, 1, [updated]}] = ets:lookup(T, K);
+        _ -> ok
+    end,
+    ok.
+
+walk(_T, '$end_of_table', _Direction) -> [];
+walk(T, K, Direction) ->
+    [_] = ets:lookup(T, K),
+    Next = case Direction of
+        next -> ets:next(T, K);
+        prev -> ets:prev(T, K)
+    end,
+    [K | walk(T, Next, Direction)].
+
+persistent_keys() ->
+    CopyKey = {?MODULE, copied_key},
+    [begin
+         %% Reading a stored value makes a new descriptor, which must be
+         %% interchangeable with the original when either is used as a key.
+         ok = persistent_term:put(CopyKey, K),
+         Copied = persistent_term:get(CopyKey),
+         ok = persistent_term:put(K, original),
+         original = persistent_term:get(Copied),
+         ok = persistent_term:put(Copied, replaced),
+         replaced = persistent_term:get(K),
+         true = persistent_term:erase(Copied),
+         missing = persistent_term:get(K, missing)
+     end || K <- keys()],
+    true = persistent_term:erase(CopyKey),
+    ok.