[PATCH] D40735: [LLD][ELF] InX::BssRelRo should check section contents before marking relro

[Peter Smith via Phabricator via llvm-commits]

peter.smith updated this revision to Diff 125331.
peter.smith added a comment.

Thanks for the comment, I've adapted the test to include parts of the previous error if mix .bss and .bss.rel.ro. I'm not sure if it adds much value in having two separate test cases, but do let me know if you'd like them split up?


https://reviews.llvm.org/D40735

Files:
  ELF/Writer.cpp
  test/ELF/relro-copyrel-bss-script.s


Index: test/ELF/relro-copyrel-bss-script.s
===================================================================
--- /dev/null
+++ test/ELF/relro-copyrel-bss-script.s
@@ -0,0 +1,43 @@
+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t.o
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/copy-in-shared.s -o %t2.o
+// RUN: ld.lld -shared %t.o %t2.o -o %t.so
+
+// A linker script that will map .bss.rel.ro into .bss.
+// RUN: echo "SECTIONS { \
+// RUN: .bss : { *(.bss) *(.bss.*) } \
+// RUN: } " > %t.script
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t3.o
+// RUN: ld.lld %t3.o %t.so -z relro -o %t --script=%t.script 2>&1
+// RUN: llvm-readobj --program-headers %t | FileCheck %s
+        .section .text, "ax", @progbits
+        .global bar
+        .global foo
+        .global _start
+_start:
+        callq bar
+        // Will produce .bss.rel.ro that will match in .bss, this will lose
+        // the relro property of the copy relocation.
+        .quad foo
+
+        .data
+        .space 4
+
+        // Non relro bss
+        .bss
+        // make large enough to affect PT_GNU_RELRO MemSize if this was marked
+        // as relro.
+        .space 0x2000
+
+// CHECK:     Type: PT_GNU_RELRO (0x6474E552)
+// CHECK-NEXT:     Offset:
+// CHECK-NEXT:     VirtualAddress:
+// CHECK-NEXT:     PhysicalAddress:
+// CHECK-NEXT:     FileSize:
+// CHECK-NEXT:     MemSize: 4096
+// CHECK-NEXT:     Flags [ (0x4)
+// CHECK-NEXT:       PF_R (0x4)
+// CHECK-NEXT:     ]
+// CHECK-NEXT:     Alignment: 1
+// CHECK-NEXT:   }
Index: ELF/Writer.cpp
===================================================================
--- ELF/Writer.cpp
+++ ELF/Writer.cpp
@@ -576,20 +576,14 @@
   if (Sec == InX::Dynamic->getParent())
     return true;
 
-  // .bss.rel.ro is used for copy relocations for read-only symbols.
-  // Since the dynamic linker needs to process copy relocations, the
-  // section cannot be read-only, but once initialized, they shouldn't
-  // change.
-  if (Sec == InX::BssRelRo->getParent())
-    return true;
-
   // Sections with some special names are put into RELRO. This is a
   // bit unfortunate because section names shouldn't be significant in
   // ELF in spirit. But in reality many linker features depend on
   // magic section names.
   StringRef S = Sec->Name;
-  return S == ".data.rel.ro" || S == ".ctors" || S == ".dtors" || S == ".jcr" ||
-         S == ".eh_frame" || S == ".openbsd.randomdata";
+  return S == ".data.rel.ro" || S == ".bss.rel.ro" || S == ".ctors" ||
+         S == ".dtors" || S == ".jcr" || S == ".eh_frame" ||
+         S == ".openbsd.randomdata";
 }
 
 // We compute a rank for each section. The rank indicates where the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40735.125331.patch
Type: text/x-patch
Size: 2770 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171204/a20b4d2d/attachment.bin>