[PATCH] D107874: [DWARF] Find offset of attribute.

[Alexander Yermolovich via Phabricator via llvm-commits]

ayermolo added inline comments.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1788
+    uint64_t OffsetVal =
+        it->getAttributeOffsetFromIndex(*AttrIndex, /* offset */ 0, *U);
+    EXPECT_TRUE(
----------------
dblaikie wrote:
> ayermolo wrote:
> > dblaikie wrote:
> > > Could probably also test the AttrIndex value before using it to lookup the attribute value.
> > Not sure I follow. I am testing it on line 1786.
> The 1786 checks that AttrIndex has a value (any value, not None), but doesn't check what value that is.
> 
> Specifically, something like:
> ```
> EXPECT_TRUE(*AttrIndex, 0);
> ```
> 
> I guess (since it seems this is only looking at the zeroth attribute, judging by line 1782?). In addition to the existing expects (I'd put this between 1786 and 1787 - after checking that AttrIndex is not None, but before using the value - so if there was a buggy value (if it returned 10 instead of 0, for instance) this check would fail before invoking UB/asserting inside getAttributeOffsetFromIndex, for instance))
Ah I see. thanks. Added. Will commit tomorrow if no other comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107874