[ELF] - Linkerscript: implemented ASSERT() keyword.

[grimar]

Summary

ASSERT(exp, message)
Ensure that exp is non-zero. If it is zero, then exit the linker with an error
code, and print message.

Looks ASSERT is useful and used by some projects. I noticed it here:
https://searchcode.com/codesearch/view/42878166/

[ruiu]

Last time we made a decision not to implement ASSERT, but now we have a good codebase to support arbitrary functions, it is probably time to do it. Thank you for doing this.

ELF/LinkerScript.cpp

909 ↗

(On Diff #65911)

Is it true that ASSERT() returns the current value of "."?

test/ELF/linkerscript/linkerscript-assert.s

4 ↗

(On Diff #65911)

I don't think we need this complex test case. This can just be something like

ASSERT(0, "fail");

and

ASSERT(1, "success");

[grimar]

ELF/LinkerScript.cpp

909 ↗

(On Diff #65911)

Hmm I was sure that yes because linkerscript from link I provided contains:

/*
	. = ASSERT(_end <= 0x8000, "Setup too big!");
	. = ASSERT(hdr == 0x1f1, "The setup header has the wrong offset!");
	/* Necessary for the very-old-loader check to work... */
	. = ASSERT(__end_init <= 5*512, "init sections too big!");

at the end. And I relied on the fact that location counter should not be moved backward as
"location counter may not be moved backwards outside of an output section if so doing creates areas with overlapping LMAs."
(https://sourceware.org/binutils/docs-2.23/ld/Location-Counter.html)

But I just checked gold behavior and it seems it just returns the result of expression instead.
So it allows moving location counter backward to the begin of address space here since assert are placed at the end and
technically no overlapping happens.

I`ll change that then.

test/ELF/linkerscript/linkerscript-assert.s

4 ↗

(On Diff #65911)

Ok + I suggest at least to check the return value of ASSERT as it probably can be used and
already had trobles above.

[ruiu]

test/ELF/linkerscript/linkerscript-assert.s

4 ↗

(On Diff #65911)

You can add one more test

ASSERT(ASSERT(42, "success") == 42, "success");

[grimar]

• Addressed review comments.

I found there are 2 forms of ASSERT that can be used in scripts:

1. Like common assert: ASSERT(1, "true")
2. Like expression that assigns result to something: xxx = ASSERT(1, "true");

I updated the code to support first form as well.

[ruiu]

ELF/LinkerScript.cpp

366 ↗

(On Diff #66102)

I didn't even look at other changes in this patch, but I think I can say the new check looks very weird. SymbolAssignment is for assignments. If it is not an assignment, you need to define a new type. Please don't give a new (obscure) meaning by defining a special case.

[grimar]

ELF/LinkerScript.cpp

366 ↗

(On Diff #66102)

Ok. Actually I was thinking about it as about expression that calculates itself but assigns to nothing.
If you thing it should be another type, I`ll do that.

[grimar]

ELF/LinkerScript.cpp

366 ↗

(On Diff #66102)

SymbolAssignment is for assignments. If it is not an assignment, you need to define a new type.

By the way, we treat PROVIDE as SymbolAssignment. I think it is also a command and not an assigment.

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

366 ↗

(On Diff #66102)

I reimplemented this. At first I was thinking about creating some generic class like MiscellaneousCommand.
But after looking at the lists of possible LS commands:
https://sourceware.org/binutils/docs/ld/Simple-Commands.html#Simple-Commands

I think that ASSERT is probably the only one we need to add into queue. All others seems to be easy to
proccess inplace (like we do with ENTRY, for example). So I ended up with AssertCommand.

[ruiu]

Almost looking good, but looks like there is a bug.

ELF/LinkerScript.cpp

675–676 ↗

(On Diff #66455)

Is this correct? I mean, you didn't consume the token "ASSERT", so readExpr() will read "ASSERT" as an expression, no?

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

675–676 ↗

(On Diff #66455)

Initially I did it to reuse new code from below:

if (Tok == "ASSERT") {
  expect("(");
  Expr E = readExpr();
  expect(",");
  StringRef Msg = next();
  expect(")");
  return [=](uint64_t Dot) {
    uint64_t V = E(Dot);
    if (!V)
      error(Msg);
    return V;
  };
}

But you right, it was not 100% correct as probably allowed things like
ASSERT(...) + 55;
And there is a better way to achieve the same. I changed that.

[ruiu]

LGTM

ELF/LinkerScript.cpp

1051–1052 ↗

(On Diff #66628)

Move this above "ALIGN".

lld/trunk/ELF/LinkerScript.h

 template <class ELFT> class InputSectionBase;
 template <class ELFT> class OutputSectionBase;
 
 // This enum is used to implement linker script SECTIONS command.
 // https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS
 enum SectionsCommandKind {
   AssignmentKind,
   OutputSectionKind,
-  InputSectionKind
+  InputSectionKind,
+  AssertKind
 };
 
 struct BaseCommand {
   BaseCommand(int K) : Kind(K) {}
   virtual ~BaseCommand() {}
   int Kind;
 };
 
struct InputSectionDescription : BaseCommand {
   InputSectionDescription() : BaseCommand(InputSectionKind) {}
   static bool classof(const BaseCommand *C);
   StringRef FilePattern;
   SortKind Sort = SortKind::None;
   std::vector<StringRef> ExcludedFiles;
   std::vector<StringRef> SectionPatterns;
 };
 
+struct AssertCommand : BaseCommand {
+  AssertCommand(Expr E) : BaseCommand(AssertKind), Expression(E) {}
+  static bool classof(const BaseCommand *C);
+  Expr Expression;
+};
+
 struct PhdrsCommand {
   StringRef Name;
   unsigned Type;
   bool HasFilehdr;
   bool HasPhdrs;
   unsigned Flags;
 };
 

lld/trunk/ELF/LinkerScript.cpp

 bool OutputSectionCommand::classof(const BaseCommand *C) {
   return C->Kind == OutputSectionKind;
 }
 
 bool InputSectionDescription::classof(const BaseCommand *C) {
   return C->Kind == InputSectionKind;
 }
 
+bool AssertCommand::classof(const BaseCommand *C) {
+  return C->Kind == AssertKind;
+}
+
 template <class ELFT> static bool isDiscarded(InputSectionBase<ELFT> *S) {
   return !S || !S->Live;
 }
 
 template <class ELFT>
 bool LinkerScript<ELFT>::shouldKeep(InputSectionBase<ELFT> *S) {
   for (StringRef Pat : Opt.KeptSections)
     if (globMatch(Pat, S->getSectionName()))
if (auto *Cmd = dyn_cast<SymbolAssignment>(Base.get())) {
       if (Cmd->Name == ".") {
         Dot = Cmd->Expression(Dot);
       } else if (Cmd->Sym) {
         cast<DefinedRegular<ELFT>>(Cmd->Sym)->Value = Cmd->Expression(Dot);
       }
       continue;
     }
 
+    if (auto *Cmd = dyn_cast<AssertCommand>(Base.get())) {
+      Cmd->Expression(Dot);
+      continue;
+    }
+
     // Find all the sections with required name. There can be more than
     // one section with such name, if the alignment, flags or type
     // attribute differs.
     auto *Cmd = cast<OutputSectionCommand>(Base.get());
     for (OutputSectionBase<ELFT> *Sec : Sections) {
       if (Sec->getName() != Cmd->Name)
         continue;
 
private:
   std::vector<StringRef> readOutputSectionPhdrs();
   InputSectionDescription *readInputSectionDescription();
   std::vector<StringRef> readInputFilePatterns();
   InputSectionDescription *readInputSectionRules();
   unsigned readPhdrType();
   SymbolAssignment *readProvide(bool Hidden);
   Expr readAlign();
   void readSort();
+  Expr readAssert();
 
   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;
while (!Error && !skip("}")) {
     BaseCommand *Cmd;
     if (peek() == "=" || peek() == "+=") {
       Cmd = readAssignment(Tok);
       expect(";");
     } else if (Tok == "PROVIDE") {
       Cmd = readProvide(false);
     } else if (Tok == "PROVIDE_HIDDEN") {
       Cmd = readProvide(true);
+    } else if (Tok == "ASSERT") {
+      Cmd = new AssertCommand(readAssert());
     } else {
       Cmd = readOutputSectionDescription(Tok);
     }
     Opt.Commands.emplace_back(Cmd);
   }
 }
 
 static int precedence(StringRef Op) {

 }
 
 void ScriptParser::readSort() {
   expect("(");
   expect("CONSTRUCTORS");
   expect(")");
 }
 
+Expr ScriptParser::readAssert() {
+  expect("(");
+  Expr E = readExpr();
+  expect(",");
+  StringRef Msg = next();
+  expect(")");
+  return [=](uint64_t Dot) {
+    uint64_t V = E(Dot);
+    if (!V)
+      error(Msg);
+    return V;
+  };
+}
+
 OutputSectionCommand *
 ScriptParser::readOutputSectionDescription(StringRef OutSec) {
   OutputSectionCommand *Cmd = new OutputSectionCommand(OutSec);
 
   // Read an address expression.
   // https://sourceware.org/binutils/docs/ld/Output-Section-Address.html#Output-Section-Address
   if (peek() != ":")
     Cmd->AddrExpr = readExpr();
Expr ScriptParser::readPrimary() {
   if (Tok == "(") {
     Expr E = readExpr();
     expect(")");
     return E;
   }
 
   // Built-in functions are parsed here.
   // https://sourceware.org/binutils/docs/ld/Builtin-Functions.html.
+  if (Tok == "ASSERT")
+    return readAssert();
   if (Tok == "ALIGN") {
     expect("(");
     Expr E = readExpr();
     expect(")");
     return [=](uint64_t Dot) { return alignTo(Dot, E(Dot)); };
   }
   if (Tok == "CONSTANT") {
     expect("(");

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

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
+
+# RUN: echo "SECTIONS {     \
+# RUN:   ASSERT(1, \"true\") \
+# RUN:  }" > %t1.script
+# RUN: ld.lld -shared -o %t1 --script %t1.script %t1.o
+# RUN: llvm-readobj %t1 > /dev/null
+
+# RUN: echo "SECTIONS {                                  \
+# RUN:   ASSERT(ASSERT(42, \"true\") == 42, \"true\") \
+# RUN:  }" > %t2.script
+# RUN: ld.lld -shared -o %t2 --script %t2.script %t1.o
+# RUN: llvm-readobj %t2 > /dev/null
+
+# RUN: echo "SECTIONS {     \
+# RUN:   ASSERT(0,\ "fail\") \
+# RUN:  }" > %t3.script
+# RUN: not ld.lld -shared -o %t3 --script %t3.script %t1.o > %t.log 2>&1
+# 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