Commit 728cc0075e made comdat symbols
from LTO objects be treated as any regular comdat symbol. This works
great for symbols that actually are IMAGE_COMDAT_SELECT_ANY, but
if the symbols have a less trivial selection type that require comparing
either the section chunk size or contents, we can't check that before
actually doing the LTO compilation.
Therefore bring back one aspect of handling from before; that comdat
resolution with a leader from an LTO symbol is essentially skipped,
like it was before 728cc0075e.
Differential Revision: https://reviews.llvm.org/D104605
This reverts commit e1adf90826.
This appears to affect the way that C++ mangled symbols appear in the
import library when using a .def file that names a C++ free function
with no name decoration. I will follow up with a reduced test case
shortly.
This is run every time around in the main linker loop. Once a match
has been found, stop trying to rematch such a symbol.
Not sure if this has any actual measurable performance impact though
(SymbolTable::findMangle() iterates over the whole symbol table for
each call and does fuzzy matching on top of that) but this makes the
code more reassuring to read at least. (This is in practice run for def
files listing undecorated stdcall functions to be exported.)
Differential Revision: https://reviews.llvm.org/D104529
The following class isn't part of the export table; there's a
second correctly placed comment about the things that actually
belong to the export table.
Make sure that comdat symbols also have a non-null dummy
SectionChunk associated.
This requires moving around an existing FIXME regarding comdats in
LTO.
Differential Revision: https://reviews.llvm.org/D103012
Before this patch, the maximum size of the GHASH table was 2^31 buckets. However we were storing the bucket index into a TypeIndex which has an input limit of (2^31)-4095 indices, see this link. Any value above that limit will improperly set the TypeIndex's high bit, which is interpreted as DecoratedItemIdMask. This used to cause bad indices on extraction when calling TypeIndex::toArrayIndex().
Differential Revision: https://reviews.llvm.org/D103297
Ghashing is probably going to be faster in most cases, even without
precomputed ghashes in object files.
Here is my table of results linking clang.pdb:
-------------------------------
| threads | GHASH | NOGHASH |
-------------------------------
| j1 | 51.031s | 25.141s |
| j2 | 31.079s | 22.109s |
| j4 | 18.609s | 23.156s |
| j8 | 11.938s | 21.984s |
| j28 | 8.375s | 18.391s |
-------------------------------
This shows that ghashing is faster if at least four cores are available.
This may make the linker slower if most cores are busy in the middle of
a build, but in that case, the linker probably isn't on the critical
path of the build. Incremental build performance is arguably more
important than highly contended batch build link performance.
The -time output indicates that ghash computation is the dominant
factor:
Input File Reading: 924 ms ( 1.8%)
GC: 689 ms ( 1.3%)
ICF: 527 ms ( 1.0%)
Code Layout: 414 ms ( 0.8%)
Commit Output File: 24 ms ( 0.0%)
PDB Emission (Cumulative): 49938 ms ( 94.8%)
Add Objects: 46783 ms ( 88.8%)
Global Type Hashing: 38983 ms ( 74.0%)
GHash Type Merging: 5640 ms ( 10.7%)
Symbol Merging: 2154 ms ( 4.1%)
Publics Stream Layout: 188 ms ( 0.4%)
TPI Stream Layout: 18 ms ( 0.0%)
Commit to Disk: 2818 ms ( 5.4%)
--------------------------------------------------
Total Link Time: 52669 ms (100.0%)
We can speed that up with a faster content hash (not SHA1).
Differential Revision: https://reviews.llvm.org/D102888
The COFF driver produces an ABSOLUTE relocation base for an ADDR32
relocation type and the system is 64 bits (machine=AMD64). The
relocation information won't be added in the output and could
produce an incorrect address access during run-time. This change
set checks if the relocation type is IMAGE_REL_AMD64_ADDR32 and
if so, adds the relocated symbol as IMAGE_REL_BASED_HIGHLOW base.
Differential Revision: https://reviews.llvm.org/D96619
Previously we simply didn't check this. Prereq to make the test suite
pass with ghash enabled by default.
Differential Revision: https://reviews.llvm.org/D102885
These symbols are long, and they tend to cause the PDB file size to
overflow. They are generally not necessary when debugging problems in
user code.
This change reduces the size of chrome.dll.pdb with coverage from
6,937,108,480 bytes to 4,690,210,816 bytes.
Differential Revision: https://reviews.llvm.org/D102719
Handle PDB writing errors like any other error in LLD: emit an error and
continue. This allows the linker to print timing data and summary data
after linking, which can be helpful for finding PDB size problems. Also
report how large the file would have been.
Example output:
lld-link: error: Output data is larger than 4 GiB. File size would have been 6,937,108,480
lld-link: error: failed to write PDB file ./chrome.dll.pdb
Summary
--------------------------------------------------------------------------------
33282 Input OBJ files (expanded from all cmd-line inputs)
4 PDB type server dependencies
0 Precomp OBJ dependencies
33396931 Input type records
... snip ...
Input File Reading: 59756 ms ( 45.5%)
GC: 7500 ms ( 5.7%)
ICF: 3336 ms ( 2.5%)
Code Layout: 6329 ms ( 4.8%)
PDB Emission (Cumulative): 46192 ms ( 35.2%)
Add Objects: 27609 ms ( 21.0%)
Type Merging: 16740 ms ( 12.8%)
Symbol Merging: 10761 ms ( 8.2%)
Publics Stream Layout: 9383 ms ( 7.1%)
TPI Stream Layout: 1678 ms ( 1.3%)
Commit to Disk: 3461 ms ( 2.6%)
--------------------------------------------------
Total Link Time: 131244 ms (100.0%)
Differential Revision: https://reviews.llvm.org/D102713
LLD already produces a nice error message when sections exceed 4GB, and
this setRVA assertion causes LLD to crash instead of diagnosing the
error properly.
No test because we don't want slow tests that create 4GB files.
Since c579a5b1d9 we don't traverse
.eh_frame when doing GC. But the exception handling personality
function needs to be included, and is only referenced from within
.eh_frame.
Differential Revision: https://reviews.llvm.org/D102138
As this isn't handled as a regular relocation, the normal handling of
maybeReportRelocationToDiscarded in Chunks.cpp doesn't apply here.
This would have caught the issue fixed by
82de4e0753.
Differential Revision: https://reviews.llvm.org/D102115
The Halide project uses `#pragma comment(linker, "/STACK:...")` to set
the stack size high enough for our embedded compiler to run in end-user
programs on Windows.
Unfortunately, lld-link.exe breaks on this when embedded in a COFF
object, despite supporting the flag on the command line. MSVC's link.exe
supports this fine. This patch extends support for this to lld-link.exe
for better compatibility with MSVC projects.
Differential Revision: https://reviews.llvm.org/D99680
This is a followup to 2b01a417d7ccb001ccc1185ef5fdc967c9fac8d7;
previously the RVAs of the exported symbols from comdats were left
zero.
Thanks to Kleis Auke Wolthuizen for the fix suggestion and pointing
out the omission.
Differential Revision: https://reviews.llvm.org/D101615
When looking for the "all" symbols that are supposed to be exported,
we can't look at the live flag - the symbols we mark as to be
exported will become GC roots even if they aren't yet marked as live.
With this in place, building an LLVM library with BUILD_SHARED_LIBS
produces the same set of symbols exported regardless of whether the
--gc-sections flag is specified, both with and without being built
with -ffunction-sections.
Differential Revision: https://reviews.llvm.org/D101522
This is a different approach from D98993 that should achieve most of the
same benefit. The two changes are:
1. Sort the list of associated child sections by section name
2. Do not consider associated sections to have children themselves
This fixes the main issue, which was that we sometimes considered an
.xdata section to have a child .pdata section. That lead to slow links
and larger binaries (less xdata folding).
Otherwise, this should be NFC: we go back to ignoring .debug/.gljmp and
other metadata sections rather than only looking at pdata/xdata. We
discovered that we do care about other associated sections, like ASan
global registration metadata.
Problem:
On SystemZ we need to open text files in text mode. On Windows, files opened in text mode adds a CRLF '\r\n' which may not be desirable.
Solution:
This patch adds two new flags
- OF_CRLF which indicates that CRLF translation is used.
- OF_TextWithCRLF = OF_Text | OF_CRLF indicates that the file is text and uses CRLF translation.
Developers should now use either the OF_Text or OF_TextWithCRLF for text files and OF_None for binary files. If the developer doesn't want carriage returns on Windows, they should use OF_Text, if they do want carriage returns on Windows, they should use OF_TextWithCRLF.
So this is the behaviour per platform with my patch:
z/OS:
OF_None: open in binary mode
OF_Text : open in text mode
OF_TextWithCRLF: open in text mode
Windows:
OF_None: open file with no carriage return
OF_Text: open file with no carriage return
OF_TextWithCRLF: open file with carriage return
The Major change is in llvm/lib/Support/Windows/Path.inc to only set text mode if the OF_CRLF is set.
```
if (Flags & OF_CRLF)
CrtOpenFlags |= _O_TEXT;
```
These following files are the ones that still use OF_Text which I left unchanged. I modified all these except raw_ostream.cpp in recent patches so I know these were previously in Binary mode on Windows.
./llvm/lib/Support/raw_ostream.cpp
./llvm/lib/TableGen/Main.cpp
./llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
./llvm/unittests/Support/Path.cpp
./clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
./clang/lib/Frontend/CompilerInstance.cpp
./clang/lib/Driver/Driver.cpp
./clang/lib/Driver/ToolChains/Clang.cpp
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D99426
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
The only known reason why ICF should not merge otherwise identical
sections with differing associated sections has to do with exception
handling tables. It's not clear what ICF should do when there are other
kinds of associated sections. In every other case when this has come up,
debug info and CF guard metadata, we have opted to make ICF ignore the
associated sections.
For comparison, ELF doesn't do anything for comdat groups. Instead,
.eh_frame is parsed to figure out if a section has an LSDA, and if so,
ICF is disabled.
Another issue is that the order of associated sections is not defined.
We have had issues in the past (crbug.com/1144476) where changing the
order of the .xdata/.pdata sections in the object file lead to large ICF
slowdowns.
To address these issues, I decided it would be best to explicitly
consider only .pdata and .xdata sections during ICF. This makes it easy
to ignore the object file order, and I think it makes the intention of
the code clearer.
I've also made the children() accessor return an empty list for
associated sections. This mostly only affects ICF and GC. This was the
behavior before I made this a linked list, so the behavior change should
be good. This had positive effects on chrome.dll: more .xdata sections
were merged that previously could not be merged because they were
associated with distinct .pdata sections.
Reviewed By: mstorsjo
Differential Revision: https://reviews.llvm.org/D98993
The S_[GL]PROC32_ID symbol records are supposed to point to function ID
records. If they don't, they are corrupt. The warning message here was
very technical, but a user has encountered it in the wild. Add some more
information and some more testing.
This reverts commit bacf9cf2c5 and
reinstates commit 1a9bd5b813.
Reverting this commit did not appear to make the problem go away, so we
can go ahead and reland it.
Remove a stray -lib argument in guardcf-lto.ll; llvm-lib doesn't
support generating import libs from a def file unlike lib.exe.
Previously this worked because the -lib argument was ignored
(printing only a warning).
Differential Revision: https://reviews.llvm.org/D96699
/reproduce: now works correctly with:
- /call-graph-ordering-file:
- /def:
- /natvis:
- /order:
- /pdbstream:
I went through all instances of MemoryBuffer::getFile() and made sure
everything that didn't already do so called takeBuffer().
For natvis, that wasn't possible since DebugInfo/PDB wants to take
owernship of the natvis buffer. For that case, I'm manually adding the
tar file entry.
/natvis: and /pdbstream: is slightly awkward, since createResponseFile()
always adds these flags to the response file but createPDB() (which
ultimately adds the files referenced by the flags) is only called if
/debug is also passed. So when using /natvis: without /debug with
/reproduce:, lld won't warn, but when linking using the response
file from the archive, it won't find the natvis file since it's not
in the tar. This isn't a new issue though, and after this patch things
at least work with using /natvis: _with_ debug with /reproduce:.
(Same for /pdbstream:)
Differential Revison: https://reviews.llvm.org/D97212
I noticed that this option was not appearing at all in the `--help`
messages for `wasm-ld` or `ld.lld`.
Add help text and make it consistent across all ports.
Differential Revision: https://reviews.llvm.org/D94925
This reverts commit 5b7aef6eb4 and relands
6529d7c5a4.
The ASan error was debugged and determined to be the fault of an invalid
object file input in our test suite, which was fixed by my last change.
LLD's project policy is that it assumes input objects are valid, so I
have added a comment about this assumption to the relocation bounds
check.
This is a pretty classic optimization. Instead of processing symbol
records and copying them to temporary storage, do a first pass to
measure how large the module symbol stream will be, and then copy the
data into place in the PDB file. This requires defering relocation until
much later, which accounts for most of the complexity in this patch.
This patch avoids copying the contents of all live .debug$S sections
into heap memory, which is worth about 20% of private memory usage when
making PDBs. However, this is not an unmitigated performance win,
because it can be faster to read dense, temporary, heap data than it is
to iterate symbol records in object file backed memory a second time.
Results on release chrome.dll:
peak mem: 5164.89MB -> 4072.19MB (-1,092.7MB, -21.2%)
wall-j1: 0m30.844s -> 0m32.094s (slightly slower)
wall-j3: 0m20.968s -> 0m20.312s (slightly faster)
wall-j8: 0m19.062s -> 0m17.672s (meaningfully faster)
I gathered similar numbers for a debug, component build of content.dll
in Chrome, and the performance impact of this change was in the noise.
The memory usage reduction was visible and similar.
Because of the new parallelism in the PDB commit phase, more cores makes
the new approach faster. I'm assuming that most C++ developer machines
these days are at least quad core, so I think this is a win.
Differential Revision: https://reviews.llvm.org/D94267
Before this patch, when using LLD with /DEBUG:GHASH and MSVC precomp.OBJ files, we had a bunch of:
lld-link: warning: S_[GL]PROC32ID record in blabla.obj refers to PDB item index 0x206ED1 which is not a LF[M]FUNC_ID record
This was caused by LF_FUNC_ID and LF_MFUNC_ID which didn't have correct mapping to the corresponding TPI records. The root issue was that the indexMapStorage was improperly re-assembled in UsePrecompSource::remapTpiWithGHashes.
After this patch, /DEBUG and /DEBUG:GHASH produce exactly the same debug infos in the PDB.
Differential Revision: https://reviews.llvm.org/D93732
It's an extension to ld64, but all the other ports have it, and
someone asked for it in PR43721.
While here, change the COFF help text to match the other ports.
Differential Revision: https://reviews.llvm.org/D93491
Martin Storsjö