[lldb-dev] Review of API and remote packets
Ravitheja Addepally via lldb-dev
lldb-dev at lists.llvm.org
Mon Apr 11 07:28:18 PDT 2016
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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20160411/1de5a29b/attachment.html>
More information about the lldb-dev
mailing list