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

I've renamed everything to just EHFrame.  Will work on restructuring the loop now.


================
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 : "");
----------------
rafael wrote:
> Will this ever return null? In the format it is always present. I can be empty.
The link I gave before suggests its optional.  LLVM itself will always generate at least "zR", but I have no idea about other tools.

The line in particular I think makes it optional is "A 'z' may be present as the first character of the string"

================
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.
----------------
rafael wrote:
> 'z' has to be the first character.
Good catch.  I'll be changing the structure of the loop so that it starts at index 0 and makes sure that 'z' is only at index 0.  That should fix this.

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:635
@@ +634,3 @@
+          uint64_t LSDA = 0;
+          if (AugmentationString.count('L')) {
+            Optional<uint32_t> Encoding = Cie->getLSDAPointerEncoding();
----------------
rafael wrote:
> 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.
Will do.


http://reviews.llvm.org/D15535