[PATCH] D77556: [DWARFDataExtractor] Add a "truncating" constructor

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDataExtractor.h:42-46
+  /// Truncating constructor
+  DWARFDataExtractor(const DWARFDataExtractor &Other, size_t Length)
+      : DataExtractor(Other.getData().substr(0, Length), Other.isLittleEndian(),
+                      Other.getAddressSize()),
+        Obj(Other.Obj), Section(Other.Section) {}
----------------
I wonder if this should be more generic than the `DWARFDataExtractor` class, i.e. be in `DataExtractor`? The concept of limiting length to a specified amount is hardly DWARF specific - ELF sections, for example, sometimes need parsing and have a limited length.

Preumably of course, we'd still want this constructor, but it would just forward to the base class one.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDataExtractorTest.cpp:148
+  EXPECT_EQ(0x42u, Truncated.getRelocatedAddress(C));
+  EXPECT_EQ(0x0u, Truncated.getRelocatedAddress(C));
+  EXPECT_THAT_ERROR(C.takeError(),
----------------
I think it would make sense to show the behaviour where an entry can be partially read, i.e. the length truncates the field. I think we'd need this tested both for a location with a relocation and for one without.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77556