This is causing use-after-frees due to references getting invalidating
after the loadedDylibs map grows, see comments on the PR.
This reverts commit 475a8a47ead32755bb1377d361afbd1928880e14.
When loading frameworks it is possible to have load commands for the
same framework through symlinks and the real path. To avoid loading
these multiple times find the real path before checking the dylib cache.
Apologies for the large change, I looked for ways to break this up and
all of the ones I saw added real complexity. This change focuses on the
option's prefixed names and the array of prefixes. These are present in
every option and the dominant source of dynamic relocations for PIE or
PIC users of LLVM and Clang tooling. In some cases, 100s or 1000s of
them for the Clang driver which has a huge number of options.
This PR addresses this by building a string table and a prefixes table
that can be referenced with indices rather than pointers that require
dynamic relocations. This removes almost 7k dynmaic relocations from the
`clang` binary, roughly 8% of the remaining dynmaic relocations outside
of vtables. For busy-boxing use cases where many different option tables
are linked into the same binary, the savings add up a bit more.
The string table is a straightforward mechanism, but the prefixes
required some subtlety. They are encoded in a Pascal-string fashion with
a size followed by a sequence of offsets. This works relatively well for
the small realistic prefixes arrays in use.
Lots of code has to change in order to land this though: both all the
option library code has to be updated to use the string table and
prefixes table, and all the users of the options library have to be
updated to correctly instantiate the objects.
Some follow-up patches in the works to provide an abstraction for this
style of code, and to start using the same technique for some of the
other strings here now that the infrastructure is in place.
ld64.lld would previously allow you to link against dylibs linked with
`-allowable_client`, even if the client's name does not match any
allowed client.
This change fixes that. See #114146 for related discussion.
The test binary `liballowable_client.dylib` was created on macOS with:
echo | clang -xc - -dynamiclib -mmacosx-version-min=10.11 -arch x86_64
-Wl,-allowable_client,allowed -o lib/liballowable_client.dylib
For COFF and ELF that are mostly free of global states, lld::errs() and
lld::outs() should not be used. This migration change allows us to
remove lld::errs, which uses the global errorHandler().
And use it to print the correct default OpenMP version for flang and
flang -fc1.
This change adds an optional `HelpTextsForVariants` to options. This
allows you to change the help text that gets shown in documentation and
`--help` based on the program its being generated for.
As `OptTable` needs to be constexpr compatible, I have used a std::array
of help text variants. Each entry is:
(list of visibilities) - > help text string
So for the OpenMP version we have (flang, fc1) -> "OpenMP version for
flang is...".
So you can have multiple visibilities use the same string. The number of
entries is currently set to 1, and the number of visibilities per entry
is 2, because that's the maximum we need for now. The code is written so
we can increase these numbers later, and the unused elements will be initialised.
I have not applied this to group descriptions just because I don't know
of one that needs changing. It could easily be enabled for those too if
needed. There are minor changes to them just to get it all to compile.
This approach of storing many help strings per option in the 1 driver
library seemed preferable to making a whole new library for Flang (even
if that would mostly be including stuff from Clang).
This reverts commit 4e3b89483a6922d3f48670bb1c50a37f342918c6, with
fixes for places I'd missed updating in lld and lldb. I've also
renamed OptionVisibility::Default to "DefaultVis" to avoid ambiguity
since the undecorated name has to be available anywhere Options.inc is
included.
Original message follows:
This splits OptTable's "Flags" field into "Flags" and "Visibility",
updates the places where we instantiate Option tables, and adds
variants of the OptTable APIs that use Visibility mask instead of
Include/Exclude flags.
We need to do this to clean up a bunch of complexity in the clang
driver's option handling - there's a whole slew of flags like
CoreOption, NoDriverOption, and FlangOnlyOption there today to try to
handle all of the permutations of flags that the various drivers need,
but it really doesn't scale well, as can be seen by things like the
somewhat recently introduced CLDXCOption.
Instead, we'll provide an additive model for visibility that's
separate from the other flags. For things like "HelpHidden", which is
used as a "subtractive" modifier for option visibility, we leave that
in "Flags" and handle it as a special case.
Note that we don't actually update the users of the Include/Exclude
APIs here or change the flags that exist in clang at all - that will
come in a follow up that refactors clang's Options.td to use the
increased flexibility this change allows.
Differential Revision: https://reviews.llvm.org/D157149
This splits OptTable's "Flags" field into "Flags" and "Visibility",
updates the places where we instantiate Option tables, and adds
variants of the OptTable APIs that use Visibility mask instead of
Include/Exclude flags.
We need to do this to clean up a bunch of complexity in the clang
driver's option handling - there's a whole slew of flags like
CoreOption, NoDriverOption, and FlangOnlyOption there today to try to
handle all of the permutations of flags that the various drivers need,
but it really doesn't scale well, as can be seen by things like the
somewhat recently introduced CLDXCOption.
Instead, we'll provide an additive model for visibility that's
separate from the other flags. For things like "HelpHidden", which is
used as a "subtractive" modifier for option visibility, we leave that
in "Flags" and handle it as a special case.
Note that we don't actually update the users of the Include/Exclude
APIs here or change the flags that exist in clang at all - that will
come in a follow up that refactors clang's Options.td to use the
increased flexibility this change allows.
Differential Revision: https://reviews.llvm.org/D157149
This avoids recomputing string length that is already known at compile time.
It has a slight impact on preprocessing / compile time, see
https://llvm-compile-time-tracker.com/compare.php?from=3f36d2d579d8b0e8824d9dd99bfa79f456858f88&to=e49640c507ddc6615b5e503144301c8e41f8f434&stat=instructions:u
This a recommit of e953ae5bbc313fd0cc980ce021d487e5b5199ea4 and the subsequent fixes caa713559bd38f337d7d35de35686775e8fb5175 and 06b90e2e9c991e211fecc97948e533320a825470.
The above patchset caused some version of GCC to take eons to compile clang/lib/Basic/Targets/AArch64.cpp, as spotted in aa171833ab0017d9732e82b8682c9848ab25ff9e.
The fix is to make BuiltinInfo tables a compilation unit static variable, instead of a private static variable.
Differential Revision: https://reviews.llvm.org/D139881
Revert "Fix lldb option handling since e953ae5bbc313fd0cc980ce021d487e5b5199ea4 (part 2)"
Revert "Fix lldb option handling since e953ae5bbc313fd0cc980ce021d487e5b5199ea4"
GCC build hangs on this bot https://lab.llvm.org/buildbot/#/builders/37/builds/19104
compiling CMakeFiles/obj.clangBasic.dir/Targets/AArch64.cpp.d
The bot uses GNU 11.3.0, but I can reproduce locally with gcc (Debian 12.2.0-3) 12.2.0.
This reverts commit caa713559bd38f337d7d35de35686775e8fb5175.
This reverts commit 06b90e2e9c991e211fecc97948e533320a825470.
This reverts commit e953ae5bbc313fd0cc980ce021d487e5b5199ea4.
We only want to rewrite paths to files that the linker looks up. Files
under RPATH are looked up at runtime by dyld.
Reviewed By: #lld-macho, keith
Differential Revision: https://reviews.llvm.org/D140167
A symbol `$ld$previous$/Another$1.2.3$1$3.0$14.0$_xxx$` means
"pretend symbol `_xxx` is in dylib `/Another` with version `1.2.3`
if the deployment target is between `3.0` and `14.0` and we're
targeting platform `1` (ie macOS)".
This means dylibs can now inject synthetic dylibs into the link, so
DylibFile needs to grow a 3rd constructor.
The only other interesting thing is that such an injected dylib
counts as a use of the original dylib. This patch gets this mostly
right (if _only_ `$ld$previous` symbols are used from a dylib,
we don't add a dep on the dylib itself, matching ld64), but one case
where we don't match ld64 yet is that ld64 even omits the original
dylib when linking it with `-needed-l`. Lld currently still adds a load
command for the original dylib in that case. (That's for a future
patch.)
Fixes#56074.
Differential Revision: https://reviews.llvm.org/D130725
This flag was introduced in ld64-609. It instructs the linker to link to
a static library while treating its symbols as if they had hidden
visibility. This is useful when building a dylib that links to static
libraries but we don't want the symbols from those to be exported.
Closes#51505
This reland adds bitcode file handling, so we won't get any compile
errors due to BitcodeFile::forceHidden being unused.
Differential Revision: https://reviews.llvm.org/D130473
This flag was introduced in ld64-609. It instructs the linker to link to
a static library while treating its symbols as if they had hidden
visibility. This is useful when building a dylib that links to static
libraries but we don't want the symbols from those to be exported.
Closes#51505
Differential Revision: https://reviews.llvm.org/D130473
This flag suppresses warnings produced by the linker. In ld64 this has
an interesting interaction with -fatal_warnings, it silences the
warnings but the link still fails. Instead of doing that here we still
print the warning and eagerly fail the link in case both are passed,
this seems more reasonable so users can understand why the link fails.
Differential Revision: https://reviews.llvm.org/D127564
Before this,
clang empty.cc -target x86_64-apple-ios13.1-macabi \
-framework CoreServices -fuse-ld=lld
would error out with
ld64.lld: error: path/to/MacOSX.sdk/System/Library/Frameworks/
CoreServices.framework/Versions/A/Frameworks/CarbonCore.framework/
Versions/A/CarbonCore.tbd(
/System/Library/Frameworks/
CoreServices.framework/Versions/A/Frameworks/CarbonCore.framework/
Versions/A/CarbonCore) is incompatible with x86_64 (macCatalyst)
Now it works, like with ld64.
Differential Revision: https://reviews.llvm.org/D124336
Move all variables at file-scope or function-static-scope into a hosting structure (lld::CommonLinkerContext) that lives at lldMain()-scope. Drivers will inherit from this structure and add their own global state, in the same way as for the existing COFFLinkerContext.
See discussion in https://lists.llvm.org/pipermail/llvm-dev/2021-June/151184.html
Differential Revision: https://reviews.llvm.org/D108850
We need to reset global state between runs, similar to the other ports.
There's some file-static state which needs to be reset as well and we
need to add some new helpers for that.
With this change, most LLD Mach-O tests pass with `LLD_IN_TEST=2` (which
runs the linker twice on each test). Some tests will be fixed by the
remainder of this stack, and the rest are fundamentally incompatible
with that mode (e.g. they intentionally throw fatal errors).
Fixes PR52070.
Reviewed By: #lld-macho, int3
Differential Revision: https://reviews.llvm.org/D112878
In Driver.cpp, addFramework used std::string instance to represent the path of a framework, which will be freed after the function returns. However, this string is stored in loadedArchive, which will be used later to compare with path of newly added frameworks. This caused https://bugs.llvm.org/show_bug.cgi?id=52133. A test is included in this commit to reproduce this bug.
Now resolveDylibPath returns a StringRef instance, and it uses StringSaver to save its data, then returns it to functions on the top. This ensures the resolved framework path is still valid after LC_LINKER_OPTION is parsed.
Reviewed By: int3, #lld-macho, oontvoo
Differential Revision: https://reviews.llvm.org/D111706
This matters for example for the iPhoneSimulator14.0.sdk, which has
a System/Library/Frameworks/UIKit.framework/UIKit that has
LC_BUILD_VERSION with minos of 14.0, so linking against that file
will produce warnings like:
.../iPhoneSimulator14.0.sdk/System/Library/Frameworks/UIKit.framework/UIKit
has version 14.0.0, which is newer than target minimum of 12.0.0
when targeting x86_64-apple-ios12.0-simulator. That doens't happen when
linking against UIKit.tbd instead, obviously.
Linking with RC_TRACE_DYLIB_SEARCHING=1 shows that ld64 also searches
the tbd file first, and we already get that right for non-framework
dylibs.
Fixes crbug.com/1249456.
Differential Revision: https://reviews.llvm.org/D109768
The previous logic was duplicated between symbol-initiated
archive loads versus flag-initiated loads (i.e. `-force_load` and
`-ObjC`). This resulted in code duplication as well as redundant work --
we would create Archive instances twice whenever we had one of those
flags; once in `getArchiveMembers` and again when we constructed the
ArchiveFile.
This was motivated by an upcoming diff where we load archive members
containing ObjC-related symbols before loading those containing
ObjC-related sections, as well as before performing symbol resolution.
Without this refactor, it would be difficult to do that while avoiding
loading the same archive member twice.
Differential Revision: https://reviews.llvm.org/D108780
Ported from COFF/ELF; test is adapted from
test/COFF/thinlto-archivecollision.ll
LTO expects every bitcode file to have a unique name. If given multiple bitcode
files with the same name, it errors with "Expected at most one ThinLTO module
per bitcode file".
This change incorporates the archive name, to disambiguate members with the
same name in different archives and the offset in archive to disambiguate
members with the same name in the same archive.
Differential Revision: https://reviews.llvm.org/D106179
findLibrary() returned a StringRef while findFramework & other helper
functions returned std::strings. Standardize on std::string.
(I initially tried making the helper functions all return StringRefs,
but I realized we shouldn't return input StringRefs since their
lifetimes would not be obvious from the calling code.)
For debugging dylib loading, it's useful to have some insight into what
the linker is doing.
ld64 has the undocumented RC_TRACE_DYLIB_SEARCHING env var
for this printing dylib search candidates.
This adds a flag --print-dylib-search to make lld print the seame information.
It's useful for users, but also for writing tests. The output is formatted
slightly differently than ld64, but we still support RC_TRACE_DYLIB_SEARCHING
to offer at least a compatible way to trigger this.
ld64 has both `-print_statistics` and `-trace_symbol_output` to enable
diagnostics output. I went with "print" since that seems like a more
straightforward name.
Differential Revision: https://reviews.llvm.org/D103985
Also adjust a few comments, and move the DylibFile comment talking about
umbrella next to the parameter again.
Differential Revision: https://reviews.llvm.org/D103783
D103423 neglected to call `parseReexports()` for nested TBD
documents, leading to symbol resolution failures when trying to look up
a symbol nested more than one level deep in a TBD file. This fixes the
regression and adds a test.
It also appears that `umbrella` wasn't being set properly when calling
`parseLoadCommands` -- it's supposed to resolve to `this` if `nullptr`
is passed. I didn't write a failing test case for this but I've made
`umbrella` a member so the previous behavior should be preserved.
Reviewed By: #lld-macho, thakis
Differential Revision: https://reviews.llvm.org/D103586
I forgot to move the message() call around as requested in D103428
before committing that change. Move it now.
Also, improve the ordinal uniq'ing comment. I hadn't realized that the
distinct-but-identical files happen with --reproduce and not in general.
No behavior change.
Differential Revision: https://reviews.llvm.org/D103522
