labath added inline comments.
================ Comment at: source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h:35-58 + //------------------------------------------------------------------ + /// Slide the data in the buffer so that access to the data will + /// start at offset \a offset. + /// + /// This is currently used to provide access to the .debug_types + /// section and pretend it starts at at offset of the size of the + /// .debug_info section. This allows a minimally invasive change to ---------------- This looks like a thing we could focus on next. Sliding the contents of the data buffer seems like a useful fiction, but the way it is implemented now, it relies on undefined behavior (out-of-range pointers, possibly even wrapping the address space) and it violates the invariants that we have some far been assuming about data extractors (namely, that for a DataExtractor, all offsets from 0 to GetByteSize()-1 are dereferencable). For example, for a "slid" data extractor: - ValidOffset(0) will return true, even though that is clearly not the case - Append(data2) will try to access data at *m_start and probably crash - so will Checksum() Since there seems to be a consensus (at least, nobody has proposed an alternative solution) that this is the way we should treat type units, I think it makes sense to spend some time to make sure we built it on solid foundations. To me, this sliding seems like a fundamental change in the nature of the data extractors, and so I think it should be done in the base DataExtractor class. So I'd propose to move this into a separate patch, where we can make sure that all other DataExtractor APIs do something sensible even if the extractor has been "slid". https://reviews.llvm.org/D32167 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits