Thanks!

Ping. Is there anything I can improve here?

• Add a test.

Not all toolchains call ToolChain::GetLinkerPath. For example, MSVC toolchain uses its own code:

void visualstudio::Linker::ConstructJob(...) {
...
  StringRef Linker = Args.getLastArgValue(options::OPT_fuse_ld_EQ, "link");
  if (Linker.equals_lower("lld"))
    Linker = "lld-link";
...
}

In my case, I am trying to cross-compile:

> ...\clang.exe a.cpp -fuse-ld=lld -target i686-pc-linux-gnu
clang.exe: error: invalid linker name in argument '-fuse-ld=lld'

• Use DenseSet<>::count(), as George suggested.

Ping. @espindola, @ruiu, is it OK to land?

I don't see anything that violates the specs. With this patch, the offset of a segment (p_offset) is still congruent to its virtual address (p_vaddr), modulo page size. This rule is enforced in lines 1850-1852 of ELF/Writer.cpp:

if (Cmd == First)
  return alignTo(Off, std::max<uint64_t>(Cmd->Alignment, Config->MaxPageSize),
                 Cmd->Addr);

and I didn't touch them.

It's not for an application. We use it for a kernel image.

• Remove "tee" in the test.

• Change type of ArchiveName to std::string.
• Add a test.

I asked @evgeny777 and he let me take this revision.

• Adjust to the tip.

Accepted by @rafael in the mailing list.

Update according to Rafael's comments:

• Replace !FirstLoadSection with Load->LastSec != Out::ProgramHeaders.
• Add a corresponding comment.

• Remove "tee". Thanks, George!

In D37626#908500, @ruiu wrote:

I started a discussion in the llvm-dev mailing list to eliminate the need to scan .eh_frame sections in a special way. Please send a comment there if you have one.

• Move Undefs and getUndefinedSymbols() into InputFile
• Change type of Undefs to DenseSet<StringRef> so that it can be used to search for the symbol without creating another filter container.

Shorten the script in the test.

Address review comments.

Thank you for the patch. I agree with Peter, the fix should be accomplished with a test.

In D39196#904887, @rengolin wrote:

I fear someone will eventually cry foul that we "broke their code".

In D41993#975015, @ruiu wrote:

I also wonder how these initial values (6 and 5) were decided.

Thanks for the review, @rafael!

Thanks for the comments!

• Add a sentinel entry earlier so that it can be arranged in a regular way.

• Limited the patch to fix only the crash in ARMExidxSentinelSection::writeTo().

Thanks for the example, Peter! It looks like I have to think on these corner cases a bit more.

• Added a comment.

• Rebased on the tip.
• This is mostly Rafael's version of the patch.
• Added a check for a weak symbol when setting IsNeeded.
• Added the test to check that a weak symbol doesn't cause adding DT_NEEDED; this test might be extracted to a separate patch.

• Do not depend on D40240.
• Mark test/ELF/gc-sections-shared.s to be in an intermediate state.

In D38790#930021, @ruiu wrote:

I feel like this patch is a bit too complicated, but I can't really point out why I felt like that. This patch actually does two different things, no? Do you mind if I ask you to split it? I think one patch can be something like this https://reviews.llvm.org/D40240, and you can create another patch not to add dead symbols to the symbol table later.

• Extract corrections in the existing tests into D40253.
• Use handling of DT_NEEDED from D40240.
• Don't try to maintain actual Live flag for all symbols. Just mark symbols used in relocations in live sections and make the final decision later in includeInSymtab.

• Rebase on the top.
• Add flag 'Live' to Symbol.
• Determine most of the alive symbols during GC phase.

I believe that the complexity in both cases is similar. Here we have two kinds of edges with distinct traversal functions. There we have to convert these edges in advance so that we can use only one traversal function. And distinct traversal function is not that different from the conversion function. But conversion always requires additional space and time to store edges, while the traversal function might be called several times. But my opinion might be biased.

• Address review comments;
• Rebase on the tip.

The symbol is not in a dead section but referenced from it. Right now I don't know any existing way to say if a symbol is referenced only from dead sections or not.

Thanks! If you don't mind, I'll leave the mentioned improvements to ones who understand python and lit better than me.

Ping. Is there anything I can do to improve this patch? I suppose I am hardly able to make D38391 quicker than this one.

• Rebased on the tip.
• Slightly improved tests.