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@lists.llvm.org> wrote:
>
>> On Mar 31, 2016, at 5:10 AM, Ravitheja Addepally via lldb-dev
>> <lldb-dev@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@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
