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

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

Great catch Pavel, I can imagine this preventing quite some confusion in the future. Thanks!

Requesting changes because I also don't think the DIEOffsetBase is correct, as explained inline.



================
Comment at: lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp:282
+    if (Optional<uint64_t> Off = OffForm->getAsSectionOffset())
+      return *Off + HdrData->DIEOffsetBase;
+  }
----------------
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?


Repository:
  rL LLVM

https://reviews.llvm.org/D44202