[PATCH] D32779: DWARF: Implementation of v5 string offsets table (.debug_str_offsets[.dwo] section)/consumer

[Wolfgang Pieb via Phabricator via llvm-commits]

wolfgangp marked 8 inline comments as done.
wolfgangp added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:118
+    // SegmentSize + 4.
+    if (!StrOffsetExt.isValidOffsetForDataOfSize(Offset, SegmentSize + 4))
+      return;
----------------
wolfgangp wrote:
> dblaikie wrote:
> > dblaikie wrote:
> > > Is there a test case for this?
> > Does this need to test if the SegmentSize is a multiple of the record size? otherwise the loop below might read a partial record, off the end/past SegmentSize?
> Will add one.
Yes, thanks for catching that.


================
Comment at: lib/DebugInfo/DWARF/DWARFFormValue.cpp:608-610
     if (!U->getStringOffsetSectionItem(Offset, StrOffset))
       return None;
+    StrOffset += U->getStringOffsetSectionRelocation(Offset);
----------------
wolfgangp wrote:
> dblaikie wrote:
> > Rather than trying, failing, falling back - should this be implemented in a way to "do the right thing" from the start?
> I was just trying to fit the new functionality into the existing structure but I'll give it a shot.
Looked at this for a while, but couldn't come up with a better way to write it. Do you have any suggestions?


https://reviews.llvm.org/D32779