[LLD][ELF] - Linkerscript: Fix FILL() expressions handling.

[grimar]

Summary

D64130 introduced a bug described in the following message:
https://reviews.llvm.org/D64130#1571560

The problem can happen with the following script:

SECTIONS {
  .out : {
...
   FILL(0x10101010)
   *(.aaa)
...
}

The current code tries to read (0x10101010) as an expression and
does not break when meets *, what results in a script parsing error.

In this patch, I verify that FILL command's expression always wrapped in ().
And at the same time =<fillexp> expression can be both wrapped or unwrapped.
I checked it matches to bfd/gold.

[MaskRay]

ELF/ScriptParser.cpp

887

This function doesn't need any change. expect("(") below should be sufficient.

readParenExpr will handle the parenthesized expression.

[grimar]

ELF/ScriptParser.cpp

887

Hmm. Right. Not sure why I did that.

ELF/ScriptParser.cpp

while (!errorCount() && !consume("}")) {
     } else if (tok == "CONSTRUCTORS") {
       // CONSTRUCTORS is a keyword to make the linker recognize C++ ctors/dtors
       // by name. This is for very old file formats such as ECOFF/XCOFF.
       // For ELF, we should ignore.
     } else if (tok == "FILL") {
       // We handle the FILL command as an alias for =fillexp section attribute,
       // which is different from what GNU linkers do.
       // https://sourceware.org/binutils/docs/ld/Output-Section-Data.html
+      expect("(");
       cmd->filler = readFill();
+      expect(")");
     } else if (tok == "SORT") {
       readSort();
     } else if (tok == "INCLUDE") {
       readInclude();
     } else if (peek() == "(") {
       cmd->sectionCommands.push_back(readInputSectionDescription(tok));
     } else {
       // We have a file name and no input sections description. It is not a

 // Reads a `=<fillexp>` expression and returns its value as a big-endian number.
 // https://sourceware.org/binutils/docs/ld/Output-Section-Fill.html
 // We do not support using symbols in such expressions.
 //
 // 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.
 std::array<uint8_t, 4> ScriptParser::readFill() {
-  uint64_t value = readExpr()().val;
+  uint64_t value;
+  if (consume("(")) {
+    value = readExpr()().val;
+    expect(")");
+  } else {
+    value = readExpr()().val;
+  }
+
   if (value > UINT32_MAX)
     setError("filler expression result does not fit 32-bit: 0x" +
              Twine::utohexstr(value));
 
   std::array<uint8_t, 4> buf;
   write32be(buf.data(), (uint32_t)value);
   return buf;
 }

test/ELF/linkerscript/fill.test

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/fill.s -o %t.o
 # RUN: ld.lld -o %t --script %s %t.o
 # RUN: llvm-objdump -s %t | FileCheck %s
 
 SECTIONS {
   .out : {
    FILL(0x11111111)
    . += 2;
+   FILL(0x10101010)
    *(.aaa)
    . += 4;
    *(.bbb)
    . += 4;
    FILL(0x22220000 + 0x2222);
    . += 4;
   }
 }
 
 # CHECK:      Contents of section .out:
 # CHECK-NEXT: 2222aa22 222222bb 22222222 22222222
+
+# RUN: echo 'SECTIONS { .out : { FILL 0x11111111 } }' > %t.script
+# RUN: not ld.lld -o /dev/null --script %t.script 2>&1 | FileCheck %s --check-prefix=ERR
+# ERR: ( expected, but got 0x11111111
 No newline at end of file

test/ELF/linkerscript/sections-padding.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 
 ## Check that padding value works:
 # RUN: echo "SECTIONS { .mysec : { *(.mysec*) } =0x1122 }" > %t.script
 # RUN: ld.lld -o %t.out --script %t.script %t
 # 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: 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
+
 ## Confirming that address was correct:
 # RUN: echo "SECTIONS { .mysec : { *(.mysec*) } =0x99887766 }" > %t.script
 # RUN: ld.lld -o %t.out --script %t.script %t
-# RUN: llvm-objdump -s %t.out | FileCheck -check-prefix=YES2 %s
-# YES2: 66998877 66998877 66998877 66998877
+# RUN: llvm-objdump -s %t.out | FileCheck -check-prefix=YES3 %s
+# YES3: 66998877 66998877 66998877 66998877
 
 ## Default padding value is 0x00:
 # RUN: echo "SECTIONS { .mysec : { *(.mysec*) } }" > %t.script
 # RUN: ld.lld -o %t.out --script %t.script %t
 # RUN: llvm-objdump -s %t.out | FileCheck -check-prefix=NO %s
 # NO: 66000000 00000000 00000000 00000000
 
 ## Decimal value.

 # RUN: ld.lld -o %t.out --script %t.script %t
 # RUN: llvm-objdump -s %t.out | FileCheck -check-prefix=YES %s
 
 ## Check case with space between '=' and expression:
 # RUN: echo "SECTIONS { .mysec : { *(.mysec*) } =((0x11 << 8) | 0x22) }" > %t.script
 # RUN: ld.lld -o %t.out --script %t.script %t
 # RUN: llvm-objdump -s %t.out | FileCheck -check-prefix=YES %s
 
-## Check we report an error if expression value is larger that 32-bits.
+## Check we report an error if expression value is larger than 32-bits.
 # RUN: echo "SECTIONS { .mysec : { *(.mysec*) } =(0x11 << 32) }" > %t.script
 # RUN: not ld.lld -o %t.out --script %t.script %t 2>&1 | FileCheck --check-prefix=ERR3 %s
 # ERR3: filler expression result does not fit 32-bit: 0x1100000000
 
 ## Check we report an error if an expression use a symbol.
 # RUN: echo "SECTIONS { foo = 0x11; .mysec : { *(.mysec*) } = foo }" > %t.script
 # RUN: not ld.lld -o %t.out %t --script %t.script 2>&1 | FileCheck --check-prefix=ERR4 %s
 # ERR4: symbol not found: foo