[idb_import] Broaden IDB import: function folders, variables, value locations, and type fidelity#8245
Open
ChrisKader wants to merge 16 commits into
Open
[idb_import] Broaden IDB import: function folders, variables, value locations, and type fidelity#8245ChrisKader wants to merge 16 commits into
ChrisKader wants to merge 16 commits into
Conversation
…ction When an IDB only records a section-relative base address (loading_base of zero, so we fall back to min_ea as a BaseSection), the rebase delta was computed against the lowest mapped *section* in the view. That over-shifts every imported address for formats where the first section starts after the file header. IDA's min_ea is the image base and maps the file header too, whereas a Mach-O's first section (__text) begins after the header and load commands. Aligning against the lowest mapped segment (segments include the header region) yields the correct delta and stops imported addresses from being shifted by the header size.
The "IDB Import refactor" introduced translate.rs (TILTranslator) as the type translator used by the mapper, but left the previous translator in types.rs behind. The module was never re-declared in lib.rs, so it has not been compiled or referenced since the refactor. Removing it drops a large block of dead code along with its stale TODOs; TILTranslator is now the single source of truth for IDB->BN type translation.
Resolve the outstanding translation TODOs in the TIL translator: - Size the variable-width C basic types (bool/short/int/long/long long/ long double) from the TIL header's compiler sizing info when a TIL is attached, falling back to the standard C ABI defaults. Both build_basic_ty and width_of_type now share these sizes so referenced-type placeholder widths stay in step with the types they stand in for. - Translate BoolSized to a real width: a 1-byte bool stays bool, any other width becomes an unsigned int of that size (BN bool is always one byte). - Honor pointer __ptr32 / __ptr64 modifiers to override the platform address size, and document that based/shifted pointers have no BN representation. - Detect variadic functions via the ellipsis calling convention instead of hardcoding has_variable_args to false. - Add udt extra_padding to the computed structure width so fixed-size UDTs occupy their true storage size. - Document the resolved design decisions for grouped (bitmask) enums, flexible array members, struct/union placeholder widths, the function return location, and the authoritative pointer address size.
- merged_types: carry an ordinal across the dedup when the kept entry lacks one, keeping name/ordinal lookups resolvable, and document that dir_tree types are clones of the same TIL definitions so no body merge is needed. - TIL decompression: read_til already inflates Zlib/Zstd sections via its section header, so document that and drop the stale "decompress til" TODO. - Function registers/stack variables: replace the dead exploratory block with a note scoping it as a follow-up feature (needs FunctionInfo and mapper support to apply named stack variables and register names).
- Populate IDBInfo.sha256 from the input file SHA256 recorded in the IDB so it is no longer always None, and drop the stale placeholder comment. - Mapper logs the recorded SHA256 and documents a future IDB verifier that would compare it against the mapped view before applying data. - Define the fallback `size_t` only when the view lacks one, so a real platform/view definition is never clobbered. - Document that the undo bracketing requires the mapper to be the sole writer, an invariant the run-once loader activity already guarantees. - Document the name-based (not range-based) section dedup rationale: the BN loader already maps the address space, so a range check would suppress every IDA segment. - Replace the remaining design-question TODOs (used-type ordering, attached TIL lookup, per-function platform tuple, OpenFileName filter naming) with decisions/notes explaining the current behavior and future direction.
The IDB records the SHA256 of its original input file. Walk to the root of the view's parent chain (the raw view, whose bytes are that on-disk file), hash it in 1 MiB chunks, and compare against the recorded hash. On mismatch we warn that the imported data may not correspond to the binary; on match we log the verification at debug level.
- Argument locations: translate IDA stack-passed argument locations (ArgLoc::Stack) into Binary Ninja parameter stack locations so explicit stack parameter placement is preserved. Register-encoded locations carry raw IDA register indices with no portable BN mapping and are left for analysis to derive. - Register variables: parse IDA "regvars" (a register renamed by the user within a function) into FunctionInfo, carrying them through the function merge, and apply them in the mapper by resolving the register by name and creating a user variable typed to the register width.
Parse each function's stack frame (named locals, saved registers and stack arguments) from the IDB along with its geometry (frsize/frregs), carry it on FunctionInfo through the function merge, and apply it in the mapper. IDA records the frame as a structure running from the bottom of the locals upward; Binary Ninja measures stack offsets from the return address, so an IDA frame offset is shifted down by local_size + saved_regs_size. Member offsets are the running sum of preceding member widths (the frame members carry no explicit offset), and the synthetic saved-register/return-address members are skipped while still advancing the offset. Variables are created as auto stack variables typed from their translated IDB types.
Two pieces of IDB data were parsed but never applied to the view: - is_no_return: mark functions IDA flags as non-returning (abort/exit/etc.) with set_auto_can_return(false) so analysis does not fall through calls to them. - Local labels: IDA's in-function named locations were folded into the name list, where map_name_to_view skips anything inside code. Route them through the dedicated map_label_to_view so they land as local-label symbols.
IDA lets users organize functions into folders in the Functions window, stored as a dirtree. Parse that hierarchy (preserving nested folders, not just the leaf functions) into FunctionFolderEntry, and recreate it in the view as Binary Ninja components: each folder becomes a component nested under its parent, and every function leaf is added to its folder's component. Functions sitting at the dirtree root are left uncomponented, matching their "no folder" state in IDA.
Expose the processor's register names (indexed by IDA register number) from the database and hand them to the type translator along with the architecture. Argument locations encoded as registers (Reg1, the Reg2 register pair, and register-relative RRel) are now resolved through those names into Binary Ninja registers and emitted as value locations, in addition to the stack locations already handled. Forms with no equivalent (distributed, static, custom) still fall back to the calling convention.
Function folders in Binary Ninja's symbol list are backed by the component API (the docs describe creating them "automatically via the API", linking to binaryninja.component.Component), so the component approach is correct. Improve the mapping so it does not depend on analysis having indexed the functions yet: capture the Ref<Function> returned when each function is created and key it by rebased address, then place those into folders directly (falling back to a view lookup only when needed). Add a summary log line reporting how many folders were created, how many functions were placed, and how many could not be found, and align terminology to "folder" to match the UI while the underlying type stays a component.
Reuse the register/stack location resolver to honor a function's explicit return location (function retloc) when the database records one, attaching it to the BN return value at full confidence. Functions without an explicit return location, or whose location cannot be resolved, keep the calling-convention-derived return as before.
A segment that covers the exact same address range as an existing section is the same region under a possibly different name, so skip it rather than add a duplicate. We still avoid an overlap-based check, which would wrongly suppress every segment because the loader maps the whole address space.
The lowest-segment rebasing fix is format agnostic; reword its comment so it no longer reads as Mach-O specific. The first section starting after the format headers (Mach-O load commands, PE headers, ELF program headers) is a general property, and aligning to the lowest segment matches IDA's image base regardless of format.
idb-rs now parses a bare unknown type (unspecified size) instead of erroring, so handle it here: a zero-width unknown has no integer representation, so map it to void rather than constructing a zero-width int.
Member
|
This is great thank you for the PR! |
Author
|
Is there anything you need me to do? I am trying to transition to BN from using IDA Pro as my main RE tool so I expect more PRs to come for this plugin. |
Member
|
Nope! We will have this reviewed and go from there, thanks for the PR! |
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.
Substantially expands what the IDB importer brings over from an IDB/I64 and applies to the Binary Ninja database, plus correctness and type-translation improvements. Verified end-to-end against a large arm64 database.
New data imported
Type translation fidelity
Integrity & correctness
Cleanup
types.rstranslator (superseded by the active translator) and resolves the plugin's outstanding TODOs.