diff --git a/CHANGELOG.md b/CHANGELOG.md index 62a335b..85bc30c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/crates/nera-runtime/src/ets.rs b/crates/nera-runtime/src/ets.rs index 4cc37b5..7b6c28f 100644 --- a/crates/nera-runtime/src/ets.rs +++ b/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) -> 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; diff --git a/crates/nera-runtime/src/ets_bitstring_tests.rs b/crates/nera-runtime/src/ets_bitstring_tests.rs new file mode 100644 index 0000000..001287d --- /dev/null +++ b/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)); + } +} diff --git a/tests/README.md b/tests/README.md index f69041e..4054cf1 100644 --- a/tests/README.md +++ b/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 diff --git a/tests/ets_bitstring_keys.erl b/tests/ets_bitstring_keys.erl new file mode 100644 index 0000000..8a35090 --- /dev/null +++ b/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 = <>, + [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. diff --git a/tests/ets_stored_bitstring.erl b/tests/ets_stored_bitstring.erl new file mode 100644 index 0000000..07f41a8 --- /dev/null +++ b/tests/ets_stored_bitstring.erl @@ -0,0 +1,119 @@ +%% EXPECT: ets_stored_bitstring_ok +%% Regression for non-byte-aligned bitstrings stored in ETS/persistent_term: +%% the writer's exit or garbage collection must not invalidate the value. +%% Copying reads must also survive deletion of the original storage. +%% Run this unchanged on native Nera and BEAM. +-module(ets_stored_bitstring). +-export([test/0]). + +test() -> + ets_lifetime(exit), + ets_lifetime(gc), + persistent_lifetime(exit), + persistent_lifetime(gc), + io:format("ets_stored_bitstring_ok~n"), + ok. + +ets_lifetime(Lifetime) -> + T = ets:new(stored_bits, [set, public]), + Writer = store(Lifetime, fun() -> + true = ets:insert(T, {bits, values(), unchanged}) + end), + churn(), + [{bits, Actual, unchanged}] = ets:lookup(T, bits), + equal({ets, Lifetime}, values(), Actual), + %% Updating another element copies the bitstrings again. + true = ets:update_element(T, bits, {3, [updated]}), + [{bits, Listed, [updated]}] = ets:tab2list(T), + true = ets:delete(T), + churn(), + equal({tab2list, Lifetime}, values(), Listed), + stop_writer(Writer), + ok. + +persistent_lifetime(Lifetime) -> + Key = {?MODULE, Lifetime}, + Writer = store(Lifetime, fun() -> + ok = persistent_term:put(Key, values()) + end), + churn(), + Actual = persistent_term:get(Key), + equal({persistent_term, Lifetime}, values(), Actual), + true = persistent_term:erase(Key), + churn(), + equal({erased, Lifetime}, values(), Actual), + stop_writer(Writer), + ok. + +%% Construct inside the writer, not in a closure captured from the reader. +%% Heap and refc binaries, sub-byte values, and tails starting inside a byte +%% all reach the shared storage copier as materialized bitstrings. +values() -> + Small = <<"abcdefgh", (id(5)):4>>, + Bytes = list_to_binary(lists:duplicate(id(200), $x)), + Large = <>, + Boundary = list_to_binary(lists:duplicate(id(64), $b)), + [{small, Small}, {large, Large}, {tiny, <<(id(5)):3>>}, + {boundary, <>}, + {small_tail, tail(<<0:3, Small/bitstring>>, 3)}, + {large_tail, tail(<<"prefix", Large/bitstring>>, 48)}]. + +id(X) -> X. +tail(Bits, Skip) -> + <<_:Skip, Rest/bitstring>> = Bits, + Rest. + +equal(Context, Expected, Actual) -> + case Actual =:= Expected of + true -> ok; + false -> erlang:error({stored_bitstring_changed, Context, Expected, Actual}) + end. + +store(exit, Body) -> + {Pid, Ref} = spawn_monitor(fun() -> Body() end), + down(Pid, Ref), + none; +store(gc, Body) -> + Parent = self(), + {Pid, Ref} = spawn_monitor(fun() -> + Body(), + _ = [lists:seq(1, 64) || _ <- lists:seq(1, 2000)], + true = erlang:garbage_collect(), + _ = [lists:seq(1, 64) || _ <- lists:seq(1, 2000)], + true = erlang:garbage_collect(), + Parent ! {self(), stored}, + receive stop -> ok end + end), + receive + {Pid, stored} -> {Pid, Ref}; + {'DOWN', Ref, process, Pid, Reason} -> erlang:error({writer_failed, Reason}) + after 5000 -> erlang:error(writer_timeout) + end. + +stop_writer(none) -> ok; +stop_writer({Pid, Ref}) -> + Pid ! stop, + down(Pid, Ref). + +down(Pid, Ref) -> + receive + {'DOWN', Ref, process, Pid, normal} -> ok; + {'DOWN', Ref, process, Pid, Reason} -> erlang:error({writer_failed, Reason}) + after 5000 -> erlang:error(writer_timeout) + end. + +%% Reuse freed writer/storage memory before reading it, with no timing sleep. +churn() -> + Parent = self(), + Workers = [spawn_monitor(fun() -> + Held = [{I, lists:seq(1, 64)} || I <- lists:seq(1, 200)], + Parent ! {self(), ready}, + receive stop -> length(Held) end + end) || _ <- lists:seq(1, 32)], + [receive {Pid, ready} -> ok after 5000 -> erlang:error(churn_timeout) end + || {Pid, _} <- Workers], + _ = [erlang:make_tuple(8, I) || I <- lists:seq(1, 20000)], + true = erlang:garbage_collect(), + [Pid ! stop || {Pid, _} <- Workers], + [down(Pid, Ref) || {Pid, Ref} <- Workers], + ok.