[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 <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>".