[lld] r315441 - [ELF] - Do not set output section flags except SHF_{ALLOC, WRITE, EXECINSTR}.

[George Rimar via llvm-commits]

Author: grimar
Date: Wed Oct 11 01:13:40 2017
New Revision: 315441

URL: http://llvm.org/viewvc/llvm-project?rev=315441&view=rev
Log:
[ELF] - Do not set output section flags except SHF_{ALLOC,WRITE,EXECINSTR}.

This is PR34546.

Currently LLD creates output sections even if it has no input sections,
but its command contains an assignment.
Committed code just assigns the same flag that was used in previous
live section.
That does not work sometimes. For example if we have following script:

.ARM.exidx : { *(.ARM.exidx*) }
.foo : { _foo = 0; } }
Then first section has SHF_LINK_ORDER flag. But section foo should not.
That was a reason of crash in OutputSection::finalize(). LLD tried to calculate
Link value, calling front() on empty input sections list.
We should only keep access flags and omit all others when creating such sections.

Patch fixes the crash observed.

Differential revision: https://reviews.llvm.org/D37736

Added:
    lld/trunk/test/ELF/linkerscript/arm-exidx-order.s
    lld/trunk/test/ELF/linkerscript/symbol-only-flags.s
Modified:
    lld/trunk/ELF/LinkerScript.cpp

Modified: lld/trunk/ELF/LinkerScript.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LinkerScript.cpp?rev=315441&r1=315440&r2=315441&view=diff
==============================================================================
--- lld/trunk/ELF/LinkerScript.cpp (original)
+++ lld/trunk/ELF/LinkerScript.cpp Wed Oct 11 01:13:40 2017
@@ -657,9 +657,25 @@ static bool isAllSectionDescription(cons
 
 void LinkerScript::adjustSectionsBeforeSorting() {
   // If the output section contains only symbol assignments, create a
-  // corresponding output section. The bfd linker seems to only create them if
-  // '.' is assigned to, but creating these section should not have any bad
-  // consequeces and gives us a section to put the symbol in.
+  // corresponding output section. The issue is what to do with linker script
+  // like ".foo : { symbol = 42; }". One option would be to convert it to
+  // "symbol = 42;". That is, move the symbol out of the empty section
+  // description. That seems to be what bfd does for this simple case. The
+  // problem is that this is not completely general. bfd will give up and
+  // create a dummy section too if there is a ". = . + 1" inside the section
+  // for example.
+  // Given that we want to create the section, we have to worry what impact
+  // it will have on the link. For example, if we just create a section with
+  // 0 for flags, it would change which PT_LOADs are created.
+  // We could remember that that particular section is dummy and ignore it in
+  // other parts of the linker, but unfortunately there are quite a few places
+  // that would need to change:
+  //   * The program header creation.
+  //   * The orphan section placement.
+  //   * The address assignment.
+  // The other option is to pick flags that minimize the impact the section
+  // will have on the rest of the linker. That is why we copy the flags from
+  // the previous sections. Only a few flags are needed to keep the impact low.
   uint64_t Flags = SHF_ALLOC;
 
   for (BaseCommand *Cmd : SectionCommands) {
@@ -667,7 +683,7 @@ void LinkerScript::adjustSectionsBeforeS
     if (!Sec)
       continue;
     if (Sec->Live) {
-      Flags = Sec->Flags;
+      Flags = Sec->Flags & (SHF_ALLOC | SHF_WRITE | SHF_EXECINSTR);
       continue;
     }
 

Added: lld/trunk/test/ELF/linkerscript/arm-exidx-order.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/linkerscript/arm-exidx-order.s?rev=315441&view=auto
==============================================================================
--- lld/trunk/test/ELF/linkerscript/arm-exidx-order.s (added)
+++ lld/trunk/test/ELF/linkerscript/arm-exidx-order.s Wed Oct 11 01:13:40 2017
@@ -0,0 +1,19 @@
+# REQUIRES: arm
+# RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %s -o %t.o
+# RUN: echo "SECTIONS { . = SIZEOF_HEADERS;    \
+# RUN:         .ARM.exidx : { *(.ARM.exidx*) } \
+# RUN:         .foo : { _foo = 0; } }" > %t.script
+# RUN: ld.lld -T %t.script %t.o -shared -o %t.so
+# RUN: llvm-readobj -s %t.so | FileCheck %s
+
+# CHECK:      Section {
+# CHECK:        Index: 
+# CHECK:        Name: .foo
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC
+# CHECK-NEXT:   ]
+
+.fnstart
+.cantunwind
+.fnend

Added: lld/trunk/test/ELF/linkerscript/symbol-only-flags.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/linkerscript/symbol-only-flags.s?rev=315441&view=auto
==============================================================================
--- lld/trunk/test/ELF/linkerscript/symbol-only-flags.s (added)
+++ lld/trunk/test/ELF/linkerscript/symbol-only-flags.s Wed Oct 11 01:13:40 2017
@@ -0,0 +1,20 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "SECTIONS { . = SIZEOF_HEADERS; \
+# RUN:         .tbss : { *(.tbss) }         \
+# RUN:         .foo : { bar = .; } }" > %t.script
+# RUN: ld.lld -o %t --script %t.script %t.o
+# RUN: llvm-readobj -s %t | FileCheck %s
+
+## Check .foo does not get SHF_TLS flag.
+# CHECK:     Section {
+# CHECK:       Index:
+# CHECK:       Name: .foo
+# CHECK-NEXT:  Type: SHT_PROGBITS
+# CHECK-NEXT:  Flags [
+# CHECK-NEXT:    SHF_ALLOC
+# CHECK-NEXT:    SHF_WRITE
+# CHECK-NEXT:  ]
+
+.section .tbss,"awT", at nobits
+.quad 0