ELF: Emit .eh_frame_hdr entries for FDEs with duplicate PCs.

[pcc]

Summary

It is possible to have FDEs with duplicate PCs if ICF was able to merge
functions with FDEs, or if the input files for some reason contained duplicate
FDEs. We previously weren't handling this correctly when producing the
contents of the .eh_frame_hdr section; we were dropping entries and leaving
null entries at the end of the section, which confused consumers of unwind
data, such as the backtrace() function.

Fix the bug by collecting the FDEs into a std::multimap rather than a std::map.

[• rafael]

How hard would it be to discard that part of the .eh_frame? Or at least not
include it in the index (reduce fde_count).

If I understand it correctly with this patch we create an empty range in
the binary search table, which is a bit odd.

[pcc]

It seems feasible, I'll see if I can do it. In the meantime, this fixes the issue with backtrace(), and also matches gold's behavior, so maybe we can land this as a first step with a FIXME.

I was also thinking about cases where the unwind info for functions with identical bodies was semantically different. For example, I suppose you might be able to have functions with identical bodies but different personality functions. If that's possible, I suppose we may need to inhibit ICF in such cases and do something completely different here.

[pcc]

Set FDE count in .eh_frame_hdr correctly instead

[• rafael]

Lgtm.

lld/trunk/ELF/OutputSections.cpp

EhFrameHeader<ELFT>::getFdePc(uintX_t EhVA, const FdeData &F) {
   default:
     fatal("unknown FDE size relative encoding");
   }
 }
 
 template <class ELFT> void EhFrameHeader<ELFT>::writeTo(uint8_t *Buf) {
   const endianness E = ELFT::TargetEndianness;
 
+  uintX_t EhVA = Sec->getVA();
+  uintX_t VA = this->getVA();
+
+  // InitialPC -> Offset in .eh_frame, sorted by InitialPC, and deduplicate PCs.
+  // FIXME: Deduplication leaves unneeded null bytes at the end of the section.
+  std::map<uintX_t, size_t> PcToOffset;
+  for (const FdeData &F : FdeList)
+    PcToOffset[getFdePc(EhVA, F)] = F.Off;
+
   const uint8_t Header[] = {1, DW_EH_PE_pcrel | DW_EH_PE_sdata4,
                             DW_EH_PE_udata4,
                             DW_EH_PE_datarel | DW_EH_PE_sdata4};
   memcpy(Buf, Header, sizeof(Header));
 
-  uintX_t EhVA = Sec->getVA();
-  uintX_t VA = this->getVA();
   uintX_t EhOff = EhVA - VA - 4;
   write32<E>(Buf + 4, EhOff);
-  write32<E>(Buf + 8, this->FdeList.size());
+  write32<E>(Buf + 8, PcToOffset.size());
   Buf += 12;
 
-  // InitialPC -> Offset in .eh_frame, sorted by InitialPC.
-  std::map<uintX_t, size_t> PcToOffset;
-  for (const FdeData &F : FdeList)
-    PcToOffset[getFdePc(EhVA, F)] = F.Off;
-
   for (auto &I : PcToOffset) {
     // The first four bytes are an offset to the initial PC value for the FDE.
     write32<E>(Buf, I.first - VA);
     // The last four bytes are an offset to the FDE data itself.
     write32<E>(Buf + 4, EhVA + I.second - VA);
     Buf += 8;
   }
 }

lld/trunk/test/ELF/eh-frame-hdr-icf.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: ld.lld %t -o %t2 --icf=all --eh-frame-hdr
+# RUN: llvm-objdump -s %t2 | FileCheck %s
+
+# CHECK: Contents of section .eh_frame_hdr:
+# CHECK-NEXT: 101a0 011b033b b4ffffff 01000000 600e0000
+#                                     ^ FDE count
+# CHECK-NEXT: 101b0 e8ffffff 00000000 00000000
+#                   ^ FDE for f2
+
+.globl _start, f1, f2
+_start:
+  ret
+
+.section .text.f1, "ax"
+f1:
+  .cfi_startproc
+  ret
+  .cfi_endproc
+
+.section .text.f2, "ax"
+f2:
+  .cfi_startproc
+  ret
+  .cfi_endproc