Commit Graph

417 Commits

Author SHA1 Message Date
Fangrui Song 7c7702b318 [ELF] Move section assignment from initializeSymbols to postParse
https://discourse.llvm.org/t/parallel-input-file-parsing/60164

initializeSymbols currently sets Defined::section and handles non-prevailing
COMDAT groups. Move the code to the parallel postParse to reduce work from the
single-threading code path and make parallel section initialization infeasible.

Postpone reporting duplicate symbol errors so that the messages have the
section information. (`Defined::section` is assigned in postParse and another
thread may not have the information).

* duplicated-synthetic-sym.s: BinaryFile duplicate definition (very rare) now
  has no section information
* comdat-binding: `%t/w.o %t/g.o` leads to an undesired undefined symbol. This
  is not ideal but we report a diagnostic to inform that this is unsupported.
  (See release note)
* comdat-discarded-lazy.s: %tdef.o is unextracted. The new behavior (discarded
  section error) makes more sense
* i386-comdat.s: switched to a better approach working around
  .gnu.linkonce.t.__x86.get_pc_thunk.bx in glibc<2.32 for x86-32.
  Drop the ancient no-longer-relevant workaround for __i686.get_pc_thunk.bx

Depends on D120640

Differential Revision: https://reviews.llvm.org/D120626
2022-03-15 19:24:41 -07:00
Fangrui Song 9b61fff0eb Revert D120626 "[ELF] Move section assignment from initializeSymbols to postParse"
This reverts commit c30e6447c0.
It exposed brittle support for __x86.get_pc_thunk.bx.
Need to think a bit how to support __x86.get_pc_thunk.bx.
2022-03-15 19:00:54 -07:00
Fangrui Song c1d4c67718 [ELF] Suppress duplicate symbol error for __x86.get_pc_thunk.bx 2022-03-15 17:20:29 -07:00
Fangrui Song c30e6447c0 [ELF] Move section assignment from initializeSymbols to postParse
https://discourse.llvm.org/t/parallel-input-file-parsing/60164

initializeSymbols currently sets Defined::section and handles non-prevailing
COMDAT groups. Move the code to the parallel postParse to reduce work from the
single-threading code path and make parallel section initialization infeasible.

Postpone reporting duplicate symbol errors so that the messages have the
section information. (`Defined::section` is assigned in postParse and another
thread may not have the information).

* duplicated-synthetic-sym.s: BinaryFile duplicate definition (very rare) now
  has no section information
* comdat-binding: `%t/w.o %t/g.o` leads to an undesired undefined symbol. This
  is not ideal but we report a diagnostic to inform that this is unsupported.
  (See release note)
* comdat-discarded-lazy.s: %tdef.o is unextracted. The new behavior (discarded
  section error) makes more sense

Depends on D120640

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D120626
2022-03-14 14:13:41 -07:00
Fangrui Song c7cf960d85 [ELF] Set the priority of STB_GNU_UNIQUE the same as STB_WEAK
In GCC -fgnu-unique output, STB_GNU_UNIQUE symbols are always defined relative
to a section in a COMDAT group. Currently `other` cannot be STB_GNU_UNIQUE for
valid input, so this patch is NFC.

If we switch to the model that ignores COMDAT resolution when performing symbol
resolution (D120626), this will fix bogus `relocation refers to a symbol in a
discarded section` errors when mixing -fno-gnu-unique objects with -fgnu-unique
objects.

Differential Revision: https://reviews.llvm.org/D120640
2022-03-14 12:00:15 -07:00
Fangrui Song b07ef4d566 [ELF] Rename Symbol::compare to shouldReplace. NFC
The return value is not a boolean instead of a tri-state.
Suggested by Peter Smith in D120640.
2022-02-28 18:25:21 +00:00
Fangrui Song d14d8664e3 [ELF] Change global variable backwardReferences to a LinkerDriver member variable. NFC
Similar to whyExtract.
2022-02-27 20:33:28 +00:00
Fangrui Song 7fd3849b35 [ELF] Move --print-archive-stats= and --why-extract= beside --warn-backrefs report
So that early errors don't suppress their output.
2022-02-27 20:23:09 +00:00
Fangrui Song 19e37a7415 [ELF] Update comment. NFC 2022-02-24 14:09:00 -08:00
Fangrui Song 6d94340809 [ELF] Simplify resolveDefined and resolveCommon
This is NFC for valid input (COMMON symbols cannot be weak or versioned).
2022-02-24 14:08:06 -08:00
Fangrui Song 15617cdb55 [ELF] Simplify --fortran-common. NFC 2022-02-24 12:21:40 -08:00
Fangrui Song 4129890dd8 [ELF] De-template Symbol::resolveLazy. NFC 2022-02-24 12:20:05 -08:00
Fangrui Song 8ca46bba23 [ELF] Move isUsedInRegularObj assignment from ctor to call sites. NFC
This removes the tricky
`isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind)`
and the copy from `Symbol::mergeProperties`.
2022-02-23 21:32:50 -08:00
Fangrui Song 88d66f6ed1 [ELF] Move duplicate symbol check after input file parsing
https://discourse.llvm.org/t/parallel-input-file-parsing/60164

To decouple symbol initialization and section initialization, `Defined::section`
assignment should be postponed after input file parsing. To avoid spurious
duplicate definition error due to two definitions in COMDAT groups of the same
signature, we should postpone the duplicate symbol check.

The function is called postScan instead of a more specific name like
checkDuplicateSymbols, because we may merge Symbol::mergeProperties into
postScan. It is placed after compileBitcodeFiles to apply to ET_REL files
produced by LTO. This causes minor diagnostic regression
for skipLinkedOutput configurations: ld.lld --thinlto-index-only a.bc b.o
(bitcode definition prevails) won't detect duplicate symbol error. I think this
is an acceptable compromise. The important cases where (a) both files are
bitcode or (b) --thinlto-index-only is unused are still detected.

Reviewed By: ikudrin

Differential Revision: https://reviews.llvm.org/D119908
2022-02-22 10:07:58 -08:00
Fangrui Song 467e1b3aaa [ELF] reportDuplicate: change Symbol * to const Symbol &. NFC 2022-02-15 11:18:31 -08:00
Fangrui Song 3d85424096 [ELF] Parse archives as --start-lib object files
https://maskray.me/blog/2022-01-16-archives-and-start-lib

For every definition in an extracted archive member, we intern the symbol twice,
once for the archive index entry, once for the .o symbol table after extraction.
This is inefficient.

Symbols in a --start-lib ObjFile/BitcodeFile are only interned once because the
result is cached in symbols[i].

Just handle an archive using the --start-lib code path. We can therefore remove
ArchiveFile and LazyArchive. For many projects, archive member extraction ratio
is high and it is a net performance win. Linking a Release build of clang is
1.01x as fast.

Note: --start-lib scans symbols in the same order that llvm-ar adds them to the
index, so in the common case the semantics should be identical. If the archive
symbol table was created in a different order, or is incomplete, this strategy
may have different semantics. Such cases are considered user error.

The `is neither ET_REL nor LLVM bitcode` error is changed to a warning.
Previously an archive may have such members without a diagnostic. Using a
warning prevents breakage.

* For some tests, the diagnostics get improved where we did not consider
  the archive member name: `b.a:` => `b.a(b.o):`.
* `no-obj.s`: the link is now allowed, matching GNU ld
* `archive-no-index.s`: the `is neither ET_REL nor LLVM bitcode` diagnostic is
  demoted to a warning.
* `incompatible.s`: even when an archive is unextracted, we may report an
  "incompatible with" error.

---

I recently decreased sizeof(SymbolUnion) by 8 and decreased memory usage quite a
bit, so retaining `symbols` for un-extracted archive members should not cause a
memory usage problem.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D119074
2022-02-15 09:38:00 -08:00
Fangrui Song 27bb799095 [ELF] Clean up headers. NFC 2022-02-07 21:53:34 -08:00
Fangrui Song 977a1a523c [ELF] Symbol::replace: use the old nameData/nameSize. NFC
Currently `this->getName() == newSym.getName()`.
By keeping the old nameData/nameSize, newSym's nameData/nameSize will be
ignored. The call sites can avoid calling getName().

printTraceSymbol needs to take the symbol name since `other`'s name is empty.
2022-02-05 16:34:02 -08:00
Fangrui Song 9e885eac54 [ELF] Remove !isLazy() condition from computeBinding
Seems applicable since we demote lazy symbols to Undefined (D111365).
2022-01-15 23:58:15 -08:00
Fangrui Song c0fc09ab91 [ELF] Remove config->relocatable condition from Symbol::computeBinding 2022-01-15 23:49:48 -08:00
Fangrui Song b3cc47006b [ELF] Speed up Symbol::computeBinding. NFC
When computeBinding is inlined into includeInDynsym and computeIsPreemptible,
the optimizer can remove the config->gnuUnique load.
2022-01-15 23:40:44 -08:00
Fangrui Song 01a51629c2 [ELF] Slightly speed up Symbol::includeInDynsym. NFC 2022-01-15 23:32:48 -08:00
Fangrui Song 7330fd236e [ELF] Simplify Symbol::includeInDynsym 2022-01-15 23:27:45 -08:00
Fangrui Song 5d3bd7f360 [ELF] Move gotIndex/pltIndex/globalDynIndex to SymbolAux
to decrease sizeof(SymbolUnion) by 8 on ELF64 platforms.

Symbols needing such information are typically 1% or fewer (5134 out of 560520
when linking clang, 19898 out of 5550705 when linking chrome). Storing them
elsewhere can decrease memory usage and symbol initialization time.
There is a ~0.8% saving on max RSS when linking a large program.

Future direction:

* Move some of dynsymIndex/verdefIndex/versionId to SymbolAux
* Support mixed TLSDESC and TLS GD without increasing sizeof(SymbolUnion)

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D116281
2022-01-09 13:43:27 -08:00
Fangrui Song 935229f66b [ELF] Symbol::getVA: assert not called on a lazy symbol
The code path is dead after D111365.
2022-01-05 00:46:48 -08:00
Luís Ferreira 10e40a4ea3 [lld] Add support for other demanglers other than Itanium
LLVM core library supports demangling other mangled symbols other than itanium,
such as D and Rust. LLD should use those demanglers in order to output pretty
demangled symbols on error messages.

Reviewed By: MaskRay, #lld-macho

Differential Revision: https://reviews.llvm.org/D116279
2022-01-05 03:25:41 +00:00
Luís Ferreira 8792cd75d0 Revert "[lld] Add support for other demanglers other than Itanium"
This reverts commit e60d6dfd5a.

clang-ppc64le-rhel buildbot failed (https://lab.llvm.org/buildbot#builders/57/builds/13424):

    tools/lld/MachO/CMakeFiles/lldMachO.dir/Symbols.cpp.o: In function `lld::demangle(llvm::StringRef, bool)':
    Symbols.cpp:(.text._ZN3lld8demangleEN4llvm9StringRefEb[_ZN3lld8demangleEN4llvm9StringRefEb]+0x90): undefined reference to `llvm::demangle(std::string const&)'
2021-12-30 18:04:21 +00:00
Luís Ferreira e60d6dfd5a [lld] Add support for other demanglers other than Itanium
LLVM core library supports demangling other mangled symbols other than itanium,
such as D and Rust. LLD should use those demanglers in order to output pretty
demangled symbols on error messages.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D116279
2021-12-30 17:52:38 +00:00
Fangrui Song e9262edf0d [ELF] SymbolTable:🔣 don't filter out PlaceholderKind
Placeholders (-y and redirectSymbols removed versioned symbols) are very rare and
the check just makes symbol table iteration slower. Most iterations filter out
placeholders anyway, so this change just drops the filter behavior.

For "Add symbols to symtabs", we need to ensure that redirectSymbols sets
isUsedInRegularObj to false when making a symbol placeholder, to avoid an
assertion failure in SymbolTableSection<ELFT>::writeTo.

My .text is 2KiB smaller. The speed-up linking chrome is 0.x%.
2021-12-26 18:11:45 -08:00
Fangrui Song 7924b3814f [ELF] Add Symbol::hasVersionSuffix
"Process symbol versions" may take 2+% time.
"Redirect symbols" may take 0.6% time.
This change speeds up the two passes and makes `*sym.getVersionSuffix()
== '@'` in the `undefined reference` diagnostic cleaner.

Linking chrome (no debug info) and another large program is 1.5% faster.

For empty-ver2.s: the behavior now matches GNU ld, though I'd consider the input
invalid and the exact behavior does not matter.
2021-12-26 17:25:54 -08:00
Kazu Hirata 62e48ed10f Use isa instead of dyn_cast (NFC) 2021-12-24 21:22:27 -08:00
Fangrui Song e1b6b5be46 [ELF] Avoid referencing SectionBase::repl after ICF
It is fairly easy to forget SectionBase::repl after ICF.
Let ICF rewrite a Defined symbol's `section` field to avoid references to
SectionBase::repl in subsequent passes. This slightly improves the --icf=none
performance due to less indirection (maybe for --icf={safe,all} as well if most
symbols are Defined).

With this change, there is only one reference to `repl` (--gdb-index D89751).
We can undo f4fb5fd752 (`Move Repl to SectionBase.`)
but move `repl` to `InputSection` instead.

Reviewed By: ikudrin

Differential Revision: https://reviews.llvm.org/D116093
2021-12-24 12:09:48 -08:00
Fangrui Song 3a5fb57393 [ELF] Replace LazyObjFile with lazy ObjFile/BitcodeFile
The new `lazy` state is the inverse of the previous `LazyObjFile::extracted`.
There are many advantages:

* previously when a LazyObjFile was extracted, a new ObjFile/BitcodeFile was created; now the file is reused, just with `lazy` cleared
* avoid the confusing transfer of `symbols` from LazyObjFile to the new file
* the `incompatible file:` diagnostic is unified with `is incompatible with`
* simpler code, smaller executable (6200+ bytes smaller on x86-64)
* make eager parsing feasible (for parallel section/symbol table initialization)
2021-12-22 17:41:50 -08:00
Fangrui Song c2f2bb066b [ELF] Remove unneeded SectionBase::repl indirection
sec->repl equals sec after rL371216.
2021-12-21 00:39:16 -08:00
Fangrui Song 101407bfaa [ELF] parseSymbolVersion: remove unussed pos == 0 check 2021-12-15 23:59:55 -08:00
Fangrui Song 2bdad16303 [ELF] SymbolTable::insert: keep @@ in the name
* Avoid the name truncation quirk in SymbolTable::insert: the truncated name will be replaced by @@ again.
* Allow foo and foo@@v1 in different files to be diagnosed as duplicate definition error (GNU ld behavior)
* Avoid potential redundant strlen on symbol name due to StringRefZ in ObjFile<ELFT>::initializeSymbols
2021-12-15 15:19:35 -08:00
Fangrui Song cf783be8d7 Reland D114783/D115603 [ELF] Split scanRelocations into scanRelocations/postScanRelocations
(Fixed an issue about GOT on a copy relocated alias.)
(Fixed an issue about not creating r_addend=0 IRELATIVE for unreferenced non-preemptible ifunc.)

The idea is to make scanRelocations mark some actions are needed (GOT/PLT/etc)
and postpone the real work to postScanRelocations. It gives some flexibility:

* Make it feasible to support .plt.got (PR32938): we need to know whether GLOB_DAT and JUMP_SLOT are both needed.
* Make non-preemptible IFUNC handling slightly cleaner: avoid setting/clearing sym.gotInIgot
* -z nocopyrel: report all copy relocation places for one symbol
* Make GOT deduplication feasible
* Make parallel relocation scanning feasible (if we can avoid all stateful operations and make Symbol attributes atomic), but parallelism may not be the appealing choice

Since this patch moves a large chunk of code out of ELFT templates. My x86-64
executable is actually a few hundred bytes smaller.

For ppc32-ifunc-nonpreemptible-pic.s: I remove absolute relocation references to non-preemptible ifunc
because absolute relocation references are incorrect in -fpie mode.

Reviewed By: peter.smith, ikudrin

Differential Revision: https://reviews.llvm.org/D114783
2021-12-14 16:28:41 -08:00
Fangrui Song ea15b862d7 Revert D114783 [ELF] Split scanRelocations into scanRelocations/postScanRelocations
May cause a failure for non-preemptible `bcmp` in a glibc -static link.
2021-12-14 14:33:50 -08:00
Fangrui Song b79686c6dc [ELF] Remove needsPltAddr in favor of needsCopy
needsPltAddr is equivalent to `needsCopy && isFunc`. In many places, it is
equivalent to `needsCopy` because the non-STT_FUNC cases are ruled out.

Reviewed By: ikudrin, peter.smith

Differential Revision: https://reviews.llvm.org/D115603
2021-12-14 09:52:43 -08:00
Fangrui Song a8024dfc06 [ELF] Avoid mutable addend parameter. NFC 2021-12-12 19:12:01 -08:00
Fangrui Song f1ba48d508 [ELF] Simplify Symbol::extract. NFC 2021-11-26 14:10:55 -08:00
Fangrui Song 09401dfcf1 [ELF] Rename fetch to extract
The canonical term is "extract" (GNU ld documentation, Solaris's `-z *extract`
options). Avoid inventing a term and match --why-extract. (ld64 prefers "load"
but the word is overloaded too much)

Mostly MFC, except for --help messages and the header row in
--print-archive-stats output.
2021-11-26 10:58:50 -08:00
Fangrui Song a954bb18b1 [ELF] Add --why-extract= to query why archive members/lazy object files are extracted
Similar to D69607 but for archive member extraction unrelated to GC. This patch adds --why-extract=.

Prior art:

GNU ld -M prints
```
Archive member included to satisfy reference by file (symbol)

a.a(a.o)                      main.o (a)
b.a(b.o)                      (b())
```

-M is mainly for input section/symbol assignment <-> output section mapping
(often huge output) and the information may appear ad-hoc.

Apple ld64
```
__Z1bv forced load of b.a(b.o)
_a forced load of a.a(a.o)
```

It doesn't say the reference file.

Arm's proprietary linker
```
Selecting member vsnprintf.o(c_wfu.l) to define vsnprintf.
...
Loading member vsnprintf.o from c_wfu.l.
              definition:  vsnprintf
              reference :  _printf_a
```

---

--why-extract= gives the user the full data (which is much shorter than GNU ld
-Map). It is easy to track a chain of references to one archive member with a
one-liner, e.g.

```
% ld.lld main.o a_b.a b_c.a c.a -o /dev/null --why-extract=- | tee stdout
reference       extracted       symbol
main.o  a_b.a(a_b.o)    a
a_b.a(a_b.o)    b_c.a(b_c.o)    b()
b_c.a(b_c.o)    c.a(c.o)        c()

% ruby -ane 'BEGIN{p={}}; p[$F[1]]=[$F[0],$F[2]] if $.>1; END{x="c.a(c.o)"; while y=p[x]; puts "#{y[0]} extracts #{x} to resolve #{y[1]}"; x=y[0] end}' stdout
b_c.a(b_c.o) extracts c.a(c.o) to resolve c()
a_b.a(a_b.o) extracts b_c.a(b_c.o) to resolve b()
main.o extracts a_b.a(a_b.o) to resolve a
```

Archive member extraction happens before --gc-sections, so this may not be a live path
under --gc-sections, but I think it is a good approximation in practice.

* Specifying a file avoids output interleaving with --verbose.
* Required `=` prevents accidental overwrite of an input if the user forgets `=`. (Most of compiler drivers' long options accept `=` but not ` `)

Differential Revision: https://reviews.llvm.org/D109572
2021-09-20 09:52:30 -07:00
Fangrui Song 00809c8889 [ELF] Apply version script patterns to non-default version symbols
Currently version script patterns are ignored for .symver produced
non-default version (single @) symbols. This makes such symbols
not localizable by `local:`, e.g.

```
.symver foo3_v1,foo3@v1
.globl foo_v1
foo3_v1:

ld.lld --version-script=a.ver -shared a.o
```

This patch adds the support:

* Move `config->versionDefinitions[VER_NDX_LOCAL].patterns` to `config->versionDefinitions[versionId].localPatterns`
* Rename `config->versionDefinitions[versionId].patterns` to `config->versionDefinitions[versionId].nonLocalPatterns`
* Allow `findAllByVersion` to find non-default version symbols when `includeNonDefault` is true. (Note: `symtab` keys do not have `@@`)
* Make each pattern check both the unversioned `pat.name` and the versioned `${pat.name}@${v.name}`
* `localPatterns` can localize `${pat.name}@${v.name}`. `nonLocalPatterns` can prevent localization by assigning `verdefIndex` (before `parseSymbolVersion`).

---

If a user notices new `undefined symbol` errors with a version script containing
`local: *;`, the issue is likely due to a missing `global:` pattern.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D107234
2021-08-04 23:52:56 -07:00
Fangrui Song a533eb7423 Revert "[ELF] Apply version script patterns to non-default version symbols"
This reverts commit 7ed22a6fa9.

buf is not cleared so the commit misses some cases.
2021-08-04 23:52:55 -07:00
Fangrui Song 7ed22a6fa9 [ELF] Apply version script patterns to non-default version symbols
Currently version script patterns are ignored for .symver produced
non-default version (single @) symbols. This makes such symbols
not localizable by `local:`, e.g.

```
.symver foo3_v1,foo3@v1
.globl foo_v1
foo3_v1:

ld.lld --version-script=a.ver -shared a.o
# In a.out, foo3@v1 is incorrectly exported.
```

This patch adds the support:

* Move `config->versionDefinitions[VER_NDX_LOCAL].patterns` to `config->versionDefinitions[versionId].localPatterns`
* Rename `config->versionDefinitions[versionId].patterns` to `config->versionDefinitions[versionId].nonLocalPatterns`
* Allow `findAllByVersion` to find non-default version symbols when `includeNonDefault` is true. (Note: `symtab` keys do not have `@@`)
* Make each pattern check both the unversioned `pat.name` and the versioned `${pat.name}@${v.name}`
* `localPatterns` can localize `${pat.name}@${v.name}`. `nonLocalPatterns` can prevent localization by assigning `verdefIndex` (before `parseSymbolVersion`).

---

If a user notices new `undefined symbol` errors with a version script containing
`local: *;`, the issue is likely due to a missing `global:` pattern.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D107234
2021-08-04 09:02:11 -07:00
Fangrui Song b06426da76 [ELF] Add -Bsymbolic-non-weak-functions
This option is a subset of -Bsymbolic-functions. It applies to STB_GLOBAL
STT_FUNC definitions.

The address of a vague linkage function (STB_WEAK STT_FUNC, e.g. an inline
function, a template instantiation) seen by a -Bsymbolic-functions linked
shared object may be different from the address seen from outside the shared
object. Such cases are uncommon. (ELF/Mach-O programs may use
`-fvisibility-inlines-hidden` to break such pointer equality.  On Windows,
correct dllexport and dllimport are needed to make pointer equality work.
Windows link.exe enables /OPT:ICF by default so different inline functions may
have the same address.)

```
// a.cc -> a.o -> a.so (-Bsymbolic-functions)
inline void f() {}
void *g() { return (void *)&f; }

// b.cc -> b.o -> exe
// The address is different!
inline void f() {}
```

-Bsymbolic-non-weak-functions is a safer (C++ conforming) subset of
-Bsymbolic-functions, which can make such programs work.

Implementations usually emit a vague linkage definition in a COMDAT group.  We
could detect the group (with more code) but I feel that we should just check
STB_WEAK for simplicity. A weak definition will thus serve as an escape hatch
for rare cases when users want interposition on definitions.

GNU ld feature request: https://sourceware.org/bugzilla/show_bug.cgi?id=27871

Longer write-up: https://maskray.me/blog/2021-05-16-elf-interposition-and-bsymbolic

If Linux distributions migrate to protected non-vague-linkage external linkage
functions by default, the linker option can still be handy because it allows
rapid experiment without recompilation. Protected function addresses currently
have deep issues in GNU ld.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D102570
2021-07-29 14:46:53 -07:00
Nathan Lanza 2f65166056 [lld:elf] Weaken the requirement for a computed binding to be STB_LOCAL
Given the following scenario:

```
// Cat.cpp
struct Animal { virtual void makeNoise() const = 0; };
struct Cat : Animal { void makeNoise() const override; };

extern "C" int puts(char const *);
void Cat::makeNoise() const { puts("Meow"); }
void doThingWithCat(Animal *a) { static_cast<Cat *>(a)->makeNoise(); }

// CatUser.cpp
struct Animal { virtual void makeNoise() const = 0; };
struct Cat : Animal { void makeNoise() const override; };

void doThingWithCat(Animal *a);

void useDoThingWithCat() {
  Cat *d = new Cat;
  doThingWithCat(d);
}

// cat.ver
{
  global: _Z17useDoThingWithCatv;
  local: *;
};

$ clang++ Cat.cpp CatUser.cpp -fpic -flto=thin -fwhole-program-vtables
-shared -O3 -fuse-ld=lld -Wl,--lto-whole-program-visibility
-Wl,--version-script,cat.ver
```

We cannot devirtualize `Cat::makeNoise`. The issue is complex:

Due to `-fsplit-lto-unit` and usage of type metadata, we place the Cat
vtable declaration into module 0 and the Cat vtable definition with type
metadata into module 1, causing duplicate entries (Undefined followed by
Defined) in the `lto::InputFile::symbols()` output.
In `BitcodeFile::parse`, after processing the `Undefined` then the
`Defined`, the final state is `Defined`.
In `BitcodeCompiler::add`, for the first symbol, `computeBinding`
returns `STB_LOCAL`, then we reset it to `Undefined` because it is
prevailing (`versionId` is `preserved`). For the second symbol, because
the state is now `Undefined`, `computeBinding` returns `STB_GLOBAL`,
causing `ExportDynamic` to be true and suppressing devirtualization.

In D77280, the `computeBinding` change used a stricter `isDefined()`
condition to make weak``Lazy` symbol work.
This patch relaxes the condition to weaker `!isLazy()` to keep it
working while making the devirtualization work as well.

Differential Revision: https://reviews.llvm.org/D98686
2021-05-24 23:32:21 -04:00
Harald van Dijk d62413452f
[lld][X86] Restore gotEntrySize.
D62727 removed GotEntrySize and GotPltEntrySize with a comment that they
are always equal to wordsize(), but that is not entirely true: X32 has a
word size of 4, but needs 8-byte GOT entries. This restores gotEntrySize
for both, adjusted for current naming conventions, but defaults it to
config->wordsize to keep things simple for architectures other than
x86_64.

This partially reverts D62727.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D102509
2021-05-17 00:13:00 +01:00
Nico Weber 279c5dc2f3 fix comment typo to cycle bots 2021-02-17 15:29:39 -05:00