[PATCH] D37852: [dwarfdump] Make .eh_frame a first class citizen

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:191-200
+          // FIXME: Parse the actual instruction.
+          *Offset += Data.getULEB128(Offset);
+          break;
         case DW_CFA_expression:
-        case DW_CFA_val_expression:
-          // TODO: implement this
-          report_fatal_error("Values with expressions not implemented yet!");
+        case DW_CFA_val_expression: {
+          // FIXME: Parse the actual instruction.
+          Data.getULEB128(Offset);
----------------
rnk wrote:
> Why does this need to be part of this change? It seems nicer to commit this without the change, XFAIL or workaround whatever test relies on these, and implement the DW_OP expression machine in a follow-up change.
I agree, and essentially this is the workaround. Unless we just want to bail and don't care about any subsequent instructions that we might understand, we need to at least increment the offset. Failing isn't really an option imho because .eh_frame is part of "all" and that might have a pretty big impact.

Maybe I can update the comment to stress that this is a workaround? 


Repository:
  rL LLVM

https://reviews.llvm.org/D37852