[PATCH] D91180: [ELF] Support multiple SORT in an input section description

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 11 01:07:35 PST 2020


grimar added inline comments.


================
Comment at: lld/ELF/ScriptParser.cpp:659
+    // SORT(.bar))) where we should break the loop.
+    while (!errorCount() && peek() != ")" && peek2() != "(")
       SectionMatcher.addPattern(unquote(next()));
----------------
I'd reformat the comment to have the sample on a single line, it is hard to read currently.

```
    // peek2() is used to detect "EXCLUDE_FILE(" or "SORT("
    // (e.g. *(.foo SORT(.bar)) ) where we should break the loop.
```

Also, our current grammar is LL(2), but we tried to use LL(1) where was possible.
See (a bit outdated) comment in ScriptLexer.cpp.

// Our grammar of the linker script is LL(2), meaning that it needs at
// most two-token lookahead to parse. The only place we need two-token
// lookahead is labels in version scripts, where we need to parse "local :"
// as if "local:".

Given above I'd go with explicit version:

```
    while (!errorCount() && peek() != ")" && peek() != "SORT" &&
           peek() != "EXCLUDE_FILE")
```

Note:
1) That "(" is a valid file name. Your version goes into infinite loop with the following input (added the `FOO (` to script from test case):
`# RUN: echo "SECTIONS { .abc : { *(SORT(.foo.*) .a* .a* SORT(.bar.*) FOO ( .b*) } }" > %t1.script`
2) Perhaps, the comment for `readInputSectionsList` should be updated.




================
Comment at: lld/test/ELF/linkerscript/sort2.s:8
 
-# CHECK:  Contents of section .abc:
-# CHECK:   01000000 00000000 02000000 00000000
-# CHECK:   03000000 00000000 04000000 00000000
-# CHECK:   06000000 00000000 05000000 00000000
+## FIXME Some input sections are duplicated in .abc and their second occurrences are zeros.
+# CHECK:      Hex dump of section '.abc'
----------------
This is what is fixed by D91127, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91180/new/

https://reviews.llvm.org/D91180



More information about the llvm-commits mailing list