[PATCH] D73887: [DebugInfo] Do not cut 64-bit values when dumping CIEs and FDEs.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin marked 2 inline comments as done.
ikudrin added a comment.

In D73887#1872416 <https://reviews.llvm.org/D73887#1872416>, @probinson wrote:

> The commit message should include a link to the Linux spec that you cited in the other review, which will help future readers understand the differences between the two formats.


This revision is mostly about 64-bit values. The links to the specs are added to the comment for `getCIEId()`, in D73886 <https://reviews.llvm.org/D73886>, which is a parent revision for this one, and to the commit message for D73627 <https://reviews.llvm.org/D73627>.



================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:191
   static bool classof(const FrameEntry *FE) { return FE->getKind() == FK_CIE; }
+  static uint64_t getCIEId(bool IsDWARF64, bool IsEH);
 
----------------
probinson wrote:
> Because this will not return the value from the data, it should have a comment along the lines of:
> `// Returns the CIE identifier to be used by the requested format.`
I've moved the method (now a function) `getCIEId()` to D73886. The comment is added there.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:328
+     << format(" FDE cie=%0*" PRIx64, IsDWARF64 && !IsEH ? 16 : 8,
+               LinkedCIEOffset)
+     << format(" pc=%08" PRIx64 "...%08" PRIx64, InitialLocation,
----------------
probinson wrote:
> This looks like you are printing LinkedCIEOffset twice?
Thanks for catching that! It was an issue in our implementation, I've prepared D74613 to fix that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73887/new/

https://reviews.llvm.org/D73887