[ELF] - Linkerscript: reimplement readSectionExcludes()

[grimar]

Summary

It is not only a bit more straightforward now, but also next 2 issues are solved:

1. It just crashed on ".foo : { *(EXCLUDE_FILE (*file1.o)) }" before.
2. It accepted multiple EXCLUDE_FILEs in a row.

[ruiu]

ELF/LinkerScript.cpp

-6–1

Move std::vector<StringRef> V just before this line to minimize the scope of the variable.

-6–1

Checking for EXCLUDE_FILE here seems too arbitrary. What if it is other reserved word such as SORT? I think we don't want to handle EXCLUDE_FILE as a special case. I'd remove && peek() == "EXCLUDE_FILE".

1

"section pattern is expected"

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

-6–1

I do not think it is possible to meet other keywords, about SORT:
there is a difference in semanics of ld/gold.

ld wants:

*(SORT(EXCLUDE_FILE (*file1.o) .foo.1))

Example of valid syntax:

SECTIONS {
foo : {
*(SORT(EXCLUDE_FILE (*file1.o) .foo.1)  .foo.3 EXCLUDE_FILE (*file1.o) .foo.1)
}
};

gold likes different order.

*(EXCLUDE_FILE (*file1.o) SORT(.foo.1))

Example of valid syntax:

SECTIONS {
foo : {
*(EXCLUDE_FILE (*file1.o) SORT(.foo.1) EXCLUDE_FILE (*file1.o) .foo.2)
}
};

I am inclined to think that means that in real world excludes probably not used with sorting. But anyways we probably should choose which semantic to support. I suggest to go with ld way: list of sections with file excludes is a subject/argument for SORT that is outside.

So basing on above, function is named readSectionExcludes(), it is about next list:
((EXCLUDE_FILE(file_pattern+))? section_pattern+)+
So it is should not be possible to meet any other key word I think and I still need that check, because if I see new "EXCLUDE_FILE" that means I need to start new [ExcludeFile, SectionsRegex] entry here.

[grimar]

ELF/LinkerScript.cpp

1074–1082

Next patch implements the syntax I am talking about: D24758

[ruiu]

LGTM

ELF/LinkerScript.cpp

 //===- LinkerScript.cpp ---------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //

 // Method reads a list of sequence of excluded files and section globs given in
 // a following form: ((EXCLUDE_FILE(file_pattern+))? section_pattern+)+
 // Example: *(.foo.1 EXCLUDE_FILE (*a.o) .foo.2 EXCLUDE_FILE (*b.o) .foo.3)
 // The semantics of that is next:
 // * Include .foo.1 from every file.
 // * Include .foo.2 from every file but a.o
 // * Include .foo.3 from every file but b.o
 void ScriptParser::readSectionExcludes(InputSectionDescription *Cmd) {
+  while (!Error && peek() != ")") {
     Regex ExcludeFileRe;
-  std::vector<StringRef> V;
-
-  while (!Error) {
-    if (skip(")")) {
-      Cmd->SectionPatterns.push_back(
-          {std::move(ExcludeFileRe), compileGlobPatterns(V)});
-      return;
-    }
-
     if (skip("EXCLUDE_FILE")) {
-      if (!V.empty()) {
-        Cmd->SectionPatterns.push_back(
-            {std::move(ExcludeFileRe), compileGlobPatterns(V)});
-        V.clear();
-      }
-
       expect("(");
       ExcludeFileRe = readFilePatterns();
-      continue;
     }
 
+    std::vector<StringRef> V;
+    while (!Error && peek() != ")" && peek() != "EXCLUDE_FILE")
       V.push_back(next());
+
+    if (!V.empty())
+      Cmd->SectionPatterns.push_back(
+          {std::move(ExcludeFileRe), compileGlobPatterns(V)});
+    else
+      setError("section pattern is expected");
   }
+  expect(")");
 }
 
 InputSectionDescription *
 ScriptParser::readInputSectionRules(StringRef FilePattern) {
   auto *Cmd = new InputSectionDescription(FilePattern);
   expect("(");
 
   // Read SORT().

test/ELF/linkerscript/exclude-multiple.s

 # CHECK-NEXT:  0130 07000000 00000000 05000000 00000000
 # CHECK-NEXT:  0140 08000000 00000000 03000000 00000000
 # CHECK-NEXT:  0150 09000000 00000000
 # CHECK-NEXT: Contents of section .foo.2:
 # CHECK-NEXT:  0158 02000000 00000000
 # CHECK-NEXT: Contents of section .foo.3:
 # CHECK-NEXT:  0160 06000000 00000000
 
+# RUN: echo "SECTIONS { .foo : { *(EXCLUDE_FILE (*file1.o) EXCLUDE_FILE (*file2.o) .foo.3) } }" > %t2.script
+# RUN: not ld.lld -script %t2.script %tfile1.o %tfile2.o %tfile3.o -o %t2.o 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=ERR
+# ERR: section pattern is expected
+
+# RUN: echo "SECTIONS { .foo : { *(EXCLUDE_FILE (*file1.o)) } }" > %t3.script
+# RUN: not ld.lld -script %t3.script %tfile1.o %tfile2.o %tfile3.o -o %t2.o 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=ERR
+
 .section .foo.1,"a"
  .quad 1
 
 .section .foo.2,"a"
  .quad 2
 
 .section .foo.3,"a"
  .quad 3