[ELF] Linkerscript: improve handling of ASSERT and symbol assignment outside SECTIONS block

[evgeny777]

Summary

This patch does the following

• ASSERT can be defined outside SECTIONS block
• In case there is no SECTIONS block, section names can still be used in ASSERT and symbol assignment expressions
• All command objects are stored and processed in a single place (createAssignments was removed)

[atanasyan]

Is it possible to do s/HasContents/HasSections/ renaming as a separate commit to reduce diffs? It looks like this change is separable from this patch.

ELF/Writer.cpp

258

Is it important that we call processCommands in the beginning of the LinkerScript::createSections but after the Writer::createSections? If it does not matter, for my taste it is better to remove the processCommands call from the LinkerScript::createSections and do it here to accentuate that we always process commands from linker scripts.

Script<ELFT>::X->processCommands(Factory);
if (ScriptConfig->HasSections)
  Script<ELFT>::X->createSections(Factory);
else
  createSections();

[evgeny777]

I have feeling that by now HasContents has no meaning at all. Even without this patch there are occasions when HasContents is false, but linker script is not empty. Like when you have only symbol assignments.

ELF/Writer.cpp

258

Yes. You can now use section size and alignment in expressions, even though you may have no SECTIONS block at all.

[ruiu]

As Simon said, I'd appreciate if you separate the renaming patch from this one.

[ruiu]

ELF/LinkerScript.cpp

52–53

This needs comment.

264–265

What is this for? Please write a comment.

771–772

Sort them alphabetically.

[evgeny777]

Addressed review comments

[• rafael]

A patch renaming HasContent to HasSections LGTM. Please commit that and rebase.

[evgeny777]

Rebased

[atanasyan]

ELF/Writer.cpp

258

Is it important that we call processCommands in the beginning of the LinkerScript::createSections but after the Writer::createSections?

Yes. You can now use section size and alignment in expressions, even though you may have no SECTIONS block at all.

OK. Is it possible to rewrite the code like that? The idea is to make clear that we always call the processCommands.

if (ScriptConfig->HasSections)
  Script<ELFT>::X->createSections(Factory);
else
  createSections();
Script<ELFT>::X->processCommands(Factory);

[evgeny777]

ELF/Writer.cpp

258

No, createSections calls processCommands(). It is not possible to create sections in LinkerScript without walking over command list and at the same time processCommands() should be called after Writer<ELFT>::createSections(), because in this case it's possible to use SIZEOF, ALIGNOF in symbol assignments.

[atanasyan]

LGTM

ELF/LinkerScript.h

public:
   virtual uint64_t getOutputSectionSize(StringRef Name) = 0;
   virtual uint64_t getOutputSectionAlign(StringRef Name) = 0;
   virtual uint64_t getHeaderSize() = 0;
   virtual uint64_t getSymbolValue(StringRef S) = 0;
 };
 
 // ScriptConfiguration holds linker script parse results.
 struct ScriptConfiguration {
-  // Used to create symbol assignments outside SECTIONS command.
-  std::vector<std::unique_ptr<SymbolAssignment>> Assignments;
   // Used to assign addresses to sections.
   std::vector<std::unique_ptr<BaseCommand>> Commands;
 
   // Used to assign sections to headers.
   std::vector<PhdrsCommand> PhdrsCommands;
 
   bool HasSections = false;
 
   llvm::BumpPtrAllocator Alloc;
 
   // List of section patterns specified with KEEP commands. They will
   // be kept even if they are unused and --gc-sections is specified.
   std::vector<llvm::Regex *> KeptSections;
 };
 
 extern ScriptConfiguration *ScriptConfig;
 
 // This is a runner of the linker script.
 template <class ELFT> class LinkerScript final : public LinkerScriptBase {
   typedef typename ELFT::uint uintX_t;
 
 public:
   LinkerScript();
   ~LinkerScript();
-  void createAssignments();
+  void processCommands(OutputSectionFactory<ELFT> &Factory);
   void createSections(OutputSectionFactory<ELFT> &Factory);
 
   std::vector<PhdrEntry<ELFT>> createPhdrs();
   bool ignoreInterpSection();
 
   ArrayRef<uint8_t> getFiller(StringRef Name);
   Expr getLma(StringRef Name);
   bool shouldKeep(InputSectionBase<ELFT> *S);
   void assignAddresses();
   int compareSections(StringRef A, StringRef B);
   bool hasPhdrsCommands();
   uint64_t getOutputSectionAddress(StringRef Name) override;
   uint64_t getOutputSectionSize(StringRef Name) override;
   uint64_t getOutputSectionAlign(StringRef Name) override;
   uint64_t getHeaderSize() override;
   uint64_t getSymbolValue(StringRef S) override;
 
   std::vector<OutputSectionBase<ELFT> *> *OutputSections;
 
 private:
   std::vector<InputSectionBase<ELFT> *>
   getInputSections(const InputSectionDescription *);
 
+  void addSection(OutputSectionFactory<ELFT> &Factory,
+                  InputSectionBase<ELFT> *Sec, StringRef Name);
   void discard(ArrayRef<InputSectionBase<ELFT> *> V);
 
   std::vector<InputSectionBase<ELFT> *>
   createInputSectionList(OutputSectionCommand &Cmd);
 
   // "ScriptConfig" is a bit too long, so define a short name for it.
   ScriptConfiguration &Opt = *ScriptConfig;
 

ELF/LinkerScript.cpp

 
 LinkerScriptBase *elf::ScriptBase;
 ScriptConfiguration *elf::ScriptConfig;
 
 template <class ELFT> static void addRegular(SymbolAssignment *Cmd) {
   Symbol *Sym = Symtab<ELFT>::X->addRegular(Cmd->Name, STB_GLOBAL, STV_DEFAULT);
   Sym->Visibility = Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;
   Cmd->Sym = Sym->body();
+
+  // If we have no SECTIONS then we don't have '.' and don't call
+  // assignAddresses(). We calculate symbol value immediately in this case.
+  if (!ScriptConfig->HasSections)
+    cast<DefinedRegular<ELFT>>(Cmd->Sym)->Value = Cmd->Expression(0);
 }
 
 template <class ELFT> static void addSynthetic(SymbolAssignment *Cmd) {
   Symbol *Sym = Symtab<ELFT>::X->addSynthetic(
       Cmd->Name, nullptr, 0, Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT);
   Cmd->Sym = Sym->body();
 }
 
for (const std::unique_ptr<BaseCommand> &Base : OutCmd.Commands) {
     // resulting vector. We do not add duplicate input sections.
     for (InputSectionBase<ELFT> *S : V)
       if (SectionIndex.insert(S).second)
         Ret.push_back(S);
   }
   return Ret;
 }
 
-template <class ELFT> void LinkerScript<ELFT>::createAssignments() {
-  for (const std::unique_ptr<SymbolAssignment> &Cmd : Opt.Assignments) {
-    if (shouldDefine<ELFT>(Cmd.get()))
-      addRegular<ELFT>(Cmd.get());
-    if (Cmd->Sym)
-      cast<DefinedRegular<ELFT>>(Cmd->Sym)->Value = Cmd->Expression(0);
-  }
-}
-
 template <class ELFT>
 static SectionKey<ELFT::Is64Bits> createKey(InputSectionBase<ELFT> *C,
                                             StringRef OutsecName) {
   // When using linker script the merge rules are different.
   // Unfortunately, linker scripts are name based. This means that expressions
   // like *(.foo*) can refer to multiple input sections that would normally be
   // placed in different output sections. We cannot put them in different
   // output sections or we would produce wrong results for
static SectionKey<ELFT::Is64Bits> createKey(InputSectionBase<ELFT> *C,
   uintX_t Alignment = 0;
   if (isa<MergeInputSection<ELFT>>(C))
     Alignment = std::max(H->sh_addralign, H->sh_entsize);
 
   return SectionKey<ELFT::Is64Bits>{OutsecName, /*Type*/ 0, Flags, Alignment};
 }
 
 template <class ELFT>
-void LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory) {
-  auto AddSec = [&](InputSectionBase<ELFT> *Sec, StringRef Name) {
-    OutputSectionBase<ELFT> *OutSec;
-    bool IsNew;
-    std::tie(OutSec, IsNew) = Factory.create(createKey(Sec, Name), Sec);
-    if (IsNew)
-      OutputSections->push_back(OutSec);
-    return OutSec;
-  };
+void LinkerScript<ELFT>::addSection(OutputSectionFactory<ELFT> &Factory,
+                                    InputSectionBase<ELFT> *Sec,
+                                    StringRef Name) {
+  OutSec->addSection(Sec);
+}
+
+template <class ELFT>
+void LinkerScript<ELFT>::processCommands(OutputSectionFactory<ELFT> &Factory) {
 
   for (const std::unique_ptr<BaseCommand> &Base1 : Opt.Commands) {
     if (auto *Cmd = dyn_cast<SymbolAssignment>(Base1.get())) {
       if (shouldDefine<ELFT>(Cmd))
         addRegular<ELFT>(Cmd);
       continue;
     }
+    if (auto *Cmd = dyn_cast<AssertCommand>(Base1.get())) {
+      // If we don't have SECTIONS then output sections have already been
+      // created by Writer<EFLT>. The LinkerScript<ELFT>::assignAddresses
+      // will not be called, so ASSERT should be evaluated now.
+      if (!Opt.HasSections)
+        Cmd->Expression(0);
+      continue;
+    }
 
     if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base1.get())) {
       std::vector<InputSectionBase<ELFT> *> V = createInputSectionList(*Cmd);
 
       if (Cmd->Name == "/DISCARD/") {
         discard(V);
         continue;
       }
 
       if (V.empty())
         continue;
 
       for (InputSectionBase<ELFT> *Sec : V) {
-        OutputSectionBase<ELFT> *OutSec = AddSec(Sec, Cmd->Name);
-        uint32_t Subalign = Cmd->SubalignExpr ? Cmd->SubalignExpr(0) : 0;
-
-        if (Subalign)
+        addSection(Factory, Sec, Cmd->Name);
+        if (uint32_t Subalign = Cmd->SubalignExpr ? Cmd->SubalignExpr(0) : 0)
           Sec->Alignment = Subalign;
-        OutSec->addSection(Sec);
-      }
-    }
-  }
+      }
 
+template <class ELFT>
+void LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory) {
+  processCommands(Factory);
   // Add orphan sections.
-  for (ObjectFile<ELFT> *F : Symtab<ELFT>::X->getObjectFiles()) {
-    for (InputSectionBase<ELFT> *S : F->getSections()) {
-      if (isDiscarded(S) || S->OutSec)
-        continue;
-      OutputSectionBase<ELFT> *OutSec = AddSec(S, getOutputSectionName(S));
-      OutSec->addSection(S);
-    }
-  }
+  for (ObjectFile<ELFT> *F : Symtab<ELFT>::X->getObjectFiles())
+    for (InputSectionBase<ELFT> *S : F->getSections())
+      if (!isDiscarded(S) && !S->OutSec)
+        addSection(Factory, S, getOutputSectionName(S));
 }
 
 // Sets value of a section-defined symbol. Two kinds of
 // symbols are processed: synthetic symbols, whose value
 // is an offset from beginning of section and regular
 // symbols whose value is absolute.
 template <class ELFT>
 static void assignSectionSymbol(SymbolAssignment *Cmd,

 }
 
 void ScriptParser::readLinkerScript() {
   while (!atEOF()) {
     StringRef Tok = next();
     if (Tok == ";")
       continue;
 
-    if (Tok == "ENTRY") {
+    if (Tok == "ASSERT") {
+      Opt.Commands.emplace_back(new AssertCommand(readAssert()));
+    } else if (Tok == "ENTRY") {
       readEntry();
     } else if (Tok == "EXTERN") {
       readExtern();
     } else if (Tok == "GROUP" || Tok == "INPUT") {
       readGroup();
     } else if (Tok == "INCLUDE") {
       readInclude();
     } else if (Tok == "OUTPUT") {
       readOutput();
     } else if (Tok == "OUTPUT_ARCH") {
       readOutputArch();
     } else if (Tok == "OUTPUT_FORMAT") {
       readOutputFormat();
     } else if (Tok == "PHDRS") {
       readPhdrs();
     } else if (Tok == "SEARCH_DIR") {
       readSearchDir();
     } else if (Tok == "SECTIONS") {
       readSections();
     } else if (Tok == "VERSION") {
       readVersion();
     } else if (SymbolAssignment *Cmd = readProvideOrAssignment(Tok, true)) {
-      if (Opt.HasSections)
-        Opt.Commands.emplace_back(Cmd);
-      else
-        Opt.Assignments.emplace_back(Cmd);
     } else {
       setError("unknown directive: " + Tok);
     }
   }
 }
 
 void ScriptParser::addFile(StringRef S) {
   if (IsUnderSysroot && S.startswith("/")) {

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::run() {
   addReservedSymbols();
 
   if (Target->NeedsThunks)
     forEachRelSec(createThunks<ELFT>);
 
   CommonInputSection<ELFT> Common(getCommonSymbols<ELFT>());
   CommonInputSection<ELFT>::X = &Common;
 
-  Script<ELFT>::X->createAssignments();
-
   Script<ELFT>::X->OutputSections = &OutputSections;
-  if (ScriptConfig->HasSections)
+  if (ScriptConfig->HasSections) {
     Script<ELFT>::X->createSections(Factory);
-  else
+  } else {
     createSections();
+    Script<ELFT>::X->processCommands(Factory);
+  }
 
   finalizeSections();
   if (HasError)
     return;
 
   if (Config->Relocatable) {
     assignFileOffsets();
   } else {

test/ELF/linkerscript/assert.s

 # RUN: FileCheck %s -check-prefix=FAIL < %t.log
 # FAIL: fail
 
 # RUN: echo "SECTIONS {     \
 # RUN:   . = ASSERT(0x1000, \"true\"); \
 # RUN:  }" > %t4.script
 # RUN: ld.lld -shared -o %t4 --script %t4.script %t1.o
 # RUN: llvm-readobj %t4 > /dev/null
+
+# RUN: echo "SECTIONS {     \
+# RUN:   .foo : { *(.foo) } \
+# RUN: } \
+# RUN: ASSERT(SIZEOF(.foo) == 8, \"true\");" > %t5.script
+# RUN: ld.lld -shared -o %t5 --script %t5.script %t1.o
+# RUN: llvm-readobj %t5 > /dev/null
+
+## Even without SECTIONS block we still use section names
+## in expressions
+# RUN: echo "ASSERT(SIZEOF(.foo) == 8, \"true\");" > %t5.script
+# RUN: ld.lld -shared -o %t5 --script %t5.script %t1.o
+# RUN: llvm-readobj %t5 > /dev/null
+.section .foo, "a"
+ .quad 0