[PATCH] D76915: [LLD][ELF] Allow empty (.init|.preinit|.fini)_array to be RELRO

[Peter Smith via Phabricator via llvm-commits]

psmith created this revision.
psmith added reviewers: ruiu, MaskRay, grimar.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.

The default GNU linker script uses the following idiom for the array sections. I'll use .init_array here, but this also applies to .preinit_array and .fini_array sections.

  .init_array    :
  {
    PROVIDE_HIDDEN (__init_array_start = .);
    KEEP (*(.init_array))
    PROVIDE_HIDDEN (__init_array_end = .);
  }

The C-library will take references to the _start and _end symbols to process the array. This will make LLD keep the OutputSection even if there are no .init_array sections. As the current check for RELRO uses the section type for .init_array the above example with no .init_array InputSections fails the checks as there are no .init_array sections to give the OutputSection a type of SHT_INIT_ARRAY. This often leads to a non-contiguous RELRO error message, which I've seen in at least PR https://bugs.llvm.org/show_bug.cgi?id=44698 when suggesting a linker script as a work-around.

The simple fix is to a textual section match as well as a section type match. Potential alternatives involve setting the type of the name of an OutputSection called .init_array to SHT_INIT_ARRAY somewhere else. Or potentially trying to work out if an OutputSection has no content and can be considered RELRO regardless of its name.


https://reviews.llvm.org/D76915

Files:
  lld/ELF/Writer.cpp
  lld/test/ELF/relro-init-fini-script.s


Index: lld/test/ELF/relro-init-fini-script.s
===================================================================
--- /dev/null
+++ lld/test/ELF/relro-init-fini-script.s
@@ -0,0 +1,30 @@
+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-none-elf %s -o %t.o
+// RUN: echo "SECTIONS { \
+// RUN:  .dynamic 0x10000 : { *(.dynamic) } \
+// RUN:  .preinit_array : { PROVIDE_HIDDEN (__preinit_array_start = .); \
+// RUN:                  KEEP (*(.preinit_array)) } \
+// RUN:  .init_array : { PROVIDE_HIDDEN (__init_array_start = .); \
+// RUN:                  KEEP (*(.init_array)) } \
+// RUN:  .fini_array : { PROVIDE_HIDDEN (__fini_array_start = .); \
+// RUN:                  KEEP (*(.fini_array)) } \
+// RUN:  .data.rel.ro : { *(.data.rel.ro) } \
+// RUN:  .data : { *(.data) } } " > %t.script
+// RUN: ld.lld %t.o -o %t.so --shared --script=%t.script
+// RUN: llvm-readobj --segments %t.so | FileCheck %s
+
+/// Check that an empty .init_array, .fini_array or .preinit_array that is
+/// kept due to symbol references, is still counted as RELRO.
+
+// CHECK:      Type: PT_GNU_RELRO
+// CHECK-NEXT: Offset: 0x2000
+// CHECK-NEXT: VirtualAddress: 0x10000
+
+        .section .data.rel.ro, "aw", %progbits
+        .global foo
+        .quad foo
+
+        .data
+        .quad __init_array_start
+        .quad __fini_array_start
+        .quad __preinit_array_start
Index: lld/ELF/Writer.cpp
===================================================================
--- lld/ELF/Writer.cpp
+++ lld/ELF/Writer.cpp
@@ -819,7 +819,8 @@
   StringRef s = sec->name;
   return s == ".data.rel.ro" || s == ".bss.rel.ro" || s == ".ctors" ||
          s == ".dtors" || s == ".jcr" || s == ".eh_frame" ||
-         s == ".openbsd.randomdata";
+         s == ".fini_array" || s == ".init_array" ||
+         s == ".openbsd.randomdata" || s == ".preinit_array";
 }
 
 // We compute a rank for each section. The rank indicates where the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76915.253070.patch
Type: text/x-patch
Size: 1935 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200327/d3ef699d/attachment.bin>