[LLD][ELF] - Linkerscript: support the case when INPUT_SECTION_FLAGS is used without section patterns.

[grimar]

Summary

This is a continuation of D72756. It uses one of test cases made by Peter Smith which was
not included to the final version of the patch (we've agreed to place a FIXME that time).

This patch suggests a change to support this missing part.
(I am not sure how much it is useful feature, but I think we do not want to have this FIXME
and IMO this change makes the code a bit cleaner).

[grimar]

• Move the new test case to a proper position in the file.

[psmith]

I've made a comment about potentially allowing nested KEEP, although I'm not 100% sure about it. Other than that this looks good.

lld/ELF/ScriptParser.cpp

702

If I've read this right, this would allow something like:

KEEP ( INPUT_SECTION_FLAGS KEEP ( ... ) )

Although not pretty, perhaps a bool parameter to readInputSectionDescription to say if we are within a KEEP so we can error if we have another. I think the ldgram.y has a separate input_section_no_keep that it uses for this purpose.

[MaskRay]

This syntax probably has no user now. The question is whether the syntax can have a reasonable use case in the future... The raw wildcard syntax is questionable because it can potentially cause parsing ambiguity. Having ) as a terminal symbol is nice.

I also left a comment on https://bugs.llvm.org/show_bug.cgi?id=39885

[grimar]

In D76765#1941605, @MaskRay wrote:

The raw wildcard syntax is questionable because it can potentially cause parsing ambiguity.
...
I also left a comment on https://bugs.llvm.org/show_bug.cgi?id=39885

What about if we drop the following piece we have then?

} else {
  // We have a file name and no input sections description. It is not a
  // commonly used syntax, but still acceptable. In that case, all sections
  // from the file will be included.
  // FIXME: GNU ld permits INPUT_SECTION_FLAGS to be used here. We do not
  // handle this case here as it will already have been matched by the
  // case above.
  auto *isd = make<InputSectionDescription>(tok);
  isd->sectionPatterns.push_back({{}, StringMatcher("*")});
  cmd->sectionCommands.push_back(isd);
}

[psmith]

In D76765#1943051, @grimar wrote:

In D76765#1941605, @MaskRay wrote:

The raw wildcard syntax is questionable because it can potentially cause parsing ambiguity.
...
I also left a comment on https://bugs.llvm.org/show_bug.cgi?id=39885

What about if we drop the following piece we have then?

} else {
  // We have a file name and no input sections description. It is not a
  // commonly used syntax, but still acceptable. In that case, all sections
  // from the file will be included.
  // FIXME: GNU ld permits INPUT_SECTION_FLAGS to be used here. We do not
  // handle this case here as it will already have been matched by the
  // case above.
  auto *isd = make<InputSectionDescription>(tok);
  isd->sectionPatterns.push_back({{}, StringMatcher("*")});
  cmd->sectionCommands.push_back(isd);
}

Given that someone can always write the pattern with ("*") which will also work on binutils then I'm happy to say we don't support it. If we do drop support we should consider a "not supported" error message, and to document it as a known limitation.

[grimar]

Abandining since seems we want to drop the support of syntax when we have a file name and no input sections description and that is not what this patch does.

lld/ELF/ScriptParser.cpp

ScriptParser::readInputSectionRules(StringRef filePattern, uint64_t withFlags,
   }
   return cmd;
 }
 
 InputSectionDescription *
 ScriptParser::readInputSectionDescription(StringRef tok) {
   // Input section wildcard can be surrounded by KEEP.
   // https://sourceware.org/binutils/docs/ld/Input-Section-Keep.html#Input-Section-Keep
-  uint64_t withFlags = 0;
-  uint64_t withoutFlags = 0;
   if (tok == "KEEP") {
     expect("(");
-    if (consume("INPUT_SECTION_FLAGS"))
-      std::tie(withFlags, withoutFlags) = readInputSectionFlags();
     InputSectionDescription *cmd =
-        readInputSectionRules(next(), withFlags, withoutFlags);
+        peek() == "INPUT_SECTION_FLAGS"
+            ? readInputSectionDescription(next())
+            : readInputSectionRules(next(), /*withFlags=*/0,
+                                    /*withoutFlags*/ 0);
     expect(")");
     script->keptSections.push_back(cmd);
     return cmd;
   }
+
+  uint64_t withFlags = 0;
+  uint64_t withoutFlags = 0;
   if (tok == "INPUT_SECTION_FLAGS") {
     std::tie(withFlags, withoutFlags) = readInputSectionFlags();
     tok = next();
   }
+
+  if (peek() != "(") {
+    // We have a file name and no input sections description. It is not a
+    // commonly used syntax, but still acceptable. In that case, all sections
+    // from the file will be included.
+    auto *isd = make<InputSectionDescription>(tok, withFlags, withoutFlags);
+    isd->sectionPatterns.push_back({{}, StringMatcher("*")});
+    return isd;
+  }
+
   return readInputSectionRules(tok, withFlags, withoutFlags);
 }
 
 void ScriptParser::readSort() {
   expect("(");
   expect("CONSTRUCTORS");
   expect(")");
 }
if (tok == ";") {
       // https://sourceware.org/binutils/docs/ld/Output-Section-Data.html
       if (peek() != "(")
         setError("( expected, but got " + peek());
       cmd->filler = readFill();
     } 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
-      // commonly used syntax, but still acceptable. In that case, all sections
-      // from the file will be included.
-      // FIXME: GNU ld permits INPUT_SECTION_FLAGS to be used here. We do not
-      // handle this case here as it will already have been matched by the
-      // case above.
-      auto *isd = make<InputSectionDescription>(tok);
-      isd->sectionPatterns.push_back({{}, StringMatcher("*")});
-      cmd->sectionCommands.push_back(isd);
+      cmd->sectionCommands.push_back(readInputSectionDescription(tok));
     }
   }
 
   if (consume(">"))
     cmd->memoryRegionName = std::string(next());
 
   if (consume("AT")) {
     expect(">");

lld/test/ELF/input-section-flags.s

 # RUN: .outsec3 : { INPUT_SECTION_FLAGS(0x1) *(.sec.*) } \
 # RUN: .outsec4 : { INPUT_SECTION_FLAGS(0x10 & !0x20) *(.sec.*) } \
 # RUN: .outsec5 : { INPUT_SECTION_FLAGS(0x20) *(.sec.*) } \
 # RUN: } " > %t3.script
 
 # RUN: ld.lld -o %t3 --script %t3.script %t.o
 # RUN: llvm-readobj --symbols %t3 | FileCheck %s
 
+## Same test but using just a filespec.
+# RUN: echo "SECTIONS { \
+# RUN: .outsec1 : { INPUT_SECTION_FLAGS(SHF_ALLOC & !SHF_EXECINSTR & \
+# RUN:                                  !SHF_WRITE & !SHF_MERGE) *.o }\
+# RUN: .outsec2 : { INPUT_SECTION_FLAGS(SHF_ALLOC & SHF_EXECINSTR & !SHF_WRITE\
+# RUN:                                  & !SHF_MERGE) *.o } \
+# RUN: .outsec3 : { INPUT_SECTION_FLAGS(SHF_WRITE) *.o } \
+# RUN: .outsec4 : { INPUT_SECTION_FLAGS(SHF_MERGE & !SHF_STRINGS) * } \
+# RUN: .outsec5 : { INPUT_SECTION_FLAGS(SHF_STRINGS) * } \
+# RUN: } " > %t4.script
+
+# RUN: ld.lld -o %t4 --script %t4.script %t.o
+# RUN: llvm-readobj --symbols %t4 | FileCheck %s
+
 ## Check that we can handle multiple InputSectionDescriptions in a single
 ## OutputSection
 # RUN: echo "SECTIONS { \
 # RUN: .outsec1 : { INPUT_SECTION_FLAGS(SHF_ALLOC & !SHF_EXECINSTR & \
 # RUN:                                  !SHF_WRITE & !SHF_MERGE) *(.sec.*) ; \
 # RUN:              INPUT_SECTION_FLAGS(SHF_ALLOC & SHF_EXECINSTR & !SHF_WRITE\
 # RUN:                                  & !SHF_MERGE)  *(.sec.* *.text) }\
-# RUN: } " > %t4.script
+# RUN: } " > %t5.script
 
-# RUN: ld.lld -o %t4 --script %t4.script %t.o
-# RUN: llvm-readobj --symbols %t4 | FileCheck --check-prefix MULTIPLE %s
+# RUN: ld.lld -o %t5 --script %t5.script %t.o
+# RUN: llvm-readobj --symbols %t5 | FileCheck --check-prefix MULTIPLE %s
 
 # MULTIPLE:  Name: _start
 # MULTIPLE:  Section: .outsec1
 # MULTIPLE:  Name: s1
 # MULTIPLE:  Section: .outsec1
 # MULTIPLE:  Name: s2
 # MULTIPLE:  Section: .outsec1
 # MULTIPLE:  Name: s3