[PATCH] D130668: [libunwind] Use `_dl_find_object` if available

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: libunwind/src/AddressSpace.hpp:640
+
+    // `_dl_find_object` gives us the start of the PT_GNU_EH_FRAME section.
+    info.dwarf_index_section =
----------------
No need to replicate the member variable name. Just say "Record the start of PT_GNU_EH_FRAME"

PT_GNU_EH_FRAME is not a section.


================
Comment at: libunwind/src/AddressSpace.hpp:653
+    }
+    // .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.
----------------
avogelsgesang wrote:
> MaskRay wrote:
> > > .eh_frame_hdr records the start of .eh_frame
> > 
> > This sentence is wrong. .eh_frame_hdr is a different section.
> `PT_GNU_EH_FRAME` is a bit of a misnormer as it refers to `eh_frame_hdr`, not `eh_frame`. From https://refspecs.linuxbase.org/LSB_3.0.0/LSB-PDA/LSB-PDA/progheader.html
> 
> > PT_GNU_EH_FRAME:
> > The array element specifies the location and size of the exception handling information as defined by the .eh_frame_hdr section.
> 
> The `eh_frame_hdr` is hence the section retrieved by `_dl_find_object`. `EHHeaderParser<LocalAddressSpace>::decodeEHHdr` a couple of lines above decodes the `eh_frame_hdr`.  Parsing the`eh_frame_hdr` gives us the pointer to the start of `eh_frame`, but doesn't give us its size. Hence, we set `dwarf_section_length` to `SIZE_MAX` and thereby rely on the zero terminator to find the end of the section.
> 
> Please let me know how to update this comment to make it more understandable.
> 
> Also, I shamelessly copied this comment from the `checkForUnwindInfoSegment` function. In case you want me to update also that comment, please let me know
I know PT_GNU_EH_FRAME. The comment just doesn't make sense to me. I suggest something aligning:

Record the start of a FDE and use SIZE_MAX to indicate that we do not know the end address.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130668