- 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
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.
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.
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`.
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
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.
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>
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
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.
Different object file formats support DWARF sections (COFF, ELF, MachO,
PE/COFF, WASM). COFF and PE/COFF only matched a subset. This caused some
GCC executables produced on MinGW to have issue later on when debugging.
One example is that `.debug_rnglists` was not matched, which caused
range-extraction to fail when printing a backtrace.
This unifies the parsing of section names in
`ObjectFile::GetDWARFSectionTypeFromName`, so all file formats can use
the same naming convention. Since the prefixes are different,
`GetDWARFSectionTypeFromName` only matches the suffixes (i.e. `.debug_`
needs to be stripped before).
I added two tests to ensure the sections are correctly identified on
Windows executables.
Lots of code around LLDB was directly accessing the target's section
load list. This NFC patch makes the section load list private so the
Target class can access it, but everyone else now uses accessor
functions. This allows us to control the resolving of addresses and will
allow for functionality in LLDB which can lazily resolve addresses in
JIT plug-ins with a future patch.
Compared to the python version, this also does type checking and error
handling, so it's slightly longer, however, it's still comfortably
under 500 lines.
Relanding with more explicit type conversions.
This reverts commit f6012a209dca6b1866d00e6b4f96279469884320.
Revert "[lldb] Add cast to fix compile error on 32-but platforms"
This reverts commit d300337e93da4ed96b044557e4b0a30001967cf0.
Revert "[lldb] Improve log message to include missing strings"
This reverts commit 0be33484853557bc0fd9dfb94e0b6c15dda136ce.
Revert "[lldb] Add comment"
This reverts commit e2bb47443d2e5c022c7851dd6029e3869fc8835c.
Revert "[lldb] Implement a formatter bytecode interpreter in C++"
This reverts commit 9a9c1d4a6155a96ce9be494cec7e25731d36b33e.
Compared to the python version, this also does type checking and error
handling, so it's slightly longer, however, it's still comfortably
under 500 lines.
Add support for type summaries embedded into the binary.
These embedded summaries will typically be generated by Swift macros,
but can also be generated by any other means.
rdar://115184658
Recently in #107731 this change was revereted due to excess memory size
in `TestSkinnyCore`. This was due to a bug where a range's end was being
passed as size. Creating massive memory ranges.
Additionally, and requiring additional review, I added more unit tests
and more verbose logic to the merging of save core memory regions.
@jasonmolenda as an FYI.
Reapplies #106293, testing identified issue in the merging code. I used
this opportunity to strip CoreFileMemoryRanges to it's own file and then
add unit tests on it's behavior.
This patch adds the option to specify specific memory ranges to be
included in a given core file. The current implementation lets user
specified ranges either be in addition to a certain save style, or
independent of them via the newly added custom enum.
To achieve being inclusive of save style, I've moved from a std::vector
of ranges to a RangeDataVector, and to join overlapping ranges to
prevent duplication of memory ranges in the core file.
As a non function bonus, when SBSavecore was initially created, the
header was included in the lldb-private interfaces, and I've fixed that
and moved it the forward declare as an oversight. CC @bulbazord in case
we need to include that into swift.
Reapply #100443 and #101770. These were originally reverted due to a
test failure and an MSAN failure. I changed the test attribute to
restrict to x86 (following the other existing tests). I could not
reproduce the test or the MSAN failure and no repo steps were provided.
In #100443, Mach-o and Minidump now only call process API's that take a
`SaveCoreOption` as the container for the style and information if a
thread should be included in the core or not. This introduced a bug
where in subsequent method calls we were not honoring the defaults of
both implementations.
~~To solve this I have made a copy of each SaveCoreOptions that is
mutable by the respective plugin. Originally I wanted to leave the
SaveCoreOptions as non const so these default value mutations could be
shown back to the user. Changing that behavior is outside of the scope
of this bugfix, but is context for why we are making a copy.~~
Removed const on the savecoreoptions so defaults can be inspected by the
user
CC: @Michael137
This PR adds `SBSaveCoreOptions`, which is a container class for options
when LLDB is taking coredumps. For this first iteration this container
just keeps parity with the extant API of `file, style, plugin`. In the
future this options object can be extended to allow users to take a
subset of their core dumps.
All `llvm::Error`s must be checked/consumed before destruction. Previously,
the errors in this patch were only consumed when logging was enabled.
Using `LLDB_LOG_ERROR` instead of `LLDB_LOG` fixes that, because it
calls `llvm::consumeError()` explicitly when logging is disabled.
In Apple's downstream fork, there is support for understanding the swift
AST sections in various binaries. Even though the lldb on llvm.org does
not have support for debugging swift, I think it makes sense to move
support for recognizing swift ast sections upstream.
Differential Revision: https://reviews.llvm.org/D159142
ConstString can be implicitly converted into a llvm::StringRef. This is
very useful in many places, but it also hides places where we are
creating a ConstString only to use it as a StringRef for the entire
lifespan of the ConstString object.
I locally removed the implicit conversion and found some of the places we
were doing this.
Differential Revision: https://reviews.llvm.org/D159237
StreamFile subclasses Stream (from lldbUtility) and is backed by a File
(from lldbHost). It does not depend on anything from lldbCore or any of its
sibling libraries, so I think it makes sense for this to live in
lldbHost instead.
Differential Revision: https://reviews.llvm.org/D157460
There can be zero padding bytes at a section end for file alignment in
PECOFF. Exclude those padding bytes when reading the section data.
Differential Revision: https://reviews.llvm.org/D157059
With D149091, ARM64X binaries are no longer reported as ARM64. This broke
lldb tests as Windows 11 system DLLs are mostly ARM64X binaries and lldb
doesn't know how to handle them. Ideally lldb would understand a bit more
about ARM64X and handle them as AMD64 in x64 processes, but this is
enough to preserve previous behavior and fix tests.
Reviewed By: mstorsjo
Differential Revision: https://reviews.llvm.org/D156268
Use templates to simplify {Get,Set}PropertyAtIndex. It has always
bothered me how cumbersome those calls are when adding new properties.
After this patch, SetPropertyAtIndex infers the type from its arguments
and GetPropertyAtIndex required a single template argument for the
return value. As an added benefit, this enables us to remove a bunch of
wrappers from UserSettingsController and OptionValueProperties.
Differential revision: https://reviews.llvm.org/D149774
As far as I can tell, this just computes the filename of the FileSpec,
which is already conveniently stored in m_filename. We can use
FileSpec::GetFilename() instead.
Differential Revision: https://reviews.llvm.org/D149663
The majority of call sites are nullptr as the execution context.
Refactor OptionValueProperties to make the argument optional and
simplify all the callers.
Similar to fdbe7c7faa54, refactor OptionValueProperties to return a
std::optional instead of taking a fail value. This allows the caller to
handle situations where there's no value, instead of being unable to
distinguish between the absence of a value and the value happening the
match the fail value. When a fail value is required,
std::optional::value_or() provides the same functionality.
Refactor OptionValue to return a std::optional instead of taking a fail
value. This allows the caller to handle situations where there's no
value, instead of being unable to distinguish between the absence of a
value and the value happening the match the fail value. When a fail
value is required, std::optional::value_or() provides the same
functionality.
llvm has a structure for maps where the key's type is a string. Using
that also means that the keys for OptionValueDictionary don't stick
around forever in ConstString's StringPool (even after they are gone).
The only thing we lose here is ordering: iterating over the map where the keys
are ConstStrings guarantees that we iterate in alphabetical order.
StringMap makes no guarantees about the ordering when you iterate over
the entire map.
Differential Revision: https://reviews.llvm.org/D149482
These probably do not need to be in the ConstString StringPool as they
don't really need any of the advantages that ConstStrings offer.
Lifetime for these things is always static and we never need to perform
comparisons for setting descriptions.
Differential Revision: https://reviews.llvm.org/D148679
We would not ensure that the error is consumed in the case that logging
is disabled. Ensure that we properly drop the error on the floor or we
would re-trigger the checked failure.
Differential Revision: https://reviews.llvm.org/D147669
Reviewed By: sgraenitz
The forwarding header is left in place because of its use in
`polly/lib/External/isl/interface/extract_interface.cc`, but I have
added a GCC warning about the fact it is deprecated, because it is used
in `isl` from where it is included by Polly.
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
It's fine right now, but will break as soon as someone else declares a
PluginProperties class in the same way.
Also tighten up the scope of the anonymous namespaces surrounding the
other PluginProperties classes.
