Review of API and remote packets

Subprojects LLDB
1

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.

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

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:,<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:,<byte_count>
Similar Packet as above except it reads meta data

2

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

3

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

4

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

5

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

6

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.

7

I used to work on debug software at Freescale (NXP?), and we had a PPC core called e6500. It had 8 IACs (Instruction Address Comparators) that could be hooked up to the trace logic in various ways, but one really useful thing you could do was set up an IAC to turn on or off trace on a given address or address range. So we implemented tracepoints, which were like breakpoints but would turn on/off trace. I'd like to see that kind of support in a trace API.

8

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.

9

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

10

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.

11

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?

12

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 ?

13

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