[libcxx-commits] [PATCH] D75480: Promote nameless lambda used by dl_iterate_phdr to named function to clean up control flow inside findUnwindSections. Also, expose the data structureto allow use by a future replacment function.

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

This broke building for windows, and I would be pretty sure it also broke things for apple platforms as well (as `_LIBUNWIND_SUPPORT_DWARF_UNWIND` is defined on both of them, at least on certain architectures).

There's a long chain of ifdefs in `LocalAddressSpace::findUnwindSections`, like this:

  #ifdef __APPLE__
  #elif defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) && defined(_LIBUNWIND_IS_BAREMETAL)
  #elif defined(_LIBUNWIND_ARM_EHABI) && defined(_LIBUNWIND_IS_BAREMETAL)
  #elif defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) && defined(_WIN32)
  #elif defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
  #elif defined(_LIBUNWIND_ARM_EHABI) && defined(__BIONIC__)
  #elif defined(_LIBUNWIND_ARM_EHABI) || defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
  // Current ELF code being refactored
  #endif

Now after this change, you've placed a separate `#if defined(_LIBUNWIND_ARM_EHABI) || defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)` standalone above all of it - while `_LIBUNWIND_SUPPORT_DWARF_UNWIND` is defined in a lot of other cases as well (both on apple targets and for some architectures on win32), that are handled by other more specialised OS specific ifdefs in `LocalAddressSpace::findUnwindSections`.

Not sure what the cleanest solution would be though. The initial thought would be to add `&& !defined(__APPLE__) && !defined(_WIN32) && !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(__BIONIC__)` to the ifdef surrounding `findUnwindSectionByPhdr`, but that doesn't quite exactly replicate the effect of the long series of ifdefs either. The safest (but less neat) solution would be to add the exact same series of ifdefs around `findUnwindSectionByPhdr`, with empty ifdef bodies for all the other cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75480/new/

https://reviews.llvm.org/D75480