Re: [Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-10-14 Thread Jan Kratochvil via lldb-commits
On Thu, 27 Sep 2018 18:21:13 +0200, Greg Clayton via lldb-commits wrote: > One fix that would work it to make the DWARFDebugInfoEntry* inside the > DWARFDie to be mutable and allow it to "fix" itself when a call is made. So > the flow would be: > > auto CUDie = cu->GetUnitDIEOnly(); > for (auto

Re: [Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-09-27 Thread Greg Clayton via lldb-commits
> On May 14, 2018, at 6:36 AM, Pavel Labath via Phabricator > wrote: > > labath added a comment. > > In https://reviews.llvm.org/D46810#1097740, @jankratochvil wrote: > >> In https://reviews.llvm.org/D46810#1097503, @labath wrote: >> >>> (If that is true, then I think this is workable, but

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Trying to be smart while being lazy and multithreaded is going to make the code complicated (possibility for bugs) and/or introduce a lot of locking overhead. A lot simpler solution is to let the caller decide if it want's the full CU or just the root DIE, and then make

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So this problem exists both in the LLDB and LLVM DWARF parsers. I am not sure this fix is safe. I would rather fix this by fixing DWARFDIE class to "do the right thing". We shoul

Re: [Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-14 Thread Pavel Labath via lldb-commits
On Mon, 14 May 2018 at 15:24, Jan Kratochvil via Phabricator < revi...@reviews.llvm.org> wrote: > 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::ParseImportedModuleswh

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-14 Thread Jan Kratochvil via Phabricator via lldb-commits
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

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D46810#1097740, @jankratochvil wrote: > In https://reviews.llvm.org/D46810#1097503, @labath wrote: > > > (If that is true, then I think this is workable, but there are still some > > details which need to be ironed out; e.g., `m_first_die.GetFi

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-14 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. In https://reviews.llvm.org/D46810#1097503, @labath wrote: > - if nothing has been parsed, m_die_array is empty and m_first_die is empty > - if the cu die has been parsed, m_die_array us empty and m_first_die is full > - if everything has been parsed m_first_die an

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I **think** I understand what is going on here, but I can't say for certain. Could you elaborate on the implementation details of this somewhere. It would be good to keep a note of this for future maintainers. My understanding of this is: - if nothing has been parsed, m

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-13 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil created this revision. jankratochvil added reviewers: clayborg, labath. Herald added subscribers: JDevlieghere, aprantl. + // GetUnitDIEPtrOnly() needs to return pointer to the first DIE. + // But the first element of m_die_array after ExtractDIEsIfNeeded(true) + // may move