[libunwind] Partially revert r297174 to fix build on at least FreeBSD.

[bsdjhb]

Summary

The changes in r297174 moved the #include of <link.h> on FreeBSD (and
probably other systems) inside of the open 'libunwind' namespace
causing various system-provided types such as pid_t to be declared in
this namespace rather than the global namespace. Fix this by moving
the relevant declarations before the 'libunwind' namespace is opened,
but still using the cleaned up declarations from r297174.

[bsdjhb]

I posted a sample compile failure on FreeBSD/amd64 in the review for the earlier change: D30696

[compnerd]

src/AddressSpace.hpp

38 ↗

(On Diff #116109)

Why does this need a #ifndef __APPLE__ guard?

[bsdjhb]

src/AddressSpace.hpp

38 ↗

(On Diff #116109)

This is to mimic the same conditionals in the existing code. The current flow is '#ifdef APPLE <do stuff> #else <the code being moved> #endif' I chose to use '#ifndef APPLE' rather than having an empty '#if APPLE' body but can use whichever is preferred. I could also push the !defined(APPLE) down into the other conditionals.

[ed]

Hey! Sorry for the breakage. I thought I did a good job of testing this change on various operating systems, but somehow this still fell through the cracks. Odd.

src/AddressSpace.hpp

268 ↗

(On Diff #116109)

Considering that this block only declares an inline function, maybe it would make sense to move this to the top of the source file as well? That way we can keep the existing order of #ifs.

[compnerd]

src/AddressSpace.hpp

38 ↗

(On Diff #116109)

_LIBUNWIND_ARM_EHABI should never be defined under a Darwin build AFAIK. I would just entirely drop the __APPLE__ check.

[bsdjhb]

src/AddressSpace.hpp

38 ↗

(On Diff #116109)

Does Darwin define _LIBUNWIND_SUPPORT_DWARF_UNWIND (line 46 below)? I also wonder what you think of @ed 's suggestion of moving the function for the #ifdef APPLE case up here as well?

[compnerd]

Moving the __APPLE__ case up to the top of the file would be nice, although, it does move a compatibility implementation too. I suppose that it isn't too terrible though.

src/AddressSpace.hpp

38 ↗

(On Diff #116109)

Yeah, it does. Although, I think we could just do:

#elif defined(_LIBUNWIND_ARM_EHABI) || (!defined(__APPLE__) && defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND))

[bsdjhb]

• Move the Apple-specific _dyld_find_unwind_sections up above the namespace

libunwind/trunk/src/AddressSpace.hpp

 #endif
 
 #include "libunwind.h"
 #include "config.h"
 #include "dwarf2.h"
 #include "EHHeaderParser.hpp"
 #include "Registers.hpp"
 
+#ifdef __APPLE__
+
+  struct dyld_unwind_sections
+  {
+    const struct mach_header*   mh;
+    const void*                 dwarf_section;
+    uintptr_t                   dwarf_section_length;
+    const void*                 compact_unwind_section;
+    uintptr_t                   compact_unwind_section_length;
+  };
+  #if (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) \
+                                 && (__MAC_OS_X_VERSION_MIN_REQUIRED >= 1070)) \
+      || defined(__IPHONE_OS_VERSION_MIN_REQUIRED)
+    // In 10.7.0 or later, libSystem.dylib implements this function.
+    extern "C" bool _dyld_find_unwind_sections(void *, dyld_unwind_sections *);
+  #else
+    // In 10.6.x and earlier, we need to implement this functionality. Note
+    // that this requires a newer version of libmacho (from cctools) than is
+    // present in libSystem on 10.6.x (for getsectiondata).
+    static inline bool _dyld_find_unwind_sections(void* addr,
+                                                    dyld_unwind_sections* info) {
+      // Find mach-o image containing address.
+      Dl_info dlinfo;
+      if (!dladdr(addr, &dlinfo))
+        return false;
+#if __LP64__
+      const struct mach_header_64 *mh = (const struct mach_header_64 *)dlinfo.dli_fbase;
+#else
+      const struct mach_header *mh = (const struct mach_header *)dlinfo.dli_fbase;
+#endif
+
+      // Initialize the return struct
+      info->mh = (const struct mach_header *)mh;
+      info->dwarf_section = getsectiondata(mh, "__TEXT", "__eh_frame", &info->dwarf_section_length);
+      info->compact_unwind_section = getsectiondata(mh, "__TEXT", "__unwind_info", &info->compact_unwind_section_length);
+
+      if (!info->dwarf_section) {
+        info->dwarf_section_length = 0;
+      }
+
+      if (!info->compact_unwind_section) {
+        info->compact_unwind_section_length = 0;
+      }
+
+      return true;
+    }
+  #endif
+
+#elif defined(_LIBUNWIND_ARM_EHABI) && defined(_LIBUNWIND_IS_BAREMETAL)
+
+// When statically linked on bare-metal, the symbols for the EH table are looked
+// up without going through the dynamic loader.
+extern char __exidx_start;
+extern char __exidx_end;
+
+#elif defined(_LIBUNWIND_ARM_EHABI) || defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
+
+// ELF-based systems may use dl_iterate_phdr() to access sections
+// containing unwinding information. The ElfW() macro for pointer-size
+// independent ELF header traversal is not provided by <link.h> on some
+// systems (e.g., FreeBSD). On these systems the data structures are
+// just called Elf_XXX. Define ElfW() locally.
+#include <link.h>
+#if !defined(ElfW)
+#define ElfW(type) Elf_##type
+#endif
+
+#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 ARM EHABI.
   uintptr_t       dso_base;
LocalAddressSpace::getEncodedP(pint_t &addr, pint_t end, uint8_t encoding,
   }
 
   if (encoding & DW_EH_PE_indirect)
     result = getP(result);
 
   return result;
 }
 
-#ifdef __APPLE__
-
-  struct dyld_unwind_sections
-  {
-    const struct mach_header*   mh;
-    const void*                 dwarf_section;
-    uintptr_t                   dwarf_section_length;
-    const void*                 compact_unwind_section;
-    uintptr_t                   compact_unwind_section_length;
-  };
-  #if (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) \
-                                 && (__MAC_OS_X_VERSION_MIN_REQUIRED >= 1070)) \
-      || defined(__IPHONE_OS_VERSION_MIN_REQUIRED)
-    // In 10.7.0 or later, libSystem.dylib implements this function.
-    extern "C" bool _dyld_find_unwind_sections(void *, dyld_unwind_sections *);
-  #else
-    // In 10.6.x and earlier, we need to implement this functionality. Note
-    // that this requires a newer version of libmacho (from cctools) than is
-    // present in libSystem on 10.6.x (for getsectiondata).
-    static inline bool _dyld_find_unwind_sections(void* addr,
-                                                    dyld_unwind_sections* info) {
-      // Find mach-o image containing address.
-      Dl_info dlinfo;
-      if (!dladdr(addr, &dlinfo))
-        return false;
-#if __LP64__
-      const struct mach_header_64 *mh = (const struct mach_header_64 *)dlinfo.dli_fbase;
-#else
-      const struct mach_header *mh = (const struct mach_header *)dlinfo.dli_fbase;
-#endif
-
-      // Initialize the return struct
-      info->mh = (const struct mach_header *)mh;
-      info->dwarf_section = getsectiondata(mh, "__TEXT", "__eh_frame", &info->dwarf_section_length);
-      info->compact_unwind_section = getsectiondata(mh, "__TEXT", "__unwind_info", &info->compact_unwind_section_length);
-
-      if (!info->dwarf_section) {
-        info->dwarf_section_length = 0;
-      }
-
-      if (!info->compact_unwind_section) {
-        info->compact_unwind_section_length = 0;
-      }
-
-      return true;
-    }
-  #endif
-
-#elif defined(_LIBUNWIND_ARM_EHABI) && defined(_LIBUNWIND_IS_BAREMETAL)
-
-// When statically linked on bare-metal, the symbols for the EH table are looked
-// up without going through the dynamic loader.
-extern char __exidx_start;
-extern char __exidx_end;
-
-#elif defined(_LIBUNWIND_ARM_EHABI) || defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
-
-// ELF-based systems may use dl_iterate_phdr() to access sections
-// containing unwinding information. The ElfW() macro for pointer-size
-// independent ELF header traversal is not provided by <link.h> on some
-// systems (e.g., FreeBSD). On these systems the data structures are
-// just called Elf_XXX. Define ElfW() locally.
-#include <link.h>
-#if !defined(ElfW)
-#define ElfW(type) Elf_##type
-#endif
-
-#endif
-
 inline bool LocalAddressSpace::findUnwindSections(pint_t targetAddr,
                                                   UnwindInfoSections &info) {
 #ifdef __APPLE__
   dyld_unwind_sections dyldInfo;
   if (_dyld_find_unwind_sections((void *)targetAddr, &dyldInfo)) {
     info.dso_base                      = (uintptr_t)dyldInfo.mh;
  #if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
     info.dwarf_section                 = (uintptr_t)dyldInfo.dwarf_section;