jankratochvil added a comment. In https://reviews.llvm.org/D46810#1097829, @labath wrote:
> I am not sure it's that simple. I've found at least one case > (`SymbolFileDWARF::ParseImportedModuleswhere we call GetFirstChild() on the > value returned by `GetUnitDIEOnly` OK, that is clearly a bug there, thanks for finding it. I see some new assertion check would be good (I may try even a compile time check by some new class wrapper). `SymbolFileDWARF::ParseImportedModules` should just call `DIE()`. DWARFDIE GetUnitDIEOnly() -> DWARFDebugInfoEntry *GetUnitDIEPtrOnly() -> ExtractDIEsIfNeeded(true ) -> CU DIE only DWARFDIE DIE () -> DWARFDebugInfoEntry * DIEPtr () -> ExtractDIEsIfNeeded(false) -> all DIEs > I was thinking of making this safer by changing the `GetUnitDIEOnly` so that > the caller has to explicitly request (either with an argument, or by > splitting it into two functions) whether it wants a CU die, which can be used > to access other DIEs, or just a bare attribute-only DIE. In second case, we > would return &m_first_die, in the first case &m_die_array[0] (after making > sure it's properly initialized). But there is already such function, it is called `DIE()`, we need no new parameter. Maybe we should rename those functions, their current names are cryptic, what about changing the second line to: DWARFDIE GetUnitDIEOnly() -> DWARFDebugInfoEntry *GetUnitDIEPtrOnly() -> ExtractDIEsIfNeeded(true ) -> CU DIE only DWARFDIE GetUnitAllDIEs() -> DWARFDebugInfoEntry *GetUnitAllDIEsPtr() -> ExtractDIEsIfNeeded(false) -> all DIEs > Then `m_first_die` can have `has_children` property set to false to enforce > that noone accesses children that way. I would prefer an assert (or even a compilation error by some wrapper class) in such case but I agree this may be enough. https://reviews.llvm.org/D46810 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
