[PATCH] D13275: [lld][elf2] Sort output sections.

[Rafael Ávila de Espíndola via llvm-commits]

rafael added inline comments.

================
Comment at: ELF/Writer.cpp:399
@@ +398,3 @@
+
+        auto rank = [](OutputSectionBase<ELFT::Is64Bits> *Sec) {
+          switch (Sec->getFlags() & (SHF_ALLOC | SHF_EXECINSTR | SHF_WRITE)) {
----------------
No auto.
Rank


================
Comment at: ELF/Writer.cpp:401
@@ +400,3 @@
+          switch (Sec->getFlags() & (SHF_ALLOC | SHF_EXECINSTR | SHF_WRITE)) {
+          case SHF_ALLOC:
+            return 0;
----------------
The rules that we must have are that

alloc is before non-alloc
read-only is the first (to combine with the first page)

The rest of the order is arbitrary, correct? Add a comment about it one way or the other.


================
Comment at: ELF/Writer.cpp:412
@@ +411,3 @@
+          default:
+            error("Unexpected section flags");
+          }
----------------
There are 8 possible values.

The ones that are missing are 

SHF_EXECINSTR:
SHF_WRITE:
SHF_EXECINSTR | SHF_WRITE:

It might be more explicit to just list them all.

================
Comment at: ELF/Writer.cpp:416
@@ +415,3 @@
+
+        // Ignore SHT_NOBITS for non-allocated sections.
+        if (AFlags && AFlags == BFlags)
----------------
Why is this relevant?

We need to put SHF_ALLOC first to reduce how much needs to be allocated and make sure debug info doesn't change actual code.

Other than that convertSectionFlagsToPHDRFlags (and therefore the creation of new PT_LOADs) only looks at SHF_WRITE and SHF_EXECINSTR, so looks like your rank function takes care of all cases.



http://reviews.llvm.org/D13275