[lldb-dev] Review of API and remote packets
Pavel Labath via lldb-dev
lldb-dev at lists.llvm.org
Wed Apr 13 03:52:49 PDT 2016
Do we need the server to know about the user ids we handed out to the
SB API client? AFAIK, you cannot have multiple traces of the same
thread running concurrently, so a thread-id should uniquely identify a
trace. The user id can then stay a client thing for abstracting the
concrete implementation details. Or am I missing something here?
On 13 April 2016 at 10:10, Ravitheja Addepally <ravithejawork at gmail.com> wrote:
> Hello,
> Ok for the thread id we may use this QThreadSuffixSupported extension
> but gdb packets also don't have userid support since gdb does not have the
> concept of user id for trace instances. Also gdb uses seperate packets for
> trace configuration and starting the trace.
>
> On Tue, Apr 12, 2016 at 12:26 PM, Pavel Labath <labath at google.com> wrote:
>>
>> LLDB already has the QThreadSuffixSupported extension, which adds the
>> ";thread:<tid>" suffix to a bunch of packets. We could say that if the
>> client requests this extension, we will support it on these packets as
>> well. Otherwise we fall back to the "Hg" thingy.
>>
>> I haven't looked at how hard it would be to implement that...
>>
>> pl
>>
>> On 12 April 2016 at 09:01, Ravitheja Addepally <ravithejawork at gmail.com>
>> wrote:
>> > One of the downsides of using the gdb protocol is that these packets are
>> > stateful meaning the thread id option is not there and the word btrace
>> > stands for branch trace which basically suggests execution tracing.
>> >
>> > On Mon, Apr 11, 2016 at 4:50 PM, Pavel Labath <labath at google.com> wrote:
>> >>
>> >> I think we should reuse packets from the gdb protocol whereever it
>> >> makes sense. So, if they fit your needs (and a quick glance seems to
>> >> confirm that), then I think you should use them.
>> >>
>> >> On 11 April 2016 at 15:28, Ravitheja Addepally
>> >> <ravithejawork at gmail.com>
>> >> wrote:
>> >> > Hello,
>> >> > Regarding the packet definitions for tracing, how about
>> >> > reusing
>> >> > the
>> >> > existing btrace packets ?
>> >> >
>> >> >
>> >> >
>> >> > https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#qXfer%20btrace%20read
>> >> >
>> >> > On Fri, Apr 1, 2016 at 7:13 PM, Greg Clayton <gclayton at apple.com>
>> >> > wrote:
>> >> >>
>> >> >> We also need to think about all other types of tracing. It might
>> >> >> make
>> >> >> more
>> >> >> sense to keep these calls on SBProcess and have the calls simply be:
>> >> >>
>> >> >>
>> >> >> lldb::SBTrace lldb::SBProcess::StartTrace(SBTraceOptions
>> >> >> &trace_options,
>> >> >> lldb::SBError &error);
>> >> >>
>> >> >> And you would need to specify which threads in the SBTraceOptions
>> >> >> object
>> >> >> itself?:
>> >> >>
>> >> >> SBTraceOptions trace_options;
>> >> >>
>> >> >> And then do some like:
>> >> >>
>> >> >> trace_options.SetTraceAllThreads();
>> >> >>
>> >> >> And maybe tracing all threads is the default. Or one can limit this
>> >> >> to
>> >> >> one
>> >> >> thread:
>> >> >>
>> >> >> trace_options.SetThreadID (tid);
>> >> >>
>> >> >> Then you start tracing using the "trace_options" which has the
>> >> >> notion
>> >> >> of
>> >> >> which threads to trace.
>> >> >>
>> >> >> lldb::SBError error;
>> >> >> lldb::SBTrace trace = process.StartTrace(trace_options, error);
>> >> >>
>> >> >> It really depends on how all different forms of trace are enabled
>> >> >> for
>> >> >> different kinds of tracing. It takes kernel support to trace only
>> >> >> specific
>> >> >> threads, but someone might be debugging a bare board that only has
>> >> >> the
>> >> >> option tracing all threads on each core. When making an API we can't
>> >> >> assume
>> >> >> we can limit this to any threads and even to any process.
>> >> >>
>> >> >> Greg
>> >> >>
>> >> >> > On Apr 1, 2016, at 2:00 AM, Pavel Labath <labath at google.com>
>> >> >> > wrote:
>> >> >> >
>> >> >> > I second Greg's suggestions, and I have some thoughts of my own:
>> >> >> >
>> >> >> > - with the proposed API, it is not possible to get a trace for
>> >> >> > newly
>> >> >> > created threads - the process needs to be stopped first, so you
>> >> >> > can
>> >> >> > enable trace collection. There certainly are cases where this
>> >> >> > could
>> >> >> > be
>> >> >> > problematic, e.g. if you get a crash early during thread creation
>> >> >> > and
>> >> >> > you want to figure out how you got there. For this to work, we
>> >> >> > might
>> >> >> > need an API like
>> >> >> > SBProcess::TraceNewThreads(...)
>> >> >> > or
>> >> >> > SBProcess::TraceAllThreads(...)
>> >> >> > with the assumption that "all" also includes newly created threads
>> >> >> > in
>> >> >> > the future.
>> >> >> >
>> >> >> > I'm not saying this needs to be done in the first implementation,
>> >> >> > but
>> >> >> > I think that we should at least design the API in a way that will
>> >> >> > not
>> >> >> > make adding this unnecessarily hard in the future (e.g. the idea
>> >> >> > of
>> >> >> > returning an SBTrace object might be problematic, since you don't
>> >> >> > know
>> >> >> > if/how many threads will be created).
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> > Also, thinking about new APIs, should we have a way to mark an API
>> >> >> > as
>> >> >> > incubating/experimental? Maybe it would be good to mark these new
>> >> >> > APIs
>> >> >> > as experimental for a while, so we have an option of changing them
>> >> >> > in
>> >> >> > the future, if it turns out we have made the wrong decision. I was
>> >> >> > thinking of either a naming convention
>> >> >> > (SBThread::StartTraceExperimental) or some annotation/comment on
>> >> >> > the
>> >> >> > methods. When we are confident this design is good, we can remove
>> >> >> > the
>> >> >> > promote the api into the "stable" set.
>> >> >> >
>> >> >> > pl
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> > On 31 March 2016 at 18:59, Greg Clayton via lldb-dev
>> >> >> > <lldb-dev at lists.llvm.org> wrote:
>> >> >> >>
>> >> >> >>> On Mar 31, 2016, at 5:10 AM, Ravitheja Addepally via lldb-dev
>> >> >> >>> <lldb-dev at lists.llvm.org> wrote:
>> >> >> >>>
>> >> >> >>> Hello all,
>> >> >> >>> I am currently working on enabling Intel (R)
>> >> >> >>> Processor
>> >> >> >>> Trace collection for lldb. I have done some previous discussions
>> >> >> >>> in
>> >> >> >>> this
>> >> >> >>> mailing list on this topic but just to summarize , the path we
>> >> >> >>> chose was to
>> >> >> >>> implement raw trace collection in lldb and the trace will be
>> >> >> >>> decoded outside
>> >> >> >>> LLDB. I wanted to expose this feature through the SB API's and
>> >> >> >>> for
>> >> >> >>> trace
>> >> >> >>> data transfer I wish to develop new communication packets. Now I
>> >> >> >>> want to get
>> >> >> >>> the new API's and packet specifications reviewed by the dev
>> >> >> >>> list.
>> >> >> >>> Please
>> >> >> >>> find the specification below ->
>> >> >> >>>
>> >> >> >>> lldb::SBError SBProcess::StartTrace(lldb::tid_t threadId, const
>> >> >> >>> SBTraceConfig &config)
>> >> >> >>> Start tracing for thread - threadId with trace configuration
>> >> >> >>> config.
>> >> >> >>> SBTraceConfig would contain the following fields-
>> >> >> >>> -> TraceType - ProcessorTrace, SoftwareTrace , any
>> >> >> >>> trace
>> >> >> >>> technology etc
>> >> >> >>> -> size of trace buffer
>> >> >> >>> -> size of meta data buffer
>> >> >> >>> Returns error in case starting trace was unsuccessful, which
>> >> >> >>> could
>> >> >> >>> occur by reasons such as
>> >> >> >>> picking non existent thread, target does not support
>> >> >> >>> TraceType
>> >> >> >>> selected etc.
>> >> >> >>
>> >> >> >> If you are going to trace on a thread, we should be putting this
>> >> >> >> API
>> >> >> >> on
>> >> >> >> SBThread. Also we have other config type classes in our public
>> >> >> >> API
>> >> >> >> and we
>> >> >> >> have suffixed them with Options so SBTraceConfig should actually
>> >> >> >> be
>> >> >> >> SBTraceOptions. Also don't bother using "const" on any public
>> >> >> >> APIs
>> >> >> >> since the
>> >> >> >> mean nothing and only cause issues. Why? All public classes
>> >> >> >> usually
>> >> >> >> contain
>> >> >> >> a std::unique_ptr or a std::shared_ptr to a private class that
>> >> >> >> exists only
>> >> >> >> within LLDB itself. The "const" is just saying don't change my
>> >> >> >> shared
>> >> >> >> pointer, which doesn't actually do anything.
>> >> >> >>
>> >> >> >> lldb::SBError SBThread::StartTrace(SBTraceOptions
>> >> >> >> &trace_options);
>> >> >> >>
>> >> >> >>>
>> >> >> >>> lldb::SBError SBProcess::StopTrace(lldb::tid_t threadId)
>> >> >> >>> Stop tracing for thread - threadId. Tracing should be enabled
>> >> >> >>> already for thread, else error is returned.
>> >> >> >>
>> >> >> >> This should be:
>> >> >> >>
>> >> >> >> lldb::SBError SBThread::StopTrace();
>> >> >> >>
>> >> >> >> One question: can there only be one kind of trace going on at the
>> >> >> >> same
>> >> >> >> time? If we ever desire to support more than one at a time, we
>> >> >> >> might
>> >> >> >> need
>> >> >> >> the above two calls to be:
>> >> >> >>
>> >> >> >>
>> >> >> >> lldb::user_id_t SBThread::StartTrace(SBTraceOptions
>> >> >> >> &trace_options,
>> >> >> >> lldb::SBError &error);
>> >> >> >> lldb::SBError SBThread::StopTrace(lldb::user_id_t trace_id);
>> >> >> >>
>> >> >> >> The StartTrace could return a unique trace token that would need
>> >> >> >> to
>> >> >> >> be
>> >> >> >> supplied back to any other trace calls like the ones below.
>> >> >> >>
>> >> >> >>> size_t SBProcess::DumpTraceData(lldb::tid_t threadId, void *buf,
>> >> >> >>> size_t size, SBError &sberror)
>> >> >> >>> Dump the raw trace data for threadId in buffer described by
>> >> >> >>> pointer
>> >> >> >>> buf and size. Tracing should be enabled already for thread else
>> >> >> >>> error
>> >> >> >>> is sent in sberror. The actual size of filled buffer is
>> >> >> >>> returned
>> >> >> >>> by
>> >> >> >>> API.
>> >> >> >>>
>> >> >> >>> size_t SBProcess::DumpTraceMetaData(lldb::tid_t threadId, void
>> >> >> >>> *buf,
>> >> >> >>> size_t size, SBError &sberror)
>> >> >> >>> Dump the raw trace meta data for threadId in buffer described
>> >> >> >>> by
>> >> >> >>> pointer buf and size. Tracing should be enabled already for
>> >> >> >>> thread
>> >> >> >>> else error is sent in sberror. The actual size of filled
>> >> >> >>> buffer
>> >> >> >>> is
>> >> >> >>> returned by API.
>> >> >> >>
>> >> >> >> These would be on lldb::SBThread and remove the lldb::tid_t
>> >> >> >> parameter,
>> >> >> >> possibly adding "lldb::user_id_t trace_id" as the first
>> >> >> >> parameter.
>> >> >> >>
>> >> >> >> The other way to do this is to create a lldb::SBTrace object.
>> >> >> >> Then
>> >> >> >> the
>> >> >> >> APIs become:
>> >> >> >>
>> >> >> >> lldb::SBTrace SBThread::StartTrace(SBTraceOptions &trace_options,
>> >> >> >> lldb::SBError &error);
>> >> >> >>
>> >> >> >> lldb::SBError SBTrace::StopTrace();
>> >> >> >> size_t SBTrace::GetData(void *buf, size_t size, SBError
>> >> >> >> &sberror);
>> >> >> >> size_t SBTrace::GetMetaData(void *buf, size_t size, SBError
>> >> >> >> &sberror);
>> >> >> >> lldb::SBThread SBTrace::GetThread();
>> >> >> >>
>> >> >> >>>
>> >> >> >>> LLDB Trace Packet Specification
>> >> >> >>>
>> >> >> >>> QTrace:1:<threadid>,<type>,<buffersize>,<metabuffersize>
>> >> >> >>> Packet for starting tracing, where -
>> >> >> >>> -> threadid - stands for thread to trace
>> >> >> >>> -> type - Type of tracing to use, it will be like type
>> >> >> >>> of
>> >> >> >>> trace mechanism to use.
>> >> >> >>> For e.g ProcessorTrace, SoftwareTrace , any
>> >> >> >>> trace
>> >> >> >>> technology etc and if
>> >> >> >>> that trace is not supported by target error
>> >> >> >>> will
>> >> >> >>> be
>> >> >> >>> returned. In Future
>> >> >> >>> we can also add more parameters in the packet
>> >> >> >>> specification, which can be type specific
>> >> >> >>> and the server can parse them based on what
>> >> >> >>> type
>> >> >> >>> it
>> >> >> >>> read in the beginning.
>> >> >> >>> -> buffersize - Size for trace buffer
>> >> >> >>> -> metabuffersize - Size of Meta Data
>> >> >> >>
>> >> >> >> If we design this, we should have the arguments be in key/value
>> >> >> >> format:
>> >> >> >>
>> >> >> >>> QTrace:1:<key>:<value>;<key>:<value>;<key>:<value>;
>> >> >> >>
>> >> >> >> Then this packet currently could be sent as:
>> >> >> >>
>> >> >> >>
>> >> >> >>
>> >> >> >>
>> >> >> >> QTrace:1:threadid:<threadid>;type:<type>;buffersize=<buffersize>;metabuffersize=<metabuffersize>;
>> >> >> >>
>> >> >> >> This way if we ever need to add new key value pairs, we don't
>> >> >> >> need
>> >> >> >> to
>> >> >> >> make a new QTrace2 packet if the args ever change.
>> >> >> >>
>> >> >> >>
>> >> >> >>> QTrace:0:<threadid>
>> >> >> >>> Stop tracing thread with threadid,{Trace needs to be started
>> >> >> >>> of-course else error}
>> >> >> >>
>> >> >> >> again, this should be key/value pair encoded
>> >> >> >>
>> >> >> >> QTrace:0:threadid:<threadid>;
>> >> >> >>>
>> >> >> >>> qXfer:trace:buffer:read:annex:<threadid>,<byte_count>
>> >> >> >>> Packet for reading the trace buffer
>> >> >> >>> -> threadid - thread ID, of-course if tracing is not
>> >> >> >>> started for this thread error will be
>> >> >> >>> returned.
>> >> >> >>> -> byte_count - number of bytes to read, in case trace
>> >> >> >>> captured
>> >> >> >>> is
>> >> >> >>> less than byte_count, then only that much
>> >> >> >>> trace
>> >> >> >>> will
>> >> >> >>> be returned in response packet.
>> >> >> >>>
>> >> >> >>> qXfer:trace:meta:read:annex:<threadid>,<byte_count>
>> >> >> >>> Similar Packet as above except it reads meta data
>> >> >> >>
>> >> >> >> Hopefully we can key/value pair encode the args text that is
>> >> >> >> "<threadid>,<byte_count>".
>> >> >> >>
>> >> >> >>
>> >> >> >>
>> >> >> >> _______________________________________________
>> >> >> >> lldb-dev mailing list
>> >> >> >> lldb-dev at lists.llvm.org
>> >> >> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>> >> >>
>> >> >
>> >
>> >
>
>
More information about the lldb-dev
mailing list