[PATCH] D32399: [LLD] Order writable executable sections before writable ones

[Mark Kettenis via Phabricator via llvm-commits]

kettenis created this revision.
Herald added subscribers: tpr, jyknight.

I'm working on SPARC support in lld.  I've got a working diff against lld 4.0.0, and I'm working on moving the changes over into the current source tree. Here is a first diff that addresses one of the more fundamental issues I ran into.

On SPARC, .plt is both writeable and executable. The current way sections are sorted means that lld puts it after .data/.bss. but it really needs to be close to .test to make sure branches into .plt don't overflow. I'd argue that because .bss is supposed to come last on all architectures, we should change the default sort order such that writable and executable sections come before sections that are just writeable. read-only executable sections should still come after sections that are just read-only of course. This diff makes this change.

With this diff, the following tests fail:

>   lld :: ELF/amdgpu-globals.s
>   lld :: ELF/gc-sections.s
>   lld :: ELF/i386-tls-ie-shared.s
>   lld :: ELF/relocation-size-shared.s
>   lld :: ELF/relocation-size.s
>   lld :: ELF/section-layout.s
>   lld :: ELF/tls-i686.s

The ELF/section-layout.s test fails because it explicitly tests that writeable and executable sections come after sections that are just writeable. Once adjusted that tests will act as a regression test for this diff.
I think most of the other tests just accidentally include a executable and writeable section. I'm not sure about amdgpu-globals.s though as I'm not really familliar with the requirements of that architecture.


https://reviews.llvm.org/D32399

Files:
  lld/ELF/Writer.cpp


Index: lld/ELF/Writer.cpp
===================================================================
--- lld/ELF/Writer.cpp
+++ lld/ELF/Writer.cpp
@@ -687,15 +687,15 @@
     return BIsWritable;
 
   if (!Config->SingleRoRx) {
-    // For a corresponding reason, put non exec sections first (the program
-    // header PT_LOAD is not executable).
+    // For a corresponding reason, put read-only non-exec sections
+    // first (the program header PT_LOAD is not executable).
     // We only do that if we are not using linker scripts, since with linker
     // scripts ro and rx sections are in the same PT_LOAD, so their relative
     // order is not important. The same applies for -no-rosegment.
     bool AIsExec = A->Flags & SHF_EXECINSTR;
     bool BIsExec = B->Flags & SHF_EXECINSTR;
     if (AIsExec != BIsExec)
-      return BIsExec;
+      return (A->Flags & SHF_WRITE) ? AIsExec : BIsExec;
   }
 
   // If we got here we know that both A and B are in the same PT_LOAD.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32399.96307.patch
Type: text/x-patch
Size: 974 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170423/4dfc894e/attachment.bin>