Codebase cleanup sweep: remove dead code, fix stale comments, dedup#866
Merged
Conversation
Replace the stale static-dispatch draft in website/content/spec/javm.md (AUIPC/JALR removed, PC as instruction-stream byte-offset, br_table jump tables) with the current RV64E differential: plain RISC-V control flow (jal/jalr/auipc), organized into Category 1 (hard spec divergences: 2^32 address wrap for data and code, jalr -> basic-block start, reserved ecall/ebreak, reserved x3/x4) and Category 2 (platform / EEI configuration). Mirrors docs/pvm-isa/05-pvm2-rv-diff.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Removes code orphaned by the PVM2 take2 redesign and corrects comments that no longer match the implementation: - delete the dead `error` module (`ProgramError`, never constructed) and drop the now-unused `thiserror` dependency - delete `rv_fast_cost` + `rv_reg_bit` (the flat per-op gas path, superseded by the per-block LUT simulation) and the dead `static_target` - drop `Predecode::valid_pc` (a per-byte bitmap written every predecode but read only by its own tests; the recompiler builds its own map) and `CopyingMemory::is_in_bounds` (no callers) - fix stale docs: AUIPC is no longer forbidden; jalr masks to 32 bits (not `& !1`) and validates against block starts (no `bb_starts` region); drop "Harvard semantics"; ecall is custom-0 funct3 (not opcode 3/10); ISA is RV64EMC+Zbb/Zba/Zbs/Zicond/Zicclsm (not "v3 keeps the PVM ISA") Smoke 12/12 bit-identical; javm-exec tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- delete dead pub items left by the Image/cnode collapse: Cap::image_from,
Cap::kind()/CapKind, CNodeCap::{remove, materialized_entries,
iter_materialized}, CNodeSlotEntry, SlotIdx::new, and the unused
ImageEndpointDef/ImageMemoryMapping re-export aliases
- fix stale docs: "code regions" -> single code region; SCALE -> SSZ
(Image is ssz_derive, not SCALE); drop the nonexistent wire.rs note
Kept the tested hash-chain API (image_content_hash/chain_genesis/
chain_extend) and MemoryMapping::path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- delete the unused frame module (MainFrame/BareFrame, 196 lines) — slot resolution moved onto InstanceEntry after the cache migration - remove zero-caller accessors: CallStack::running_mut, Entry::as_instance, Entry::as_instance_mut, ImageCache::insert - fix stale docs: ecall is custom-0 funct3=001/010 (not "opcode 3/10"), drop the unread phi[12] claim; qualify enforce_invariants as test-only Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- delete CODE_CAP_INDEX (code is the Image's fixed-base `code` region now, not cap index 64) and the false "init_cap field" doc - fix the low-code/high-data layout docs: data is relocated to [DATA_BASE, ..), not its page-0 address; add Zicclsm to the ISA list Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- remove the artificial `let _ = cap_abi::BARE_GAS_SLOT;` keep-alive and its import (crate::abi stays live via real uses) - fix stale docs: the placeholder image sidesteps validation by being non-empty (no instruction-start bitmap exists); SCALE -> SSZ; fix the duplicated genesis step numbering Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- delete dead leaves_at_depth, SparseList::entries_count, and the `let _ = read_slice;` / `let _ = pair;` no-op residue - fix stale docs: the sparse subtree walk is recursive (not "iterative with a fixed-size stack"); zero_hash is recomputed per call (not cached) Kept the codec library API (U256, from_vec/into_inner, Bitlist::data_bytes) and the subtree-root cache. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
map_args encoded a removed kernel ABI (MGMT_MAP/set_args, phi[11]/phi[12] operands, a nonexistent javm_legacy crate) and had zero callers. Deleting it also clears every stale comment it carried; the crate's public surface is now EndpointDescriptor + the endpoint macro. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- fix a latent build break: Nub::heap_stats matched `Backend::Local(_)` but Backend::Local became a struct variant — `cargo build -p nub --features heap-diag` (exercised by javm-bench) failed with E0164 - delete dead GUEST_FN_TABLE_SIZE (no fixed-size dispatch table exists) - fix stale ABI docs: the guest cap store is CACHE (CacheDirectory<Arc<Cap>>), not a `Mutex<HashMap<.., Box<Cap>>>` DIRECTORY; directory_va points at the whole CacheDirectory; run_pvm_with_mem -> enter_frame; SCALE -> opaque; "byte-PVM" -> "PVM2 (RISC-V)" Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
javm-recompiler-x86: - delete ~30 unused pub Assembler emit helpers (legacy memory-gas, perm-table probe, SIB base+index load/store, native call/push, and unused rotate/shift wrappers) plus the now-orphaned private modrm/rex helpers - delete dead consts CTX_HEAP_BASE/CTX_HEAP_TOP/CTX_FAST_REENTRY, the unused Predecode re-export, and the write-only HelperFns::sbrk_helper field (and its two initializers in nub-arch-x86 / the compile_profile example) - gate test-only sync_len behind cfg(test) - fix stale docs: no callf/retf native call frames (jalr is jmp_reg); the mmap/std-gated emit path was retired (Vec only); CTX is at 512 GiB; drop the "PVM path" framing (the only path is RV streaming) nub-arch-x86: - delete unused Perm::kernel_rw - doc sweep: arena is DISPATCH|JIT|TRAMP (no BB region); CTX/arena at 512 GiB with a real [0, CODE_BASE) null guard; run_pvm_with_mem -> enter_frame; publish_instance -> publish_transient_instance; lookup_handle -> CACHE.get; CapHandle -> CapHashOrRef::Ref; fix the scratch-sweep / refcount contradictions Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- delete dead run_recompiler_cold and the redundant cfg_attr(allow(unused)) - fix stale docs/strings: real bench file names (bench.rs, sub_vm_*); SCALE -> SSZ; compile_profile code_base -> CODE_BASE (0x0040_0000, not the old 1 GiB); complete the ISA strings (RV64EMC + Zbb/Zba/Zbs/Zicond/Zicclsm); attribute the JIT recompiler to nub::Nub / javm-recompiler-x86 (not javm-exec) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nub-host-kvm / nub-host-common have diverged far from upstream Hyperlight into a bespoke KVM-ioctls host; we are the sole maintainer with no merge path back. This removes everything outside the supported config (KVM + Linux + x86_64), ~5.8k lines: - delete always-false cfg gates whose alias/feature was already removed: cfg(mshv3), cfg(gdb), cfg(crashdump), cfg(unshared_snapshot_mem), cfg(trace_guest), cfg(mem_profile), cfg(nanvix*), feature="i686-guest"; unwrap each paired live cfg(not(..)) branch - delete unsupported-platform code: all cfg(target_os="windows") / cfg(target_arch="aarch64") branches, incl. whole files (the aarch64 hyperlight_vm / kvm / regs backends, the i686/aarch64 arch page-walkers in nub-host-common, the hw-interrupts x86_64 module, the Windows-only wrappers.rs) - drop the now-unused features (function_call_metrics, hw-interrupts) and the stale unexpected_cfgs allow-list; remove the orphaned guest-counter test helper - fix stale comments referencing the removed backends cargo build/clippy -D warnings/test clean; smoke 12/12 bit-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- remove the unused pub host_comm::encode_response (a local addition with no callers; the live error-response path uses the private encoder in guest_function/call.rs) and trim the now-unused Response import - fix stale docs: entrypoint sets up GDT/TSS/IDT + stack (the in-guest CoW handler was removed in F3.3); the host no longer enforces a version match (F4.2), so the 0.15.0 pin is for dep parity, not an ABI lock Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lace The `(mem_size, overlays)` derivation from an Image's memory_mappings + pinned/initial slot contents was copy-pasted in four spots — the recomp bench harness, the kernel genesis path, and the two guest-test paths (incl. the conformance oracle). The recompiler runtime and the interpreter conformance oracle MUST agree on Instance memory layout, yet each kept its own copy that could silently drift. Replace all four with a single `Image::data_overlays()` method in javm-cap, the canonical source of truth. Smoke 12/12; conformance + sub_vm tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CodeBuf had one variant (Vec) since the host-targeted mmap-direct emit path was retired. Replace the field with a plain Vec<u8>, drop the six irrefutable-let matches, and fix the stale "Vec or mmap region" note in the safety model. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The host is KVM-only (Linux); it never runs on macOS, so the macOS guest-VA-reservation path (dynamic kernel-chosen base + retry loop) was dead. Remove it and simplify the unsupported-OS guard to `not(linux)`. The runtime base stays parameterizable via `guest_va_base()` — `JAR_GUEST_VA_BASE` env override, else `GUEST_VA_BASE_DEFAULT`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Trim forked-but-unused public items (no callers anywhere; the crate is guest-only so the host build never lints them): - the free `virt_to_phys` / `phys_to_virt` wrappers + the AsRef impl that only served them (the `phys_to_virt`/`try_phys_to_virt` *methods* stay) - `OS_PAGE_SIZE` (write-only) and its init-time store - the `error` re-export module (consumers import hyperlight_guest::error directly) Smoke 12/12 (guest builds + runs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The fpu/sregs/debug_regs/xsave getters on the VirtualMachine trait had zero callers once the gdb/crashdump consumers were stripped — they only survived behind #[allow(dead_code)]. Remove them from the trait + the KvmVm impl. This makes the whole `regs/x86_64/debug_regs.rs` module dead (CommonDebugRegs was only its return type), so delete it and its wiring, and trim RegisterError to the four variants still constructed (GetRegs/SetRegs/SetFpu/SetSregs). The live path keeps regs() + the setters. clippy -D warnings + nub-host-kvm tests pass; smoke 12/12. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
InstBuf is an asm.rs-internal emission detail (no consumer elsewhere). Make it and its methods private; that drops the `len_without_is_empty` and `new_without_default` clippy obligations, so the unused `is_empty` method and the `Default` impl can go. Privatizing also lets the dead_code lint police InstBuf's internals going forward. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sub_vm_recurse.rs and sub_vm_data_recurse.rs were ~95% identical (Built struct, build_and_publish, invoke, hex_short, criterion driver) and each re-derived Instance overlays inline. Promote `criterion` to a normal (target-scoped) dependency — javm-bench is a benchmark crate — so the whole driver can live in src/lib.rs: - add `build_sub_vm_top` (using the canonical `Image::data_overlays()`), `invoke_sub_vm`, `hex_short`, and the `run_recurse_bench` criterion driver to the lib - collapse each bench file to a blob const + a one-line call (237/209 -> ~40 lines), dropping the dead `depth_seed` parameter - fix two stale "byte-PVM" mentions (it is the PVM2 / RISC-V interpreter) Both benches pass `cargo bench -- --test`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Genesis ReviewComparison targets:
How to reviewPost a comment with the following format (rank from best to worst): Use the short commit hashes above and To meta-review another reviewer's comment, react with 👍 or 👎. |
Contributor
Author
|
/review Whole- One real bug surfaced and fixed along the way: The strongest design signal is the explicit Deliberately not done — ssz offset-table dedup callout with the reason ("merging would change malformed-input rejection behavior in the consensus serialization codec"). Knowing where to stop and saying why you stopped is harder than doing the dedup. |
Contributor
|
JAR Bot: Quorum reached — triggering merge. |
Contributor
Author
|
JAR Bot: Merged (quorum reached). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A whole-
rust/cleanup sweep: removes dead code, corrects stale comments left by the PVM2 take2 redesign, prunes the diverged vendored host stack down to what we actually build, and lands a few worthwhile refactors. Net −7.5k lines (14 files deleted), no behavior change to the supported path.Highlights
Nub::heap_statsfailed to compile (E0164) under theheap-diagfeature (matchedBackend::Local(_)after it became a struct variant) — a pathjavm-bench'sheap_drifttest exercises.Our crates — dead code + stale comments
error/framemodules,rv_fast_cost,CODE_CAP_INDEX, ABI-stalemap_args, the collapsed-Image cnode/cap orphans, ssz residue), dropped a now-unusedthiserrordep, and corrected ~80 stale comments (br_table/bb_starts/Harvard/run_pvm_with_mem/SCALE→SSZ/opcode-3-vs-custom-0/CTX-at-4 GiB).Assembleremit helpers + dead consts + the write-onlysbrk_helperfield; doc sweep for the take2 vocabulary drift.Vendored
nub-host-*— strip to the supported config (KVM + Linux + x86_64)These have diverged far from upstream Hyperlight into a bespoke KVM-ioctls host (sole-maintained, no merge path back):
mshv3/gdb/crashdump/unshared_snapshot_mem/trace_guest/mem_profile/nanvix/i686-guest) and all Windows/aarch64/macOS code — ~5.8k lines incl. 14 whole files (aarch64 backends, hw-interrupts, Windowswrappers.rs, i686/aarch64 page-walkers).unexpected_cfgsallow-list; removed the deadVirtualMachineregister getters +debug_regs.rs+ the host's macOS VA-reservation path (the runtime base stays parameterizable viaguest_va_base()/JAR_GUEST_VA_BASE).nub-arch-guestbinupstream-parity API surface.Refactors
Image::data_overlays()— hoisted the Instance memory-overlay derivation (copied in 4–5 places, incl. the recompiler path and the conformance oracle, which must agree) into one canonical method in javm-cap.CodeBufenum and privatizedInstBufin the recompiler.run_recurse_benchdriver (promotingcriterionto a normal dep for this benchmark crate).Deliberately not done
DecodeErrorvariants,expected_lenhandling, and entry shape; merging would change malformed-input rejection behavior in the consensus serialization codec. Left as-is (matches the audit's "flag only").Verification
cargo fmt --check,clippy --workspace --all-targets -D warnings,cargo doc -D warnings,cargo test --workspace --release, and the PVM2smokegate (interp == recomp, 12 workloads, bit-identical) all pass.🤖 Generated with Claude Code