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

[Rafael Ávila de Espíndola via llvm-commits]

rafael added a comment.

General direction looks good.  Just have some comments.


================
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
 ///
----------------
The whitespace change looks ood.

================
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
----------------
This comment is a copy and paste and is probably out of place for .eh_frame. It is based on debug_frame.

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:542
@@ +541,3 @@
+      StringRef AugmentationString(Augmentation ? Augmentation : "");
+      uint64_t EHData = 0;
+      if (AugmentationString.count("eh"))
----------------
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

================
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
 
----------------
Can you update the test to use the new command line option and delete the old one?


Repository:
  rL LLVM

http://reviews.llvm.org/D15535