[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.

222–223

What is this for? Please write a comment.

749–750

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/InputFiles.cpp

bool elf::ObjectFile<ELFT>::shouldMerge(const Elf_Shdr &Sec) {
   if (Config->Optimize == 0)
     return false;
 
   // We don't merge if linker script has SECTIONS command. When script
   // do layout it can merge several sections with different attributes
   // into single output sections. We currently do not support adding
   // mergeable input sections to regular output ones as well as adding
   // regular input sections to mergeable output.
-  if (ScriptConfig->HasContents)
+  if (ScriptConfig->HasSections)
     return false;
 
   // A mergeable section with size 0 is useless because they don't have
   // any data to merge. A mergeable string section with size 0 can be
   // argued as invalid because it doesn't end with a null character.
   // We'll avoid a mess by handling them as if they were non-mergeable.
   if (Sec.sh_size == 0)
     return false;

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 HasContents = false;
+  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);

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>
-void LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory) {
+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 (auto *Cmd = dyn_cast<OutputSectionCommand>(Base1.get())) {
         uint32_t Subalign = Cmd->SubalignExpr ? Cmd->SubalignExpr(0) : 0;
 
         if (Subalign)
           Sec->Alignment = Subalign;
         OutSec->addSection(Sec);
       }
     }
   }
+}
 
+template <class ELFT>
+void LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory) {
+  processCommands(Factory);
   // Add orphan sections.
   for (const std::unique_ptr<ObjectFile<ELFT>> &F :
        Symtab<ELFT>::X->getObjectFiles()) {
     for (InputSectionBase<ELFT> *S : F->getSections()) {
       if (isDiscarded(S) || S->OutSec)
         continue;
       OutputSectionBase<ELFT> *OutSec;
       bool IsNew;

 }
 
 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.HasContents)
-        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("/")) {
void ScriptParser::readSearchDir() {
   expect("(");
   StringRef Tok = next();
   if (!Config->Nostdlib)
     Config->SearchPaths.push_back(unquote(Tok));
   expect(")");
 }
 
 void ScriptParser::readSections() {
-  Opt.HasContents = true;
+  Opt.HasSections = true;
   expect("{");
   while (!Error && !skip("}")) {
     StringRef Tok = next();
     BaseCommand *Cmd = readProvideOrAssignment(Tok, true);
     if (!Cmd) {
       if (Tok == "ASSERT")
         Cmd = new AssertCommand(readAssert());
       else

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->HasContents)
+  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 {
     Phdrs = Script<ELFT>::X->hasPhdrsCommands() ? Script<ELFT>::X->createPhdrs()
                                                 : createPhdrs();
     fixHeaders();
-    if (ScriptConfig->HasContents) {
+    if (ScriptConfig->HasSections) {
       Script<ELFT>::X->assignAddresses();
     } else {
       fixSectionAlignments();
       assignAddresses();
     }
 
     if (!Config->OFormatBinary)
       assignFileOffsets();
template <class ELFT> void Writer<ELFT>::addReservedSymbols() {
   // __tls_get_addr, so it's not defined anywhere. Create a hidden definition
   // to avoid the undefined symbol error. As usual as special case is MIPS -
   // MIPS libc defines __tls_get_addr itself because there are no TLS
   // optimizations for this target.
   if (!Out<ELFT>::DynSymTab && Config->EMachine != EM_MIPS)
     Symtab<ELFT>::X->addIgnored("__tls_get_addr");
 
   // If linker script do layout we do not need to create any standart symbols.
-  if (ScriptConfig->HasContents)
+  if (ScriptConfig->HasSections)
     return;
 
   ElfSym<ELFT>::EhdrStart = Symtab<ELFT>::X->addIgnored("__ehdr_start");
 
   auto Define = [this](StringRef S, DefinedRegular<ELFT> *&Sym1,
                        DefinedRegular<ELFT> *&Sym2) {
     Sym1 = Symtab<ELFT>::X->addIgnored(S, STV_DEFAULT);
 
if (needsPtLoad(Sec))
       Sec->PageAlign = true;
   }
 }
 
 // We should set file offsets and VAs for elf header and program headers
 // sections. These are special, we do not include them into output sections
 // list, but have them to simplify the code.
 template <class ELFT> void Writer<ELFT>::fixHeaders() {
-  uintX_t BaseVA = ScriptConfig->HasContents ? 0 : Config->ImageBase;
+  uintX_t BaseVA = ScriptConfig->HasSections ? 0 : Config->ImageBase;
   Out<ELFT>::ElfHeader->setVA(BaseVA);
   uintX_t Off = Out<ELFT>::ElfHeader->getSize();
   Out<ELFT>::ProgramHeaders->setVA(Off + BaseVA);
   Out<ELFT>::ProgramHeaders->setSize(sizeof(Elf_Phdr) * Phdrs.size());
 }
 
 // Assign VAs (addresses at run-time) to output sections.
 template <class ELFT> void Writer<ELFT>::assignAddresses() {

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