[ELF] - Linkerscript: ignore SORT(CONSTRUCTORS)

[grimar]

Summary

Some scripts can contain SORT(CONSTRUCTORS) expression:
https://svnweb.freebsd.org/base/head/sys/conf/ldscript.amd64?revision=284870&view=markup#l152

for ELF it just a nop:
"When linking object file formats which do not support arbitrary sections, such as ECOFF and XCOFF, the linker will automatically recognize C++ global constructors and destructors by name. For these object file formats, the CONSTRUCTORS command tells the linker to place constructor information in the output section where the CONSTRUCTORS command appears. The CONSTRUCTORS command is ignored for other object file formats."
(http://www.sourceware.org/binutils/docs-2.10/ld_3.html)

So patch implements ignoring.

[davide]

This LGTM, but, doesn't this collide with your other SORT() patch?

[grimar]

In D22848#497954, @davide wrote:

This LGTM, but, doesn't this collide with your other SORT() patch?

Good question. Technically as far I understand, CONSTRUCTORS is outputsection keyword, which should be used inside SORT() at the top level.
All files wildcards I saw starts from '*', D22852 relies on that (and I think it is reasonable).
We are also do not plan to support sort by filenames AFAIK as nothing uses that.

So I think we can handle SORT(CONSTRUCTORS) just like it would be a single command to simplify processing, like that patch do.

[grimar]

Lets see what Rui think.

ELF/LinkerScript.cpp

445 ↗

(On Diff #65688)

probably name could be skipSortConstructors() or something alike.

[ruiu]

This change seems correct in terms of semantics, but I got to ask why FreeBSD's linker scripts still have it if it is only for ECOFF or XCOFF file formats.

[emaste]

This change seems correct in terms of semantics, but I got to ask why FreeBSD's linker scripts still have it if it is only for ECOFF or XCOFF file formats.

I presume it was just based on the default linker script provided by GNU ld (from ld --verbose) years ago and nobody's had a reason to revisit it until now. I suspect most uses of it in the wild will have the same reason.

I should probably just remove it from FreeBSD's ldscript.

[davide]

In D22848#498532, @emaste wrote:

This change seems correct in terms of semantics, but I got to ask why FreeBSD's linker scripts still have it if it is only for ECOFF or XCOFF file formats.

I presume it was just based on the default linker script provided by GNU ld (from ld --verbose) years ago and nobody's had a reason to revisit it until now. I suspect most uses of it in the wild will have the same reason.

I should probably just remove it from FreeBSD's ldscript.

There's a lot of software I have to link that I don't have control of which use this feature. Please consider an implementation.

[ruiu]

LGTM. I had no objection to include this, was just wondering why FreeBSD needed this in the first place. It'd be a nice cleanup for FreeBSD to remove the call of SORT(CONSTURCTORS) from their linker scripts.

[emaste]

It'd be a nice cleanup for FreeBSD to remove the call of SORT(CONSTURCTORS) from their linker scripts.

Agreed, patch here for reference https://reviews.freebsd.org/D7343
Note that some are just CONSTRUCTORS, not inside of SORT.

lld/trunk/ELF/LinkerScript.cpp

private:
   void readSections();
 
   SymbolAssignment *readAssignment(StringRef Name);
   void readOutputSectionDescription(StringRef OutSec);
   std::vector<StringRef> readOutputSectionPhdrs();
   unsigned readPhdrType();
   void readProvide(bool Hidden);
   void readAlign(OutputSectionCommand *Cmd);
+  void readSort();
 
   Expr readExpr();
   Expr readExpr1(Expr Lhs, int MinPrec);
   Expr readPrimary();
   Expr readTernary(Expr Cond);
   Expr combine(StringRef Op, Expr Lhs, Expr Rhs);
 
   const static StringMap<Handler> Cmd;

 }
 
 void ScriptParser::readAlign(OutputSectionCommand *Cmd) {
   expect("(");
   Cmd->AlignExpr = readExpr();
   expect(")");
 }
 
+void ScriptParser::readSort() {
+  expect("(");
+  expect("CONSTRUCTORS");
+  expect(")");
+}
+
 void ScriptParser::readOutputSectionDescription(StringRef OutSec) {
   OutputSectionCommand *Cmd = new OutputSectionCommand(OutSec);
   Opt.Commands.emplace_back(Cmd);
 
   // Read an address expression.
   // https://sourceware.org/binutils/docs/ld/Output-Section-Address.html#Output-Section-Address
   if (peek() != ":")
     Cmd->AddrExpr = readExpr();
if (Tok == "*") {
       while (!Error && !skip(")"))
         InCmd->Patterns.push_back(next());
     } else if (Tok == "KEEP") {
       readKeep(Cmd);
     } else if (Tok == "PROVIDE") {
       readProvide(false);
     } else if (Tok == "PROVIDE_HIDDEN") {
       readProvide(true);
+    } else if (Tok == "SORT") {
+      readSort();
     } else {
       setError("unknown command " + Tok);
     }
   }
   Cmd->Phdrs = readOutputSectionPhdrs();
 
   StringRef Tok = peek();
   if (Tok.startswith("=")) {

lld/trunk/test/ELF/linkerscript/linkerscript-sort-constructors.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
+# RUN: echo "SECTIONS { .aaa : { SORT(CONSTRUCTORS) } }" > %t1.script
+# RUN: ld.lld -shared -o %t1 --script %t1.script %t1.o
+# RUN: llvm-readobj %t1 > /dev/null