Fix .so build-id remap (absolute offsets) + register lock; add regression tests#1
Merged
Merged
Conversation
… lock
Two real bugs found while proving source-free loading + remap of Piccolissimo
package images end-to-end:
1. header.jl: _parse_so_header_by_scan parsed the embedded JI header from an
IOBuffer view starting at the header's location in the .so, so the recorded
build-id file offsets were RELATIVE to that location, not absolute. remap()
then seeked those tiny offsets in the real file and clobbered the ELF program
headers ("ELF load command not page-aligned"). Fix: _shift_header_offsets()
converts .so offsets to absolute. .so remap is now readelf-valid and
read_verify_mod_list passes.
2. loader.jl: register_restored_modules was called outside Base.require_lock,
but it asserts the lock is held -> ConcurrencyViolationError on Julia 1.12.
Wrapped in @lock Base.require_lock.
Also: fix placeholder UUIDs (package UUID + TestItemRunner test-dep UUID) so the
suite is resolvable, and add test/test_so_remap.jl regression tests asserting
.so offsets point at the actual build-id bytes and remap keeps the file intact.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ulia CI Adds test/test_register_lock.jl, a regression test for the loader.jl register fix (`restored = @lock Base.require_lock Base.register_restored_modules(...)`). On Julia 1.12, register_restored_modules opens with assert_havelock(require_lock) and mutates Base's global module registry (loaded_modules_order, loaded_precompiles, pkgorigins); calling it without the lock throws ConcurrencyViolationError, and concurrent unsynchronized loads would race the registry. The test exercises register_restored_modules directly (the exact call the fix wraps) via a synthetic, depot-independent SimpleVector: - asserts the no-lock call throws ConcurrencyViolationError (the original bug) and the @lock call succeeds; - spawns many concurrent locked registrations (@sync + Threads.@Spawn) and asserts none raise and the global registry stays consistent. A full end-to-end load_package_image round-trip is intentionally not exercised: it needs a precompiled package-image fixture built with session-matching cache flags whose deps are already loaded; arbitrary depot images fail in C deserialization ("Pkgimage flags mismatch") before the locked line is reached. This limitation is documented in the test. These tests may need a precompiled depot fixture for any future end-to-end coverage. Adds .github/workflows/CI.yml (the repo had no .github/): minimal Julia CI on 1.12 / ubuntu-latest / x64 (checkout, setup-julia, cache, buildpkg, runtest), mirroring sibling Harmoniqs Julia repos. NOTE: several existing tests scan Base.DEPOT_PATH for real .ji/.so fixtures, which a clean CI runner lacks unless a precompiled-depot fixture is provisioned; noted inline in the workflow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…xture); add gated R2 bundle download to CI Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Two real bugs found proving source-free load + remap end-to-end: (1) header.jl recorded .so build-id offsets relative to the embedded-header location, so remap clobbered the ELF program headers — fixed with _shift_header_offsets(); (2) loader.jl called register_restored_modules outside Base.require_lock → ConcurrencyViolationError on 1.12. Also fixes placeholder UUIDs + adds test/test_so_remap.jl. 135 tests pass; bundle loads 516 modules + solves.
🤖 Generated with Claude Code