| Age | Commit message (Collapse) | Author |
|---|
|
Also fixes a BinaryViewRef leak
|
|
This is a meaningless commit, i'm throwing this in for when someone inevitably calls out the inconsistent "formatting" (what style is this again?)
|
|
Previously, `MMappedFileAccessor::Open` attempted to impose a fixed limit
on the number of file accessors that were live at one time. This was
done because the default file descriptor limit on some platforms is
relatively low (256 on macOS). Given that recent iOS shared caches can
contain 60+ files it is easy to tie up a large percentage of this limit
by opening one or two shared caches.
This is problematic as if the limit imposed by `MMappedFileAccessor::Open`
is reached, the attempt to access the file will block waiting for
another file accessor to be closed. This can lead to a deadlock.
This commit makes three changes to this strategy:
1. It attempts to raise the file descriptor limit to 1024. Unix systems
support both soft and hard file descriptor limits. The soft file
descriptor limit is what is enforced, but a process can explicitly
raise the limit to any value below the hard limit if it wishes.
2. The fixed limit on open files accessors is changed to a soft limit.
`FileAccessorCache` is introduced to manage the caching of file
accessors. It provides a basic LRU cache. Whenever a new file is
opened, the cache of open accessors is pruned to stay below the
target limit (50% of the soft file descriptor limit). This limiting
is primarily done to allow files containing dirty pages to be
unmapped if they're no longer being used.
LRU isn't the optimal strategy for this, but it is simple to
implement and understand. Ideally there'd be some access time
component to the cache so files that haven't been accessed can be
released.
3. File accessors are cached even for sessions corresponding to views
that have been closed. The "Open Selection with Options..." context
menu hits this case as a view is created and destroyed as part of
populating the options dialog, and the real view is then created in
the same session.
The most significant benefit of this change is that the logic around
opening / closing file accessors is simpler and can no longer deadlock.
While working on this I noticed that `SharedCache` opened some files via
`MMappedFileAccessor::Open` without specifying a post-open operation to
apply slide information. Instead, it would explicitly apply the slide
information after opening the file. This works fine so long as the file
accessor limit is never hit. If it is hit and the accessor is closed,
the next time the file is opened it will not have any slide information
applied. This would lead to very confusing bugs.
|
|
This change is mostly motivated by simplifying the code, but it also
brings minor correctness and performance benefits.
1. The pointer returned by mmap is stored as a uint8_t* rather than
void* as that is how it is used. This reduces how often it needs to
be cast to a different type before it is used.
2. Read methods for primitives delegate to a new Read template function
that in turn delegates to the general-purpose `Read(void* dest, size_t
address, size_t length)`. This improves the consistency of bounds
checking and simplifies the code. The compiler is more than willing
to inline this so we get less repetition with no overhead.
3. ReadNullTermString now uses std::find to find the nul byte and
directly constructs the string from that range of bytes. This removes
an unnecessary allocation that was previously being forced by the use
reserve followed by shrink_to_fit. It also avoids repeated
reallocation for longer strings as they grew past the the reserved
size as they were being built up a character at a time.
|
|
`ReadExportNode` is called a lot during the initial load of the shared
cache and thus impacts how long it takes for the UI to become
responsive. This is a collection of optimizations that cut the time
spent within `ReadExportNode` by 50%:
1. Pass iterators to `ReadExportNode` rather than a `DataBuffer` + offset.
The lack of inlining in `DataBuffer`'s `operator[]` kills performance.
Ideally this would have used `std::span`, but that would require
bumping the minimum C++ version to C++20.
2. Add `MMappedFileAccessor::ReadSpan` so that `ReadExportNode` can
operate directly on the mapped data without first copying it.
3. Removes a call to `GetAnalysisFunctionsForAddress` whose result was
unused.
4. Use `std::find` to find the nul at the end of strings rather than
assembling the string a character at a time. This avoids repeatedly
growing the string.
5. Avoid the usual `Symbol` constructor in favor of `BNCreateSymbol`.
The `Symbol` constructor has over head in two forms:
1. It has a `NameSpace` as an argument. Creating / destroying this
allocates and deallocates memory We could create a single
instance and reuse it for all calls, but...
2. The constructor copies all fields of the `NameSpace` to the heap
before calling `BNCreateSymbol` and then deallocates them
afterwards. This seems unnecessary, and adds a non-trivial amount
of overhead.
This can go back to directly constructing the `Symbol` once the
constructors are improved.
|
|
`PageMapping` was storing the path to the file it points within. This
was causing unnecessary work within `VM::MappingAtAddress` as the
`PageMapping`, and thus the path, is copied into the return value. This
copying was more expensive than the map lookup.
The file path is now stored within a `LazyMappedFileAccessor` class that
wraps the `SelfAllocatingWeakPtr`. This means the path is still
available via the `PageMapping`, but it does not need to be copied as
often.
This includes two additional improvements / optimizations while I was
touching the code in question:
1. `MMappedFileAccessor::Open` no longer performs two hash lookups when
a file accessor already exists.
2. `VM::MapPages` takes the path by const reference to avoid an
unnecessary copy.
|
|
There are typically only a few dozen mappings, while there can be
millions of pages. This reduces the amount of time spent populating the
mapping from region to file accessor along with the memory usage of the
same.
|
|
`MMappedFileAccessor::ReadBuffer` was returning a heap-allocated
`DataBuffer`, but no callers were ever deleting it. There does not
appear to be any reason to heap allocate the `DataBuffer` as the type is
effectively a smart pointer wrapper around `BNDataBuffer`. Switch to
returning it by value instead.
Additionally, `MMappedFileAccessor::ReadBuffer` was allocating a buffer,
copying data into it, and then handing that allocation to the `DataBuffer`
constructor. The constructor copies data into a new allocation it
owns so this allocation is unnecessary and was being leaked.
|
|
|
|
|
|
|
|
This is an early release of our DSC processing plugin. We're still hard at work improving this feature. You should be able to just drop in a dyld_shared_cache and use the 'Shared Cache Triage' view to load and analyze images.
|
