[ELF] - Eliminate AssertCommand.

[grimar]

Summary

Currently, LLD supports ASSERT as a separate command.

We support two forms now.

1. Assign expression-view: . = ASSERT(0x100)

(old GNU ld required it and some scripts in the wild are still using
something like . = ASSERT((_end - _text <= (512 * 1024 * 1024)), "kernel image bigger than KERNEL_IMAGE_SIZE");

2. Nowadays above is not a mandatory form and command-like form is commonly used:

ASSERT(<expr>, "text);

The return value of the ASSERT is Dot. That was implemented in D30171.
It looks like (2) is just a short version of (1) then.

GNU ld does *not* list ASSERT as a SECTIONS command:
https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS

Given above I think we can change ASSERT to be an assignment to Dot.
That makes the rest of the code much simpler.

[ruiu]

ELF/ScriptParser.cpp

764 ↗

(On Diff #141614)

This change might be okay, but this function name has to be changed.

[• espindola]

I am OK with it once a name is agreed on. I would also be OK with readProvideOrAssignment since ASSERT can be viewed as a odd form of assignment.

ELF/ScriptParser.cpp

765 ↗

(On Diff #141614)

Add a comment saying that the Assert expression returns ., so this is just ". = .".

[grimar]

• Addressed review comments.

ELF/ScriptParser.cpp

764 ↗

(On Diff #141614)

What about readAssignment? Since PROVIDE, HIDDEN and ASSERT are an a variants of assignment command.

[grimar]

Ping. Rui?

[• espindola]

LGTM

[grimar]

In D45434#1077174, @espindola wrote:

LGTM

It turns out there was one readAssignment already. Not sure how that happened when I posted the patch :/
I'll rename the original readAssignment to readSymbolAssignment as readAssignment introduced in this patch is a wider thing.

lld/trunk/ELF/LinkerScript.h

 typedef std::function<ExprValue()> Expr;
 
 // This enum is used to implement linker script SECTIONS command.
 // https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS
 enum SectionsCommandKind {
   AssignmentKind, // . = expr or <sym> = expr
   OutputSectionKind,
   InputSectionKind,
-  AssertKind, // ASSERT(expr)
   ByteKind    // BYTE(expr), SHORT(expr), LONG(expr) or QUAD(expr)
 };
 
 struct BaseCommand {
   BaseCommand(int K) : Kind(K) {}
   int Kind;
 };
 
struct InputSectionDescription : BaseCommand {
   std::vector<InputSection *> Sections;
 
   // Temporary record of synthetic ThunkSection instances and the pass that
   // they were created in. This is used to insert newly created ThunkSections
   // into Sections at the end of a createThunks() pass.
   std::vector<std::pair<ThunkSection *, uint32_t>> ThunkSections;
 };
 
-// Represents an ASSERT().
-struct AssertCommand : BaseCommand {
-  AssertCommand(Expr E) : BaseCommand(AssertKind), Expression(E) {}
-
-  static bool classof(const BaseCommand *C) { return C->Kind == AssertKind; }
-
-  Expr Expression;
-};
-
 // Represents BYTE(), SHORT(), LONG(), or QUAD().
 struct ByteCommand : BaseCommand {
   ByteCommand(Expr E, unsigned Size, std::string CommandString)
       : BaseCommand(ByteKind), CommandString(CommandString), Expression(E),
         Size(Size) {}
 
   static bool classof(const BaseCommand *C) { return C->Kind == ByteKind; }
 

lld/trunk/ELF/LinkerScript.cpp

for (BaseCommand *Base : Sec->SectionCommands) {
     // Handle BYTE(), SHORT(), LONG(), or QUAD().
     if (auto *Cmd = dyn_cast<ByteCommand>(Base)) {
       Cmd->Offset = Dot - Ctx->OutSec->Addr;
       Dot += Cmd->Size;
       expandOutputSection(Cmd->Size);
       continue;
     }
 
-    // Handle ASSERT().
-    if (auto *Cmd = dyn_cast<AssertCommand>(Base)) {
-      Cmd->Expression();
-      continue;
-    }
-
     // Handle a single input section description command.
     // It calculates and assigns the offsets for each section and also
     // updates the output section size.
     auto *Cmd = cast<InputSectionDescription>(Base);
     for (InputSection *Sec : Cmd->Sections) {
       // We tentatively added all synthetic sections at the beginning and
       // removed empty ones afterwards (because there is no way to know
       // whether they were going be empty or not other than actually running
void LinkerScript::assignAddresses() {
 
   for (BaseCommand *Base : SectionCommands) {
     if (auto *Cmd = dyn_cast<SymbolAssignment>(Base)) {
       Cmd->Addr = Dot;
       assignSymbol(Cmd, false);
       Cmd->Size = Dot - Cmd->Addr;
       continue;
     }
-
-    if (auto *Cmd = dyn_cast<AssertCommand>(Base)) {
-      Cmd->Expression();
-      continue;
-    }
-
     assignOffsets(cast<OutputSection>(Base));
   }
   Ctx = nullptr;
 }
 
 // Creates program headers as instructed by PHDRS linker script command.
 std::vector<PhdrEntry *> LinkerScript::createPhdrs() {
   std::vector<PhdrEntry *> Ret;

lld/trunk/ELF/ScriptParser.cpp

private:
   void readOutputFormat();
   void readPhdrs();
   void readRegionAlias();
   void readSearchDir();
   void readSections();
   void readVersion();
   void readVersionScriptCommand();
 
-  SymbolAssignment *readAssignment(StringRef Name);
+  SymbolAssignment *readSymbolAssignment(StringRef Name);
   ByteCommand *readByteCommand(StringRef Tok);
   uint32_t readFill();
   uint32_t parseFill(StringRef Tok);
   void readSectionAddressType(OutputSection *Cmd);
   OutputSection *readOutputSectionDescription(StringRef OutSec);
   std::vector<StringRef> readOutputSectionPhdrs();
   InputSectionDescription *readInputSectionDescription(StringRef Tok);
   StringMatcher readFilePatterns();
   std::vector<SectionPattern> readInputSectionsList();
   InputSectionDescription *readInputSectionRules(StringRef FilePattern);
   unsigned readPhdrType();
   SortSectionPolicy readSortKind();
   SymbolAssignment *readProvideHidden(bool Provide, bool Hidden);
-  SymbolAssignment *readProvideOrAssignment(StringRef Tok);
+  SymbolAssignment *readAssignment(StringRef Tok);
   void readSort();
-  AssertCommand *readAssert();
-  Expr readAssertExpr();
+  Expr readAssert();
   Expr readConstant();
   Expr getPageSize();
 
   uint64_t readMemoryAssignment(StringRef, StringRef, StringRef);
   std::pair<uint32_t, uint32_t> readMemoryAttributes();
 
   Expr combine(StringRef Op, Expr L, Expr R);
   Expr readExpr();

 }
 
 void ScriptParser::readLinkerScript() {
   while (!atEOF()) {
     StringRef Tok = next();
     if (Tok == ";")
       continue;
 
-    if (Tok == "ASSERT") {
-      Script->SectionCommands.push_back(readAssert());
-    } else if (Tok == "ENTRY") {
+    if (Tok == "ENTRY") {
       readEntry();
     } else if (Tok == "EXTERN") {
       readExtern();
     } else if (Tok == "GROUP") {
       readGroup();
     } else if (Tok == "INCLUDE") {
       readInclude();
     } else if (Tok == "INPUT") {
while (!atEOF()) {
     } else if (Tok == "REGION_ALIAS") {
       readRegionAlias();
     } else if (Tok == "SEARCH_DIR") {
       readSearchDir();
     } else if (Tok == "SECTIONS") {
       readSections();
     } else if (Tok == "VERSION") {
       readVersion();
-    } else if (SymbolAssignment *Cmd = readProvideOrAssignment(Tok)) {
+    } else if (SymbolAssignment *Cmd = readAssignment(Tok)) {
       Script->SectionCommands.push_back(Cmd);
     } else {
       setError("unknown directive: " + Tok);
     }
   }
 }
 
 void ScriptParser::readDefsym(StringRef Name) {
void ScriptParser::readSections() {
   // their own segment. We do the same if SECTIONS command is present in linker
   // script. See comment for computeFlags().
   Config->SingleRoRx = true;
 
   expect("{");
   std::vector<BaseCommand *> V;
   while (!errorCount() && !consume("}")) {
     StringRef Tok = next();
-    BaseCommand *Cmd = readProvideOrAssignment(Tok);
-    if (!Cmd) {
-      if (Tok == "ASSERT")
-        Cmd = readAssert();
-      else
-        Cmd = readOutputSectionDescription(Tok);
-    }
+    if (BaseCommand *Cmd = readAssignment(Tok))
       V.push_back(Cmd);
+    else
+      V.push_back(readOutputSectionDescription(Tok));
   }
 
   if (!atEOF() && consume("INSERT")) {
     std::vector<BaseCommand *> *Dest = nullptr;
     if (consume("AFTER"))
       Dest = &Script->InsertAfterCommands[next()];
     else if (consume("BEFORE"))
       Dest = &Script->InsertBeforeCommands[next()];

 }
 
 void ScriptParser::readSort() {
   expect("(");
   expect("CONSTRUCTORS");
   expect(")");
 }
 
-AssertCommand *ScriptParser::readAssert() {
-  return make<AssertCommand>(readAssertExpr());
-}
-
-Expr ScriptParser::readAssertExpr() {
+Expr ScriptParser::readAssert() {
   expect("(");
   Expr E = readExpr();
   expect(",");
   StringRef Msg = unquote(next());
   expect(")");
 
   return [=] {
     if (!E().getValue())
OutputSection *ScriptParser::readOutputSectionDescription(StringRef OutSec) {
   if (consume("ONLY_IF_RW"))
     Cmd->Constraint = ConstraintKind::ReadWrite;
   expect("{");
 
   while (!errorCount() && !consume("}")) {
     StringRef Tok = next();
     if (Tok == ";") {
       // Empty commands are allowed. Do nothing here.
-    } else if (SymbolAssignment *Assign = readProvideOrAssignment(Tok)) {
+    } else if (SymbolAssignment *Assign = readAssignment(Tok)) {
       Cmd->SectionCommands.push_back(Assign);
     } else if (ByteCommand *Data = readByteCommand(Tok)) {
       Cmd->SectionCommands.push_back(Data);
-    } else if (Tok == "ASSERT") {
-      Cmd->SectionCommands.push_back(readAssert());
-      expect(";");
     } else if (Tok == "CONSTRUCTORS") {
       // CONSTRUCTORS is a keyword to make the linker recognize C++ ctors/dtors
       // by name. This is for very old file formats such as ECOFF/XCOFF.
       // For ELF, we should ignore.
     } else if (Tok == "FILL") {
       Cmd->Filler = readFill();
     } else if (Tok == "SORT") {
       readSort();
uint32_t ScriptParser::parseFill(StringRef Tok) {
 
   uint32_t Buf;
   write32be(&Buf, V);
   return Buf;
 }
 
 SymbolAssignment *ScriptParser::readProvideHidden(bool Provide, bool Hidden) {
   expect("(");
-  SymbolAssignment *Cmd = readAssignment(next());
+  SymbolAssignment *Cmd = readSymbolAssignment(next());
   Cmd->Provide = Provide;
   Cmd->Hidden = Hidden;
   expect(")");
   return Cmd;
 }
 
-SymbolAssignment *ScriptParser::readProvideOrAssignment(StringRef Tok) {
+SymbolAssignment *ScriptParser::readAssignment(StringRef Tok) {
+  // Assert expression returns Dot, so this is equal to ".=."
+  if (Tok == "ASSERT")
+    return make<SymbolAssignment>(".", readAssert(), getCurrentLocation());
+
   size_t OldPos = Pos;
   SymbolAssignment *Cmd = nullptr;
   if (peek() == "=" || peek() == "+=")
-    Cmd = readAssignment(Tok);
+    Cmd = readSymbolAssignment(Tok);
   else if (Tok == "PROVIDE")
     Cmd = readProvideHidden(true, false);
   else if (Tok == "HIDDEN")
     Cmd = readProvideHidden(false, true);
   else if (Tok == "PROVIDE_HIDDEN")
     Cmd = readProvideHidden(true, true);
 
   if (Cmd) {
     Cmd->CommandString =
         Tok.str() + " " +
         llvm::join(Tokens.begin() + OldPos, Tokens.begin() + Pos, " ");
     expect(";");
   }
   return Cmd;
 }
 
-SymbolAssignment *ScriptParser::readAssignment(StringRef Name) {
+SymbolAssignment *ScriptParser::readSymbolAssignment(StringRef Name) {
   StringRef Op = next();
   assert(Op == "=" || Op == "+=");
   Expr E = readExpr();
   if (Op == "+=") {
     std::string Loc = getCurrentLocation();
     E = [=] { return add(Script->getSymbolValue(Name, Loc), E()); };
   }
   return make<SymbolAssignment>(Name, E, getCurrentLocation());
if (Tok == "ALIGNOF") {
     StringRef Name = readParenLiteral();
     OutputSection *Cmd = Script->getOrCreateOutputSection(Name);
     return [=] {
       checkIfExists(Cmd, Location);
       return Cmd->Alignment;
     };
   }
   if (Tok == "ASSERT")
-    return readAssertExpr();
+    return readAssert();
   if (Tok == "CONSTANT")
     return readConstant();
   if (Tok == "DATA_SEGMENT_ALIGN") {
     expect("(");
     Expr E = readExpr();
     expect(",");
     readExpr();
     expect(")");

lld/trunk/test/ELF/linkerscript/assert.s

 # RUN: ld.lld -shared -o %t5 --script %t5.script %t1.o
 # RUN: llvm-readobj %t5 > /dev/null
 
 ## Test assertions inside of output section decriptions.
 # RUN: echo "SECTIONS { .foo : { *(.foo) ASSERT(SIZEOF(.foo) == 8, \"true\"); } }" > %t6.script
 # RUN: ld.lld -shared -o %t6 --script %t6.script %t1.o
 # RUN: llvm-readobj %t6 > /dev/null
 
+## Unlike the GNU ld, we accept the ASSERT without the semicolon.
+## It is consistent with how ASSERT can be written outside of the
+## output section declaration.
 # RUN: echo "SECTIONS { .foo : { ASSERT(1, \"true\") } }" > %t7.script
-# RUN: not ld.lld -shared -o %t7 --script %t7.script %t1.o > %t.log 2>&1
-# RUN: FileCheck %s -check-prefix=CHECK-SEMI < %t.log
-# CHECK-SEMI: error: {{.*}}.script:1: ; expected, but got }
+# RUN: ld.lld -shared -o %t7 --script %t7.script %t1.o
 
 .section .foo, "a"
  .quad 0