In the non-ARM case, the offset was left unset, so the symbol
synthesized for the entry point pointed to the start of the containing
section.
As a drive-by change, simplify offset adjustment in ARM case.
- it is always passed as zero
- a lot of plugins aren't using it correctly
- the data extractor class already has the capability to look at a
subset of bytes
After this change, Address::SetSection() had only one use left (in a
unit test) and was removed. Address::ClearSection() had no uses, now
also removed. (It is unlikely that someone needs to change the section
without simultaneously changing the section offset, and for that we have
a constructor.)
This is to highlight places where we (probably unintentionally)
construct an `Address` object from an already resolved address, making
it unresolved again.
See the changes in `DynamicLoaderDarwin.cpp` for a quick example.
Also, use this constructor instead of `Address(lldb::addr_t file_addr,
const SectionList *section_list)` when `section_list` is `nullptr`.
The warning message intended to print the mismatched file addresses, but
was passing GetByteSize() instead of GetFileAddress() for both the
expected and actual values.
Some plugins were returning the number of specifications they have
added, while others were returning the total final number. Particularly
devious plugins (Minidump) were clearing the specification list
altogether. This resulted in nondeterministic failures (depending on
plugin ininitialization order) in TestSBModule.
This PR defines the problem away by having each plugin only return the
specifications it is responsible for. If the caller wants to merge them,
it is free to do so. This *might* be slighly less efficient, but this is
hardly hot code.
I'm not touching the ObjectFile::GetModuleSpecifications function (the
caller of all these functions) as the PR is big enough, although the
same approach might be warranted there as well.
Fixes https://github.com/llvm/llvm-project/issues/178625.
Fetch the size of the shared cache address range in VM in the inferior
process and return it, if possible. This allows for a simple call to
DynamicLoader::GetSharedCacheInformation to get the virtual address
extent of the shared cache in the inferior. It's a minor addition to the
method that fetches the shared cache filepath from the inferior, plus
piping it up through lldb's layers.
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
lldb had three preprocessor defines for logging,
LLDB_LOG - formatv style argument
LLDB_LOGF - printf style argument
LLDB_LOGV - formatv style argument, only when verbose enabled
If you weren't looking at Log.h and the definition of these three, and
wanted to log something with formatv, it was easy to use LLDB_LOGV by
accident. We just had a situation where an important log statement
wasn't logging and it turned out to be this. This is fragile if you
aren't looking at the header directly, so I'd like to make this more
explicit. My proposal:
LLDB_LOG - formatv style argument
LLDB_LOG_VERBOSE - formatv style argument, only when verbose enabled
LLDB_LOGF - printf style argument
LLDB_LOGF_VERBOSE - printf style argument, only when verbose enabled
The new fouth one is to remove several places where we do `if (log &&
log->GetVerbose()) LLDB_LOGF (...)` in the sources today, and make both
styles consistent.
This PR implements that change, mechanically changing all LLDB_LOGV's to
LLDB_LOG_VERBOSE.
It also updates many of the `if (log && log->GetVerbose()) LLDB_LOGF`'s.
Some uses of this conditional expression do extra calculations in
addition to logging, and so those were left as-is so we're not doing
throwaway work when running without verbose logging.
There were many instances throughout lldb where callers are still doing
`if (log) LLDB_LOG*(...)`, a remnant of when all calls were to the `Log`
object's `Printf()` method, and you had to check if your local Log*
pointer was non-nullptr before calling the method. I removed those,
again keeping ones where work for logging is done in the block of code.
The code changes are all mechanical and uninteresting, but the question
of whether this naming change is widely agreed on is maybe worth
discussing.
Minimal infrastructure for a the SymbolLocator plugin that fetches debug
info from Microsoft SymStore repositories. This can work cross-platform
and for various debug info formats in principle, but the current plan is
focussed on PE/COFF on Windows with debug info in PDB files. Once we
have a stable first version, we'd like to add features like download,
environment variables, caching and progress feedback for users.
SymbolVendorPECOFF was tailored towards DWARF debug info so far. I added
code to load the PDB path from the executable (it only checked
gnu_debuglink so far) and not bail out if DWARF sections are missing, so
that in the PDB case we still call AddSymbolFileRepresentation() in the
very end of CreateInstance().
The API test in this patch mocks the directory layout from SymStore, so
it doesn't depend on SymStore.exe from the Windows SDK. It runs on all
platforms that link debug info in a PDB file, which is still just
Windows, but it could be cross-platform in principle.
-----
Relands with minor fixes: API tests create mocked SymStore in the test's
build directory. One log instruction was moved. One more object
access goes through module in SymbolFile.
Minimal infrastructure for a the SymbolLocator plugin that fetches debug
info from Microsoft SymStore repositories. More features will follow.
SymbolVendorPECOFF was tailored towards DWARF debug info so far. This
patch adds code to load the PDB path from the executable and not bail
out if DWARF sections are missing, so that in the PDB case we still
call `AddSymbolFileRepresentation()` in the very end of
`CreateInstance()`.
The API test in this patch mocks the directory layout from SymStore, so
it doesn't depend on `SymStore.exe` from the Windows SDK. It runs on all
platforms that link debug info in a PDB file, which is still just
Windows, but it could be cross-platform in principle.
Symbols added by linker scripts do not have debug information or types,
but they do have addresses. Sometimes users need to see the addresses of
these symbols. Currently these symbols end up being assigned
'eSymbolTypeInvalid', so any attempt to look at them fails. This fixes
that issue.
I was reading through ObjectContainerBSDArchive and came across some
dead method decls, a less-than-completely-clear `shared_ptr` typedef in
`ObjectContainerBSDArchive::Archive` for a shared_ptr<Archive> which was
a little unclear when reading a decl like `shared_ptr archive_sp;` for a
local variable.
This was originally introduced to support kalimba DSPs featuring 24-bit
bytes by f03e6d84 and also c928de3e, but the kalimba support was mostly
removed by f8819bd5. This change removes the rest of the support, which
was far from complete.
This patch introduces SubtargetFeatures in ArchSpec to enable consistent
handling of architecture-specific extensions in LLDB. For RISCV targets
in particular, it allows LLDB to propagate target extension features
from ELF attributes to expression evaluation and disassembly.
Fix LLDB header guards using clang-tidy's llvm-header-guard check. A
bunch of headers have been moved or renamed and we often forget to
update the header guard.
In #168245, I attempted to dump the available settings to Markdown. That
required a full build of LLDB. However, to build the docs, only the swig
wrappers should need to be compiled. The comment was that we should be
able to use the definitions from the TableGen files.
Currently, the property definitions in don't have information about the
path where they will be available. They only contain a `Definition`
which groups properties, so they can be added to
`OptionValueProperties`.
With this PR, I'm adding the path for each property definition. For
example, `symbols.enable-external-lookup` would have `Name =
enable-external-lookup, Path = symbols`. In LLDB itself, we don't need
this path, we only need it for the documentation. To avoid mismatches
between the actual path and the declared one, I added a debug-only check
when a property group is added to a parent
(`OptionValueProperties::AppendProperty`).
The TableGen emitter for the properties now additionally emits
`g_{definition}_properties_def`, which includes both the array of
properties and the expected path. This constant has to be used to
initialize a `OptionValueProperties`.
I couldn't test this for everything (e.g. IntelPT or ProcessKDP), but
the necessary changes are simple: (1) set the `Path` in the TableGen
file, (2) update `initialize` to use `_def`.
Get the shared cache filepath and uuid that the inferior process is
using from debugserver, try to open that shared cache on the lldb host
mac and if the UUID matches, index all of the binaries in that shared
cache. When looking for binaries loaded in the process, get them from
the already-indexed shared cache.
Every time a binary is loaded, PlatformMacOSX may query the shared cache
filepath and uuid from the Process, and pass that to
HostInfo::GetSharedCacheImageInfo() if available (else fall back to the
old HostInfo::GetSharedCacheImageInfo method which only looks at lldb's
own shared cache), to get the file being requested.
ProcessGDBRemote caches the shared cache filepath and uuid from the
inferior, once it has a non-zero UUID. I added a lock for this ivar
specifically, so I don't have 20 threads all asking for the shared cache
information from debugserver and updating the cached answer. If we never
get back a non-zero UUID shared cache reply, we will re-query at every
library loaded notification. debugserver has been providing the shared
cache UUID since 2013, although I only added the shared cache filepath
field last November.
Note that a process will not report its shared cache filepath or uuid at
initial launch. As dyld gets a chance to execute a bit, it will start
returning binaries -- it will be available at the point when libraries
start loading. (it won't be available yet when the binary & dyld are the
only two binaries loaded in the process)
I tested this by disabling lldb's scan of its own shared cache
pre-execution -- only loading the system shared cache when the inferior
process reports that it is using that. I got 6-7 additional testsuite
failures running lldb like that, because no system binaries were loaded
before exeuction start, and the tests assumed they would be.
rdar://148939795
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
Add handling for `STT_TLS` (thread-local storage) symbols in the ELF
symbol parsing code. Previously, TLS symbols like `errno` from glibc
were not recognized because `STT_TLS` was not handled in the symbol type
switch statement.
This treats TLS symbols as data symbols (`eSymbolTypeData`), similar to
`STT_OBJECT`. The actual TLS address resolution is already implemented
in `DynamicLoaderPOSIXDYLD::GetThreadLocalData()` which uses the DWARF
`DW_OP_form_tls_address` opcode to calculate thread-local addresses.
This changes VirtualDataExtractor's GetByteSize to return the virtual
byte size of the buffer (external users only understand the data
contents in terms of the virtual sizes & offsets). There are check
methods in DataExtractor that check they are not going off the end of a
buffer, they usually use the BytesLeft() method. There are a couple of
callers of BytesLeft() externally, but it is predominantly an internal
use API. I have BytesLeft() use the physical size of the buffer, not the
virtual size, for the benefit of the DataExtractor methods. (and to
avoid duplicating all of them down in VirtualDataExtractor)
Another problem is the we call SetData on DataExtractorSP's (e.g. see
the ObjectFile ctor) with the DataBuffer it already has, an offset of 0,
and the GetByteSize. A no-op for a DataExtractor that is already using
that DataBuffer. But SetData would try to use that length as a physical
size, and truncate the buffer that the DataExtractor would accept.
I added VirtualDataExtractor subclass methods for the SetData's, detect
(1) data being added to an uninitialized DataExtractor, (2) the same
data / offset / length as currently being used is added to the
DataExtractor (a no-op), or (3) we're genuinely changing the data source
or setting an offset / length that is different. This final case we're
not ready to handle today, I added asserts for them so we can catch it
in debug builds, and then I clear the LookupTable and add a no-op entry
so this extractor will behave like a plain DataExtractor -- because I
don't know better to do. If we genuinely need to handle this case, and
I'm pretty sure we don't need to, I'd have to assume that we're taking a
subset of the original data source (an offset & length), so we'd need to
update all of the LookupTable entries to reflect the new offsets, and
remove entries that are no longer referring to the subsetted range. I'll
leave that until there's any evidence it's actually needed.
rdar://148939795
In a PR last month I changed the ObjectFile CreateInstance etc methods
to accept an optional DataExtractorSP instead of a DataBufferSP, and
retain the extractor in a shared pointer internally in all of the
ObjectFile subclasses. This is laying the groundwork for using a
VirtualDataExtractor for some Mach-O binaries on macOS, where the
segments of the binary are out-of-order in actual memory, and we add a
lookup table to make it appear that the TEXT segment is at offset 0 in
the Extractor, etc. Working on the actual implementation, I realized we
were still using DataBufferSP's in ModuleSpec and Module, as well as in
ObjectFile::GetModuleSpecifications.
I originally was making a much larger NFC change where I had all
ObjectFile subclasses operating on DataExtractors throughout their
implementation, as well as in the DWARF parser. It was a very large
patchset. Many subclasses start with their DataExtractor, then create
smaller DataExtractors for parts of the binary image - the string table,
the symbol table, etc., for processing.
After consideration and discussion with Jonas, we agreed that a
segment/section of a binary will never require a lookup table to access
the bytes within it, so I changed
VirtualDataExtractor::GetSubsetExtractorSP to (1) require that the
Subset be contained within a single lookup table entry, and (2) return a
simple DataExtractor bounded on that byte range. By doing this, I was
able to remove all of my very-invasive changes to the ObjectFile
subclass internals; it's only when they are operating on the entire
binary image that care is needed.
One pattern that subclasses like ObjectFileBreakpad use is to take an
ArrayRef of the DataBuffer for a binary, then create a StringRef of
that, then look for strings in it. With a VirtualDataExtractor and
out-of-order binary segments, with gaps between them, this allows us to
search the entire buffer looking for a string, and segfault when it gets
to an unmapped region of the buffer. I added a
VirtualDataExtractor::GetSubsetExtractorSP(0) which gets the largest
contiguous memory region starting at offset 0 for this use case, and I
added a comment about what was being done there because I know it is not
obvious, and people not working on macOS wouldn't be familiar with the
requirement. (when we have a ModuleSpec with a DataExtractor, any of the
ObjectFile subclasses get a shot at Creating, so they all have to be
able to iterate on these)
rdar://148939795
This fixes a regression in `ObjectFile` and `ObjectFileELF` introduced
by #171574.
The original code created a `DataBuffer` using `MapFileDataWritable`.
```
data_sp = MapFileDataWritable(*file, length, file_offset);
if (!data_sp)
return nullptr;
data_offset = 0;
```
The new code requires converting the `DataBuffer` to a `DataExtractor`:
```
DataBufferSP buffer_sp = MapFileDataWritable(*file, length, file_offset);
if (!buffer_sp)
return nullptr;
extractor_sp = std::make_shared<DataExtractor>();
extractor_sp->SetData(buffer_sp, data_offset, buffer_sp->GetByteSize());
data_offset = 0;
```
The issue is that once we get a data buffer back from
MapFileDataWritable, we don't have to adjust for the `data_offset` again
when calling `SetData` as the `DataBuffer` is already normalized to have
a zero start offset. A similar issue exists in `ObjectFile`.
rdar://168317174
Avoid redundant calls to `std::shared_ptr::get()`. The class provides a
dereference operator and using that is the standard, idiomatic way to
access the underlying object.
Corrected various spelling mistakes such as 'occurred', 'receiver',
'initialized', 'length', and others in comments, variable names,
function names, and documentation throughout the project. These
changes improve code readability and maintain consistency in naming
and documentation.
Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
The ObjectFile plugin interface accepts an optional DataBufferSP
argument. If the caller has the contents of the binary, it can provide
this in that DataBufferSP. The ObjectFile subclasses in their
CreateInstance methods will fill in the DataBufferSP with the actual
binary contents if it is not set.
ObjectFile base class creates an ivar DataExtractor from the
DataBufferSP passed in.
My next patch will be a caller that creates a VirtualDataExtractor with
the binary data, and needs to pass that in to the ObjectFile plugin,
instead of the bag-of-bytes DataBufferSP. It builds on the previous
patch changing ObjectFile's ivar from DataExtractor to DataExtractorSP
so I could pass in a subclass in the shared ptr. And it will be using
the VirtualDataExtractor that Jonas added in
https://github.com/llvm/llvm-project/pull/168802
No behavior is changed by the patch; we're simply moving the creation of
the DataExtractor to the caller, instead of a DataBuffer that is
immediately used to set up the ObjectFile DataExtractor. The patch is a
bit complicated because all of the ObjectFile subclasses have to
initialize their DataExtractor to pass in to the base class.
I ran the testsuite on macOS and on AArch64 Ubutnu. (btw David, I ran it
under qemu on my M4 mac with SME-no-SVE again, Ubuntu 25.10, checked
lshw(1) cpu capabilities, and qemu doesn't seem to be virtualizing the
SME, that explains why the testsuite passes)
rdar://148939795
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
LLDB can use the wasm name section to populate its symbol table and get
names for functions. However the index space used in the name section is
the "function index space" which includes imported as well as locally
defined functions.
ObjectFile has an m_data DataExtractor ivar which may be default
constructed initially, or initialized with a DataBuffer passed in to its
ctor. If the DataExtractor does not get a DataBuffer source passed in,
the subclass will initialize it with access to the object file's data.
When a DataBuffer is passed in to the base class ctor, the DataExtractor
only has its buffer initialized; ObjectFile doesn't yet know the address
size and endianness to fully initialize the DataExtractor.
This patch changes ObjectFile to instead have a DataExtractorSP ivar
which is always initialized with at least a default-constructed
DataExtractor object in the base class ctor. The next patch I will be
writing is to change the ObjectFile ctor to take an optional
DataExtractorSP, so the caller can pass a DataExtractor subclass -- the
VirtualizeDataExtractor being added via
https://github.com/llvm/llvm-project/pull/168802
instead of a DataBuffer which is trivially saved into the DataExtractor.
The change is otherwise mechanical; all `m_data.` changed to
`m_data_sp->` and all the places where `m_data` was passed in for a
by-ref call were changed to `*m_data_sp.get()`. The shared pointer is
always initialized to contain an object.
I built & ran the testsuite on macOS and on aarch64-Ubuntu (thanks for
getting the Linux testsuite to run on SME-only systems David). All of
the ObjectFile subclasses I modifed compile cleanly, but I haven't
tested them beyond any unit tests they may have (prob breakpad).
rdar://148939795
Recently I've been deep diving ELF cores in LLDB, aspiring to move LLDB
closer to GDB in capability. One issue I encountered was a system lib
losing it's unwind plan when loading the debuginfo. The reason for this
was the debuginfo has the eh_frame section stripped and the main
executable did not.
The root cause of this was this line in
[ObjectFileElf](163933e9e7/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (L1972))
```
// For eTypeDebugInfo files, the Symbol Vendor will take care of updating the
// unified section list.
if (GetType() != eTypeDebugInfo)
unified_section_list = *m_sections_up;
```
This would always be executed because CalculateType can never return an
eTypeDebugInfo
```
ObjectFile::Type ObjectFileELF::CalculateType() {
switch (m_header.e_type) {
case llvm::ELF::ET_NONE:
// 0 - No file type
return eTypeUnknown;
case llvm::ELF::ET_REL:
// 1 - Relocatable file
return eTypeObjectFile;
case llvm::ELF::ET_EXEC:
// 2 - Executable file
return eTypeExecutable;
case llvm::ELF::ET_DYN:
// 3 - Shared object file
return eTypeSharedLibrary;
case ET_CORE:
// 4 - Core file
return eTypeCoreFile;
default:
break;
}
return eTypeUnknown;
}
```
This makes sense as there isn't a explicit sh_type to denote that this
file is a debuginfo. After some discussion with @clayborg and
@GeorgeHuyubo we settled on joining the exciting unified section list
with whatever new sections were being added. Adding each new unique
section, or taking the section with the maximum file size. We picked
this strategy to pick the section with the most information. In most
scenarios, LHS should be SHT_NOBITS and RHS would be SHT_PROGBITS.
Here is a diagram documenting the existing vs proposed new way.
<img width="1666" height="1093" alt="image"
src="https://github.com/user-attachments/assets/73ba9620-c737-439e-9934-ac350d88a3b5"
/>
A global offset table is a section that holds the address of functions
that are dynamically linked. The Swift plugin needs to know if sections
are a global offset table or not.
The code to apply relocations was sometimes creating unaligned
destination pointers. Instead of giving them an explicit type (i.e.
`uint64_t *`) and forcing the compiler to generate unaligned stores,
mark the pointer as `void *`. The compiler will figure out the correct
series of store instructions.
Update all uses of variadic `.Cases` to use the initializer list
overload instead. I plan to mark variadic `.Cases` as deprecated in a
followup PR.
For more context, see https://github.com/llvm/llvm-project/pull/163117.
llvm-dsymutil can produce mach-o files where some sections in __DWARF
exceed the 4GB barrier and subsequent sections in the dSYM will be
inaccessible because the mach-o section_64 structure only has a 32 bit
file offset. This patch enables LLDB to load a large dSYM file by
figuring out when this happens and properly adjusting the file offset of
the LLDB sections.
I was unable to add a test as obj2yaml and yaml2obj are broken for
mach-o files and they can't convert a yaml file back into a valid mach-o
object file. Any suggestions for adding a test would be appreciated.
This reverts `5a80fb9177e3c831c9c574400a13d77393397f2a`. The original
change got reverted because of failing tests on macOS.
The issue was that I changed the scope of setting `type =
eSymbolTypeData` during the cleanup. This patch relands the original
patch but doesn't change the `else` branch to an `else if` branch.
Tested that macOS test-suite passes.
On Darwin C-symbols are prefixed with a '_'. The LLDB Macho-O parses
handles Objective-C metadata symbols starting with '_OBJC' specially.
Previously global symbols starting with a '_O' prefix were lost because
of incorrectly scoped if-guards. This patch removes those checks.
There is more cleanup that can be done in this file because there's a
bunch of duplicated checks for these ObjC symbols. I decided to leave
that for an NFC follow-up.
Depends on https://github.com/llvm/llvm-project/pull/161520
rdar://158159242
The ObjectFileELF parser was not able to load ELF notes from PT_NOTE
program headers. This patch fixes ObjectFileELF::GetUUID() to check the
program header and parse the notes in any PT_NOTE segments. This will
allow memory ELF files to extract the UUID from an in memory image that
has no section headers. Added a test that creates an ELF file, strips
all section headers, and then makes sure that LLDB can see the UUID
value.
The Mach-O file format has several load commands which specify the
location of data in the file in UInt32 offsets. lldb uses these same
structures to track the offsets of the binary in virtual address space
when it is running. Normally a binary is loaded in memory contiguously,
so this is fine, but on Darwin systems there is a "system shared cache"
where all system libraries are combined into one region of memory and
pre-linked. The shared cache has the TEXT segments for every binary
loaded contiguously, then the DATA segments, and finally a shared common
LINKEDIT segment for all binaries. The virtual address offset from the
TEXT segment for a libray to the LINKEDIT may exceed 4GB of virtual
address space depending on the structure of the shared cache, so this
use of a UInt32 offset will not work.
There was an initial instance of this issue that I fixed last November
in https://github.com/llvm/llvm-project/pull/117832 where I fixed this
issue for the LC_SYMTAB / `symtab_command` structure. But we have the
same issue now with three additional structures;
`linkedit_data_command`, `dyld_info_command`, and `dysymtab_command`.
For all of these we can see the pattern of `dyld_info.export_off +=
linkedit_slide` applied to the offset fields in ObjectFileMachO.
This defines local structures that mirror the Mach-O structures, except
that it uses UInt64 offset fields so we can reuse the same field for a
large virtual address offset at runtime. I defined ctor's from the
genuine structures, as well as operator= methods so the structures can
be read from the Mach-O binary into the standard object, then copied
into our local expanded versions of them. These structures are ABI in
Mach-O and cannot change their layout.
The alternative is to create local variables alongside these Mach-O load
command objects for the offsets that we care about, adjust those by the
correct VA offsets, and only use those local variables instead of the
fields in the objects. I took the approach of the local enhanced
structure in November and I think it is the cleaner approach.
rdar://160384968
When a processor faults/is interrupted/gets an exception, it will stop
running code and jump to an exception catcher routine. Most processors
will store the pc that was executing in a system register, and the
catcher functions have special instructions to retrieve that & possibly
other registers. It may then save those values to stack, and the author
can add .cfi directives to tell lldb's unwinder where to find those
saved values.
ARM Cortex-M (microcontroller) processors have a simpler mechanism where
a fixed set of registers are saved to the stack on an exception, and a
unique value is put in the link register to indicate to the caller that
this has taken place. No special handling needs to be written into the
exception catcher, unless it wants to inspect these preserved values.
And it is possible for a general stack walker to walk the stack with no
special knowledge about what the catch function does.
This patch adds an Architecture plugin method to allow an Architecture
to override/augment the UnwindPlan that lldb would use for a stack
frame, given the contents of the return address register. It resembles a
feature where the LanguageRuntime can replace/augment the unwind plan
for a function, but it is doing it at offset by one level. The
LanguageRuntime is looking at the local register context and/or symbol
name to decide if it will override the unwind rules. For the Cortex-M
exception unwinds, we need to modify THIS frame's unwind plan if the
CALLER's LR had a specific value. RegisterContextUnwind has to retrieve
the caller's LR value before it has completely decided on the UnwindPlan
it will use for THIS stack frame.
This does mean that we will need one additional read of stack memory
than we currently do when unwinding, on Armv7 Cortex-M targets. The
unwinder walks the stack lazily, as stack frames are requested, and so
now if you ask for 2 stack frames, we will read enough stack to walk 2
frames, plus we will read one extra word of memory, the spilled LR value
from the stack. In practice, with 512-byte memory cache reads, this is
unlikely to be a real performance hit.
This PR includes a test with a yaml corefile description and a JSON
ObjectFile, incorporating all of the necessary stack memory and symbol
names from a real debug session I worked on. The architectural default
unwind plans are used for all stack frames except the 0th because
there's no instructions for the functions, and no unwind info. I may
need to add an encoding of unwind fules to ObjectFileJSON in the future
as we create more test cases like this.
This PR depends on the yaml2macho-core utility from
https://github.com/llvm/llvm-project/pull/153911 to run its API test.
rdar://110663219
My original implementation for parsing Wasm segments was wrong in two
related ways. I had a bug in calculating the file vm address and I
didn't fully understand the difference between active and passive
segments and how that impacted their file vm address.
With this PR, we now support parsing init expressions for active
segments, rather than just skipping over them. This is necessary to
determine where they get loaded.
Similar to llvm-objdump, we currently only support simple opcodes (i.e.
constants). We also currently do not support active segments that use a
non-zero memory index. However this covers all segments for a
non-trivial Swift binary compiled to Wasm.
Note: This is a resubmission of #106791. I had to revert this a year ago
for a failing test that I could not understand. I have time now to try
and get this in again.
Summary:
This improves the performance of ObjectFileMacho::ParseSymtab by
removing eager and expensive work in favor of doing it later in a
less-expensive fashion.
Experiment:
My goal was to understand LLDB's startup time.
First, I produced a Debug build of LLDB (no dSYM) and a
Release+NoAsserts build of LLDB. The Release build debugged the Debug
build as it debugged a small C++ program. I found that
ObjectFileMachO::ParseSymtab accounted for somewhere between 1.2 and 1.3
seconds consistently. After applying this change, I consistently
measured a reduction of approximately 100ms, putting the time closer to
1.1s and 1.2s on average.
Background:
ObjectFileMachO::ParseSymtab will incrementally create symbols by
parsing nlist entries from the symtab section of a MachO binary. As it
does this, it eagerly tries to determine the size of symbols (e.g. how
long a function is) using LC_FUNCTION_STARTS data (or eh_frame if
LC_FUNCTION_STARTS is unavailable). Concretely, this is done by
performing a binary search on the function starts array and calculating
the distance to the next function or the end of the section (whichever
is smaller).
However, this work is unnecessary for 2 reasons:
1. If you have debug symbol entries (i.e. STABs), the size of a function
is usually stored right after the function's entry. Performing this work
right before parsing the next entry is unnecessary work.
2. Calculating symbol sizes for symbols of size 0 is already performed
in `Symtab::InitAddressIndexes` after all the symbols are added to the
Symtab. It also does this more efficiently by walking over a list of
symbols sorted by address, so the work to calculate the size per symbol
is constant instead of O(log n).
