[PATCH] D15535: Use dwarfdump's frame parser instead of writing a new one in llvm-obdjump

[Pete Cooper via llvm-commits]

pete added a comment.

Thanks for the review.  Will get you an updated version shortly.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:22
@@ -21,3 +21,3 @@
 
-/// \brief A parsed .debug_frame section
+  /// \brief A parsed .debug_frame or .eh_frame section
 ///
----------------
rafael wrote:
> The whitespace change looks ood.
Good catch.  It was automatically indented.  Will fix.

================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:371
@@ +370,3 @@
+
+  // There's a "bug" in the DWARFv3 standard with respect to the target address
+  // size within debug frame sections. While DWARF is supposed to be independent
----------------
rafael wrote:
> This comment is a copy and paste and is probably out of place for .eh_frame. It is based on debug_frame.
Ah yeah, i'll remove the bad comment.

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:542
@@ +541,3 @@
+      StringRef AugmentationString(Augmentation ? Augmentation : "");
+      uint64_t EHData = 0;
+      if (AugmentationString.count("eh"))
----------------
rafael wrote:
> Looks like EHData is never used.
> Also, where is "eh" from? I don't see it in
> 
> https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
I'd been using this site.  But yeah, no idea where 'eh' comes from.  We don't generate it in LLVM so i'll remove it from this code.

https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-PDA/LSB-PDA/ehframechpt.html

================
Comment at: test/tools/llvm-objdump/eh_frame-arm64.test:1
@@ -1,2 +1,2 @@
 # RUN: llvm-objdump -unwind-info %p/Inputs/eh_frame.macho-arm64 2>/dev/null | FileCheck %s
 
----------------
rafael wrote:
> Can you update the test to use the new command line option and delete the old one?
-unwind-info is still used for other stuff.  But I will remove .eh_frame dumping from that method and make this test use -debug=frames.

If we want to remove -unwind-info completely then i'm happy to, but will do in a follow up patch.


Repository:
  rL LLVM

http://reviews.llvm.org/D15535