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 <ravithejaw...@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 <lab...@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 <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 <lab...@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 >> >> <ravithejaw...@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 <gclay...@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 <lab...@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@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
