[ELF] - Improve diagnostic messages for move location counter errors.

[grimar]

Summary

Previously LLD would error out just "ld.lld: error: unable to move location counter backward"
What does not really reveal the place of issue,
Patch adds location to the output.

[ruiu]

lld/trunk/ELF/LinkerScript.h

92–94 ↗

(On Diff #89205)

Performance is not important at all here, so just pass a copy of a std::string instead of std::string &&.

109 ↗

(On Diff #89205)

This needs a comment.

[grimar]

lld/trunk/ELF/LinkerScript.h

109 ↗

(On Diff #89205)

I wonder if it worth to make Location a member of BaseCommand.
Since we also use it in OutputSectionCommand.
At the same time other commands seems do not need it.

[ruiu]

lld/trunk/ELF/LinkerScript.h

109 ↗

(On Diff #89205)

That's a good point. I guess it might make sense to move it to BaseCommand because basically we need debug info for all commands to print out user-friendly error messages.

ELF/LinkerScript.h

struct BaseCommand {
 
   virtual ~BaseCommand() = default;
 
   int Kind;
 };
 
 // This represents ". = <expr>" or "<symbol> = <expr>".
 struct SymbolAssignment : BaseCommand {
-  SymbolAssignment(StringRef Name, Expr E)
-      : BaseCommand(AssignmentKind), Name(Name), Expression(E) {}
+  SymbolAssignment(StringRef Name, Expr E, std::string &&Loc)
+      : BaseCommand(AssignmentKind), Name(Name), Expression(E),
+        Location(std::move(Loc)) {}
 
   static bool classof(const BaseCommand *C);
 
   // The LHS of an expression. Name is either a symbol name or ".".
   StringRef Name;
   SymbolBody *Sym = nullptr;
 
   // The RHS of an expression.
   Expr Expression;
 
   // Command attributes for PROVIDE, HIDDEN and PROVIDE_HIDDEN.
   bool Provide = false;
   bool Hidden = false;
+
+  std::string Location;
 };
 
 // Linker scripts allow additional constraints to be put on ouput sections.
 // If an output section is marked as ONLY_IF_RO, the section is created
 // 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 };
 
public:
   std::vector<OutputSectionBase *> *OutputSections;
 
   int getSectionIndex(StringRef Name);
 
 private:
   void assignSymbol(SymbolAssignment *Cmd, bool InSec = false);
   void addSymbol(SymbolAssignment *Cmd);
   void computeInputSections(InputSectionDescription *);
-  void setDot(Expr E, bool InSec = false);
+  void setDot(Expr E, const Twine &Loc, bool InSec = false);
 
   void discard(ArrayRef<InputSectionBase<ELFT> *> V);
 
   std::vector<InputSectionBase<ELFT> *>
   createInputSectionList(OutputSectionCommand &Cmd);
 
   // "ScriptConfig" is a bit too long, so define a short name for it.
   ScriptConfiguration &Opt = *ScriptConfig;

ELF/LinkerScript.cpp

static bool isUnderSysroot(StringRef Path) {
   if (Config->Sysroot == "")
     return false;
   for (; !Path.empty(); Path = sys::path::parent_path(Path))
     if (sys::fs::equivalent(Config->Sysroot, Path))
       return true;
   return false;
 }
 
-template <class ELFT> void LinkerScript<ELFT>::setDot(Expr E, bool InSec) {
+template <class ELFT>
+void LinkerScript<ELFT>::setDot(Expr E, const Twine &Loc, bool InSec) {
   uintX_t Val = E(Dot);
   if (Val < Dot) {
     if (InSec)
-      error("unable to move location counter backward for: " + CurOutSec->Name);
+      error(Loc + ": unable to move location counter backward for: " +
+            CurOutSec->Name);
     else
-      error("unable to move location counter backward");
+      error(Loc + ": unable to move location counter backward");
   }
   Dot = Val;
   // Update to location counter means update to section size.
   if (InSec)
     CurOutSec->Size = Dot - CurOutSec->Addr;
 }
 
 // Sets value of a symbol. Two kinds of symbols are processed: synthetic
 // symbols, whose value is an offset from beginning of section and regular
 // symbols whose value is absolute.
 template <class ELFT>
 void LinkerScript<ELFT>::assignSymbol(SymbolAssignment *Cmd, bool InSec) {
   if (Cmd->Name == ".") {
-    setDot(Cmd->Expression, InSec);
+    setDot(Cmd->Expression, Cmd->Location, InSec);
     return;
   }
 
   if (!Cmd->Sym)
     return;
 
   if (auto *Body = dyn_cast<DefinedSynthetic>(Cmd->Sym)) {
     Body->Section = Cmd->Expression.Section();

 void LinkerScript<ELFT>::assignOffsets(OutputSectionCommand *Cmd) {
   if (Cmd->LMAExpr)
     LMAOffset = Cmd->LMAExpr(Dot) - Dot;
   OutputSectionBase *Sec = findSection<ELFT>(Cmd->Name, *OutputSections);
   if (!Sec)
     return;
 
   if (Cmd->AddrExpr && Sec->Flags & SHF_ALLOC)
-    setDot(Cmd->AddrExpr);
+    setDot(Cmd->AddrExpr, Cmd->Location);
 
   // Handle align (e.g. ".foo : ALIGN(16) { ... }").
   if (Cmd->AlignExpr)
     Sec->updateAlignment(Cmd->AlignExpr(0));
 
   // Try and find an appropriate memory region to assign offsets in.
   CurMemRegion = findMemoryRegion(Cmd, Sec);
   if (CurMemRegion)
if (consume("ABSOLUTE")) {
     E = readExpr();
   }
   if (Op == "+=") {
     std::string Loc = getCurrentLocation();
     E = [=](uint64_t Dot) {
       return getSymbolValue(Loc, Name, Dot) + E(Dot);
     };
   }
-  return new SymbolAssignment(Name, E);
+  return new SymbolAssignment(Name, E, getCurrentLocation());
 }
 
 // This is an operator-precedence parser to parse a linker
 // script expression.
 Expr ScriptParser::readExpr() {
   // Our lexer is context-aware. Set the in-expression bit so that
   // they apply different tokenization rules.
   bool Orig = InExpr;

test/ELF/linkerscript/locationcountererr.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 # RUN: echo "SECTIONS { .text 0x2000 : {. = 0x10 ; *(.text) } }" > %t.script
 # RUN: not ld.lld %t --script %t.script -o %t1 2>&1 | FileCheck %s
-# CHECK: unable to move location counter backward for: .text
+# CHECK: {{.*}}.script:1: unable to move location counter backward for: .text
 
 .globl _start
 _start:
 nop

test/ELF/linkerscript/locationcountererr2.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: echo "SECTIONS { . = 0x20; . = 0x10; }" > %t.script
 # RUN: not ld.lld %t.o --script %t.script -o %t -shared 2>&1 | FileCheck %s
-# CHECK: unable to move location counter backward
+# CHECK: {{.*}}.script:1: unable to move location counter backward
 
 # RUN: echo "SECTIONS { . = 0x20; . = ASSERT(0x1, "foo"); }" > %t2.script
 # RUN: ld.lld %t.o --script %t2.script -o %t -shared

test/ELF/linkerscript/out-of-order.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-linux %s -o %t.o
 # RUN: echo "SECTIONS { .data 0x4000 : { *(.data) } .text 0x2000 : { *(.text) } }" > %t.script
 # RUN: not ld.lld -o %t.so --script %t.script %t.o -shared 2>&1 | FileCheck %s
 
-# CHECK:  error: unable to move location counter backward
+# CHECK:  error: {{.*}}.script:1: unable to move location counter backward
 
 .quad 0
 .data
 .quad 0