For what it's worth, with the temporal/logical backtrace feature we added SBThread::GetExtendedBacktraceThread() which hands out a fake SBThread similar to the ones Kuba is doing here. (in that case, we don't want to fetch all of the extended backtraces, e.g. to support a NumExtendedBacktraceThreads style API, because it's expensive to do. We chain them individually as needed by the driver program)
In that case I have the Process own all of these created Threads - it has an m_extended_thread_list ivar which is cleared every time we have a public stop event. This means if the driver gets one of these SBThreads back, it will be invalid if the inferior resumes execution. I don't think any of this is really applicable to the discussion here but I wanted to mention it as something similar. > On Sep 3, 2014, at 7:15 PM, Enrico Granata <[email protected]> wrote: > > Inlined! > > Sent from my iPhone > > On Sep 3, 2014, at 6:48 PM, Kuba Brecka <[email protected]> wrote: > >> Hi Enrico, >> >> > However, a HistoryThreads hardly seems to fit in [ValueImpl] >> > I am assuming this is mostly because: >> > - this is a pointer-based lookup, so it makes little sense on the >> > ValueObject proper >> > - but you still want to store it away somewhere once you computed it >> >> Exactly. >> >> > I admit not having followed through your previous patch in depth, so take >> > my >> > suggestion with a grain of salt.. couldn’t your MemoryHistory plugin handle >> > the caching? i.e. all I would ever do when asked for a HistoryThreadsSP is >> > plugin->GetHistoryThreads(pointer_value); and the plugin would internally >> > handle a per-process cache of pointer_value —> thread list >> >> I would also like to be able to "renew" that thread list, so you don't get >> stuck with one list that is no longer correct. The SBValue API did the >> caching and you could always create a new SBValue to "renew". >> > > When would the list be invalidated? > If the list can turn wrong without anyone being made aware of it, and your > only hope is to fetch the data again, or risk failure, then no level of > caching makes sense. > If I have a way to detect that the list is now wrong, and I am supposed to > make a new SBValue, why not just detect it automatically and fetch valid data > again inside the plugin? > If detection of changes is entirely impossible, I would argue against caching > entirely - slow but correct wins over fast but potentially (and undetectably > so) wrong IMO. > >> Although I think you're right, a generic "here's a pointer, give me a thread >> list" sounds reasonable. > > I would argue in favor of that, yes. > You can stick this API on ValueObject and hence SBValue as an helper, I ended > up seeing the value in that, so no objection there. But I do believe that at > the end of the day, your API is really pointer --> threads, anything else > being a convenience for users. > >> The issue with that is that SB doesn't know about a thread list. Mostly >> because even internally, ThreadList is not really just a container for >> Thread objects, but has a lot of process-related logic. >> >> What would you suggest? Having a SBThreadList that would be just a container >> (and thus would differ from ThreadList)? >> > > That's not a bad idea. Just like SBValue has a ValueImpl, SBThreadList would > have an Impl object. The Impl would contain a set of ThreadSP/SBThread > objects and return those when asked. > Just don't put a vector<> or anything like that directly in the SB object if > you make one, that causes problems! > >> Kuba >> >> >> >> On Wed, Sep 3, 2014 at 2:54 PM, Enrico Granata <[email protected]> wrote: >> I think I mentioned that I was not sure SBValue was the best place to place >> this functionality (it’s address based, not ValueObject based), but it >> looked like others on the team felt differently, so no big deal there >> >> One thing that worries me a little bit more is how the history threads are >> being stored on the ValueImpl - I am not sure I agree with that >> >> The ValueImpl is essentially a way to reconstruct the underlying ValueObject >> for an SBValue given choices on dynamic/synthetic values; I could see us >> adding more axes to that class if we ended up with more features that could >> potentially generate different “views” of the same underlying root >> ValueObject >> >> However, a HistoryThreads hardly seems to fit in there - I am assuming this >> is mostly because: >> - this is a pointer-based lookup, so it makes little sense on the >> ValueObject proper >> - but you still want to store it away somewhere once you computed it >> >> I admit not having followed through your previous patch in depth, so take my >> suggestion with a grain of salt.. couldn’t your MemoryHistory plugin handle >> the caching? i.e. all I would ever do when asked for a HistoryThreadsSP is >> plugin->GetHistoryThreads(pointer_value); and the plugin would internally >> handle a per-process cache of pointer_value —> thread list >> >> The advantage is that now the question could be asked from anywhere; I could >> ask it of an internal ValueObject, or of an SBValue, and the right thing >> would happen consistently (w.r.t. caching and whatnot) because now the >> plugin is handling this in a sane manner >> >> This last one is minor but, >> +typedef std::shared_ptr<HistoryThreads> HistoryThreadsSP; >> + >> >> why do you need to define this in SBValue.cpp? >> Can’t HistoryThreadsSP be defined with all the other FooSP typedefs, so it’s >> visible everywhere? >> >>> On Sep 3, 2014, at 11:41 AM, Kuba Brecka <[email protected]> wrote: >>> >>> This patch depends on http://reviews.llvm.org/D4596. >>> >>> As a continuation of the previous patch that adds a MemoryHistory plugin >>> and implementation for ASan-provided malloc/free stack traces, this patch >>> exposes this into the SB API. In short, these two new methods are added >>> into SBValue: >>> >>> * uint32_t SBValue::GetNumMemoryHistoryThreads (); >>> * SBThread SBValue::GetMemoryHistoryThreadAtIndex (uint32_t idx); >>> >>> This corresponds to how we provide objects for which we don't have >>> containers (SBFrame and GetNumFrames + GetFrameAtIndex). Note that exposing >>> ThreadList into a generic SBThreadList container would not be >>> straightforward, because currently ThreadList is not a generic container of >>> threads, but instead holds functionality tied to a process and can >>> currently only be used to hold all threads in a process. >>> >>> http://reviews.llvm.org/D5175 >>> >>> Files: >>> include/lldb/API/SBValue.h >>> scripts/Python/interface/SBValue.i >>> source/API/SBValue.cpp >>> test/functionalities/asan/TestAsan.py >>> <D5175.13219.patch>_______________________________________________ >>> lldb-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits >> >> Thanks, >> - Enrico >> 📩 egranata@.com ☎️ 27683 >> >> >> >> >> > _______________________________________________ > lldb-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
