[lld] bb7d2b1 - [LLD][ELF] - Disambiguate "=fillexp" with a primary expression to allow =0x90 /DISCARD/

[Georgii Rymar via llvm-commits]

Author: Georgii Rymar
Date: 2020-03-19T12:49:25+03:00
New Revision: bb7d2b178022e8590f483714bd9bafa9d3f86e24

URL: https://github.com/llvm/llvm-project/commit/bb7d2b178022e8590f483714bd9bafa9d3f86e24
DIFF: https://github.com/llvm/llvm-project/commit/bb7d2b178022e8590f483714bd9bafa9d3f86e24.diff

LOG: [LLD][ELF] - Disambiguate "=fillexp" with a primary expression to allow =0x90 /DISCARD/

Fixes https://bugs.llvm.org/show_bug.cgi?id=44903

It is about the following case:

```
SECTIONS {
  .foo : { *(.foo) } =0x90909090
  /DISCARD/ : { *(.bar) }
}
```

Here while parsing the fill expression we treated the
"/" of "/DISCARD/" as operator.

With this change, suggested by Fangrui Song, we do
not allow expressions with operators (e.g. "0x1100 + 0x22")
that are not wrapped into round brackets. It should not
be an issue for users, but helps to resolve parsing ambiguity.

Differential revision: https://reviews.llvm.org/D74687

Added: 
    

Modified: 
    lld/ELF/ScriptParser.cpp
    lld/test/ELF/linkerscript/sections-padding.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index 0a041202f278..92768bae0c83 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -848,9 +848,9 @@ OutputSection *ScriptParser::readOutputSectionDescription(StringRef outSec) {
       // We handle the FILL command as an alias for =fillexp section attribute,
       // which is 
diff erent from what GNU linkers do.
       // https://sourceware.org/binutils/docs/ld/Output-Section-Data.html
-      expect("(");
+      if (peek() != "(")
+        setError("( expected, but got " + peek());
       cmd->filler = readFill();
-      expect(")");
     } else if (tok == "SORT") {
       readSort();
     } else if (tok == "INCLUDE") {
@@ -905,8 +905,11 @@ OutputSection *ScriptParser::readOutputSectionDescription(StringRef outSec) {
 // When reading a hexstring, ld.bfd handles it as a blob of arbitrary
 // size, while ld.gold always handles it as a 32-bit big-endian number.
 // We are compatible with ld.gold because it's easier to implement.
+// Also, we require that expressions with operators must be wrapped into
+// round brackets. We did it to resolve the ambiguity when parsing scripts like:
+// SECTIONS { .foo : { ... } =120+3 /DISCARD/ : { ... } }
 std::array<uint8_t, 4> ScriptParser::readFill() {
-  uint64_t value = readExpr()().val;
+  uint64_t value = readPrimary()().val;
   if (value > UINT32_MAX)
     setError("filler expression result does not fit 32-bit: 0x" +
              Twine::utohexstr(value));

diff  --git a/lld/test/ELF/linkerscript/sections-padding.s b/lld/test/ELF/linkerscript/sections-padding.s
index 5ec0ddbe767a..4d147d79c63e 100644
--- a/lld/test/ELF/linkerscript/sections-padding.s
+++ b/lld/test/ELF/linkerscript/sections-padding.s
@@ -7,7 +7,7 @@
 # RUN: llvm-objdump -s %t.out | FileCheck --check-prefix=YES %s
 # YES: 66000011 22000011 22000011 22000011
 
-# RUN: echo "SECTIONS { .mysec : { *(.mysec*) } =0x1100+0x22 }" > %t.script
+# RUN: echo "SECTIONS { .mysec : { *(.mysec*) } =(0x1100+0x22) }" > %t.script
 # RUN: ld.lld -o %t.out --script %t.script %t
 # RUN: llvm-objdump -s %t.out | FileCheck --check-prefix=YES2 %s
 # YES2: 66000011 22000011 22000011 22000011
@@ -66,6 +66,11 @@
 # RUN: not ld.lld -o /dev/null %t --script %t.script 2>&1 | FileCheck --check-prefix=ERR4 %s
 # ERR4: symbol not found: foo
 
+## Check we are able to parse scripts where "/DISCARD/" follows a section fill expression.
+# RUN: echo "SECTIONS { .mysec : { *(.mysec*) } =0x1122 /DISCARD/ : { *(.text) } }" > %t.script
+# RUN: ld.lld -o %t.out --script %t.script %t
+# RUN: llvm-objdump -s %t.out | FileCheck --check-prefix=YES %s
+
 .section        .mysec.1,"a"
 .align  16
 .byte   0x66