[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

1001

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

test/ELF/linkerscript/linkerscript-assert.s

5

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

1001

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

5

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

5

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

27–1

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

27–1

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

27–1

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

27–1

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

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

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

28–29

Move this above "ALIGN".

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;
   bool Sort = false;
   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;
 };
 

ELF/LinkerScript.cpp

 #include "LinkerScript.h"
 #include "Config.h"
 #include "Driver.h"
 #include "InputSection.h"
 #include "OutputSections.h"
 #include "ScriptParser.h"
 #include "Strings.h"
 #include "Symbols.h"
 #include "SymbolTable.h"
 #include "Target.h"
 #include "Writer.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ELF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 

 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;
 
void ScriptParser::readSearchDir() {
   Config->SearchPaths.push_back(next());
   expect(")");
 }
 
 void ScriptParser::readSections() {
   Opt.HasContents = true;
   expect("{");
   while (!Error && !skip("}")) {
+    if (peek() == "ASSERT") {
+      Opt.Commands.emplace_back(new AssertCommand(readExpr()));
+      continue;
+    }
+
     StringRef Tok = next();
     if (peek() == "=" || peek() == "+=") {
       readAssignment(Tok);
       expect(";");
     } else if (Tok == "PROVIDE") {
       readProvide(false);
     } else if (Tok == "PROVIDE_HIDDEN") {
       readProvide(true);
if (Tok == "DATA_SEGMENT_RELRO_END") {
     return [](uint64_t Dot) { return alignTo(Dot, Target->PageSize); };
   }
   if (Tok == "SIZEOF") {
     expect("(");
     StringRef Name = next();
     expect(")");
     return [=](uint64_t Dot) { return getSectionSize(Name); };
   }
+  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;
+    };
+  }
 
   // Parse a symbol name or a number literal.
   uint64_t V = 0;
   if (Tok.getAsInteger(0, V)) {
     if (Tok != "." && !isValidCIdentifier(Tok))
       setError("malformed number: " + Tok);
     return [=](uint64_t Dot) { return getSymbolValue(Tok, Dot); };
   }

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