Handle more crt*.o filename variants.

[saugustine]

Summary

Crt files are special cased by name when dealing with ctor and dtor
sections, but the current code misses certain variants. In particular, those
named when clang takes the code path in
clang/lib/Driver/ToolChain.cpp:416, where crtfiles are named:

clang_rt.<component>-<arch>-<env>.<suffix>

Previously, the code only handled:
clang_rt.<component>.<suffix>
<component>.<suffix>

This revision fixes that.

[ruiu]

lld/ELF/OutputSections.cpp

390–391

At this point maybe we should consider start using regexps?

[saugustine]

lld/ELF/OutputSections.cpp

390–391

Entirely up to you. My preference is not strong.

[ruiu]

lld/ELF/OutputSections.cpp

390–391

Let's try that to see how it looks. You can create a regex object by doing this

std::regex re(R"(^crtbegin[ST].o|^clang_rtbegin[ST]-.*\.o)");

and use re.match to see if an argument matches the regex.

[saugustine]

• Switch crt file matching to regular expressions.

[saugustine]

Thanks. Please take another look.

The biggest issue here is that embedding a string in the regex takes more code than it saves, so I just promoted the regex into the calling functions.

[ruiu]

lld/ELF/OutputSections.cpp

389

Please remove the mention about crtend.

391

ditto

396

Add static so that the regex is compiled only once.

396

Escaping dot?

^(clang_rt\.)?

Also I blieve [\-.*]? has to be

(-.*)?

[MaskRay]

lld/ELF/OutputSections.cpp

9

Place standard includes after llvm includes. clang-format can fix the problem.

396

^ and $ can be deleted because std::regex_match checks the entire target character sequence.

[saugustine]

• Address additional comments.

[saugustine]

All comments addressed. Please take another look when you get a chance.

[ruiu]

Could you add a test?

lld/ELF/OutputSections.cpp

396

nit: You don't need to escape - as it is not a metacharacter

402

ditto

[MaskRay]

You may need to update the file test/ELF/ctors_dtors_priority.s, delete the existing %t-crtbegin.o RUN lines, mkdir %t, add crtbegin.o and clang_rt.crtbegin.o RUN lines. You can run the ELF specific tests with ninja check-lld-elf.

lld/ELF/OutputSections.cpp

388–397

driver -> compiler driver

[ruiu]

lld/ELF/OutputSections.cpp

395

Remove llvm::.

Please swap this line with the following line so that static variable definitions are at the beginning of the function.

[ruiu]

lld/ELF/OutputSections.cpp

397

A char array returned by StringRef::data() is not guaranteed to be null-terminated, so I don't think this line is safe.

[saugustine]

• Add test. Address other comments.
• remove llvm namespace.

[ruiu]

lld/ELF/OutputSections.cpp

396

I believe you can avoid creating a copy of a string here. Does something like this work?

s = sys::path::filename(s);
return std::regex_match(s.begin(), s.end(), re);

[saugustine]

• Avoid a string copy.

[ruiu]

LGTM

[saugustine]

Switch to new repo.

[saugustine]

Committed in rG118ceea5c364, with the git transition obscuring the auto-reporting.

[MaskRay]

You may install the PHP (yes..) tool arcanist. I usually do:

# say you are in branch foobar
% arc diff 'HEAD^' # answer yes, to create a differential (Dxxxxx)
# get approved
% arc amend  # will amend your git description with a `Reviewed By:` line. I usually delete `Subscribers:` and `Tags:` lines because they are really useless. Make sure to keep the `Differential Revision:` line.
% git rebase --onto origin/master 'HEAD^' HEAD
% ninja -C Release check-lld-elf  # Make sure tests are still good after the rebase.

% git checkout master
% git merge --ff foobar
% git push

The Differential Revision: line is important. It associates a git commit with a differential and causes the differential to be closed automatically.

(My arc lives in ~/projects/arc/arcanist/bin/arc so I actually use PATH=~/projects/arc/arcanist/bin:$PATH arc diff 'HEAD^'.)

lld/ELF/OutputSections.cpp

 //===- OutputSections.cpp -------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "OutputSections.h"
 #include "Config.h"
 #include "LinkerScript.h"
 #include "SymbolTable.h"
 #include "SyntheticSections.h"
 #include "Target.h"
 #include "lld/Common/Memory.h"
 #include "lld/Common/Strings.h"
 #include "lld/Common/Threads.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SHA1.h"
+#include <regex>
 
 using namespace llvm;
 using namespace llvm::dwarf;
 using namespace llvm::object;
 using namespace llvm::support::endian;
 using namespace llvm::ELF;
 
 namespace lld {
void OutputSection::finalize() {
   link = in.symTab->getParent()->sectionIndex;
   // sh_info for SHT_REL[A] sections should contain the section header index of
   // the section to which the relocation applies.
   InputSectionBase *s = first->getRelocatedSection();
   info = s->getOutputSection()->sectionIndex;
   flags |= SHF_INFO_LINK;
 }
 
-// Returns true if S matches /Filename.?\.o$/.
-static bool isCrtBeginEnd(StringRef s, StringRef filename) {
-  if (!s.endswith(".o"))
-    return false;
-  s = s.drop_back(2);
-  if (s.endswith(filename))
-    return true;
-  return !s.empty() && s.drop_back().endswith(filename);
-}
+// Returns true if S is in one of the many forms the compiler driver may pass
+// crtbegin files.
+//
+// Gcc uses any of crtbegin[<empty>|S|T].o.
+// Clang uses Gcc's plus clang_rt.crtbegin[<empty>|S|T][-<arch>|<empty>].o.
 
-static bool isCrtbegin(StringRef s) { return isCrtBeginEnd(s, "crtbegin"); }
-static bool isCrtend(StringRef s) { return isCrtBeginEnd(s, "crtend"); }
+static bool isCrtbegin(StringRef s) {
+  static std::regex re(R"((clang_rt\.)?crtbegin[ST]?(-.*)?\.o)");
+  s = sys::path::filename(s);
+  return std::regex_match(s.begin(), s.end(), re);
+}
+
+static bool isCrtend(StringRef s) {
+  static std::regex re(R"((clang_rt\.)?crtend[ST]?(-.*)?\.o)");
+  s = sys::path::filename(s);
+  return std::regex_match(s.begin(), s.end(), re);
+}
 
 // .ctors and .dtors are sorted by this priority from highest to lowest.
 //
 //  1. The section was contained in crtbegin (crtbegin contains
 //     some sentinel value in its .ctors and .dtors so that the runtime
 //     can find the beginning of the sections.)
 //
 //  2. The section has an optional priority value in the form of ".ctors.N"

lld/test/ELF/ctors_dtors_priority.s

 // REQUIRES: x86
 
 // Test .ctors* and .dtors* are sorted by priority.
 
 // RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1
+// RUN: mkdir -p %t
 // RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux \
-// RUN:   %p/Inputs/ctors_dtors_priority1.s -o %t-crtbegin.o
+// RUN:   %p/Inputs/ctors_dtors_priority1.s -o %t/crtbegin.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux \
 // RUN:   %p/Inputs/ctors_dtors_priority2.s -o %t2
 // RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux \
-// RUN:   %p/Inputs/ctors_dtors_priority3.s -o %t-crtend.o
-// RUN: ld.lld %t1 %t2 %t-crtend.o %t-crtbegin.o -o %t.exe
+// RUN:   %p/Inputs/ctors_dtors_priority3.s -o %t/crtend.o
+// RUN: ld.lld %t1 %t2 %t/crtend.o %t/crtbegin.o -o %t.exe
 // RUN: llvm-objdump -s %t.exe | FileCheck %s
 
+// RUN: cp %t/crtbegin.o %t/clang_rt.crtbegin.o
+// RUN: cp %t/crtend.o %t/clang_rt.crtend.o
+// RUN: ld.lld %t1 %t2 %t/clang_rt.crtend.o %t/clang_rt.crtbegin.o -o %t.clang_rt.exe
+// RUN: llvm-objdump -s %t.clang_rt.exe | FileCheck %s
+
+// RUN: cp %t/crtbegin.o %t/clang_rt.crtbegin-x86_64.o
+// RUN: cp %t/crtend.o %t/clang_rt.crtend-x86_64.o
+// RUN: ld.lld %t1 %t2 %t/clang_rt.crtend-x86_64.o %t/clang_rt.crtbegin-x86_64.o -o %t.clang_rt-arch.exe
+// RUN: llvm-objdump -s %t.clang_rt-arch.exe | FileCheck %s
+
+  
 .globl _start
 _start:
   nop
 
 .section .ctors, "aw", @progbits
   .quad 1
 .section .ctors.100, "aw", @progbits
   .quad 2