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

Peter Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 4 02:52:19 PST 2017


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

>> peter.smith created this revision.
>>  Herald added a subscriber: emaste.
>> 
>> When a linker script is used with a pattern like { *(.bss .bss.*) } the InX::BssRelRo section will match against .bss.*. We want to make sure that when InX::BssRelRo hasn't created >>any .bss.rel.ro InputSections we don't report the .bss OutputSection as relro. We also want to give an error message if relro and non relro bss is placed within the same >>OutputSection.
> 
> I wonder if we need to do that much.
> 
> Any thoughts on the attached patch? It uses the idea of depending on the
> output section name for .bss.rel.ro but doesn't issue an error message
> if the user puts .bss.rel.ro in a rw section. That is similar to what
> happens with .data.rel.ro, no?

I'm happy with Rafael's patch and have incorporated it into the review. This passes the important test case, I've removed the test case for the error message as this is less likely to happen as a user would have to explicitly name a .bss.rel.ro output section and then put .bss in it.


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,37 @@
+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t.o
+// RUN: ld.lld -shared %t.o -o %t.so
+// A linker script that will map .bss.rel.ro into .bss. If we don't have any
+// .bss.rel.ro then we don't want to mark .bss as being relro.
+// RUN: echo "SECTIONS { \
+// RUN: .bss : { *(.bss) *(.bss.*) } \
+// RUN: } " > %t.script
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t2.o
+// RUN: ld.lld %t2.o %t.so -z relro -o %t --script=%t.script
+// RUN: llvm-readobj --program-headers %t | FileCheck %s
+        .section .text, "ax", @progbits
+        .global _start
+_start:
+        callq bar
+
+        .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.125305.patch
Type: text/x-patch
Size: 2553 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171204/ab30d1ed/attachment.bin>


More information about the llvm-commits mailing list