- 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.
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
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>
Note that llvm::support::endianness has been renamed to
llvm::endianness while becoming an enum class as opposed to an enum.
This patch replaces llvm::support::{big,little,native} with
llvm::endianness::{big,little,native}.
There are two age fields in a PDB file. One from the PDB Stream and another one
from the DBI stream.
According to https://randomascii.wordpress.com/2011/11/11/source-indexing-is-underused-awesomeness/#comment-34328,
The age in DBI stream is used to against the binary's age. `Pdbstr.exe` is used
to only increment the age from PDB stream without changing the DBI age. I
also verified this by manually changing the DBI age of a PDB file and let
`windbg.exe` to load it. It shows the following logs before and after changing:
Before:
```
SYMSRV: BYINDEX: 0xA
c:\symbols*https://msdl.microsoft.com/download/symbols
nlaapi.pdb
D72AA69CD5ABE5D28C74FADB17DE3F8C1
SYMSRV: PATH: c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
SYMSRV: RESULT: 0x00000000
*** WARNING: Unable to verify checksum for NLAapi.dll
DBGHELP: NLAapi - public symbols
c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
...
```
After:
```
SYMSRV: BYINDEX: 0xA
c:\symbols*https://msdl.microsoft.com/download/symbols
nlaapi.pdb
D72AA69CD5ABE5D28C74FADB17DE3F8C1
SYMSRV: PATH: c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
SYMSRV: RESULT: 0x00000000
DBGHELP: c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb - mismatched pdb
SYMSRV: BYINDEX: 0xB
c:\symbols*https://chromium-browser-symsrv.commondatastorage.googleapis.com
nlaapi.pdb
D72AA69CD5ABE5D28C74FADB17DE3F8C1
SYMSRV: PATH: c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
SYMSRV: RESULT: 0x00000000
DBGHELP: c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb - mismatched pdb
SYMSRV: BYINDEX: 0xC
c:\src\symbols*https://msdl.microsoft.com/download/symbols
nlaapi.pdb
D72AA69CD5ABE5D28C74FADB17DE3F8C1
SYMSRV: PATH: c:\src\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
SYMSRV: RESULT: 0x00000000
*** WARNING: Unable to verify checksum for NLAapi.dll
DBGHELP: NLAapi - public symbols
c:\src\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
```
So, `windbg.exe` uses the DBI age to detect mismatched pdb, but it still loads
the pdb even if the age mismatched. Probably lldb should do the same and give
some warnings.
This fixes a bug that lldb can't load some windows system pdbs due to mismatched
uuid.
Reviewed By: rnk
Differential Revision: https://reviews.llvm.org/D152189
Previously, depending on how you constructed a UUID from data or a
StringRef, an input value of all zeros was valid (e.g. setFromData)
or not (e.g. setFromOptionalData). Since there was no way to tell
which interpretation to use, it was done somewhat inconsistently.
This standardizes the meaning of a UUID of all zeros to Not Valid,
and removes all the Optional methods and their uses, as well as the
static factories that supported them.
Differential Revision: https://reviews.llvm.org/D132191
When an object file returns multiple architectures, it is treated
as a fat binary - which really isn't the case of i386 vs i686 where
the object file actually has one architecture.
This allows getting rid of hardcoded architecture triples in
PlatformWindows.
The parallel i386 and i686 architecture strings stem from
5e6f45201f0b62c1e7a24fc396f3ea6e10dc880d / D7120 and
ad587ae4ca143d388c0ec4ef2faa1b5eddedbf67 / D4658.
Differential Revision: https://reviews.llvm.org/D128617
The current design allows that the object file contents could be mapped
by one object file plugin and then used by another. Presumably the idea
here was to avoid mapping the same file twice.
This becomes an issue when one object file plugin wants to map the file
differently from the others. For example, ObjectFileELF needs to map its
memory as writable while others likeObjectFileMachO needs it to be
mapped read-only.
This patch prevents plugins from changing the buffer by passing them is
by value rather than by reference.
Differential revision: https://reviews.llvm.org/D122944
This patch deals with ObjectFile, ObjectContainer and OperatingSystem
plugins. I'll convert the other types in separate patches.
In order to enable piecemeal conversion, I am leaving some ConstStrings
in the lowest PluginManager layers. I'll convert those as the last step.
Differential Revision: https://reviews.llvm.org/D112061
In future patches I will be setting the IsText parameter frequently so I will refactor the args to be in the following order. I have removed the FileSize parameter because it is never used.
```
static ErrorOr<std::unique_ptr<MemoryBuffer>>
getFile(const Twine &Filename, bool IsText = false,
bool RequiresNullTerminator = true, bool IsVolatile = false);
static ErrorOr<std::unique_ptr<MemoryBuffer>>
getFileOrSTDIN(const Twine &Filename, bool IsText = false,
bool RequiresNullTerminator = true);
static ErrorOr<std::unique_ptr<MB>>
getFileAux(const Twine &Filename, uint64_t MapSize, uint64_t Offset,
bool IsText, bool RequiresNullTerminator, bool IsVolatile);
static ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
getFile(const Twine &Filename, bool IsVolatile = false);
```
Reviewed By: jhenderson
Differential Revision: https://reviews.llvm.org/D99182
