[PATCH] D44202: [DebugInfo/AccelTable] Fix inconsistency in getDIEOffset implementations

[Pavel Labath via Phabricator via llvm-commits]

labath added a reviewer: aprantl.
labath added a comment.

+ at aprantl to weigh in on the comment.



================
Comment at: lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp:282
+    if (Optional<uint64_t> Off = OffForm->getAsSectionOffset())
+      return *Off + HdrData->DIEOffsetBase;
+  }
----------------
JDevlieghere wrote:
> labath wrote:
> > Actually, after reading the "spec" <https://llvm.org/docs/SourceLevelDebugging.html#name-accelerator-tables>, I'm starting to be unsure about this part. The document says: "base DIE offset that should be added to any atoms that are encoded using the DW_FORM_ref1, DW_FORM_ref2, DW_FORM_ref4, DW_FORM_ref8 or DW_FORM_ref_udata", but we our producers encode this field as DW_FORM_data4.
> Indeed, I don't think this is correct. While we currently don't have any atoms using this encoding, technically nothing prevents you from defining one with DW_FORM_ref* which would require using the DIEOffsetBase. I don't know the history behind this though, maybe @aprantl knows more?
So, to be correct, do you think we should check explicitly check for the specified forms here?
(Also, whatever the solution, it should probably be also applied to the getCUOffset function below).


Repository:
  rL LLVM

https://reviews.llvm.org/D44202