[ELF] Handle multiple discontiguous .note sections

[phosek]

Summary

There could be multiple discontiguous output .note sections in which case we need to put these into separate PT_NOTE segments rather then placing them into a single segment. Where possible, we could reorder the input sections to make sure that all .note are layed out next to each other to avoid creation multiple PT_NOTE segments, but even in that case, it's still possible to construct a discontiguous case e.g. by using a linker script.

[ruiu]

ELF/Writer.cpp

1236–1240

Can you move this code out of this for loop and create a new loop only for .notes? I think doing that improves readability.

[grimar]

I think this also needs testcase which shows that next case still creates single PT_NOTE segment:

.note : { *(.note.a) *(.note.b) }

[ruiu]

LGTM

Please add a comment saying that that code creates one PT_NOTE per a group of contiguous .note sections.

ELF/Writer.cpp

template <class ELFT> std::vector<PhdrEntry> Writer<ELFT>::createPhdrs() {
   }
 
   // Add the first PT_LOAD segment for regular output sections.
   uintX_t Flags = computeFlags<ELFT>(PF_R);
   PhdrEntry *Load = AddHdr(PT_LOAD, Flags);
 
   PhdrEntry TlsHdr(PT_TLS, PF_R);
   PhdrEntry RelRo(PT_GNU_RELRO, PF_R);
-  PhdrEntry Note(PT_NOTE, PF_R);
+  std::vector<PhdrEntry> Notes;
   for (OutputSectionBase *Sec : OutputSections) {
     if (!(Sec->Flags & SHF_ALLOC))
       break;
 
     // If we meet TLS section then we create TLS header
     // and put all TLS sections inside for further use when
     // assign addresses.
     if (Sec->Flags & SHF_TLS)
       TlsHdr.add(Sec);
 
     if (!needsPtLoad<ELFT>(Sec))
       continue;
 
     // Segments are contiguous memory regions that has the same attributes
     // (e.g. executable or writable). There is one phdr for each segment.
     // Therefore, we need to create a new phdr when the next section has
     // different flags or is loaded at a discontiguous address using AT linker
     // script command.
     uintX_t NewFlags = computeFlags<ELFT>(Sec->getPhdrFlags());
     if (Script<ELFT>::X->hasLMA(Sec->getName()) || Flags != NewFlags) {
       Load = AddHdr(PT_LOAD, NewFlags);
       Flags = NewFlags;
     }
 
+    if (Sec->Type == SHT_NOTE) {
+      if (!Load->Last || Load->Last->Type != SHT_NOTE)
+        Notes.emplace_back(PT_NOTE, PF_R);
+      Notes.back().add(Sec);
+    }
+
     Load->add(Sec);
 
     if (isRelroSection<ELFT>(Sec))
       RelRo.add(Sec);
-    if (Sec->Type == SHT_NOTE)
-      Note.add(Sec);
   }
 
   // Add the TLS segment unless it's empty.
   if (TlsHdr.First)
     Ret.push_back(std::move(TlsHdr));
 
   // Add an entry for .dynamic.
   if (In<ELFT>::DynSymTab) {
template <class ELFT> std::vector<PhdrEntry> Writer<ELFT>::createPhdrs() {
 
   // PT_OPENBSD_WXNEEDED is a OpenBSD-specific header to mark the executable
   // is expected to perform W^X violations, such as calling mprotect(2) or
   // mmap(2) with PROT_WRITE | PROT_EXEC, which is prohibited by default on
   // OpenBSD.
   if (Config->ZWxneeded)
     AddHdr(PT_OPENBSD_WXNEEDED, PF_X);
 
-  if (Note.First)
-    Ret.push_back(std::move(Note));
+  Ret.insert(Ret.end(), Notes.begin(), Notes.end());
   return Ret;
 }
 
 template <class ELFT>
 void Writer<ELFT>::addPtArmExid(std::vector<PhdrEntry> &Phdrs) {
   if (Config->EMachine != EM_ARM)
     return;
   auto I = std::find_if(

test/ELF/note-loadaddr.c

+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+// RUN: echo "SECTIONS { \
+// RUN:  .note.a : AT(0x1000) { *(.note.a) } \
+// RUN:  .note.b : AT(0x2000) { *(.note.b) } \
+// RUN: }" > %t.script
+// RUN: ld.lld %t.o --script %t.script -o %t
+// RUN: llvm-readobj -program-headers %t | FileCheck %s
+
+// CHECK:      Type: PT_NOTE
+// CHECK-NEXT: Offset: 0x1000
+// CHECK-NEXT: VirtualAddress: 0x0
+// CHECK-NEXT: PhysicalAddress: 0x1000
+// CHECK-NEXT: FileSize: 8
+// CHECK-NEXT: MemSize: 8
+// CHECK-NEXT:   Flags [
+// CHECK-NEXT:   PF_R
+// CHECK-NEXT: ]
+// CHECK-NEXT: Alignment: 1
+// CHECK:      Type: PT_NOTE
+// CHECK-NEXT: Offset: 0x1008
+// CHECK-NEXT: VirtualAddress: 0x8
+// CHECK-NEXT: PhysicalAddress: 0x2000
+// CHECK-NEXT: FileSize: 8
+// CHECK-NEXT: MemSize: 8
+// CHECK-NEXT:   Flags [
+// CHECK-NEXT:   PF_R
+// CHECK-NEXT: ]
+// CHECK-NEXT: Alignment: 1
+
+.section .note.a, "a", @note
+.quad 0
+
+.section .note.b, "a", @note
+.quad 0

test/ELF/note-multiple.s

+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+// RUN: echo "SECTIONS { \
+// RUN:  .note.a : { *(.note.a) } \
+// RUN:  .b : { *(.b) } \
+// RUN:  .c : { *(.c) } \
+// RUN:  .note.d : { *(.note.d) } \
+// RUN: }" > %t.script
+// RUN: ld.lld %t.o --script %t.script -o %t
+// RUN: llvm-readobj -program-headers %t | FileCheck %s
+
+// CHECK:      Type: PT_NOTE
+// CHECK-NEXT: Offset: 0x1000
+// CHECK-NEXT: VirtualAddress: 0x0
+// CHECK-NEXT: PhysicalAddress: 0x0
+// CHECK-NEXT: FileSize: 8
+// CHECK-NEXT: MemSize: 8
+// CHECK-NEXT:   Flags [
+// CHECK-NEXT:   PF_R
+// CHECK-NEXT: ]
+// CHECK-NEXT: Alignment: 1
+// CHECK:      Type: PT_NOTE
+// CHECK-NEXT: Offset: 0x1018
+// CHECK-NEXT: VirtualAddress: 0x18
+// CHECK-NEXT: PhysicalAddress: 0x18
+// CHECK-NEXT: FileSize: 8
+// CHECK-NEXT: MemSize: 8
+// CHECK-NEXT:   Flags [
+// CHECK-NEXT:   PF_R
+// CHECK-NEXT: ]
+// CHECK-NEXT: Alignment: 1
+
+.section .note.a, "a", @note
+.quad 0
+
+.section .b, "a"
+.quad 0
+
+.section .c, "a"
+.quad 0
+
+.section .note.d, "a", @note
+.quad 0