[ELF] Properly handle .eh_frame in linker scripts

[phosek]

Summary

Using .eh_frame input section pattern in linker script currently causes a crash; this is because .eh_frame input sections require special handling since they're all combined into a synthetic section rather than regular output section.

[ruiu]

SHF_MERGE sections are also handled in a special way because they are aggregated one synthetic section (which is done in combineMergableSections in Writer.cpp). Maybe we should do the same thing for .eh_frame? At least they need to be consistent.

[phosek]

I tried to use the same handling as in case of combineMergableSections, let me know if this makes sense.

[ruiu]

ELF/SyntheticSections.cpp

507–508 ↗

(On Diff #90783)

What is this for?

ELF/Writer.cpp

227 ↗

(On Diff #90783)

EHFrame -> EhFrame since we have EhFrameSection.

935–938 ↗

(On Diff #90783)

What is this?

[phosek]

ELF/SyntheticSections.cpp

507–508 ↗

(On Diff #90783)

.eh_frame can have relocations, so we need to ensure that if the EhFrameSection gets GC'ed, .rel.eh_frame/.rela.eh_frame gets GC'ed as well.

ELF/Writer.cpp

935–938 ↗

(On Diff #90783)

To process .eh_frame relocations.

[ruiu]

ELF/SyntheticSections.cpp

507–508 ↗

(On Diff #90783)

But when the control reached Writer::run function, all garbage has already been gc'ed, no?

[phosek]

ELF/SyntheticSections.cpp

507–508 ↗

(On Diff #90783)

Not necessarily, for example you can have /DISCARD/ : { *(.eh_frame) } in the linker script which only gets processed in LinkerScript::processCommands.

[ruiu]

LGTM

ELF/SyntheticSections.cpp

507–508 ↗

(On Diff #90783)

Ah, that's right.

Somewhat orthogonal, but can you avoid using std::move and just copy? I don't think we need memory here.

[evgeny777]

This patch causes crash, when creating PT_GNU_EH_FRAME segment in Writer<ELFT>::createPhdrs (OutSec is nullptr) in case .eh_frame section is discarded in linker script.

ELF/LinkerScript.cpp

for (InputSectionBase *IB : ICmd.Sections) {
     // We tentatively added all synthetic sections at the beginning and removed
     // empty ones afterwards (because there is no way to know whether they were
     // going be empty or not other than actually running linker scripts.)
     // We need to ignore remains of empty sections.
     if (auto *Sec = dyn_cast<SyntheticSection>(IB))
       if (Sec->empty())
         continue;
 
+    // We skip all eh_frame input sections since those are part of and handled
+    // by the EhFrameSection synthetic section.
+    if (IB->kind() == InputSectionBase::EHFrame)
+      continue;
+
     if (!IB->Live)
       continue;
     assert(CurOutSec == IB->OutSec || AlreadyOutputOS.count(IB->OutSec));
     output(cast<InputSection>(IB));
   }
 }
 
 template <class ELFT>

ELF/OutputSections.cpp

 // single progbits section. This is typically done by linker
 // scripts. Merging nobits and progbits will force disk space
 // to be allocated for nobits sections. Other ones don't require
 // any special treatment on top of progbits, so there doesn't
 // seem to be a harm in merging them.
 static bool canMergeToProgbits(unsigned Type) {
   return Type == SHT_NOBITS || Type == SHT_PROGBITS || Type == SHT_INIT_ARRAY ||
          Type == SHT_PREINIT_ARRAY || Type == SHT_FINI_ARRAY ||
-         Type == SHT_NOTE;
+         Type == SHT_NOTE || Type == SHT_X86_64_UNWIND;
 }
 
 template <class ELFT> static void reportDiscarded(InputSectionBase *IS) {
   if (!Config->PrintGcSections)
     return;
   message("removing unused section from '" + IS->Name + "' in file '" +
           IS->getFile<ELFT>()->getName());
 }
 
 template <class ELFT>
 void OutputSectionFactory::addInputSec(InputSectionBase *IS,
                                        StringRef OutsecName) {
   if (!IS->Live) {
     reportDiscarded<ELFT>(IS);
     return;
   }
+  if (IS->kind() == InputSectionBase::EHFrame) {
+    In<ELFT>::EhFrame->addSection(IS);
+    return;
+  }
 
   SectionKey Key = createKey<ELFT>(IS, OutsecName);
   uint64_t Flags = getOutFlags<ELFT>(IS);
   OutputSection *&Sec = Map[Key];
   if (Sec) {
     if (getIncompatibleFlags(Sec->Flags) != getIncompatibleFlags(IS->Flags))
       error("Section has flags incompatible with others with the same name " +
             toString(IS));
     if (Sec->Type != IS->Type) {
       if (canMergeToProgbits(Sec->Type) && canMergeToProgbits(IS->Type))
         Sec->Type = SHT_PROGBITS;
       else
         error("Section has different type from others with the same name " +
               toString(IS));
     }
     Sec->Flags |= Flags;
   } else {
-    uint32_t Type = IS->Type;
-    if (IS->kind() == InputSectionBase::EHFrame) {
-      In<ELFT>::EhFrame->addSection(IS);
-      return;
-    }
-    Sec = make<OutputSection>(Key.Name, Type, Flags);
+    Sec = make<OutputSection>(Key.Name, IS->Type, Flags);
     OutputSections.push_back(Sec);
   }
 
   Sec->addSection(IS);
 }
 
 OutputSectionFactory::~OutputSectionFactory() {}
 

test/ELF/linkerscript/eh-frame.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "SECTIONS { \
+# RUN:          .eh_frame : { *(.eh_frame) } \
+# RUN:       }" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN: llvm-objdump -s -section=".eh_frame" %t1 | FileCheck %s
+
+# CHECK: 0000 14000000 00000000 017a5200 01781001
+# CHECK-NEXT: 0010 1b0c0708 90010000
+
+.global _start
+_start:
+ nop
+
+.section .dah,"ax",@progbits
+.cfi_startproc
+ nop
+.cfi_endproc