[ELF] - Linkerscript: implemented += operator.

[grimar]

Summary

Sometimes += is used to move the location counter.
Example from the wild is:

.dbg_excpt _DBG_EXCPT_ADDR (NOLOAD) :
{
  . += (DEFINED (_DEBUGGER) ? 0x8 : 0x0);

https://github.com/chipKIT32/pic32-Arduino-USB-Bootloader-original/blob/master/boot-linkerscript.ld

Patch implements it and opens way for others type of assignments (-= *= etc), though I think only += is
actual to support.

[ruiu]

ELF/LinkerScript.cpp

818 ↗

(On Diff #65936)

It can be like this. You don't have to call combine() to handle operators that is known at compile time.

StringRef Op = next();
assert(Op == "=" || Op == "+=");
Expr E = readExpr();
if (Op == "+=")
  E = [=](uint64_t Dot) { return getSymbolValue(Name, Dot) + E(Dot); };
auto *Cmd = new SymbolAssignment(Name, E);
...

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

818 ↗

(On Diff #65936)

Fixed.

[ruiu]

LGTM with a nit.

ELF/LinkerScript.cpp

922–923 ↗

(On Diff #65989)

I think you can remove this error check. If Tok is not a valid symbol name, getSymbolValue will find it.

[grimar]

ELF/LinkerScript.cpp

922–923 ↗

(On Diff #65989)

Looks few tests begins to fail if I remove. Since this line is not added by this patch, I suggest to leave it for now, I'll check how to remove it and prepare a patch tomorrow.

[grimar]

ELF/LinkerScript.cpp

922–923 ↗

(On Diff #65989)

After little investigation I am not sure will removing of check be good change or not now. Imagine we have the next script:

SECTIONS {
boom .temp : { *(.temp) } }
}

And imagine we have no .temp sections. While check is here, parser will catch the ".temp" and report mailformed number. Without that check since we never will create boom as there is no .temp sections, error will never shown.
But at the same time:

SECTIONS {
boom temp : { *(.temp) } }
}

Here temp is valid C identifier. So we will never see any error in that case. So that check helps only sometimes here.
But there are cases when this check really helps. Example:

SECTIONS {   . = ;  }

It finds that ";" is not a number and not a symbol and error outs. Without check it writes:
"; expected but got }" because of:

if (peek() == "=" || peek() == "+=") {
  readAssignment(Tok);
  expect(";");

Probably we should check somehow that expression that readExpr() returns is not empty.

lld/trunk/ELF/LinkerScript.cpp

void ScriptParser::readSearchDir() {
   expect(")");
 }
 
 void ScriptParser::readSections() {
   Opt.DoLayout = true;
   expect("{");
   while (!Error && !skip("}")) {
     StringRef Tok = next();
-    if (peek() == "=") {
+    if (peek() == "=" || peek() == "+=") {
       readAssignment(Tok);
       expect(";");
     } else if (Tok == "PROVIDE") {
       readProvide(false);
     } else if (Tok == "PROVIDE_HIDDEN") {
       readProvide(true);
     } else {
       readOutputSectionDescription(Tok);
void ScriptParser::readProvide(bool Hidden) {
   if (SymbolAssignment *Assignment = readAssignment(next())) {
     Assignment->Provide = true;
     Assignment->Hidden = Hidden;
   }
   expect(")");
   expect(";");
 }
 
-SymbolAssignment *ScriptParser::readAssignment(StringRef Name) {
-  expect("=");
-  Expr E = readExpr();
-  auto *Cmd = new SymbolAssignment(Name, E);
-  Opt.Commands.emplace_back(Cmd);
-  return Cmd;
-}
+static uint64_t getSymbolValue(StringRef S, uint64_t Dot) {
+  if (S == ".")
+    return Dot;
 
-// This is an operator-precedence parser to parse a linker
-// script expression.
-Expr ScriptParser::readExpr() { return readExpr1(readPrimary(), 0); }
-
-static uint64_t getSymbolValue(StringRef S) {
   switch (Config->EKind) {
   case ELF32LEKind:
     if (SymbolBody *B = Symtab<ELF32LE>::X->find(S))
       return B->getVA<ELF32LE>();
     break;
   case ELF32BEKind:
     if (SymbolBody *B = Symtab<ELF32BE>::X->find(S))
       return B->getVA<ELF32BE>();
     break;
   case ELF64LEKind:
     if (SymbolBody *B = Symtab<ELF64LE>::X->find(S))
       return B->getVA<ELF64LE>();
     break;
   case ELF64BEKind:
     if (SymbolBody *B = Symtab<ELF64BE>::X->find(S))
       return B->getVA<ELF64BE>();
     break;
   default:
     llvm_unreachable("unsupported target");
   }
   error("symbol not found: " + S);
   return 0;
 }
 
+SymbolAssignment *ScriptParser::readAssignment(StringRef Name) {
+  StringRef Op = next();
+  assert(Op == "=" || Op == "+=");
+  Expr E = readExpr();
+  if (Op == "+=")
+    E = [=](uint64_t Dot) { return getSymbolValue(Name, Dot) + E(Dot); };
+  auto *Cmd = new SymbolAssignment(Name, E);
+  Opt.Commands.emplace_back(Cmd);
+  return Cmd;
+}
+
+// This is an operator-precedence parser to parse a linker
+// script expression.
+Expr ScriptParser::readExpr() { return readExpr1(readPrimary(), 0); }
+
 // This is a part of the operator-precedence parser. This function
 // assumes that the remaining token stream starts with an operator.
 Expr ScriptParser::readExpr1(Expr Lhs, int MinPrec) {
   while (!atEOF() && !Error) {
     // Read an operator and an expression.
     StringRef Op1 = peek();
     if (Op1 == "?")
       return readTernary(Lhs);
if (S == "COMMONPAGESIZE" || S == "MAXPAGESIZE")
     return Target->PageSize;
   error("unknown constant: " + S);
   return 0;
 }
 
 Expr ScriptParser::readPrimary() {
   StringRef Tok = next();
 
-  if (Tok == ".")
-    return [](uint64_t Dot) { return Dot; };
-
   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 == "DATA_SEGMENT_RELRO_END") {
     readExpr();
     expect(")");
     return [](uint64_t Dot) { return alignTo(Dot, Target->PageSize); };
   }
 
   // Parse a symbol name or a number literal.
   uint64_t V = 0;
   if (Tok.getAsInteger(0, V)) {
-    if (!isValidCIdentifier(Tok))
+    if (Tok != "." && !isValidCIdentifier(Tok))
       setError("malformed number: " + Tok);
-    return [=](uint64_t Dot) { return getSymbolValue(Tok); };
+    return [=](uint64_t Dot) { return getSymbolValue(Tok, Dot); };
   }
   return [=](uint64_t Dot) { return V; };
 }
 
 Expr ScriptParser::readTernary(Expr Cond) {
   next();
   Expr L = readExpr();
   expect(":");

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

 # RUN:  .neq : { *(.neq) } \
 # RUN:  . = CONSTANT (MAXPAGESIZE) * 0x24; \
 # RUN:  .maxpagesize : { *(.maxpagesize) } \
 # RUN:  . = CONSTANT (COMMONPAGESIZE) * 0x25; \
 # RUN:  .commonpagesize : { *(.commonpagesize) } \
 # RUN:  . = DATA_SEGMENT_ALIGN (CONSTANT (MAXPAGESIZE), CONSTANT (COMMONPAGESIZE)); \
 # RUN:  .datasegmentalign : { *(.datasegmentalign) } \
 # RUN:  . = DATA_SEGMENT_END (.); \
+# RUN:  . = 0x27000; \
+# RUN:  . += 0x1000; \
+# RUN:  .plusassign : { *(.plusassign) } \
 # RUN: }" > %t.script
 # RUN: ld.lld %t --script %t.script -o %t2
 # RUN: llvm-readobj -s %t2 | FileCheck %s
 
 # CHECK: Section {
 # CHECK:   Index: 1
 # CHECK:   Name: .plus
 # CHECK-NEXT:   Type: SHT_PROGBITS

 # CHECK-NEXT:   Address: 0x26000
 # CHECK-NEXT:   Offset:
 # CHECK-NEXT:   Size:
 # CHECK-NEXT:   Link:
 # CHECK-NEXT:   Info:
 # CHECK-NEXT:   AddressAlignment:
 # CHECK-NEXT:   EntrySize:
 # CHECK-NEXT: }
+# CHECK-NEXT: Section {
+# CHECK-NEXT:   Index:
+# CHECK-NEXT:   Name: .plusassign
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address: 0x28000
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size:
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   AddressAlignment:
+# CHECK-NEXT:   EntrySize:
+# CHECK-NEXT: }
 
 ## Mailformed number error.
 # RUN: echo "SECTIONS { \
 # RUN:  . = 0x12Q41; \
 # RUN: }" > %t.script
 # RUN: not ld.lld %t --script %t.script -o %t2 2>&1 | \
 # RUN:  FileCheck --check-prefix=NUMERR %s
 # NUMERR: malformed number: 0x12Q41

 .section .maxpagesize, "a"
 .quad 0
 
 .section .commonpagesize, "a"
 .quad 0
 
 .section .datasegmentalign, "a"
 .quad 0
+
+.section .plusassign, "a"
+.quad 0