[libcxx-commits] [PATCH] D130668: [libunwind] Use `_dl_find_object` if available

[Saleem Abdulrasool via Phabricator via libcxx-commits]

compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.


================
Comment at: libunwind/src/AddressSpace.hpp:630
+        (char *)findResult.dlfo_map_end - (char *)findResult.dlfo_map_start);
+    static_assert(DLFO_STRUCT_HAS_EH_DBASE == 0, "unexpected base address");
+
----------------
This static assertion doesn't make sense to me.  You expect to define `DLFO_STRUCT_HAS_EH_DBASE` as `0` when it is available?


================
Comment at: libunwind/src/AddressSpace.hpp:634
+    // PT_GNU_EH_FRAME section. Setting length to `SIZE_MAX` effectively
+    // disables all range checks.
+    static_assert(DLFO_EH_SEGMENT_TYPE == PT_GNU_EH_FRAME,
----------------
Could you move the comment to the assignment below please.


================
Comment at: libunwind/src/AddressSpace.hpp:635
+    // disables all range checks.
+    static_assert(DLFO_EH_SEGMENT_TYPE == PT_GNU_EH_FRAME,
+                  "unexpected segment retrieved by `_dl_find_object`");
----------------
Hmm, I don't understand this assertion either.


================
Comment at: libunwind/src/AddressSpace.hpp:642
+    if (!EHHeaderParser<LocalAddressSpace>::decodeEHHdr(
+            *this, info.dwarf_index_section, SIZE_MAX, hdrInfo)) {
+      return false;
----------------
I wonder if it makes sense to use `info.dwarf_index_section_length` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130668