Skip to content

fix(vm): grow-only dropped-segment bitset in data.drop/elem.drop - #172

Open
dmitry123 wants to merge 1 commit into
develfrom
claude/datadrop-elemdrop-bitset-693a70
Open

fix(vm): grow-only dropped-segment bitset in data.drop/elem.drop#172
dmitry123 wants to merge 1 commit into
develfrom
claude/datadrop-elemdrop-bitset-693a70

Conversation

@dmitry123

Copy link
Copy Markdown
Member

Fixes FLU-1100.

Problem

visit_data_drop and visit_element_drop used BitVec::resize(idx + 1, false) to make room for the bit they set. resize also shrinks, so dropping a lower-indexed segment after a higher-indexed one truncated the bitset and discarded the record of the earlier drop:

data.drop 5   -> bitset len 6, bit 5 = true
data.drop 2   -> resize(3) drops bit 5
memory.init 5 -> `.get(5).unwrap_or(false)` reads "not dropped", copies real data

Per the Wasm spec a dropped segment must behave as zero-length, so memory.init / table.init on it has to trap. rwasm instead succeeded and copied data — a state divergence from wasmtime on the same module.

Fix

Grow the bitset only when the index is beyond its current length, then set the bit. Applied identically in visit_data_drop (src/vm/executor/memory.rs) and visit_element_drop (src/vm/executor/table.rs).

Segment indices are bounded by N_MAX_DATA_SEGMENTS / N_MAX_ELEM_SEGMENTS (100_000) in verify_opcode for all four opcodes that reach these paths (MemoryInit, DataDrop, TableInit, ElemDrop), so worst-case growth stays at ~12.5 KiB per bitset.

Tests

New tests/segment_drop.rs covers both opcodes with three cases each: no drop (succeeds), single drop (traps), and descending drop 5 then 2 (must still trap). Both descending cases fail on devel and pass with the fix.

The fuzz generator is nudged toward the pattern that was missed: min_data_segments / min_element_segments are set to 4 in fuzz/fuzz_targets/differential.rs, so generated modules reliably contain several segments and can emit descending drop sequences. Note the rwasm-fuzz crate does not compile on devel for an unrelated reason (a missing Context trait import at differential.rs:709), so that change is unverified by a build.

Verification

  • cargo test — full suite green
  • cargo clippy --all-targets — clean

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@dmitry123, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dcb39c38-517b-43aa-8643-84dd8ee0c5f1

📥 Commits

Reviewing files that changed from the base of the PR and between b8f6091 and 07c10ba.

📒 Files selected for processing (4)
  • fuzz/fuzz_targets/differential.rs
  • src/vm/executor/memory.rs
  • src/vm/executor/table.rs
  • tests/segment_drop.rs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Criterion results (vs baseline)


running 75 tests
test compiler::compiled_expr::tests::compiledexpr_eval_const_returns_none_for_global_or_funcref ... ignored
test compiler::compiled_expr::tests::compiledexpr_from_const_roundtrips ... ignored
test compiler::compiled_expr::tests::compiledexpr_funcref_and_global_introspection ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_global_get_uses_context ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_i32_add_mixed_const_and_global ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_i32_add_mixed_global_and_funcref ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_i32_add_wraps ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_i32_const ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_i32_sub_wraps ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_i64_const ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_i64_mul_wraps ... ignored
test compiler::compiled_expr::tests::compiledexpr_new_ref_func_uses_context ... ignored
test compiler::compiled_expr::tests::compiledexpr_zero_is_zero ... ignored
test compiler::compiled_expr::tests::constop_eval_returns_value ... ignored
test compiler::compiled_expr::tests::empty_eval_context_always_none ... ignored
test compiler::compiled_expr::tests::eval_with_context_reads_globals_and_funcs ... ignored
test compiler::compiled_expr::tests::expr_op_combines_operands_and_propagates_none ... ignored
test compiler::compiled_expr::tests::funcrefop_reads_from_context ... ignored
test compiler::compiled_expr::tests::globalop_maps_value_kinds_correctly ... ignored
test compiler::compiled_expr::tests::op_clone_panics_for_expr_variant - should panic ... ignored
test compiler::compiled_expr::tests::op_clone_works_for_non_expr_variants ... ignored
test compiler::compiled_expr::tests::op_constant_encodes_f32_f64_bits ... ignored
test compiler::compiled_expr::tests::op_constant_encodes_funcref_externref_ids ... ignored
test compiler::compiled_expr::tests::op_constant_encodes_i32_i64 ... ignored
test compiler::drop_keep::tests::test_drop_keep_translation ... ignored
test compiler::func_type_registry::tests::deduplicates_matching_signatures ... ignored
test compiler::func_type_registry::tests::index_lookup_is_stable ... ignored
test compiler::func_type_registry::tests::resolves_unique_signatures_correctly ... ignored
test compiler::parser::tests::unsupported_component_model_returns_error ... ignored
test module::tests::test_decode_exact_rejects_trailing_garbage ... ignored
test module::tests::test_decode_module_wo_source_pc ... ignored
test module::tests::test_decode_rejects_partial_source_pc ... ignored
test module::tests::test_endianness ... ignored
test module::tests::test_module_encoding ... ignored
test module::verification::tests::accepts_verified_encoded_module ... ignored
test module::verification::tests::regular_construction_does_not_verify ... ignored
test module::verification::tests::regular_decode_does_not_verify ... ignored
test module::verification::tests::rejects_branch_target_outside_code_section ... ignored
test module::verification::tests::rejects_call_target_outside_code_section ... ignored
test module::verification::tests::rejects_missing_table_index_payload ... ignored
test module::verification::tests::rejects_section_index_outside_limits ... ignored
test module::verification::tests::rejects_source_pc_outside_code_section ... ignored
test module::verification::tests::rejects_zero_local_depth ... ignored
test strategy::types::tests::checked_memory_range_end_rejects_overflow ... ignored
test types::nan_preserving_float::tests::test_neg_nan_f32 ... ignored
test types::nan_preserving_float::tests::test_neg_nan_f64 ... ignored
test types::nan_preserving_float::tests::test_ops_f32 ... ignored
test types::nan_preserving_float::tests::test_ops_f64 ... ignored
test types::opcode::tests::test_fpu_opcode_encoding_uses_offset ... ignored
test types::opcode::tests::test_opcode_code_values ... ignored
test types::opcode::tests::test_opcode_encoding ... ignored
test types::opcode::tests::test_opcode_encoding_uses_explicit_code ... ignored
test types::opcode::tests::test_opcode_size ... ignored
test types::units::tests::bytes_new16 ... ignored
test types::units::tests::bytes_new32 ... ignored
test types::units::tests::bytes_new64 ... ignored
test types::units::tests::pages_checked_add ... ignored
test types::units::tests::pages_checked_sub ... ignored
test types::units::tests::pages_max ... ignored
test types::units::tests::pages_new ... ignored
test types::units::tests::pages_to_bytes ... ignored
test types::value::copysign_regression_works ... ignored
test types::value::wasm_float_max_regression_works ... ignored
test types::value::wasm_float_min_regression_works ... ignored
test vm::store::tests::clamps_runtime_memory_limit_to_global_maximum ... ignored
test wasmtime::tests::test_call_with_charging_linear_wasmtime ... ignored
test wasmtime::tests::test_call_with_charging_param_overflow_wasmtime ... ignored
test wasmtime::tests::test_call_with_charging_quadratic_wasmtime ... ignored
test wasmtime::tests::test_wasmtime_caller_memory_read_into_vec_checks_bounds_before_allocating ... ignored
test wasmtime::tests::test_wasmtime_caller_missing_memory_returns_trap ... ignored
test wasmtime::tests::test_wasmtime_executor_memory_read_into_vec_checks_bounds_before_allocating ... ignored
test wasmtime::tests::test_wasmtime_executor_missing_entrypoint_returns_trap ... ignored
test wasmtime::tests::test_wasmtime_snapshot_missing_memory_returns_trap ... ignored
test wasmtime::types::tests::maps_unknown_wasmtime_error_to_illegal_opcode ... ignored
test wasmtime::types::tests::maps_wasmtime_traps_to_rwasm_traps ... ignored

test result: ok. 0 passed; 0 failed; 75 ignored; 0 measured; 0 filtered out; finished in 0.00s

Comparisons/bench_native
                        time:   [5.8101 ns 6.0349 ns 6.2852 ns]
Found 142 outliers among 1000 measurements (14.20%)
  19 (1.90%) high mild
  123 (12.30%) high severe
Comparisons/bench_strategy_wasmtime
                        time:   [17.591 µs 18.007 µs 18.460 µs]
Found 92 outliers among 1000 measurements (9.20%)
  67 (6.70%) high mild
  25 (2.50%) high severe
Comparisons/bench_strategy_rwasm
                        time:   [14.203 µs 14.509 µs 14.852 µs]
Found 81 outliers among 1000 measurements (8.10%)
  7 (0.70%) low mild
  32 (3.20%) high mild
  42 (4.20%) high severe

Heads-up: runner perf is noisy; treat deltas as a smoke check.

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants