[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 inline comments.

================
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:51
@@ -50,2 +50,3 @@
   std::unique_ptr<DWARFDebugFrame> DebugFrame;
+  std::unique_ptr<DWARFDebugFrame> DebugEHFrame;
   std::unique_ptr<DWARFDebugMacro> Macro;
----------------
Just EHFrame maybe?

================
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:174
@@ +173,3 @@
+  /// Get a pointer to the parsed eh frame information object.
+  const DWARFDebugFrame *getDebugEHFrame();
+
----------------
getEHFrame

================
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:199
@@ -193,2 +198,3 @@
   virtual StringRef getDebugFrameSection() = 0;
+  virtual StringRef getDebugEHFrameSection() = 0;
   virtual const DWARFSection &getLineSection() = 0;
----------------
getEHFrameSection

================
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:251
@@ -244,2 +250,3 @@
   StringRef DebugFrameSection;
+  StringRef DebugEHFrameSection;
   DWARFSection LineSection;
----------------
EHFrameSection

================
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:291
@@ -283,2 +290,3 @@
   StringRef getDebugFrameSection() override { return DebugFrameSection; }
+  StringRef getDebugEHFrameSection() override { return DebugEHFrameSection; }
   const DWARFSection &getLineSection() override { return LineSection; }
----------------
getEHFrameSection

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:540
@@ -475,2 +539,3 @@
       uint8_t Version = Data.getU8(&Offset);
       const char *Augmentation = Data.getCStr(&Offset);
+      StringRef AugmentationString(Augmentation ? Augmentation : "");
----------------
Will this ever return null? In the format it is always present. I can be empty.

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:564
@@ +563,3 @@
+        // Walk the augmentation string to get all the augmentation data.
+        for (unsigned i = 1, e = AugmentationString.size(); i != e; ++i) {
+          char Char = AugmentationString[i];
----------------
You can use a range loop, no?

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:626
@@ +625,3 @@
+        StringRef AugmentationString = Cie->getAugmentationString();
+        if (AugmentationString.count('z')) {
+          // Parse the augmentation length and data for this FDE.
----------------
'z' has to be the first character.

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:635
@@ +634,3 @@
+          uint64_t LSDA = 0;
+          if (AugmentationString.count('L')) {
+            Optional<uint32_t> Encoding = Cie->getLSDAPointerEncoding();
----------------
Instead of checking the string again, can't you check the variables you set when parsing? If LSDAPointerEncoding has a value for this case for example.


http://reviews.llvm.org/D15535