[PATCH] D37013: [ELF] - Add additional information about location when emiting linkerscript's parsing errors.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
Herald added a subscriber: emaste.

This is PR31128 which complains that we show unclear message when
parsing script like

  .= foo

(note: there is no space between `.=')

When LLD see `.= foo;` it starts reading output section declaration. 
It reads section name `.=`, addess expression `1` and expects to see `:` right after that
all and fails with `: expected, but got ;` message, which is fully correct in this case,
but not very useful for user who writes simple assignment expression.

FWIW scripts like `SECTIONS {  .= : { *(.text*) }  }` are correct, both GNU linkers
would create output section `.=` in that case, and LLD do the same.

What I suggest is to provide additional error context if available, like this patch do.


https://reviews.llvm.org/D37013

Files:
  ELF/ScriptLexer.cpp
  ELF/ScriptLexer.h
  ELF/ScriptParser.cpp
  test/ELF/linkerscript/operators.s


Index: test/ELF/linkerscript/operators.s
===================================================================
--- test/ELF/linkerscript/operators.s
+++ test/ELF/linkerscript/operators.s
@@ -90,6 +90,13 @@
 # RUN:  FileCheck --check-prefix=TERNERR %s
 # TERNERR: : expected, but got ;
 
+## Broken assignment.
+# RUN: echo "SECTIONS { .= 1; }" > %t.script
+# RUN: not ld.lld %t --script %t.script -o %t2 2>&1 | \
+# RUN:  FileCheck --check-prefix=ASSIGNERR %s
+# ASSIGNERR: unable to parse description for output section '.=':
+# ASSIGNERR: : expected, but got ;
+
 .globl _start
 _start:
 nop
Index: ELF/ScriptParser.cpp
===================================================================
--- ELF/ScriptParser.cpp
+++ ELF/ScriptParser.cpp
@@ -604,6 +604,8 @@
 }
 
 OutputSection *ScriptParser::readOutputSectionDescription(StringRef OutSec) {
+  setLocationContext("unable to parse description for output section '" +
+                     OutSec + "':");
   OutputSection *Cmd =
       Script->createOutputSection(OutSec, getCurrentLocation());
 
@@ -666,6 +668,7 @@
   // Consume optional comma following output section command.
   consume(",");
 
+  dropLocationContext();
   return Cmd;
 }
 
Index: ELF/ScriptLexer.h
===================================================================
--- ELF/ScriptLexer.h
+++ ELF/ScriptLexer.h
@@ -24,6 +24,9 @@
   explicit ScriptLexer(MemoryBufferRef MB);
 
   void setError(const Twine &Msg);
+  void setLocationContext(const Twine &Msg) { Context = Msg.str(); }
+  void dropLocationContext() { Context.clear(); }
+
   void tokenize(MemoryBufferRef MB);
   static StringRef skipSpace(StringRef S);
   bool atEOF();
@@ -39,6 +42,7 @@
   std::vector<StringRef> Tokens;
   bool InExpr = false;
   size_t Pos = 0;
+  std::string Context;
 
 private:
   void maybeSplitExpr();
Index: ELF/ScriptLexer.cpp
===================================================================
--- ELF/ScriptLexer.cpp
+++ ELF/ScriptLexer.cpp
@@ -78,6 +78,9 @@
   if (ErrorCount)
     return;
 
+  if (!Context.empty())
+    error(Context);
+
   if (!Pos) {
     error(getCurrentLocation() + ": " + Msg);
     return;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37013.112171.patch
Type: text/x-patch
Size: 2130 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170822/65702463/attachment.bin>