[lld/elf] support quote usage in section names

[royger]

Summary

Section names used in ELF linker scripts can be quoted, but such
quotes must not be propagated to the binary ELF section names. As
such strip the quotes from the section names when processing them, and
also strip them from linker script functions that take section names
as parameters.

[MaskRay]

lld/test/ELF/linkerscript/section-quotes.test

5

Use split-file to provide the linker script

20

Just keep ALIGNOF for .text and keep LOADADDR for .data? That provides sufficient coverage without being too verbose.

24

Unused. Delete

30

Unused. Delete

36

Unused. Delete

[royger]

Thanks for the review, let me know if you agree with the reply. Will look into using split-file if that's desirable.

lld/test/ELF/linkerscript/section-quotes.test

20

The point is to test all possible combinations of quoted section name definition and usage of quotes section names in functions. Hence the tree combinations for .text .data and .bss:

• text: quoted section name used for both definition and as function parameter
• data: section definition not quoted, section parameter in functions quoted.
• bss: section definition quoted, section parameter in functions not quoted.

24

It's unused, but the point (here and in usages below) is to test that the SIZEOF function can correctly deal with quoted and unquoted section names.

[MaskRay]

lld/test/ELF/linkerscript/section-quotes.test

20

One LONG (ALIGNOF("...")) and one LONG (LOADADDR(".text")) are sufficient. No need to test LONG (ALIGNOF("...")) in two output section descriptions.

24

OK, keeping text_size and data_text sounds good. But change llvm-readelf -S %t to llvm-readelf -S -s %t and additionally test the values of the two symbols.

[royger]

• Use split-file.
• Remove .bss test case.
• Test presence of {text,data}_size in the output.
• Add comment about purpose of tests.

[MaskRay]

[lld/elf] fix quote usage in section names

"Support " is more appropriate. We don't claim this was supported so we cannot consider this as a bug.

[royger]

In D124266#3472842, @MaskRay wrote:

[lld/elf] fix quote usage in section names

"Support " is more appropriate. We don't claim this was supported so we cannot consider this as a bug.

OK, I've used fix because GNU ld does support quoted section names, and lld claims to be compatible.

Can this be fixed at commit please? Are there any other steps I should do to get this committed?

Thanks

[MaskRay]

In D124266#3474336, @royger wrote:

In D124266#3472842, @MaskRay wrote:

[lld/elf] fix quote usage in section names

"Support " is more appropriate. We don't claim this was supported so we cannot consider this as a bug.

OK, I've used fix because GNU ld does support quoted section names, and lld claims to be compatible.

Can this be fixed at commit please? Are there any other steps I should do to get this committed?

Thanks

You may change the subject of the differential.

There are number of comments for tests which haven't been marked as done.

[royger]

Since you approved the change I take you are happy with the code as-is, and marked all the comments as done. Thanks.

[royger]

Ping?

[MaskRay]

In D124266#3536371, @royger wrote:

Ping?

I adjusted the test a bit and landed it. Sorry for being so late. You can ping once per week if no response...

lld/ELF/ScriptParser.cpp

 //===- ScriptParser.cpp ---------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
while (!errorCount() && !consume("}")) {
     } else if (tok == "INCLUDE") {
       readInclude();
       continue;
     }
 
     if (SectionCommand *cmd = readAssignment(tok))
       v.push_back(cmd);
     else
-      v.push_back(readOutputSectionDescription(tok));
+      v.push_back(readOutputSectionDescription(unquote(tok)));
   }
   script->sectionCommands.insert(script->sectionCommands.end(), v.begin(),
                                  v.end());
 
   if (atEOF() || !consume("INSERT")) {
     script->hasSectionsCommand = true;
     return;
   }
if (tok == "ABSOLUTE") {
     Expr inner = readParenExpr();
     return [=] {
       ExprValue i = inner();
       i.forceAbsolute = true;
       return i;
     };
   }
   if (tok == "ADDR") {
-    StringRef name = readParenLiteral();
+    StringRef name = unquote(readParenLiteral());
     OutputSection *osec = &script->getOrCreateOutputSection(name)->osec;
     osec->usedInExpression = true;
     return [=]() -> ExprValue {
       checkIfExists(*osec, location);
       return {osec, false, 0, location};
     };
   }
   if (tok == "ALIGN") {
     expect("(");
     Expr e = readExpr();
     if (consume(")")) {
       e = checkAlignment(e, location);
       return [=] { return alignTo(script->getDot(), e().getValue()); };
     }
     expect(",");
     Expr e2 = checkAlignment(readExpr(), location);
     expect(")");
     return [=] {
       ExprValue v = e();
       v.alignment = e2().getValue();
       return v;
     };
   }
   if (tok == "ALIGNOF") {
-    StringRef name = readParenLiteral();
+    StringRef name = unquote(readParenLiteral());
     OutputSection *osec = &script->getOrCreateOutputSection(name)->osec;
     return [=] {
       checkIfExists(*osec, location);
       return osec->alignment;
     };
   }
   if (tok == "ASSERT")
     return readAssert();
if (tok == "LENGTH") {
     StringRef name = readParenLiteral();
     if (script->memoryRegions.count(name) == 0) {
       setError("memory region not defined: " + name);
       return [] { return 0; };
     }
     return script->memoryRegions[name]->length;
   }
   if (tok == "LOADADDR") {
-    StringRef name = readParenLiteral();
+    StringRef name = unquote(readParenLiteral());
     OutputSection *osec = &script->getOrCreateOutputSection(name)->osec;
     osec->usedInExpression = true;
     return [=] {
       checkIfExists(*osec, location);
       return osec->getLMA();
     };
   }
   if (tok == "LOG2CEIL") {
if (tok == "SEGMENT_START") {
     expect("(");
     skip();
     expect(",");
     Expr e = readExpr();
     expect(")");
     return [=] { return e(); };
   }
   if (tok == "SIZEOF") {
-    StringRef name = readParenLiteral();
+    StringRef name = unquote(readParenLiteral());
     OutputSection *cmd = &script->getOrCreateOutputSection(name)->osec;
     // Linker script does not create an output section if its content is empty.
     // We want to allow SIZEOF(.foo) where .foo is a section which happened to
     // be empty.
     return [=] { return cmd->size; };
   }
   if (tok == "SIZEOF_HEADERS")
     return [=] { return elf::getHeaderSize(); };

lld/test/ELF/linkerscript/section-quotes.test

+# REQUIRES: x86
+## Test quotation when specifying section names.
+
+# RUN: split-file %s %ts
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %ts/s -o %t.o
+# RUN: ld.lld -T %ts/t %t.o -o %t
+# RUN: llvm-readelf -s -S %t | FileCheck %s
+
+# CHECK:      Name         Type     Address          Off    Size   ES Flg Lk Inf Al
+# CHECK-NEXT:              NULL     0000000000000000 000000 000000 00      0   0  0
+# CHECK-NEXT: .text        {{.*}}
+# CHECK-NEXT: .data        {{.*}}
+# CHECK:        Num:    Value          Size Type    Bind   Vis       Ndx Name
+# CHECK-NEXT:     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND
+# CHECK-NEXT:     1: 0000000000000009     0 NOTYPE  GLOBAL DEFAULT   ABS text_size
+# CHECK-NEXT:     2: 0000000000000009     0 NOTYPE  GLOBAL DEFAULT   ABS data_size
+
+#--- s
+  .text
+  nop
+
+  .data
+  .byte 0
+
+#--- t
+SECTIONS {
+  # Check quoted section names are accepted and quotes are removed in the
+  # output binary section name.
+  #
+  # Also check that functions taking a section name as parameter work correctly
+  # when quoted section names are provided as inputs.
+  ".text" : AT(ADDR(".text")) {
+    LONG (ALIGNOF(".text"))
+    LONG (LOADADDR(".text"))
+    *(.text)
+  }
+  text_size = SIZEOF(".text");
+
+  # Check that functions that take a section name can correctly match a quoted
+  # section name input parameter against a non-quoted section definition.
+  .data : AT(ADDR(".data")) {
+    LONG (ALIGNOF(".data"))
+    LONG (LOADADDR(".data"))
+    *(.data)
+  }
+  data_size = SIZEOF(".data");
+}