Conversation
messense
force-pushed
the
macho-pe
branch
2 times, most recently
from
cc74f99 to
f334ab1
Compare
MachO: - Handle @rpath/, @loader_path/, @executable_path/ prefixes - Load DYLD_LIBRARY_PATH and DYLD_FALLBACK_LIBRARY_PATH - Default fallback paths: ~/lib, /usr/local/lib, /lib, /usr/lib - Support fat/universal binaries (select native arch) PE: - Search application directory first - Search Windows system directories (System32, System, Windows) - Search current directory and PATH environment variable - Support Wine-style directory layouts Also refactored: - Format-aware path loading (load_elf_paths, load_macho_paths, load_pe_paths) - Format-dispatched find_library (find_elf_library, find_macho_library, find_pe_library) - Shared try_library_candidates for compatibility checking - Added BinaryFormat enum and format() method to InspectDylib trait
- elf.rs: remove unnecessary clone() on Copy type (Option<&str>) - ld_so_conf.rs: use strip_prefix() instead of manual slicing - lib.rs: use if-let with .next() instead of for loop that never loops
Only search current directory and PATH environment variable when analyzing against the real filesystem root on Windows (e.g., C:\). This mirrors how ELF only uses LD_LIBRARY_PATH when root is '/'. Previously, PATH was always added to search paths, causing the analyzer to find real system DLLs (like KERNEL32.dll) and recursively resolve all their transitive dependencies, even when analyzing with a custom sysroot.
MachO fixes: - Track @loader_path per-dependency: each library in the dependency chain now correctly uses its own path for @loader_path resolution, instead of always using the top-level binary's path - Use per-library rpaths: transitive dependencies are resolved using the intermediate library's LC_RPATH entries, not the top-level binary's - Handle fat/universal Mach-O binaries in dependency resolution: when a dependent library is a fat binary, extract the compatible architecture slice instead of skipping it entirely PE fixes: - Skip API set DLLs (api-ms-win-*, ext-ms-win-*): these are virtual DLLs resolved by Windows at runtime via an API set schema. They never exist on disk, so searching for them is pointless and expensive - Case-insensitive DLL matching: Windows filesystems are case-insensitive but cross-platform analysis may run on case-sensitive filesystems (Linux). Added find_file_case_insensitive() with fast exact-match path and slow directory-scan fallback Also: - Removed unused self.rpaths field from DependencyAnalyzer (was never populated); rpaths now flow through the dependency stack per-library - Extracted try_single_candidate() from try_library_candidates() to enable PE's case-insensitive search to reuse the parsing/compat logic - Added not_found_library() and is_api_set_dll() helpers - Added unit test for API set DLL detection - Thorough documentation of tricky behaviors in code comments
MachO - @rpath fallback with full suffix: When @rpath/ resolution fails against all rpaths, also try the rpath suffix (e.g., 'subdir/libfoo.dylib' from '@rpath/subdir/libfoo.dylib') against DYLD_FALLBACK_LIBRARY_PATH and additional paths. Previously only the leaf filename was tried in fallback, which would miss libraries installed in subdirectories. This matches delocate's behavior of appending /usr/local/lib and /usr/lib as fallback @rpath search dirs. Also restructured fallback search to only apply leaf-filename fallback for non-@rpath install names (absolute paths, @executable_path, etc.), keeping the two code paths clearly separated. PE - SysWOW64 support: Added Windows/SysWOW64 (and lowercase/Wine variants) to the system directory search list. On 64-bit Windows, System32 contains 64-bit DLLs while SysWOW64 contains 32-bit DLLs. The WoW64 File System Redirector transparently maps System32 accesses from 32-bit processes to SysWOW64, but since we don't emulate this redirector, we search both directories and rely on compatible() to select the correct architecture match.
…andling - Remove unused 'format' field from DependencyAnalyzer (was set but never read; dylib.format() is used for dispatch instead) - Extract check_compatible() helper to deduplicate the identical 'if compatible → read_rpath + libraries' pattern across ELF, MachO::Binary, and PE arms in try_single_candidate - Replace redundant if-let with let-else in fat binary handling (the inner match is always true since we just constructed the Object) - Add LibInfo type alias to avoid clippy type_complexity warning
On Windows, real system DLLs (KERNEL32.dll etc.) are found in C:\Windows\System32, parsed, and their transitive dependencies discovered — so libraries.len() grows well beyond the 4 direct deps. On Linux/macOS those system dirs don't exist, so all non-API-set libs are just recorded as not-found. Replace the fragile assert_eq!(libraries.len(), 4) with: - Assert all needed libs exist in the dependency map - Assert API set DLLs are never found (they're virtual) - Assert libraries.len() >= 4 (allows transitive deps on Windows)
- actions/checkout@v2 → @v4 - Replace deprecated actions-rs/toolchain@v1 and actions-rs/cargo@v1 with dtolnay/rust-toolchain@stable and direct cargo commands - Add clippy job with -D warnings - Install rustfmt/clippy via toolchain components instead of rustup
Prefix dylib_path with underscore in load_elf_paths — the parameter is only used inside a #[cfg(unix)] block for LD_LIBRARY_PATH, so it's unused on Windows.
- Fat binary: select arch directly in single pass instead of collecting into Vec, selecting by index, then re-iterating with nth() - PE find_library: merge duplicate search loops into single chained iterator - Fix clippy nonminimal_bool warning in cfg-gated arch detection
There was a problem hiding this comment.
Pull request overview
This PR expands lddtree from ELF-only dependency analysis to support Mach-O (macOS) and PE (Windows) binaries, updating the analyzer to parse binaries via goblin::Object and adding new test fixtures to validate behavior across formats.
Changes:
- Refactors dependency inspection behind a new internal
InspectDylibtrait and adds format-specific implementations for ELF/Mach-O/PE. - Adds Mach-O and PE resolution logic (including macOS
@loader_path/@executable_pathhandling and Windows API set DLL skipping / case-insensitive lookup). - Extends CI and adds new binary fixtures + tests for Mach-O and PE parsing.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib.rs |
Core refactor: multi-format parsing, per-format path loading, new resolution stack model, PE helpers (API sets + case-insensitive lookup). |
src/elf.rs |
Implements InspectDylib for ELF and restores ELF compatibility checks in the new model. |
src/macho.rs |
Implements InspectDylib for Mach-O and Mach-O compatibility logic. |
src/pe.rs |
Implements InspectDylib for PE and PE compatibility logic. |
src/errors.rs |
Adds UnsupportedBinary error variant used by the new multi-format dispatcher. |
src/ld_so_conf.rs |
Small parsing improvement using strip_prefix for include lines. |
tests/test_lddtree.rs |
Renames ELF test and adds Mach-O and PE tests. |
tests/test.macho |
Adds Mach-O fixture binary for tests. |
tests/test.pe |
Adds PE fixture binary for tests. |
.github/workflows/CI.yml |
Updates actions/toolchain setup and expands checks to include fmt + clippy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Guard MachO::libraries() against empty self.libs to prevent panic - Use is_file() instead of exists() in find_file_case_insensitive to skip directories and non-regular files - Resolve Mach-O absolute install names through sysroot (root) for cross-compilation SDK support - Relax test_macho assertion to >= 4 (transitive deps found on macOS) - Add .gitattributes to mark test fixtures as binary (prevents line-ending corruption with core.autocrlf)
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
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.
find_libraryfind_library