Re: [lldb-dev] Review of API and remote packets

[Ravitheja Addepally via lldb-dev]

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 <
ravithejaw...@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  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 
>> 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 
>> wrote:
>> >>
>> >> LLDB already has the QThreadSuffixSupported extension, which adds the
>> >> ";thread:" 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 <
>> ravithejaw...@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 
>> 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
>> >> >> 
>> >> >> 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 
>> >> >> > 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
>> >> >> >> _options,
>> >> >> >> lldb::SBError );
>> >> >> >>
>> >> >> >> 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);
>> >> >> >>
>> >> >> >> 

Re: [lldb-dev] Review of API and remote packets

[Pavel Labath via lldb-dev]

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  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  wrote:
>>
>> LLDB already has the QThreadSuffixSupported extension, which adds the
>> ";thread:" 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 
>> 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  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
>> >> 
>> >> 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 
>> >> > 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
>> >> >> _options,
>> >> >> lldb::SBError );
>> >> >>
>> >> >> 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 
>> >> >> > 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 

Re: [lldb-dev] Review of API and remote packets

[Ravitheja Addepally via lldb-dev]

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  wrote:

> LLDB already has the QThreadSuffixSupported extension, which adds the
> ";thread:" 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 
> 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  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  >
> >> 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 
> 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
> >> >> _options,
> >> >> lldb::SBError );
> >> >>
> >> >> 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 
> 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
> >> >> >
> >> >> >

Re: [lldb-dev] Review of API and remote packets

[Pavel Labath via lldb-dev]

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  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  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 _options,
>> lldb::SBError );
>>
>> 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  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
>> >  wrote:
>> >>
>> >>> On Mar 31, 2016, at 5:10 AM, Ravitheja Addepally via lldb-dev
>> >>>  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 )
>> >>>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 

Re: [lldb-dev] Review of API and remote packets

[Greg Clayton via lldb-dev]

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 _options, 
lldb::SBError );

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  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
>  wrote:
>> 
>>> On Mar 31, 2016, at 5:10 AM, Ravitheja Addepally via lldb-dev 
>>>  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 )
>>>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 _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 

Re: [lldb-dev] Review of API and remote packets

[Pavel Labath via lldb-dev]

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
 wrote:
>
>> On Mar 31, 2016, at 5:10 AM, Ravitheja Addepally via lldb-dev 
>>  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 )
>> 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 _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 _options, 
> lldb::SBError );
> 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 )
>> 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 )
>> 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 

Re: [lldb-dev] Review of API and remote packets

[Greg Clayton via lldb-dev]

> On Mar 31, 2016, at 5:10 AM, Ravitheja Addepally via lldb-dev 
>  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 
> )
> 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 _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 _options, 
lldb::SBError );
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 )
> 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 )
> 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 _options, lldb::SBError 
);

lldb::SBError SBTrace::StopTrace();
size_t SBTrace::GetData(void *buf, size_t size, SBError );
size_t SBTrace::GetMetaData(void *buf, size_t size, SBError );
lldb::SBThread SBTrace::GetThread();

> 
> LLDB Trace Packet Specification
> 
> QTrace:1:,,,
> 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::;:;:;

Then this packet currently could be sent as:

QTrace:1:threadid:;type:;buffersize=;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:
> 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:;
> 
> 

[lldb-dev] Review of API and remote packets

[Ravitheja Addepally via lldb-dev]

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

lldb::SBError SBProcess::StopTrace(lldb::tid_t threadId)
Stop tracing for thread - threadId. Tracing should be enabled already
for thread, else error is returned.


size_t SBProcess::DumpTraceData(lldb::tid_t threadId, void *buf, size_t
size, 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 )
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.


LLDB Trace Packet Specification

QTrace:1:,,,
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

QTrace:0:
Stop tracing thread with threadid,{Trace needs to be started of-course
else error}

qXfer:trace:buffer:read:annex:,
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:,
Similar Packet as above except it reads meta data
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev