[lldb-dev] Review of API and remote packets
Ravitheja Addepally via lldb-dev
lldb-dev at lists.llvm.org
Thu Jun 16 05:10:59 PDT 2016
Hello All,
In the context of Intel® Processor Trace support in LLDB I
asked, a while ago, about the syntax of remote packets.
Directions were mixed:
1. Taking GDBSERVER/GDB packets (available from 7.10)
2. Going to a brand new packets for lldb/lldbserver (as described by
Greg).
We consider also:
1. Use case lldb/lldbserver can use new packets, while LLDB/gdbserver
uses the GDB ones.
2. Use case LLDB/GDBSERVER is decreasing in importance. We see a
gradual increase in the solutions and systems providing lldbserver as
default.
3. GDB/lldserver is not supported.
(for all those observations please correct me if I am wrong)
In this sense we intent to follow the development using new packets for the
use case lldb/lldbserver.
But we still need a feedback on: Is the use LLDB/GDBSERVER still important?
Looking forward for your feedback!
Thanks and regards,
A Ravi Theja
On Wed, Apr 13, 2016 at 1:55 PM, Ravitheja Addepally <
ravithejawork at gmail.com> wrote:
> Well the point of the user ids was to support multiple trace technologies
> for the same thread, so in that case the thread id is not sufficient for
> uniquely identifying the trace. Now I guess the server would need to be
> aware of the user-id if multiple trace technologies are active on the same
> thread ?
>
> On Wed, Apr 13, 2016 at 12:52 PM, Pavel Labath <labath at google.com> wrote:
>
>> 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
>> >> >> >>
>> >> >> >
>> >> >
>> >> >
>> >
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20160616/60b38cd2/attachment-0001.html>
More information about the lldb-dev
mailing list