In D58848#1440177, @avl wrote:

Thank you!

Oops it is used by unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp but not in library code. Let me see how to update the tests to use the other DWAAARFContext::create.

In D59531#1439899, @jakehehrlich wrote:

Roland pointed out that this will be a regression after those special high alignment notes are flushed from the ecosystem (this is apparently a recent addition and the issue will be resolved in the months to come). A possibility is that if we see a note section with greater than 4-byte alignment then we use this method but otherwise fallback to the old method. This means that on Linux binaries for the time being (while this new ABI thing is being flushed out of the ecosystem) will be slightly larger but as soon as its fixed ld.lld will be back to fully functioning. This is a general strategy too because no matter size the notes are, the reader will bump by 4-byte alignment. That seems fine to me and I'll upload that change today hopefully.

Fix typo in the title

In D59594#1439028, @MaskRay wrote:

This gives me more confidence..

ninja -C Release clang lld  # build lld with this change

cmake -GNinja -H. -BGOT -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_COMPILER=$HOME/llvm/Release/bin/clang++ -DCMAKE_C_COMPILER=$HOME/llvm/Release/bin/clang -DLLVM_ENABLE_LLD=On -DLLVM_INCLUDE_GO_TESTS=OFF -DLLVM_INCLUDE_DOCS=OFF -DLLVM_INCLUDE_EXAMPLES=OFF -DBUILD_SHARED_LIBS=On
ninja -C GOT check-all  # all (llvm+clang+extra+lld+compiler-rt) passed

/*DiscardedSecIdx*/ 0 -> /*DiscardedSecIdx=*/0

@rocallahan I find that people are discussing a generic approach in D59553

In D59333#1432520, @grimar wrote:

In D59333#1432442, @MaskRay wrote:

In D59333#1432433, @grimar wrote:

In D59333#1428331, @ruiu wrote:

Maybe we should also update the helptext of Bstatic? It currently says that "Do not link against shared libraries" which doesn't sound like it captures the exact meaning of the option.

"Do not search for dynamic libraries when linking"?

Is it worth mentioning the positional behavior? "Do not search for shared libraries mentioned on the command line after the option"

The default behavior can be restored with -Bdynamic, which makes the wording
"mentioned on the command line after the option" not so precise perhaps?
We do not mention the position for other positional options as far I can see too (like for -as-needed and -whole-archive).

For -whole-archive we have just:
"Force load of all members in a static library."

And then for -no-whole-archive:
"Restores the default behavior of loading archive members."

Maybe use words "disable/enable", i.e change the -Bstatic to
"Disable the lookup of dynamic libraries when linking".

and then -Bdynamic to something like
"Enable the lookup of dynamic libraries when linking (default)"?

0 -> /*DiscardedSecIdx=*/0

Address grimar's comments

It looks Phabricator doesn't have a good diff algorithm... // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \

This gives me more confidence..

.

Binding != STB_WEAK

.

generalize to SHF_EXCLUDE and other discarded sections

Simplify In.GotPlt->HasGotPltOffRel = true; after reading the interaction again...

Update code/tests related to R_GOTPLT R_TLSGD_GOTPLT

Update title

Update description

In D59649#1438837, @smeenai wrote:

In D59649#1438809, @MaskRay wrote:

In D59649#1438330, @smeenai wrote:

Thank you!

If I'm understanding correctly, for the case where the relocation is not in the same object file as the discarded section, we'll still get the "undefined symbol" error message. I don't know if it would be possible to enhance that case to print the "relocation against discarded section" message as well. (gold does print that message in this case; it just doesn't print the additional information about the section group signature and prevailing definition.)

Symbol has the File field but not the section index (st_shndx). If the relocation is not in the same object as the discarded section, we can't get the defined section easily.

Could we do so via symbol table lookup? Rui said in https://bugs.llvm.org/show_bug.cgi?id=41133 that "We already insert all symbols (including ones for discarded sections) to the symbol table".

In D57940#1438854, @arichardson wrote:

In D57940#1438813, @MaskRay wrote:

dispatch using Config->IsRela

The value of getAddend<ELFT>(Rel); now only seems to be in Ref.getRawDataRefImpl().p now. Shouldn't the rel case also be adding the implicit addend? I'm not sure I'm reading the code currently but it looks to me like REL no longer gets the addend added.

Dispatch on RelTy

In D57940#1438313, @ruiu wrote:

Not sure if I follow. Looks like you always instantiate the template using ELFT::Rela, so effectively you are assuming that all targets are using RELA (which is a wrong assumption by the way)? I think you need to dispatch using Config->IsRela.

dispatch using Config->IsRela

In D59649#1438330, @smeenai wrote:

Thank you!

If I'm understanding correctly, for the case where the relocation is not in the same object file as the discarded section, we'll still get the "undefined symbol" error message. I don't know if it would be possible to enhance that case to print the "relocation against discarded section" message as well. (gold does print that message in this case; it just doesn't print the additional information about the section group signature and prevailing definition.)

Factor out maybeReportDiscardedComdat

Update

In D54747#1342666, @rocallahan wrote:

Here are some results for the rusoto test in https://github.com/rust-lang/rust/issues/56068#issue-382175735:

LLD	Binary size in bytes
LLD 6.0.1	43,791,192
LLD 8.0.0 1cfcf404f6b23943405bc3c5bb5940b10b914624	43,861,056
LLD 8.0.0 1cfcf404f6b23943405bc3c5bb5940b10b914624 --start-lib	43,844,760
LLD 8.0.0 1cfcf404f6b23943405bc3c5bb5940b10b914624 + this patch	6,281,488

For --start-lib I wrapped --start-lib/--end-lib around all object files.

As I expected --start-lib didn't make much difference. The dependencies containing debuginfo that I want to be GCed away are already packaged into static libraries before being passed to the final link.

We don't know addresses at this point, and indeed we can't because the layout of the rest of the output file depends on how many program headers there are, which is decided by this function. I would be in favour of the one PT_NOTE per SHT_NOTE as it is simpler and handles more cases.

In D57940#1437293, @ruiu wrote:

I thought that I sent it before, but it's gone somewhere...

I wonder why you had to use template. Looks like you could just pass a plain function pointer. Is there any reason you didn't do that?

.

Update the comment to

Improve 3 tests

In D59559#1435876, @sivachandra wrote:

In D59559#1435872, @MaskRay wrote:

If there isn't a convincing reason .got and .got.plt should be adjacent, I think we probably don't need end(.got) == start(.got.plt) (I've changed my mind about that after investigating this more).

Yes, I am likely mixing up things from other linkers vs how it is implemented in ld.lld. In ld.lld, for the reloc expression like G + GOT + A - P, we conveniently have Sym.getGotVA() as an equivalent of G + GOT eliminating a need for negative indices. So, I think end(.got) == start(.got.plt) is really not required.

If there isn't a convincing reason .got and .got.plt should be adjacent, I think we probably don't need end(.got) == start(.got.plt) (I've changed my mind about that after investigating this more).

[2] - Glibc's X86_64 code uses extern long _DYNAMIC[] attribute((visibility("hidden"))); just like the test in question. However, it refers to only &_DYNAMIC, for which the compiler emits R_X86_64_PC32. Hence, we do not hit the problem with

About "will break some _GLOBAL_OFFSET_TABLE_ use cases", can you elaborate which ones?

I believe the patch in the current form will break some _GLOBAL_OFFSET_TABLE_ use cases (see https://reviews.llvm.org/D48095) so I volunteer to be a reviewer :)

I surveyed ld.bfd's architecture-varying rules:

Re-title :)

Improve weak-undef-no-shared-libs.s

Comment S and A

Add supports*

Add a test with spaces around patterns

std::list<int> Sections; -> int Next, Prev;

@aaron.ballman Does the release note look good now? 😊

As a side note, sh_size%sh_entsize==0 is required and that is tested by test/ELF/invalid/merge-invalid-size.s

Update title

In D59333#1432433, @grimar wrote:

In D59333#1428331, @ruiu wrote:

Maybe we should also update the helptext of Bstatic? It currently says that "Do not link against shared libraries" which doesn't sound like it captures the exact meaning of the option.

"Do not search for dynamic libraries when linking"?