| Age | Commit message (Collapse) | Author |
|---|
|
The `FileAccessorCache`'s LRU eviction could discard a file's mapped
data after slide info had already been applied to it. Future accesses to
the file produced a fresh mapping, but failed to reapply the slide info.
This could result in pointers not correctly being slid, such as in
https://github.com/Vector35/binaryninja-api/issues/7689.
The LRU cache existed to stay within OS file descriptor limits, since
the old `MappedFile` held its fd open for the lifetime of the mapping.
There's no real reason for it to hold the file descriptor open like
this. Closing it after `mmap` is sufficient to avoid the file descriptor
limits.
`MappedFileRegion` replaces the combination of `FileAccessorCache`,
`WeakFileAccessor`, and `MappedFileAccessor`. It closes the fd
immediately after mmap, so all files can stay mapped without consuming
descriptors, making the cache unnecessary.
`MappedFileRegion` is owned directly by the `CacheEntry` for its full
lifetime. Slide info is applied exactly once to each `MappedFileRegion`.
|
|
|
|
In some iOS 15 caches, the .symbols file's mapping has an address of 0.
This would cause it to be returned by `SharedCache::GetEntryContaining`
and loaded into the view.
The .symbols file contains the local symbol tables for images in the
shared cache. It is not intended to be mapped into the same address
space as the rest of the shared cache.
`SharedCache` now tracks the symbols cache entry separately from other
entries. A dedicated `VirtualMemory` region is used when accessing the
data it contains. This could be a `FileAccessor`, but that would require
additional changes within `SharedCacheMachOHeader`.
`SharedCacheMachOProcessor` now directly accesses the local symbols
cache entry rather than needing to search for it.
|
|
|
|
`GetRegionAt` was really providing the semantics of
`GetRegionContaining` due to `m_regions` being an `AddressRangeMap` and
using transparent comparators to allow `find` to work with any address
in the range.
`GetRegionAt` is updated to verify that the start address of the region
that was found matches the requested address. `GetRegionContaining` is
updated to use `AddressRangeMap::find` rather than doing a linear search
over all regions.
|
|
This occurred because prior to the change made in build `7484` we short-circuited when the pointer pointed to a read only segment, now that we consult the section the sections semantics were incorrect and failed, this fixes the dyld data sections to have correct semantics.
|
|
Fixed branch island regions being skipped
|
|
of the header
We did not truncate the buffer length when reading the header using the mappingOffset size, resulting in a buffer overflow.
|
|
Following the conversation in https://github.com/Vector35/binaryninja-api/issues/6631 we are going to make the edited alerts warnings and then also fix the warning showing on certain shared caches even when all entries are present.
|
|
- Make `SharedCache::m_entries` a simple std::vector we never lookup based off the entry ID
- Remove some old comments
- Rename some old variables so that they are accurate
- Fix compiler warnings
- Remove some unneeded copying
|
|
Prior to this we would not have meaningful errors shown to the user when we fail to construct a CacheEntry from a file.
Apart of trying to make the opening of shared caches clearer.
|
|
This improves symbol recovery drastically on newer shared caches
Related PR: https://github.com/Vector35/binaryninja-api/pull/6210
|
|
We already had separated this part out into the `CacheProcessor` but its more like a builder than anything, so this refactors that out into a `SharedCacheBuilder`.
- Separate out the `CacheProcessor` and simplify its implementation
- Move more implementation specific stuff to the shared cache view
- Prepare for more validation of the shared cache to detect irregular or incomplete shared cache information
- Deduplicate a lot of file vs. project file lookup stuff
- Stop copying all images when getting the number of images to log
- Allow user to select another directory to scan for shared cache files, might make this a warning instead to prevent users from relying on this behavior
We will likely follow this commit up soon with better open behavior, maybe open with options can specify a list of cache entry files? Shouldn't be too much effort aside from making the UI modal for that.
|
|
Fixes the case of stub functions being applied with mangled names.
|
|
This gives the user the ability to take a database with no original file path continue processing entries with the user supplied base file.
This also will set the original file path for future loads, we might want to make that optional. And the behavior with remote projects for that might be undesired.
|
|
Fixes https://github.com/Vector35/binaryninja-api/issues/6582
More considerations might need to be made for databases saved without the original file path. Likely solved through some shared cache specific metadata.
|
|
It is modified on a worker thread so if a user tries to call `GetSymbolWithName` after view init but before the processing on the worker thread finishes, there would have been issues!
|
|
Also swapped out CacheEntry::m_images to a vector as its never actually used as a lookup
|
|
slide info
This improves performance by ~10x with the only real issue being a copy of the cache entry existing for each weak ref.
Also fixes
- Some old TODO's that are no longer relevant
- Some function signatures not being const
- FileAccessorCache not evicting enough accessors to fit under an adjusted cache size
- Added a warning if we are over the global cache size in view initialization
- Logger name not having a `.` in `SlideInfoProcessor::SlideInfoProcessor`
- Re-added the accessor dirty check before applying slide info to fix https://github.com/Vector35/binaryninja-api/issues/6570
|
|
Fixes https://github.com/Vector35/binaryninja-api/issues/6561
- Also tightens the SharedCache class to move only, to prevent accidental copies.
- Also removes some extra copies in FFI when should pass by ref
- Also adds `get_symbol_with_name` to python API
The current named symbol map is populated in a worker thread spawned in the view init. This is because populating the map can take about 1 second.
If we are fine with another 1 second added to the view init time then we can add it serially but I don't think this way is _that_ bad, no analysis consults
this, however a user might add a workflow that would be racing this. So we need to add a mutex.
|
|
|
|
|
|
|
|
In absence of a better name, this commit refactors the shared cache code.
|
|
This makes it so that we persist the symbols within the regular symbol list in storage
|
|
gracefully now
|
|
Slide info is parsed and applied on the main thread, below the
`SharedCache` constructor, when a shared cache is opened. Time spent
applying the slide is time that the main thread is blocked.
These changes eliminate some unnecessary overhead so what work remains
is dominated by kernel work (paging in data, copying pages when the
first modification is made).
The changes are:
1. Read slide pointers via `ReadULong` rather than the variable-length
`Read` method. The compiler isn't able to eliminate the call to
`memcpy` in the variable-length `Read` function, and the function
call overhead is noticeable given the small size of the read and
number of times it is called.
2. Remove the file member from `MappingInfo`. Slide info is applied for
a single file at a time so there's no reason to track the file it
belongs to. This removes unnecesssary reference counting on the
`std::shared_ptr` that holds the `MMappedFileAccessor`.
|
|
Still need to decide on how we want handle this, because we need multiple files we probably just want to prompt the user for a directory?
|
|
This warning will happen anytime you load an image with already processed regions.
|
|
Both the Macho and DSC views need to process Objective-C but have separate processor classes. It would appear that the DSC version was largely a copy and paste of the Macho view one, with some modifications. The majority of code overlaps between the 2 so it doesn't make sense to maintain 2 and copy and paste improvements/fixes between them.
This commit fixes that by creating a base Objective-C processor that contains the shared code. View specific code is implemented in the respective subclasses for the views. Although there is very little view specific code for each.
|
|
|
|
`MemoryRegion` now tracks the start address of the image to which it
belongs. `SharedCache::HeaderForAddress` uses the existing address range
map to quickly find the `MemoryRegion` for a given address, and from
there can look up the image via its start address.
Some extra logic is added to `CacheInfo::Load` to populate the image
start address on `MemoryRegion` when loading from metadata that predates
this change.
The result is that `HeaderForAddress` is no longer the most expensive
part of `FindSymbolAtAddrAndApplyToAddr`.
|
|
Use the LLVM demangler to demangle relevant symbols
|
|
|
|
Also we now initialize m_cacheInfo when calling `DeserializeFromRawView` the responsibility is on the caller to handle
|
|
version
This does not actually give the user control to exit early, that looks to need core changes.
|
|
to the regions being loaded
The logic before would default to adding a section unless it was from a region in the `regionsToLoad` argument and its header had already been initialized. However if a user does `Load Section by Address` then there's only one region in `regionsToLoad`. All sections not in that region would be automatically added, even if they had already been or the user hadn't requested them to be loaded.
|
|
Also fixes a BinaryViewRef leak
|
|
MMappedFileAccessor::Open
Nothing guarantees that the `SharedCache` lives long enough.
|
|
There was a second BeginBulkModifySymbols after rebasing the new processing symbols commit
|
|
This is _probably_ the fix? I am not actually sure I am pushing so the CI can figure it out for me :)
|
|
Split out from https://github.com/WeiN76LQh/binaryninja-api/tree/process-local-symbols
|
|
Split out from https://github.com/WeiN76LQh/binaryninja-api/tree/process-local-symbols
|
|
The initial state is initialized during `PerformInitialLoad` and is
immutable after that point. This required some slight restructuring of
how information about memory regions is tracked as that was previously
modified as regions were loaded. Memory regions are now stored in a map
from their address range to the `MemoryRegion` object. This makes it
cheap to look them up by address which is a common operation.
The modified state consists of changes since the last save to the
`DSCView` / `ViewSpecificState`. This means it is no longer necessary to
copy any state when mutating a `SharedCache` instance for the first
time. Instead, its data structures start off empty and are populated as
images, sections, or symbol information is loaded.
The loaded state consists of all modified state that has since been
saved. It lives on the `ViewSpecificState`. Saving modified state
merges it into the the existing loaded state.
This pattern is carried over to the `Metadata` stored on the `DSCView`.
The initial state is stored under its own metadata key, and each
modified state is stored under a key with an incrementing number. This
means each save of the state only needs to serialize the state that
changed, rather than reserializing all of the state all of the time.
There are two huge benefits from these changes:
1. At no point does `SharedCache` have to copy its in memory state.
The basic copy-on-write approach introduced in #6129 reduced how
often these copies are made, but they're still frequent and very
expensive.
1. At no point does `SharedCache` have to re-serialize state to JSON
that it has already serialized. JSON serialization previously added
hundreds of milliseconds to any mutating operation on `SharedCache`.
As a result, this speeds up the initial load of the shared cache by
around 2x and loading of subsequent images improves by about the same.
One trade-off is that the serialization / deserialization logic is more
complicated. There are two reasons for this:
1. The state is now split across multiple metadata keys and needs to be
merged when it is loaded.
2. The in-memory representation uses pointers to identify memory regions.
These relationships have to be re-established after the JSON is
deserialized.
As a future direction it is worth considering whether the logic owned by
`SharedCache` could be split in a similar manner to the data. The
initial loading of the cache header, loading of images, and handling of
symbol information are all mostly independent and work on separate data.
If the logic were split into separate classes it would be easier to
reason about which data is valid when, and would easily permit
concurrent loading of multiple images from the shared library in a
thread-safe manner.
|
|
|
|
Fixes MSVC C++17 failing to compile, hopefully.
|
|
These are from clang so there are still a bunch of warnings on GCC and MSVC
|
|
Potential nullptr deref on invalid view type (see the error log right after the logger init)
|
|
This is a meaningless commit, i'm throwing this in for when someone inevitably calls out the inconsistent "formatting" (what style is this again?)
|
|
A logic error in `FindSymbolAtAddrAndApplyToAddr` meant that a code path
that would attempt to re-use existing symbol definitions in the view
would still fall back to searching the exported symbol list.
`FindSymbolAtAddrAndApplyToAddr` is updated to return early if a symbol
already exists at the target location and the target location is the
same as the symbol location. Additionally, in the event a symbol was found
in the view at the symbol location, it is applied to the target location
and then the function returns.
Additionally, `WillMutateState` is moved after these initial checks so
it is only called in the codepaths that attempt to locate the symbol
amongst the exported symbols.
After these changes it is now possible to load QuartzCore from the iOS
14 shared cache in around 70 seconds. Previously it would run for hours
without the load completing.
|
