[Lldb-commits] [PATCH] [lldb] ASan history threads SB API

[Kuba Brecka]

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".

Although I think you're right, a generic "here's a pointer, give me a
thread list" sounds reasonable. 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)?

Kuba



On Wed, Sep 3, 2014 at 2:54 PM, Enrico Granata <egranata at apple.com> 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 <kuba.brecka at gmail.com> 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
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>
>
> Thanks,
> *- Enrico*
> 📩 egranata@.com ☎️ 27683
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140903/de4d7eb1/attachment.html>