[ELF] Postpone evaluation of ORIGIN/LENGTH in a MEMORY command

[MaskRay]

Summary

createFiles(args)
 readDefsym
 readerLinkerScript(*mb)
  ...
   readMemory
    readMemoryAssignment("ORIGIN", "org", "o") // eagerly evaluated
target = getTarget();
link(args)
 writeResult<ELFT>()
  ...
   finalizeSections()
    script->processSymbolAssignments()
     addSymbol(cmd) // with this patch, evaluated here

readMemoryAssignment eagerly evaluates ORIGIN/LENGTH and returns an uint64_t.
This patch postpones the evaluation to make

• --defsym and symbol assignments
• CONSTANT(COMMONPAGESIZE) (requires a non-null lld::elf::target)

work. If the expression somehow requires interaction with memory
regions, the circular dependency may cause the expression to evaluate to
a strange value. See the new test added to memory-err.s

[MaskRay]

Add tests

[grimar]

LGTM with 2 comments addressed.

lld/ELF/ScriptParser.cpp

1526

It calls std::function(nullptr_t) constructor.
Such Expr will fail during evaluation (exception: std::bad_function_call).

You should probably use return [] { return 0; };

lld/test/ELF/linkerscript/memory.s

49 ↗

(On Diff #248835)

I'd probably add a short description comment here and for the test below.

[MaskRay]

Address comments

lld/ELF/ScriptParser.cpp

1526

Thanks for this tip!

return 0 does not crash (not testable) because:

createFiles(args);  // parse linker scripts
if (errorCount())
  return;

Anyway, return [] { return 0; }; removes dependency on errorCount() check.

[aykevl]

Adding myself as reviewer so I don't lose this patch. I hope to test this out soon.

[MaskRay]

Add documentation docs/ELF/linker_script.rst

[MaskRay]

Restore the previous Diff. Added docs/ELF/linker_script.rst to the wrong Differential.

lld/ELF/LinkerScript.h

 // only if its input sections are read-only. Likewise, an output section
 // with ONLY_IF_RW is created if all input sections are RW.
 enum class ConstraintKind { NoConstraint, ReadOnly, ReadWrite };
 
 // This struct is used to represent the location and size of regions of
 // target memory. Instances of the struct are created by parsing the
 // MEMORY command.
 struct MemoryRegion {
-  MemoryRegion(StringRef name, uint64_t origin, uint64_t length, uint32_t flags,
+  MemoryRegion(StringRef name, Expr origin, Expr length, uint32_t flags,
                uint32_t negFlags)
       : name(std::string(name)), origin(origin), length(length), flags(flags),
         negFlags(negFlags) {}
 
   std::string name;
-  uint64_t origin;
-  uint64_t length;
+  Expr origin;
+  Expr length;
   uint32_t flags;
   uint32_t negFlags;
   uint64_t curPos = 0;
 };
 
 // This struct represents one section match pattern in SECTIONS() command.
 // It can optionally have negative match pattern for EXCLUDED_FILE command.
 // Also it may be surrounded with SORT() command, so contains sorting rules.

lld/ELF/LinkerScript.cpp

if (!cmdRef)
     cmdRef = make<OutputSection>(name, SHT_PROGBITS, 0);
   return cmdRef;
 }
 
 // Expands the memory region by the specified size.
 static void expandMemoryRegion(MemoryRegion *memRegion, uint64_t size,
                                StringRef regionName, StringRef secName) {
   memRegion->curPos += size;
-  uint64_t newSize = memRegion->curPos - memRegion->origin;
-  if (newSize > memRegion->length)
+  uint64_t newSize = memRegion->curPos - (memRegion->origin)().getValue();
+  uint64_t length = (memRegion->length)().getValue();
+  if (newSize > length)
     error("section '" + secName + "' will not fit in region '" + regionName +
-          "': overflowed by " + Twine(newSize - memRegion->length) + " bytes");
+          "': overflowed by " + Twine(newSize - length) + " bytes");
 }
 
 void LinkerScript::expandMemoryRegions(uint64_t size) {
   if (ctx->memRegion)
     expandMemoryRegion(ctx->memRegion, size, ctx->memRegion->name,
                        ctx->outSec->name);
   // Only expand the LMARegion if it is different from memRegion.
   if (ctx->lmaRegion && ctx->memRegion != ctx->lmaRegion)
void LinkerScript::allocateHeaders(std::vector<PhdrEntry *> &phdrs) {
 
   llvm::erase_if(phdrs,
                  [](const PhdrEntry *e) { return e->p_type == PT_PHDR; });
 }
 
 LinkerScript::AddressState::AddressState() {
   for (auto &mri : script->memoryRegions) {
     MemoryRegion *mr = mri.second;
-    mr->curPos = mr->origin;
+    mr->curPos = (mr->origin)().getValue();
   }
 }
 
 // Here we assign addresses as instructed by linker script SECTIONS
 // sub-commands. Doing that allows us to use final VA values, so here
 // we also handle rest commands like symbol assignments and ASSERTs.
 // Returns a symbol that has changed its section or value, or nullptr if no
 // symbol has changed.

lld/ELF/ScriptParser.cpp

private:
   SortSectionPolicy readSortKind();
   SymbolAssignment *readProvideHidden(bool provide, bool hidden);
   SymbolAssignment *readAssignment(StringRef tok);
   void readSort();
   Expr readAssert();
   Expr readConstant();
   Expr getPageSize();
 
-  uint64_t readMemoryAssignment(StringRef, StringRef, StringRef);
+  Expr readMemoryAssignment(StringRef, StringRef, StringRef);
   std::pair<uint32_t, uint32_t> readMemoryAttributes();
 
   Expr combine(StringRef op, Expr l, Expr r);
   Expr readExpr();
   Expr readExpr1(Expr lhs, int minPrec);
   StringRef readParenLiteral();
   Expr readPrimary();
   Expr readTernary(Expr cond);
if (tok == "DEFINED") {
     return [=] { return symtab->find(name) ? 1 : 0; };
   }
   if (tok == "LENGTH") {
     StringRef name = readParenLiteral();
     if (script->memoryRegions.count(name) == 0) {
       setError("memory region not defined: " + name);
       return [] { return 0; };
     }
-    return [=] { return script->memoryRegions[name]->length; };
+    return script->memoryRegions[name]->length;
   }
   if (tok == "LOADADDR") {
     StringRef name = readParenLiteral();
     OutputSection *cmd = script->getOrCreateOutputSection(name);
     cmd->usedInExpression = true;
     return [=] {
       checkIfExists(cmd, location);
       return cmd->getLMA();
if (tok == "MAX" || tok == "MIN") {
     return [=] { return std::max(a().getValue(), b().getValue()); };
   }
   if (tok == "ORIGIN") {
     StringRef name = readParenLiteral();
     if (script->memoryRegions.count(name) == 0) {
       setError("memory region not defined: " + name);
       return [] { return 0; };
     }
-    return [=] { return script->memoryRegions[name]->origin; };
+    return script->memoryRegions[name]->origin;
   }
   if (tok == "SEGMENT_START") {
     expect("(");
     skip();
     expect(",");
     Expr e = readExpr();
     expect(")");
     return [=] { return e(); };
if (consume("}"))
       return ret;
     expect(";");
   }
 
   expect("}");
   return ret;
 }
 
-uint64_t ScriptParser::readMemoryAssignment(StringRef s1, StringRef s2,
+Expr ScriptParser::readMemoryAssignment(StringRef s1, StringRef s2,
                                             StringRef s3) {
   if (!consume(s1) && !consume(s2) && !consume(s3)) {
     setError("expected one of: " + s1 + ", " + s2 + ", or " + s3);
     return 0;
   }
   expect("=");
-  return readExpr()().getValue();
+  return readExpr();
 }
 
 // Parse the MEMORY command as specified in:
 // https://sourceware.org/binutils/docs/ld/MEMORY.html
 //
 // MEMORY { name [(attr)] : ORIGIN = origin, LENGTH = len ... }
 void ScriptParser::readMemory() {
   expect("{");
   while (!errorCount() && !consume("}")) {
     StringRef tok = next();
     if (tok == "INCLUDE") {
       readInclude();
       continue;
     }
 
     uint32_t flags = 0;
     uint32_t negFlags = 0;
     if (consume("(")) {
       std::tie(flags, negFlags) = readMemoryAttributes();
       expect(")");
     }
     expect(":");
 
-    uint64_t origin = readMemoryAssignment("ORIGIN", "org", "o");
+    Expr origin = readMemoryAssignment("ORIGIN", "org", "o");
     expect(",");
-    uint64_t length = readMemoryAssignment("LENGTH", "len", "l");
+    Expr length = readMemoryAssignment("LENGTH", "len", "l");
 
     // Add the memory region to the region map.
     MemoryRegion *mr = make<MemoryRegion>(tok, origin, length, flags, negFlags);
     if (!script->memoryRegions.insert({tok, mr}).second)
       setError("region '" + tok + "' already defined");
   }
 }