[libunwind] Combine dl_iterate_phdr codepaths for DWARF and EHABI

[rprichard]

Summary

dl_iterate_phdr is used to search for unwind info provided by either
PT_GNU_EH_FRAME or PT_ARM_EXIDX. Most of the code between the two is
the same, so combine them, and factor out what's different into
checkForUnwindInfoSegment.

Details:

• The FrameHeaderCache can now be enabled for ARM EHABI.
• findUnwindSectionsByPhdr now finds the last PT_ARM_EXIDX rather than the first. There should only be one segment.
• The dso_base and text_segment_length fields of UnwindInfoSections are now needed for dl_iterate_phdr when using EHABI, to hold the low and high PC values for a cache entry.

[rprichard]

Apply clang-format fix.

[rprichard]

This change enables the FrameHeaderCache for EHABI.

[danielkiss]

LGTM, refactor looks correct. in case of _LIBUNWIND_ARM_EHABI the director of the iteration over the phdrs changed but that is fine.

[rprichard]

Thanks for the review. It looks like I still need someone from the libunwind group to accept it, though.

libunwind/src/AddressSpace.hpp

 #include <link.h>
 
 #endif
 
 namespace libunwind {
 
 /// Used by findUnwindSections() to return info about needed sections.
 struct UnwindInfoSections {
-#if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) || defined(_LIBUNWIND_SUPPORT_DWARF_INDEX) ||       \
-    defined(_LIBUNWIND_SUPPORT_COMPACT_UNWIND)
-  // No dso_base for SEH or ARM EHABI.
+#if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) ||                                \
+    defined(_LIBUNWIND_SUPPORT_COMPACT_UNWIND) ||                              \
+    defined(_LIBUNWIND_USE_DL_ITERATE_PHDR)
+  // No dso_base for SEH.
   uintptr_t       dso_base;
 #endif
-#if defined(_LIBUNWIND_USE_DL_ITERATE_PHDR) &&                                 \
-    defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)
+#if defined(_LIBUNWIND_USE_DL_ITERATE_PHDR)
   uintptr_t       text_segment_length;
 #endif
 #if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
   uintptr_t       dwarf_section;
   uintptr_t       dwarf_section_length;
 #endif
 #if defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)
   uintptr_t       dwarf_index_section;

 }
 
 struct _LIBUNWIND_HIDDEN dl_iterate_cb_data {
   LocalAddressSpace *addressSpace;
   UnwindInfoSections *sects;
   uintptr_t targetAddr;
 };
 
-#if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
-  #if !defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)
-    #error "_LIBUNWIND_SUPPORT_DWARF_UNWIND requires _LIBUNWIND_SUPPORT_DWARF_INDEX on this platform."
-  #endif
-
 #if defined(_LIBUNWIND_USE_FRAME_HEADER_CACHE)
 #include "FrameHeaderCache.hpp"
 
 // Typically there is one cache per process, but when libunwind is built as a
 // hermetic static library, then each shared object may have its own cache.
 static FrameHeaderCache TheFrameHeaderCache;
 #endif
 
 static bool checkAddrInSegment(const Elf_Phdr *phdr, size_t image_base,
                                dl_iterate_cb_data *cbdata) {
   if (phdr->p_type == PT_LOAD) {
     uintptr_t begin = image_base + phdr->p_vaddr;
     uintptr_t end = begin + phdr->p_memsz;
     if (cbdata->targetAddr >= begin && cbdata->targetAddr < end) {
       cbdata->sects->dso_base = begin;
       cbdata->sects->text_segment_length = phdr->p_memsz;
       return true;
     }
   }
   return false;
 }
 
+static bool checkForUnwindInfoSegment(const Elf_Phdr *phdr, size_t image_base,
+                                      dl_iterate_cb_data *cbdata) {
+#if defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)
+  if (phdr->p_type == PT_GNU_EH_FRAME) {
+    EHHeaderParser<LocalAddressSpace>::EHHeaderInfo hdrInfo;
+    uintptr_t eh_frame_hdr_start = image_base + phdr->p_vaddr;
+    cbdata->sects->dwarf_index_section = eh_frame_hdr_start;
+    cbdata->sects->dwarf_index_section_length = phdr->p_memsz;
+    if (EHHeaderParser<LocalAddressSpace>::decodeEHHdr(
+            *cbdata->addressSpace, eh_frame_hdr_start, phdr->p_memsz,
+            hdrInfo)) {
+      // .eh_frame_hdr records the start of .eh_frame, but not its size.
+      // Rely on a zero terminator to find the end of the section.
+      cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
+      cbdata->sects->dwarf_section_length = UINTPTR_MAX;
+      return true;
+    }
+  }
+  return false;
+#elif defined(_LIBUNWIND_ARM_EHABI)
+  if (phdr->p_type == PT_ARM_EXIDX) {
+    uintptr_t exidx_start = image_base + phdr->p_vaddr;
+    cbdata->sects->arm_section = exidx_start;
+    cbdata->sects->arm_section_length = phdr->p_memsz;
+    return true;
+  }
+  return false;
+#else
+#error Need one of _LIBUNWIND_SUPPORT_DWARF_INDEX or _LIBUNWIND_ARM_EHABI
+#endif
+}
+
 static int findUnwindSectionsByPhdr(struct dl_phdr_info *pinfo,
                                     size_t pinfo_size, void *data) {
   auto cbdata = static_cast<dl_iterate_cb_data *>(data);
   if (pinfo->dlpi_phnum == 0 || cbdata->targetAddr < pinfo->dlpi_addr)
     return 0;
 #if defined(_LIBUNWIND_USE_FRAME_HEADER_CACHE)
   if (TheFrameHeaderCache.find(pinfo, pinfo_size, data))
     return 1;
 #else
   // Avoid warning about unused variable.
   (void)pinfo_size;
 #endif
 
   Elf_Addr image_base = calculateImageBase(pinfo);
-  bool found_obj = false;
-  bool found_hdr = false;
+  bool found_text = false;
+  bool found_unwind = false;
 
   // Third phdr is usually the executable phdr.
   if (pinfo->dlpi_phnum > 2)
-    found_obj = checkAddrInSegment(&pinfo->dlpi_phdr[2], image_base, cbdata);
+    found_text = checkAddrInSegment(&pinfo->dlpi_phdr[2], image_base, cbdata);
 
-  // PT_GNU_EH_FRAME is usually near the end. Iterate backward. We already know
-  // that there is one or more phdrs.
+  // PT_GNU_EH_FRAME and PT_ARM_EXIDX are usually near the end. Iterate
+  // backward. We already know that there is one or more phdrs.
   for (Elf_Half i = pinfo->dlpi_phnum; i > 0; i--) {
     const Elf_Phdr *phdr = &pinfo->dlpi_phdr[i - 1];
-    if (!found_hdr && phdr->p_type == PT_GNU_EH_FRAME) {
-      EHHeaderParser<LocalAddressSpace>::EHHeaderInfo hdrInfo;
-      uintptr_t eh_frame_hdr_start = image_base + phdr->p_vaddr;
-      cbdata->sects->dwarf_index_section = eh_frame_hdr_start;
-      cbdata->sects->dwarf_index_section_length = phdr->p_memsz;
-      found_hdr = EHHeaderParser<LocalAddressSpace>::decodeEHHdr(
-          *cbdata->addressSpace, eh_frame_hdr_start, phdr->p_memsz,
-          hdrInfo);
-      if (found_hdr) {
-        // .eh_frame_hdr records the start of .eh_frame, but not its size.
-        // Rely on a zero terminator to find the end of the section.
-        cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
-        cbdata->sects->dwarf_section_length = UINTPTR_MAX;
-      }
-    } else if (!found_obj) {
-      found_obj = checkAddrInSegment(phdr, image_base, cbdata);
-    }
-    if (found_obj && found_hdr) {
+    if (!found_unwind && checkForUnwindInfoSegment(phdr, image_base, cbdata))
+      found_unwind = true;
+    else if (!found_text && checkAddrInSegment(phdr, image_base, cbdata))
+      found_text = true;
+    if (found_text && found_unwind) {
 #if defined(_LIBUNWIND_USE_FRAME_HEADER_CACHE)
       TheFrameHeaderCache.add(cbdata->sects);
 #endif
       return 1;
     }
   }
   return 0;
 }
 
-#elif defined(_LIBUNWIND_ARM_EHABI)
-
-static int findUnwindSectionsByPhdr(struct dl_phdr_info *pinfo, size_t,
-                                    void *data) {
-  auto *cbdata = static_cast<dl_iterate_cb_data *>(data);
-  bool found_obj = false;
-  bool found_hdr = false;
-
-  assert(cbdata);
-  assert(cbdata->sects);
-
-  if (cbdata->targetAddr < pinfo->dlpi_addr)
-    return 0;
-
-  Elf_Addr image_base = calculateImageBase(pinfo);
-
-  for (Elf_Half i = 0; i < pinfo->dlpi_phnum; i++) {
-    const Elf_Phdr *phdr = &pinfo->dlpi_phdr[i];
-    if (phdr->p_type == PT_LOAD) {
-      uintptr_t begin = image_base + phdr->p_vaddr;
-      uintptr_t end = begin + phdr->p_memsz;
-      if (cbdata->targetAddr >= begin && cbdata->targetAddr < end)
-        found_obj = true;
-    } else if (phdr->p_type == PT_ARM_EXIDX) {
-      uintptr_t exidx_start = image_base + phdr->p_vaddr;
-      cbdata->sects->arm_section = exidx_start;
-      cbdata->sects->arm_section_length = phdr->p_memsz;
-      found_hdr = true;
-    }
-  }
-  return found_obj && found_hdr;
-}
-
-#endif
 #endif  // defined(_LIBUNWIND_USE_DL_ITERATE_PHDR)
 
 
 inline bool LocalAddressSpace::findUnwindSections(pint_t targetAddr,
                                                   UnwindInfoSections &info) {
 #ifdef __APPLE__
   dyld_unwind_sections dyldInfo;
   if (_dyld_find_unwind_sections((void *)targetAddr, &dyldInfo)) {

libunwind/test/frameheadercache_test.pass.cpp

 // The other libunwind tests don't test internal interfaces, so the include path
 // is a little wonky.
 #include "../src/config.h"
 
 // Only run this test under supported configurations.
 
 #if defined(_LIBUNWIND_USE_DL_ITERATE_PHDR) &&                                 \
-    defined(_LIBUNWIND_SUPPORT_DWARF_INDEX) &&                                 \
     defined(_LIBUNWIND_USE_FRAME_HEADER_CACHE)
 
 #include <link.h>
 #include <stdio.h>
 
 // This file defines several of the data structures needed here,
 // and includes FrameHeaderCache.hpp as well.
 #include "../src/AddressSpace.hpp"