Implementation of Intel(R) Processor Trace support for Linux

[ravitheja]

Summary

This patch implements support for Intel(R) Processor Trace
in lldb server. The changes have support for
starting/stopping and reading the trace data. The code
is only available on Linux versions where the perf
attributes for aux buffers are available.

The patch also consists of Unit tests for testing the
core buffer reading function.

[labath]

First batch of comments from me, I think I will have some more once I gain more insight into this. The main one is about the constructor/initialize, destructor/destroy duality, which we should abolish. The rest is mostly stylistic.

source/Plugins/Process/Linux/NativeProcessLinux.cpp

616

if(traceMonitor)

617

As far as, I can tell, every call to StartProcessorTracing is followed by setUserId. Can we move the logic of setting the id inside the StartProcessorTracing function

2479

Please use llvm file API for this (see how getProcFile works -- it's just a very thin wrapper around the llvm functions)

2506

Please use LLDB_LOG (here and everywhere else in this patch)

2843

You are calling this function with uid == m_pt_process_uid, which means this parameter is redundant, and leads to redundant sanity checks. Please remove it.

2961

You are modifying the config, but the caller is ignoring these modifications. If you don't need these, we could remove the modifications, make the config argument const and end up with a much cleaner interface.

3052

I don't see a getProcFile overload with this signature (although I am not opposed to adding one). Are you sure this compiles?

3060

Is this scope necessary? I find it just confuses the reader...

3066

add: columns[1] = columns[1].trim();
Then you can skip calling trim everywhere else.

3071

columns[0].contains(foo)

3096

return here. Then you can avoid duplicating the check outside the loop.

source/Plugins/Process/Linux/NativeProcessLinux.h

156

How about void ReadCyclicBuffer(ArrayRef<uint8_t> buffer, size_t position, MutableArrayRef<uint8_t> &dest)

160

This seems suspicious. How did you come to choose this number?

263

Please move this into a separate file. NativeProcessLinux.cpp is big enough as it is.

277

This would be much cleaner, if we could avoid creating an "invalid trace" object. The way that's usually done is to have a factory function, returning say llvm::Expected<std::unique_ptr<T>> (or even llvm::Expected<T> if you are able to make your type movable easily). The factory function then does all the operations that can fail and returns an error in this case. Once it has everything ready, it just calls the constructor, forwarding all the necessary arguments. The constructor can perform any additional non-fallable initialization, if necessary.

This way all the code inside and around the object can assume that once it has an object of this type, it is a valid object. We don't have many examples of this pattern, but you can for example look at MinidumpParser::Create or the code in (D32930) for guidance. I am also working on a patch to switch NativeProcess creation to use this pattern.

294

I believe we decided to call this "trace ID"

unittests/CMakeLists.txt

77 ↗

(On Diff #100699)

Please put this under unittests/Process/Linux (to match unittests/Process/gdb-remote)

unittests/Linux/CMakeLists.txt

1 ↗

(On Diff #100699)

Delete this. Just include the files as "Plugins/Process/Linux/XXX.h"

[clayborg]

I trust Pavel to review this since it is in the Linux native plugins mostly.

[zturner]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

726–730

This function returns a Status. Can't we assume that traceMonitor will be valid if and only if the returned Status is a success condition? And if it's not a success condition, shouldn't you log the error?

2513–2518

Please use a range based for here.

2622

Can you do an early return here if the condition is not true?

2623

And here

2634–2638

Nothing specifically wrong with this, but it's implicitly convertible, so if you like it you can simply just pass these strings into the AddIntegerItem function as string literals.

2713

std::make_pair(thread, traceMonitor) might allow this to fit on one line. m_processor_trace_monitor.emplace(thread, traceMonitor); almost definitely would.

2740

Early return here.

2741

Who is responsible for freeing this memory?

2790–2791

Seems like this would be more straightforward to just say:

if (trace_type != eTraceTypeProcessorTrace)
  return NativeProcessProtocol::StartTrace(config, error);

2866–2867

Bunch of opportunities in this function for early returning on error conditions.

2872–2873

Range based for with an inverted conditional and early continue inside the loop.

2927–2934

Delete and replace callsites with llvm::Log2_64

2936–2939

What happens if you call this function twice in a row? Or from different threads? Is that something you care about?

3005–3006

Perhaps you can use llvm::MemoryBuffer here? It mmaps internally

3042

It looks to me like vendor_id param could be a StringRef&.

source/Plugins/Process/Linux/NativeProcessLinux.h

156

Better yet, drop the position argument entirely. ReadCyclicBuffer(src, N, dest) is equivalent to ReadCyclicBuffer(src.drop_front(N), dest);

265–266

Instead of having

void *m_mmap_data;
void *m_mmap_aux;
void *m_mmap_base;

and then doing some ugly casting every time someone calls getAuxBufferSize or getDataBufferSize, instead just write:

MutableArrayRef<uint8_t> m_mmap_data;
MutableArrayRef<uint8_t> m_mmap_aux;

and initializing these array refs up front using the size from the header. This way you don't have to worry about anyone using the buffers incorrectly, and the ReadPerfTraceAux(size_t offset) function no longer needs to return a Status, but instead it can just return MutableArrayRef<uint8_t> since nothing can go wrong.

343–344

Can you use an llvm::DenseMap here?

346–348

And an llvm::DenseSet here?

[zturner]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

1318–1320

for (const auto &I : buf)
  response.PutHex8(I);

[ravitheja]

source/Plugins/Process/Linux/NativeProcessLinux.h

156

So there are couple of things i would like to mention ->

1. The data and aux buffers allocated are cyclic in nature, so we need to consider the cyc_start or starting index in order to correctly read them.
2. The function ReadCyclicBuffer is very generic and is capable of reading a certain chunk of memory starting from a certain offset.
3. Now I could have kept this function in the private domain so that the cyc_start need not be passed everytime, but then I also wanted to unit test this function.

Now keeping these in mind my questions are the following ->
@zturner @labath I need the offset and cyc_start , both are required so the position argument won't suffice ?

Please correct me if i am wrong.

160

Sometime ago I asked in the devlist about the error codes in lldb and got the answer that they were completely arbitrary, so 0x23 has no special meaning. The question that I would like to ask is do u think these error codes should be here ? coz in https://reviews.llvm.org/D33035 it was suggested by @clayborg that tools working on top of the SB API's should only receive Error Strings and not codes.

Currently anyone who would encounter these codes would have to refer here for their meaning. Also the remote packets don't allow me to send Error Strings instead of error codes.

265–266

As mentioned in my previous comments, the m_mmap_data and m_mmap_aux are cyclic buffers and unfortunately the kernel keeps overwriting the buffer in a cyclic manner. The last position written by the kernel can only be obtained by inspecting the data_head and aux_head fields in the perf_event_mmap_page structure (pointed by m_mmap_base).

Because of this scenario you see the ugly type casting. Now regarding the casting in getAuxBufferSize and getDataBufferSize I did not want to store the buffer sizes as even if store them since they are not the biggest contributors to the total type casting ugliness. plus if the correct sizes are already store in the perf_event_mmap_page structure why store them myself ?.

Now regarding ReadPerfTraceAux(size_t offset) , i still need the size argument coz what if the user does not want to read the complete data from offset to the end and just wants a chunk in between the offset and the end ?

[ravitheja]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

3005–3006

In my case, I only want to allocate it using mmap with the options i use here, with
llvm::MemoryBuffer I see no option to force it to only use mmap and with the options i need ?

[labath]

source/Plugins/Process/Linux/NativeProcessLinux.h

156

I believe zachary did not understand what the function does. The position is indeed necessary. However, I believe the prototype I suggested will work for you.

As for the function itself, I think it is way over-engineered. More than half of the function is sanity-checking, and more than half of the tests are tests of the sanity checks.

With ArrayRef, the function could be implemented as:

auto remaining = buffer.drop_front(position);
if(remaining.size() > dest.size())
  std::copy(remaining.begin(), dest.size(), dest.begin());
else
  std::copy_n(buffer.begin(), remaining.size() - dest.size(), 
    std::copy(remaining.begin(), remaining.end(), dest.begin());

(this will be slightly more complicated if you need to handle the dest.size() > buffer.size() case, which I am not sure if you need)

160

If they're completely arbitrary, then how about starting with number one?

Not being able to send complex error messages across the gdb-protocol is a bit of a drag. I would not be opposed to adding such a facility, as I wished for that a couple of times in past. If you wish to do that, then we should start a separate discussion.

265–266

So, if this refers to a structure of type perf_event_mmap_page, why let the type of this be perf_event_mmap_page *? That way you can have just one cast, when you initialize the member, and not each time you access it.

[ravitheja]

Changes suggested in previous round of feedback.

[ravitheja]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2843

Well I wanted this function to be capable of being called from other contexts as well, but i never ended up using that way.

2936–2939

What happens if you call this function twice in a row

Second call will fail,

Or from different threads

If called for different threads, then they will start to be traced, ofcourse assuming they are not being traced.

3052

Yes sorry I forgot to add this new file.

source/Plugins/Process/Linux/NativeProcessLinux.h

265–266

The thing is this structure is only available from a certain version of the perf_event.h. Although I have put macro guards everywhere I use this structure. But on previous Linux versions it won't be there, so I was afraid if i would run into issues.

[labath]

Second round of comments. I still see a lot of places with redundant checks for what appear to be operational invariants. I'd like to get those cleared up to make the code flow better.

source/Plugins/Process/Linux/NativeProcessLinux.cpp

612

Every call to AddThread is followed by this block. Is there a reason we cannot just move it inside the AddThread function?

622

LLDB_LOG will already print the function name (same thing everywhere else you use __FUNCTION__), so you don't need to write it out yourself. Also the AsCString is unnecessary, as Status has a pretty-printer.

2524

unused variable

2617

As of r305462 you can write error = traceMonitor.takeError(), so you shouldn't need the extra variable.

2634

Should you delete the iterator from the map after this? In fact, the mere act of deleting the iterator should probably be what triggers the trace stop, in line with RAII principles.

2668

This is dead code. The caller has already checked this condition. This should be at most an assertion.

source/Plugins/Process/Linux/NativeProcessLinux.h

263

ErrorOr<T> is a better way to return a value *or* an error. Although, in this case, it sounds like you're really only interested in the found-or-not-found distinction, so you could just drop the Status argument and let a null return value signify an error.

277

I'd like to downgrade these to unique pointers to ProcessTraceMonitor. There's no reason for these to ever outlive or escape the process instance, so it's natural to say they are strongly owned by it. In other places where you use ProcessorTraceMonitorSP you can just use regular pointers or references (depending on whether they can be null or not).

282

I am confused about the purpose of this member variable. As far as I can tell, it will always contain *either* all of the threads in the process *or* none of them. Is there a situation where this is not the case?

source/Plugins/Process/Linux/ProcessorTrace.cpp

159

eErrorTypePOSIX is used for errno error values. Please don't try to pass your invented error codes as these.

322

dead code

413

Just put llvm_unreachable here. There shouldn't be any way to construct this object if tracing is not supported.

481

Dead code? The construction of the object shouldn't have succeeded if you failed to allocate the memory or initialize the trace.

source/Plugins/Process/Linux/ProcessorTrace.h

21

All the other files in the folder are in an additional process_linux namespace. This file should go there as well.

71

Make this MutableArrayRef<uint8_t>. Then you don't need the ugly cast when calling readcyclicbuffer, nor the GetAuxBufferSize function (as it just becomes m_mmap_aux.size()). Same for m_mmap_data.

80

Instead of global variables (we may want to debug more than one process eventually), we could have a member variable std::pair<user_id_t, TraceOptions> m_process_trace; (you can have a special class for that if you don't like .first and .second everywhere).

unittests/Linux/ProcessorTraceTest.cpp

1 ↗

(On Diff #102540)

You seem to have forgotten to delete this.

unittests/Process/Linux/ProcessorTraceTest.cpp

42

I don't understand why you're testing these. Why would anyone create an ArrayRef pointing to null? If you get handed an array ref, you should be free to assume it points to valid data.

[ravitheja]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

612

Yes that could be done.

source/Plugins/Process/Linux/NativeProcessLinux.h

282

Yes there can be situations like that, for e.g if a user starts tracing on an individual thread followed by tracing the whole process (meaning in two separate start trace calls) then this Set will contain the thread ids of all the threads except the one on which individual tracing was started. This can be extended further.

unittests/Process/Linux/ProcessorTraceTest.cpp

42

Well I see that ArrayRef can be constructed with a nullptr and a positive size, like

ArrayRef(nullptr, 5);

hence I added the that check . If u want I can remove these checks ?

[ravitheja]

source/Plugins/Process/Linux/ProcessorTrace.cpp

159

Yes I did not want to use eErrorTypePOSIX but when transitioning from Status to llvm::Error, the m_code is only retained for eErrorTypePOSIX else its not retained.

[labath]

source/Plugins/Process/Linux/NativeProcessLinux.h

282

Interesting... Is that actually useful for something? I find the semantic of "trace all threads in the process except those that happen to be traced already" a bit confusing.

If you have a use for it in mind then fine, but otherwise, I'd like go for the simpler option of failing the request to trace the whole process if some individual threads are traced already. "you can either trace the whole process *OR* any number of individual threads" sounds much easier to understand. Otherwise, you'd have to think about corner cases like "What if the individual thread trace is stopped but the process trace request remains?" Do you then automatically start tracing the remaining thread based on the previous "whole process trace" request, and so on...

source/Plugins/Process/Linux/ProcessorTrace.cpp

159

That's a good point. When I wrote the conversion function, there was no use case for this -- I think you're the first person who actually want's to use the error codes in some meaningful way.

What is your further plan for these error codes? Judging by the state of the D33035 you won't be able to use them to display the error messages to the user?

If you still want to preserve the error codes, we can definitely make this happen. Actually, llvm::Error makes this even easier, as it allows you to define new error categories in a distributed way. Frankly, I think the your use of the "generic" error category with custom error codes is a bit of a hack. I think the intended usage of the Status class was to define your own ErrorType enum value and tag your errors with that (but that does not scale well to many different sources of error codes).

unittests/Process/Linux/ProcessorTraceTest.cpp

42

Yes, please. That was never the intention of array ref -- it should be used in a way that in always refers to valid memory. Making that assumption will allow you to cut the size of your code in half (both the test and the implementation)

[ravitheja]

Just to make things clear, I will explain a use case

Suppose if we are debugging an application where the main thread spawns a second new thread ->

int main() {
int i = 0;   // user starts tracing on main thread -> gets traceid 1
.....  // Some statements
i++; // Here he starts tracing the whole process -> gets traceid 2.
        // Now traceid=2 represents the tracing instance on the 
        // whole process, meaning all new threads spawned in 
        //  the process will be traced with this id. Although the 
        //main thread is already being traced hence 
        // will not be a part of traceid =2
std::thread (thread_function) // New thread spawned, this will be traced with id =2
.....  // Some statements
        // To obtain the trace data or stop the trace on main thread
       //  the user can simply work with trace id =1 and not specify the
      // thread id anywhere.
        // For threads under the process trace id hood, the thread id needs to be specified for
        // obtaining the trace data or if the tracing needs to be stopped only for that thread.
       // Now if the tracing on the process is switched off then the tracing on the main thread
      // is not affected.
}

Now the set of threads

m_pt_traced_thread_group

will track threads that are being traced under the process trace id. Hence when a thread exits,
we need to update this set accordingly.

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2634

Yes, but the thing is i have to update the Set of threads

m_pt_traced_thread_group

which is tracking how many threads are being traced under the process trace id. The StopTrace calls StopProcessorTracingOnThread eventually which deletes the element from the various containers. I can directly call StopProcessorTracingOnThread instead of StopTrace.

[ravitheja]

source/Plugins/Process/Linux/NativeProcessLinux.h

282

Yes thats what will happen. All threads spawned will be traced unless the tracing on the whole process is stopped.

[ravitheja]

source/Plugins/Process/Linux/ProcessorTrace.cpp

159

My plan is to perhaps implement a way to pass error strings along with the error packets, so that the tool in D33035 can directly use those strings. I guess then I can just use the eErrorTypeGeneric .

So keeping that in mind I can just remove the error codes and replace them everywhere with the strings ? Although the tool will not work unless the error strings are transported .

[labath]

Thanks for spelling that out. However, it still does not sound like a convincing use case to me. Why would the user start to trace just one thread, only to later change his mind and trace the whole process instead. I'm not saying that can't happen, but it seems like something that we should discourage rather than try to support. The messaging I'd give to the user is: "you should trace the whole process unless you have a good reason not to". In case of a single threaded application, it won't make a difference, and if that app suddenly turns multithreaded, it will be traced as well. If he happens to have a single-thread trace running and later chooses to have a full process traced, he can always cancel that one and initiate a full trace instead.

In D33674#782291, @ravitheja wrote:

Just to make things clear, I will explain a use case

Suppose if we are debugging an application where the main thread spawns a second new thread ->

int main() {
int i = 0;   // user starts tracing on main thread -> gets traceid 1
.....  // Some statements
i++; // Here he starts tracing the whole process -> gets traceid 2.
        // Now traceid=2 represents the tracing instance on the 
        // whole process, meaning all new threads spawned in 
        //  the process will be traced with this id. Although the 
        //main thread is already being traced hence 
        // will not be a part of traceid =2

Yes, and this is what makes it confusing to me. At this point trace 2 will not be tracing anything, even though in the comments and the code you use the terms like "process trace" and "trace all threads".

Another thing I find confusing is what would happen if the events were reversed:

1. Start tracing the whole process
2. Ok. Traceid = 1
3. Start tracing thread 47
4. Error. Thread 47 is already traced. (???)

So now tracing a single thread is forbidden if you already have a process trace running, but starting a process trace is ok even if there is a thread trace already. This seems more complicated to grasp than "you cannot trace the full process as you already trace one of it's threads".

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2634

That makes it slightly better, but I still find extremely verbose -- I have to read through 50 lines of error checks, only to execute two lines of code:

m_pt_traced_thread_group.erase(thread);
m_processor_trace_monitor.erase(iter);

It looks like there is some problem with abstractions there.

source/Plugins/Process/Linux/ProcessorTrace.cpp

159

Ok, then let's drop the error enum, and use just strings for now(?) For what it's worth, at least it makes this consisted with the rest of the code base.

[ravitheja]

Although a bit confusing, there is more flexibility for the user.I must also point out that the trace buffer available is not unlimited and there can be situations where a user might simultaneously want to trace newly spawned threads with a smaller buffer and trace an individual thread with perhaps a bigger buffer size. Tracing all threads is definitely important coz the user might not want to separately start tracing on each new thread. Now the current design might be a bit confusing but I am willing to document it well with examples and uses cases.

source/Plugins/Process/Linux/NativeProcessLinux.h

277

Hi, I don't see the advantage of changing to unique pointers ? coz when the process dies they will be destroyed anyhow, plus using shared pointers makes it easier for functions operating with the ProcessTraceMonitor to work.

[ravitheja]

Start tracing the whole process
Ok. Traceid = 1
Start tracing thread 47
Error. Thread 47 is already traced. (???)

The error is because its already being traced, so if someone wants the trace log, its already available and the user need not do anything extra.

I think if we should first sort out the functionality ?

[labath]

In D33674#783861, @ravitheja wrote:

Although a bit confusing, there is more flexibility for the user.I must also point out that the trace buffer available is not unlimited and there can be situations where a user might simultaneously want to trace newly spawned threads with a smaller buffer and trace an individual thread with perhaps a bigger buffer size. Tracing all threads is definitely important coz the user might not want to separately start tracing on each new thread. Now the current design might be a bit confusing but I am willing to document it well with examples and uses cases.

I agree that having the ability to trace new threads from the moment they spawn is important. The confusion arises from the fact that we are treating "trace all threads" and "trace new threads" as a single concept. I think the cleanest solution for me would be to split those up, so that we would have three operations: (1) trace a specific thread, (2) trace new threads, (3) trace all threads. Then (1) and (2) can be combined in arbitrary ways, and (3) is mutually exclusive with anything else. It could be I am over-designing this though. I'd like to hear the opinion of anyone else that is reading this.

source/Plugins/Process/Linux/NativeProcessLinux.h

277

It makes it clear that the Process is the owner of these objects (and not for example "sharing" them with anyone else). Plus you should use the simplest tool that gets the job done and unique_ptr is definitely simpler. So I'd reverse the question: If there is no need for using shared_ptr, why do it?

I disagree with the statement that it makes it harder for the functions to work. Please provide an example.

[ravitheja]

source/Plugins/Process/Linux/NativeProcessLinux.h

277

Ok I was thinking that working with shared_pointers was a bit cleaner approach than working with references or raw pointers, but in this case won't make much of a difference.

I will make them unique pointers. No problem.

[ravitheja]

In D33674#783893, @labath wrote:

In D33674#783861, @ravitheja wrote:

Although a bit confusing, there is more flexibility for the user.I must also point out that the trace buffer available is not unlimited and there can be situations where a user might simultaneously want to trace newly spawned threads with a smaller buffer and trace an individual thread with perhaps a bigger buffer size. Tracing all threads is definitely important coz the user might not want to separately start tracing on each new thread. Now the current design might be a bit confusing but I am willing to document it well with examples and uses cases.

I agree that having the ability to trace new threads from the moment they spawn is important. The confusion arises from the fact that we are treating "trace all threads" and "trace new threads" as a single concept. I think the cleanest solution for me would be to split those up, so that we would have three operations: (1) trace a specific thread, (2) trace new threads, (3) trace all threads. Then (1) and (2) can be combined in arbitrary ways, and (3) is mutually exclusive with anything else. It could be I am over-designing this though. I'd like to hear the opinion of anyone else that is reading this.

Well that's the whole thread group idea. Currently we have only one thread group i.e the process group (all existing un traced threads + newly spawned ones).
With separate "trace all threads" and "trace new threads", it will be multiple thread groups. For e.g

Lets say an application spawns 10 threads, now here we can end up with 9 thread groups in the worst case if the user calls "trace all threads" after each
new spawned thread.

Now I wanted to avoid having multiple thread groups coz the implementation will become more complex.

[labath]

Well that's the whole thread group idea. Currently we have only one thread group i.e the process group (all existing un traced threads + newly spawned ones).
With separate "trace all threads" and "trace new threads", it will be multiple thread groups. For e.g

Lets say an application spawns 10 threads, now here we can end up with 9 thread groups in the worst case if the user calls "trace all threads" after each
new spawned thread.

Now I wanted to avoid having multiple thread groups coz the implementation will become more complex.

That's not exactly how I imagined it. Calling "trace all threads" could still only be done once. You can only combine tracing a single thread and tracing newly created threads. So you could end up with ~10 trace instances, if you start tracing threads one by one, but the user would probably give up after the first two, and just start tracing all future threads in once instance.

However, this talk of groups has interested me. If we phrase it in terms of groups, I think the current behavior makes some sense -- the user can create a "group trace", and this group would contain all current untraced threads, as well as any threads that are created in the future. If noone else has any opinion on this, then I guess we can go with this. We should just make sure to document it and avoid function names like "trace_ALL_threads" and similar. :)

[ravitheja]

Changes for last feedback.

[labath]

I like the direction this is going in, but I see places for more cleanup. The main three items are:

• making destruction cleaner
• avoiding global variables
• making ReadCyclicBuffer readable

the rest are just general nits.

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2548

return process_config.takeError() is two lines shorter

2565

This is still called trace*ALL*threads

2637

the formatting of the bracket is wrong

2723

s/UID/traceid/

source/Plugins/Process/Linux/NativeProcessLinux.h

113

having default argument values on virtual functions is dangerous. Please remove these. AFAIK, noone is going to be calling these through a NativeProcessLinux pointer anyway.

source/Plugins/Process/Linux/ProcessorTrace.cpp

34

Are these functions still useful?

130

You can consider return ret.getError(), which will probably give you a more detailed error message.

149

drop AsCString()

244

drop c_str()

308

I'd like this work to be done in the destructor. Just like nobody should see a constructed-but-not-yet-initialized object, so the destroyed-but-not-destructed state should not exist. Then you don't need to worry about setting the state of the object to some sane values.

In fact, if you utilize std::unique_ptr capabilities, then you may not even need a destructor at all. Just define a class

class munmap_delete {
  size_t m_length;

public:
  munmap_delete(size_t length) : m_length(length) {}
  void operator()(void *ptr) { munmap(ptr, m_length); }
};

and then you can just declare `
std::unique_ptr<perf_event_mmap_page, munmap_delete> mmap_base(mmap_result, munmap_delete(mmap_size))

This way, the allocation and deallocation code are close together, and you don't need to write yourself // we allocated extra page reminders.

Since we store m_aux in a ArrayRef already, this would require duplicating the pointer in the unique_ptr member. I am fine with that, but if you want to avoid it you can just have a getAux() member function which sythesizes the arrayref on demand.

359

ReadCyclicBuffer will handle the case of a zero-sized buffer gracefully, right? You don't need to check the size at every level.

439

buffer = buffer.drop_back(bytes_remaining) ?

476

I think there are waaaaaay too many nested conditionals here. Let's try this implementation for a change:

SmallVector<ArrayRef<uint8_t>, 2> parts{ src.slice(src_cyc_index), src.drop_back(src_cyc_index)};
if (offset >= parts[0].size()) {
  offset -= parts[0].size();
  parts.drop_front();
}
parts[0] = parts[0].drop_front(offset);

That takes care of offset. Now on to the copy.

dst = dst.take_front(size_to_read);
auto next = dst.begin();
for(auto part: parts) {
  size_t chunk_size = std::min(size_to_read, part.size());
  next = std::copy_n(part.begin(), chunk_size, next);
  size_to_read -= chunk_size;
}

Much easier to reason about and less than half the size of the original implementation.

source/Plugins/Process/Linux/ProcessorTrace.h

80

I guess I wasn't too clear about this, but I meant a member variable in the Process class. This solves the "multiple process" issue, but creates a couple of other ones. All the call sites are now more complicated, and you have to worry about managing the lifetime of the entries in this map. If this was a Process member variable, those would be handled automatically (and it makes more sense, since it is the process class that owns these traces instances).

[ravitheja]

source/Plugins/Process/Linux/ProcessorTrace.cpp

308

I can get the desired effect, by moving this function to the destructor, but we won't be able to catch munmap errors. I guess logging should be fine ? right now we were able to report errors in munmap.

[ravitheja]

source/Plugins/Process/Linux/ProcessorTrace.h

80

Ok sorry now I understand what u meant.

[labath]

source/Plugins/Process/Linux/ProcessorTrace.cpp

308

They only way I see that munmap can fail is if you pass it a garbage argument.

On success, munmap() returns 0, on failure -1, and errno is set (probably to EINVAL). :D

That is a programmer error, so I would be fine with even asserting on it, but logging and ignoring is fine as well. It's not like you can do anything better at that point anyway -- you're already ignoring the return value of Destroy...

[ravitheja]

More changes from past feedback.

[labath]

Should I start looking at this or you have more changes planned? I still see manual memory management in the Destroy function...

BTW, what's your testing strategy for this?

source/Plugins/Process/Linux/ProcessorTrace.cpp

393

Isn't the drop-back expression equivalent to src.take_front(src_cyc_index)?

[ravitheja]

Yes you can start looking at it. The thing with munmap stuff is the following, you basically don't want to give the user an opportunity to have an uninitialized or instances which have been destroyed but not deleted.
So I removed the Destroy function from the public space. Now the user need not worry about Destroy function.

Regarding the testing strategy, we have tests at two levels, one at the SB API level and the other at the tool level.

[ravitheja]

source/Plugins/Process/Linux/ProcessorTrace.cpp

393

The problem with that is I don't see the documentation of some functions and I have to dig in the code.
Which is why I did not used it.

[labath]

In D33674#790595, @ravitheja wrote:

Yes you can start looking at it. The thing with munmap stuff is the following, you basically don't want to give the user an opportunity to have an uninitialized or instances which have been destroyed but not deleted.
So I removed the Destroy function from the public space. Now the user need not worry about Destroy function.

Ok, so that was one of my reasons for doing this. The other one is internal code cleanlyness -- it's much easier to verify that the code is healthy by just looking at it when the initialization and deinitialization is close together. unique_ptr allows you to do that. In this code

if ((result = mmap(..., size)) != MAP_FAILED)
  ptr_up = std::unique_ptr(result, mmap_deleter(size));

the init and deinit code is on adjecant line, and it's guaranteed than memory will be freed. Here:

first_function() {
ptr = mmap(..., size);
ref=ArrayRef(ptr, size-1);
...
}

...

second_function() {
  ...
  // Remember to increment size by one
  munmap(ref.data(), ref.size()+1);
  ... 
}

it's not so obvious because the code is far apart and you need to carry state around. To verify correctness I need to look at the two pieces of code, and then find all possible code paths between them.

Regarding the testing strategy, we have tests at two levels, one at the SB API level and the other at the tool level.

Cool, are you going to put some of those tests in this patch?

source/Plugins/Process/Linux/ProcessorTrace.cpp

393

That's fine, there are a lot of these APIs and it's not possible to remember all of them. However, now that you know about the function, wouldn't you agree that it's cleaner to use it?

[ravitheja]

Regarding the testing strategy, we have tests at two levels, one at the SB API level and the other at the tool level.

Cool, are you going to put some of those tests in this patch?

The problem with uploading tests is that they need minimum Broadwell machine and a newer Linux kernel (> 4.2 something )

[ravitheja]

In D33674#790653, @labath wrote:

In D33674#790595, @ravitheja wrote:

Yes you can start looking at it. The thing with munmap stuff is the following, you basically don't want to give the user an opportunity to have an uninitialized or instances which have been destroyed but not deleted.
So I removed the Destroy function from the public space. Now the user need not worry about Destroy function.

Ok, so that was one of my reasons for doing this. The other one is internal code cleanlyness -- it's much easier to verify that the code is healthy by just looking at it when the initialization and deinitialization is close together. unique_ptr allows you to do that. In this code

if ((result = mmap(..., size)) != MAP_FAILED)
  ptr_up = std::unique_ptr(result, mmap_deleter(size));

the init and deinit code is on adjecant line, and it's guaranteed than memory will be freed. Here:

first_function() {
ptr = mmap(..., size);
ref=ArrayRef(ptr, size-1);
...
}

...

second_function() {
  ...
  // Remember to increment size by one
  munmap(ref.data(), ref.size()+1);
  ... 
}

it's not so obvious because the code is far apart and you need to carry state around. To verify correctness I need to look at the two pieces of code, and then find all possible code paths between them.

Ok I will write it like that. Please tell me if u want more changes, I will do them all together for the next patch.

source/Plugins/Process/Linux/ProcessorTrace.cpp

393

Ok I will use that.

[ravitheja]

Removing Destroy() function.

[labath]

I still see some room for improvement, but some of those require some infrastructure improvements, like being able to construct llvm::StringError (or equivalent easily), which I guess will have to wait until I fix those. I think this patch has gone on long enough.

In D33674#790681, @ravitheja wrote:

Regarding the testing strategy, we have tests at two levels, one at the SB API level and the other at the tool level.

Cool, are you going to put some of those tests in this patch?

The problem with uploading tests is that they need minimum Broadwell machine and a newer Linux kernel (> 4.2 something )

Some kind of test is better than nothing, so I would still recommend you go ahead with the test. This wouldn't be the first piece of functionality without a proper test, but I hope you realize it is in your interest to have the functionality tested as much as possible -- people will be refactoring this code, and code around it -- if it's not tested, it will be broken sooner or later. In any case, I am going to leave that up to you.

include/lldb/Host/common/NativeProcessProtocol.h

virtual lldb::user_id_t StartTrace(const TraceOptions &config,
                                      Status &error) {
     error.SetErrorString("Not implemented");
     return LLDB_INVALID_UID;
   }
 
   //------------------------------------------------------------------
   /// StopTracing API as the name suggests stops a tracing instance.
   ///
-  /// @param[in] uid
+  /// @param[in] traceid
   ///     The user id of the trace intended to be stopped. Now a
   ///     user_id may map to multiple threads in which case this API
   ///     could be used to stop the tracing for a specific thread by
   ///     supplying its thread id.
   ///
   /// @param[in] thread
   ///     Thread is needed when the complete process is being traced
   ///     and the user wishes to stop tracing on a particular thread.
   ///
   /// @return
   ///     Status indicating what went wrong.
   //------------------------------------------------------------------
-  virtual Status StopTrace(lldb::user_id_t uid,
+  virtual Status StopTrace(lldb::user_id_t traceid,
                            lldb::tid_t thread = LLDB_INVALID_THREAD_ID) {
     return Status("Not implemented");
   }
 
   //------------------------------------------------------------------
   /// This API provides the trace data collected in the form of raw
   /// data.
   ///
-  /// @param[in] uid thread
-  ///     The uid and thread provide the context for the trace
+  /// @param[in] traceid thread
+  ///     The traceid and thread provide the context for the trace
   ///     instance.
   ///
   /// @param[in] buffer
   ///     The buffer provides the destination buffer where the trace
   ///     data would be read to. The buffer should be truncated to the
   ///     filled length by this function.
   ///
   /// @param[in] offset
   ///     There is possibility to read partially the trace data from
   ///     a specified offset where in such cases the buffer provided
   ///     may be smaller than the internal trace collection container.
   ///
   /// @return
   ///     The size of the data actually read.
   //------------------------------------------------------------------
-  virtual Status GetData(lldb::user_id_t uid, lldb::tid_t thread,
+  virtual Status GetData(lldb::user_id_t traceid, lldb::tid_t thread,
                          llvm::MutableArrayRef<uint8_t> &buffer,
                          size_t offset = 0) {
     return Status("Not implemented");
   }
 
   //------------------------------------------------------------------
   /// Similar API as above except it aims to provide any extra data
   /// useful for decoding the actual trace data.
   //------------------------------------------------------------------
-  virtual Status GetMetaData(lldb::user_id_t uid, lldb::tid_t thread,
+  virtual Status GetMetaData(lldb::user_id_t traceid, lldb::tid_t thread,
                              llvm::MutableArrayRef<uint8_t> &buffer,
                              size_t offset = 0) {
     return Status("Not implemented");
   }
 
   //------------------------------------------------------------------
   /// API to query the TraceOptions for a given user id
   ///
-  /// @param[in] uid
+  /// @param[in] traceid
   ///     The user id of the tracing instance.
   ///
   /// @param[in] config
   ///     The thread id of the tracing instance, in case configuration
   ///     for a specific thread is needed should be specified in the
   ///     config.
   ///
   /// @param[out] error
   ///     Status indicates what went wrong.
   ///
   /// @param[out] config
   ///     The actual configuration being used for tracing.
   //------------------------------------------------------------------
-  virtual Status GetTraceConfig(lldb::user_id_t uid, TraceOptions &config) {
+  virtual Status GetTraceConfig(lldb::user_id_t traceid, TraceOptions &config) {
     return Status("Not implemented");
   }
 
 protected:
   lldb::pid_t m_pid;
 
   std::vector<NativeThreadProtocolSP> m_threads;
   lldb::tid_t m_current_thread_id;

include/lldb/Host/linux/Support.h

 namespace lldb_private {
 
 llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
 getProcFile(::pid_t pid, ::pid_t tid, const llvm::Twine &file);
 
 llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
 getProcFile(::pid_t pid, const llvm::Twine &file);
 
+llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
+getProcFile(const llvm::Twine &file);
+
 } // namespace lldb_private
 
 #endif // #ifndef LLDB_HOST_LINUX_SUPPORT_H

source/Host/linux/Support.cpp

 lldb_private::getProcFile(::pid_t pid, const llvm::Twine &file) {
   Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
   std::string File = ("/proc/" + llvm::Twine(pid) + "/" + file).str();
   auto Ret = llvm::MemoryBuffer::getFileAsStream(File);
   if (!Ret)
     LLDB_LOG(log, "Failed to open {0}: {1}", File, Ret.getError().message());
   return Ret;
 }
+
+llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
+lldb_private::getProcFile(const llvm::Twine &file) {
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+  std::string File = ("/proc/" + file).str();
+  auto Ret = llvm::MemoryBuffer::getFileAsStream(File);
+  if (!Ret)
+    LLDB_LOG(log, "Failed to open {0}: {1}", File, Ret.getError().message());
+  return Ret;
+}

source/Plugins/Process/Linux/CMakeLists.txt

 include_directories(.)
 include_directories(../POSIX)
 include_directories(../Utility)
 
 add_lldb_library(lldbPluginProcessLinux PLUGIN
   NativeProcessLinux.cpp
   NativeRegisterContextLinux.cpp
   NativeRegisterContextLinux_arm.cpp
   NativeRegisterContextLinux_arm64.cpp
   NativeRegisterContextLinux_x86_64.cpp
   NativeRegisterContextLinux_mips64.cpp
   NativeRegisterContextLinux_s390x.cpp
   NativeThreadLinux.cpp
+  ProcessorTrace.cpp
   SingleStepCheck.cpp
 
   LINK_LIBS
     lldbCore
     lldbHost
     lldbSymbol
     lldbTarget
     lldbUtility
     lldbPluginProcessPOSIX
     lldbPluginProcessUtility
   LINK_COMPONENTS
     Support
   )

source/Plugins/Process/Linux/NativeProcessLinux.h

 #include "lldb/Host/Debug.h"
 #include "lldb/Host/HostThread.h"
 #include "lldb/Host/linux/Support.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-types.h"
 
 #include "NativeThreadLinux.h"
+#include "ProcessorTrace.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 
 namespace lldb_private {
 class Status;
 class Scalar;
 
 namespace process_linux {
 /// @class NativeProcessLinux
public:
 
   NativeThreadLinuxSP GetThreadByID(lldb::tid_t id);
 
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
   GetAuxvData() const override {
     return getProcFile(GetID(), "auxv");
   }
 
+  lldb::user_id_t StartTrace(const TraceOptions &config,
+                             Status &error) override;
+
+  Status StopTrace(lldb::user_id_t traceid,
+                   lldb::tid_t thread) override;
+
+  Status GetData(lldb::user_id_t traceid, lldb::tid_t thread,
+                 llvm::MutableArrayRef<uint8_t> &buffer,
+                 size_t offset = 0) override;
+
+  Status GetMetaData(lldb::user_id_t traceid, lldb::tid_t thread,
+                     llvm::MutableArrayRef<uint8_t> &buffer,
+                     size_t offset = 0) override;
+
+  Status GetTraceConfig(lldb::user_id_t traceid, TraceOptions &config) override;
+
   // ---------------------------------------------------------------------
   // Interface used by NativeRegisterContext-derived classes.
   // ---------------------------------------------------------------------
   static Status PtraceWrapper(int req, lldb::pid_t pid, void *addr = nullptr,
                               void *data = nullptr, size_t data_size = 0,
                               long *result = nullptr);
 
   bool SupportHardwareSingleStepping() const;
private:
   std::vector<std::pair<MemoryRegionInfo, FileSpec>> m_mem_region_cache;
 
   lldb::tid_t m_pending_notification_tid;
 
   // List of thread ids stepping with a breakpoint with the address of
   // the relevan breakpoint
   std::map<lldb::tid_t, lldb::addr_t> m_threads_stepping_with_breakpoint;
 
   // ---------------------------------------------------------------------
   // Private Instance Methods
   // ---------------------------------------------------------------------
   NativeProcessLinux();
 
   Status LaunchInferior(MainLoop &mainloop, ProcessLaunchInfo &launch_info);
 
   /// Attaches to an existing process.  Forms the
   /// implementation of Process::DoAttach
   void AttachToInferior(MainLoop &mainloop, lldb::pid_t pid, Status &error);
 
   ::pid_t Attach(lldb::pid_t pid, Status &error);
 
#endif
   Status ResumeThread(NativeThreadLinux &thread, lldb::StateType state,
                       int signo);
 
   void ThreadWasCreated(NativeThreadLinux &thread);
 
   void SigchldHandler();
 
   Status PopulateMemoryRegionCache();
+
+  lldb::user_id_t StartTraceGroup(const TraceOptions &config,
+                                         Status &error);
+
+  // This function is intended to be used to stop tracing
+  // on a thread that exited.
+  Status StopTracingForThread(lldb::tid_t thread);
+
+  // The below function as the name suggests, looks up a ProcessorTrace
+  // instance from the m_processor_trace_monitor map. In the case of
+  // process tracing where the traceid passed would map to the complete
+  // process, it is mandatory to provide a threadid to obtain a trace
+  // instance (since ProcessorTrace is tied to a thread). In the other
+  // scenario that an individual thread is being traced, just the traceid
+  // is sufficient to obtain the actual ProcessorTrace instance.
+  llvm::Expected<ProcessorTraceMonitor &>
+  LookupProcessorTraceInstance(lldb::user_id_t traceid, lldb::tid_t thread);
+
+  // Stops tracing on individual threads being traced. Not intended
+  // to be used to stop tracing on complete process.
+  Status StopProcessorTracingOnThread(lldb::user_id_t traceid,
+                                      lldb::tid_t thread);
+
+  // Intended to stop tracing on complete process.
+  // Should not be used for stopping trace on
+  // individual threads.
+  void StopProcessorTracingOnProcess();
+
+  llvm::DenseMap<lldb::tid_t, ProcessorTraceMonitorUP>
+      m_processor_trace_monitor;
+
+  // Set for tracking threads being traced under
+  // same process user id.
+  llvm::DenseSet<lldb::tid_t> m_pt_traced_thread_group;
+
+  lldb::user_id_t m_pt_proces_trace_id;
+  TraceOptions m_pt_process_trace_config;
 };
 
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef liblldb_NativeProcessLinux_H_

source/Plugins/Process/Linux/NativeProcessLinux.cpp

 
 // -----------------------------------------------------------------------------
 // Public Instance Methods
 // -----------------------------------------------------------------------------
 
 NativeProcessLinux::NativeProcessLinux()
     : NativeProcessProtocol(LLDB_INVALID_PROCESS_ID), m_arch(),
       m_supports_mem_region(eLazyBoolCalculate), m_mem_region_cache(),
-      m_pending_notification_tid(LLDB_INVALID_THREAD_ID) {}
+      m_pending_notification_tid(LLDB_INVALID_THREAD_ID),
+      m_pt_proces_trace_id(LLDB_INVALID_UID) {}
 
 void NativeProcessLinux::AttachToInferior(MainLoop &mainloop, lldb::pid_t pid,
                                           Status &error) {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   LLDB_LOG(log, "pid = {0:x}", pid);
 
   m_sigchld_handle = mainloop.RegisterSignal(
       SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, error);
if (info_err.Fail()) {
                pid, info_err);
       return;
     }
 
     LLDB_LOG(log, "tid {0}, si_code: {1}, si_pid: {2}", pid, info.si_code,
              info.si_pid);
 
     auto thread_sp = AddThread(pid);
+
     // Resume the newly created thread.
     ResumeThread(*thread_sp, eStateRunning, LLDB_INVALID_SIGNAL_NUMBER);
     ThreadWasCreated(*thread_sp);
     return;
   }
 
   // Get details on the signal raised.
   if (info_err.Success()) {
     // We have retrieved the signal info.  Dispatch appropriately.
     if (info.si_signo == SIGTRAP)
       MonitorSIGTRAP(info, *thread_sp);
     else
       MonitorSignal(info, *thread_sp, exited);
   } else {
     if (info_err.GetError() == EINVAL) {
       // This is a group stop reception for this tid.
       // We can reach here if we reinject SIGSTOP, SIGSTP, SIGTTIN or SIGTTOU
       // into the tracee, triggering the group-stop mechanism. Normally
       // receiving these would stop the process, pending a SIGCONT. Simulating
LLDB_LOG(log,
              "tracking the thread.",
              tid);
     // Also a very improbable event.
     return;
   }
 
   LLDB_LOG(log, "pid = {0}: tracking new thread tid {1}", GetID(), tid);
   new_thread_sp = AddThread(tid);
+
   ResumeThread(*new_thread_sp, eStateRunning, LLDB_INVALID_SIGNAL_NUMBER);
   ThreadWasCreated(*new_thread_sp);
 }
 
 void NativeProcessLinux::MonitorSIGTRAP(const siginfo_t &info,
                                         NativeThreadLinux &thread) {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   const bool is_main_thread = (thread.GetID() == GetID());
 
   assert(info.si_signo == SIGTRAP && "Unexpected child signal!");
 
   switch (info.si_code) {
   // TODO: these two cases are required if we want to support tracing of the
   // inferiors' children.  We'd need this to debug a monitor.
Status NativeProcessLinux::Detach() {
 
   for (auto thread_sp : m_threads) {
     Status e = Detach(thread_sp->GetID());
     if (e.Fail())
       error =
           e; // Save the error, but still attempt to detach from other threads.
   }
 
+  m_processor_trace_monitor.clear();
+  m_pt_proces_trace_id = LLDB_INVALID_UID;
+
   return error;
 }
 
 Status NativeProcessLinux::Signal(int signo) {
   Status error;
 
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   LLDB_LOG(log, "sending signal {0} ({1}) to pid {1}", signo,
bool NativeProcessLinux::StopTrackingThread(lldb::tid_t thread_id) {
   for (auto it = m_threads.begin(); it != m_threads.end(); ++it) {
     if (*it && ((*it)->GetID() == thread_id)) {
       m_threads.erase(it);
       found = true;
       break;
     }
   }
 
+  if (found)
+    StopTracingForThread(thread_id);
   SignalIfAllThreadsStopped();
   return found;
 }
 
 NativeThreadLinuxSP NativeProcessLinux::AddThread(lldb::tid_t thread_id) {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
   LLDB_LOG(log, "pid {0} adding thread with tid {1}", GetID(), thread_id);
 
   assert(!HasThreadNoLock(thread_id) &&
          "attempted to add a thread by id that already exists");
 
   // If this is the first thread, save it as the current thread
   if (m_threads.empty())
     SetCurrentThreadID(thread_id);
 
   auto thread_sp = std::make_shared<NativeThreadLinux>(this, thread_id);
   m_threads.push_back(thread_sp);
+
+  if (m_pt_proces_trace_id != LLDB_INVALID_UID) {
+    auto traceMonitor = ProcessorTraceMonitor::Create(
+        GetID(), thread_id, m_pt_process_trace_config, true);
+    if (traceMonitor) {
+      m_pt_traced_thread_group.insert(thread_id);
+      m_processor_trace_monitor.insert(
+          std::make_pair(thread_id, std::move(*traceMonitor)));
+    } else {
+      LLDB_LOG(log, "failed to start trace on thread {0}", thread_id);
+      Status error(traceMonitor.takeError());
+      LLDB_LOG(log, "error {0}", error);
+    }
+  }
+
   return thread_sp;
 }
 
 Status
 NativeProcessLinux::FixupBreakpointPCAsNeeded(NativeThreadLinux &thread) {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_BREAKPOINTS));
 
   Status error;
Status NativeProcessLinux::PtraceWrapper(int req, lldb::pid_t pid, void *addr,
 
   PtraceDisplayBytes(req, data, data_size);
 
   if (error.Fail())
     LLDB_LOG(log, "ptrace() failed: {0}", error);
 
   return error;
 }
+
+llvm::Expected<ProcessorTraceMonitor &>
+NativeProcessLinux::LookupProcessorTraceInstance(lldb::user_id_t traceid,
+                                                 lldb::tid_t thread) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  if (thread == LLDB_INVALID_THREAD_ID && traceid == m_pt_proces_trace_id) {
+    LLDB_LOG(log, "thread not specified: {0}", traceid);
+    return Status("tracing not active thread not specified").ToError();
+  }
+
+  for (auto& iter : m_processor_trace_monitor) {
+    if (traceid == iter.second->GetTraceID() &&
+        (thread == iter.first || thread == LLDB_INVALID_THREAD_ID))
+      return *(iter.second);
+  }
+
+  LLDB_LOG(log, "traceid not being traced: {0}", traceid);
+  return Status("tracing not active for this thread").ToError();
+}
+
+Status NativeProcessLinux::GetMetaData(lldb::user_id_t traceid,
+                                       lldb::tid_t thread,
+                                       llvm::MutableArrayRef<uint8_t> &buffer,
+                                       size_t offset) {
+  TraceOptions trace_options;
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  Status error;
+
+  LLDB_LOG(log, "traceid {0}", traceid);
+
+  auto perf_monitor = LookupProcessorTraceInstance(traceid, thread);
+  if (!perf_monitor) {
+    LLDB_LOG(log, "traceid not being traced: {0}", traceid);
+    buffer = buffer.slice(buffer.size());
+    error = perf_monitor.takeError();
+    return error;
+  }
+  return (*perf_monitor).ReadPerfTraceData(buffer, offset);
+}
+
+Status NativeProcessLinux::GetData(lldb::user_id_t traceid, lldb::tid_t thread,
+                                   llvm::MutableArrayRef<uint8_t> &buffer,
+                                   size_t offset) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  Status error;
+
+  LLDB_LOG(log, "traceid {0}", traceid);
+
+  auto perf_monitor = LookupProcessorTraceInstance(traceid, thread);
+  if (!perf_monitor) {
+    LLDB_LOG(log, "traceid not being traced: {0}", traceid);
+    buffer = buffer.slice(buffer.size());
+    error = perf_monitor.takeError();
+    return error;
+  }
+  return (*perf_monitor).ReadPerfTraceAux(buffer, offset);
+}
+
+Status NativeProcessLinux::GetTraceConfig(lldb::user_id_t traceid,
+                                          TraceOptions &config) {
+  Status error;
+  if (config.getThreadID() == LLDB_INVALID_THREAD_ID &&
+      m_pt_proces_trace_id == traceid) {
+    if (m_pt_proces_trace_id == LLDB_INVALID_UID) {
+      error.SetErrorString("tracing not active for this process");
+      return error;
+    }
+    config = m_pt_process_trace_config;
+  } else {
+    auto perf_monitor =
+        LookupProcessorTraceInstance(traceid, config.getThreadID());
+    if (!perf_monitor) {
+      error = perf_monitor.takeError();
+      return error;
+    }
+    error = (*perf_monitor).GetTraceConfig(config);
+  }
+  return error;
+}
+
+lldb::user_id_t
+NativeProcessLinux::StartTraceGroup(const TraceOptions &config,
+                                           Status &error) {
+
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  if (config.getType() != TraceType::eTraceTypeProcessorTrace)
+    return LLDB_INVALID_UID;
+
+  if (m_pt_proces_trace_id != LLDB_INVALID_UID) {
+    error.SetErrorString("tracing already active on this process");
+    return m_pt_proces_trace_id;
+  }
+
+  for (const auto &thread_sp : m_threads) {
+    if (auto traceInstance = ProcessorTraceMonitor::Create(
+            GetID(), thread_sp->GetID(), config, true)) {
+      m_pt_traced_thread_group.insert(thread_sp->GetID());
+      m_processor_trace_monitor.insert(
+          std::make_pair(thread_sp->GetID(), std::move(*traceInstance)));
+    }
+  }
+
+  m_pt_process_trace_config = config;
+  error = ProcessorTraceMonitor::GetCPUType(m_pt_process_trace_config);
+
+  // Trace on Complete process will have traceid of 0
+  m_pt_proces_trace_id = 0;
+
+  LLDB_LOG(log, "Process Trace ID {0}", m_pt_proces_trace_id);
+  return m_pt_proces_trace_id;
+}
+
+lldb::user_id_t NativeProcessLinux::StartTrace(const TraceOptions &config,
+                                               Status &error) {
+  if (config.getType() != TraceType::eTraceTypeProcessorTrace)
+    return NativeProcessProtocol::StartTrace(config, error);
+
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+
+  lldb::tid_t threadid = config.getThreadID();
+
+  if (threadid == LLDB_INVALID_THREAD_ID)
+    return StartTraceGroup(config, error);
+
+  auto thread_sp = GetThreadByID(threadid);
+  if (!thread_sp) {
+    // Thread not tracked by lldb so don't trace.
+    error.SetErrorString("invalid thread id");
+    return LLDB_INVALID_UID;
+  }
+
+  const auto &iter = m_processor_trace_monitor.find(threadid);
+  if (iter != m_processor_trace_monitor.end()) {
+    LLDB_LOG(log, "Thread already being traced");
+    error.SetErrorString("tracing already active on this thread");
+    return LLDB_INVALID_UID;
+  }
+
+  auto traceMonitor =
+      ProcessorTraceMonitor::Create(GetID(), threadid, config, false);
+  if (!traceMonitor) {
+    error = traceMonitor.takeError();
+    LLDB_LOG(log, "error {0}", error);
+    return LLDB_INVALID_UID;
+  }
+  lldb::user_id_t ret_trace_id = (*traceMonitor)->GetTraceID();
+  m_processor_trace_monitor.insert(
+      std::make_pair(threadid, std::move(*traceMonitor)));
+  return ret_trace_id;
+}
+
+Status NativeProcessLinux::StopTracingForThread(lldb::tid_t thread) {
+  Status error;
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  LLDB_LOG(log, "Thread {0}", thread);
+
+  const auto& iter = m_processor_trace_monitor.find(thread);
+  if (iter == m_processor_trace_monitor.end()) {
+    error.SetErrorString("tracing not active for this thread");
+    return error;
+  }
+
+  if (iter->second->GetTraceID() == m_pt_proces_trace_id) {
+    // traceid maps to the whole process so we have to erase it from the
+    // thread group.
+    LLDB_LOG(log, "traceid maps to process");
+    m_pt_traced_thread_group.erase(thread);
+  }
+  m_processor_trace_monitor.erase(iter);
+
+  return error;
+}
+
+Status NativeProcessLinux::StopTrace(lldb::user_id_t traceid,
+                                     lldb::tid_t thread) {
+  Status error;
+
+  TraceOptions trace_options;
+  trace_options.setThreadID(thread);
+  error = NativeProcessLinux::GetTraceConfig(traceid, trace_options);
+
+  if (error.Fail())
+    return error;
+
+  switch (trace_options.getType()) {
+  case lldb::TraceType::eTraceTypeProcessorTrace:
+    if (traceid == m_pt_proces_trace_id &&
+        thread == LLDB_INVALID_THREAD_ID)
+      StopProcessorTracingOnProcess();
+    else
+      error = StopProcessorTracingOnThread(traceid, thread);
+    break;
+  default:
+    error.SetErrorString("trace not supported");
+    break;
+  }
+
+  return error;
+}
+
+void NativeProcessLinux::StopProcessorTracingOnProcess() {
+  for (auto thread_id_iter : m_pt_traced_thread_group)
+    m_processor_trace_monitor.erase(thread_id_iter);
+  m_pt_traced_thread_group.clear();
+  m_pt_proces_trace_id = LLDB_INVALID_UID;
+}
+
+Status NativeProcessLinux::StopProcessorTracingOnThread(lldb::user_id_t traceid,
+                                                        lldb::tid_t thread) {
+  Status error;
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+
+  if (thread == LLDB_INVALID_THREAD_ID) {
+    for (auto& iter : m_processor_trace_monitor) {
+      if (iter.second->GetTraceID() == traceid) {
+        // Stopping a trace instance for an individual thread
+        // hence there will only be one traceid that can match.
+        m_processor_trace_monitor.erase(iter.first);
+        return error;
+      }
+      LLDB_LOG(log, "Trace ID {0}", iter.second->GetTraceID());
+    }
+
+    LLDB_LOG(log, "Invalid TraceID");
+    error.SetErrorString("invalid trace id");
+    return error;
+  }
+
+  // thread is specified so we can use find function on the map.
+  const auto& iter = m_processor_trace_monitor.find(thread);
+  if (iter == m_processor_trace_monitor.end()) {
+    // thread not found in our map.
+    LLDB_LOG(log, "thread not being traced");
+    error.SetErrorString("tracing not active for this thread");
+    return error;
+  }
+  if (iter->second->GetTraceID() != traceid) {
+    // traceid did not match so it has to be invalid.
+    LLDB_LOG(log, "Invalid TraceID");
+    error.SetErrorString("invalid trace id");
+    return error;
+  }
+
+  LLDB_LOG(log, "UID - {0} , Thread -{1}", traceid, thread);
+
+  if (traceid == m_pt_proces_trace_id) {
+    // traceid maps to the whole process so we have to erase it from the
+    // thread group.
+    LLDB_LOG(log, "traceid maps to process");
+    m_pt_traced_thread_group.erase(thread);
+  }
+  m_processor_trace_monitor.erase(iter);
+
+  return error;
+}

source/Plugins/Process/Linux/ProcessorTrace.h

+//===-- ProcessorTrace.h -------------------------------------- -*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_ProcessorTrace_H_
+#define liblldb_ProcessorTrace_H_
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+
+#include "lldb/Core/TraceOptions.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-types.h"
+
+#include <linux/perf_event.h>
+
+namespace lldb_private {
+
+namespace process_linux {
+
+// ---------------------------------------------------------------------
+// This class keeps track of one tracing instance of
+// Intel(R) Processor Trace on Linux OS. There is a map keeping track
+// of different tracing instances on each thread, which enables trace
+// gathering on a per thread level.
+//
+// The tracing instance is linked with a trace id. The trace id acts like
+// a key to the tracing instance and trace manipulations could be
+// performed using the trace id.
+//
+// The traace id could map to trace instances for a group of threads
+// (spanning to all the threads in the process) or a single thread.
+// The kernel interface for us is the perf_event_open.
+// ---------------------------------------------------------------------
+
+class ProcessorTraceMonitor;
+typedef std::unique_ptr<ProcessorTraceMonitor> ProcessorTraceMonitorUP;
+
+class ProcessorTraceMonitor {
+  int m_fd;
+  perf_event_mmap_page *m_mmap_base;
+  lldb::user_id_t m_traceid;
+  lldb::tid_t m_thread_id;
+
+  llvm::MutableArrayRef<uint8_t> m_data;
+  llvm::MutableArrayRef<uint8_t> m_aux;
+
+  // Counter to track trace instances.
+  static lldb::user_id_t m_trace_num;
+
+  void Destroy();
+
+  void SetTraceID(lldb::user_id_t traceid) { m_traceid = traceid; }
+
+  Status StartTrace(lldb::pid_t pid, lldb::tid_t tid,
+                    const TraceOptions &config);
+
+  ProcessorTraceMonitor()
+      : m_fd(-1),
+        m_mmap_base(nullptr), m_traceid(LLDB_INVALID_UID),
+        m_thread_id(LLDB_INVALID_THREAD_ID){};
+
+public:
+  static Status GetCPUType(TraceOptions &config);
+
+  static llvm::Expected<ProcessorTraceMonitorUP>
+  Create(lldb::pid_t pid, lldb::tid_t tid, const TraceOptions &config,
+      bool useProcessSettings);
+
+  Status ReadPerfTraceAux(llvm::MutableArrayRef<uint8_t> &buffer,
+                          size_t offset = 0);
+
+  Status ReadPerfTraceData(llvm::MutableArrayRef<uint8_t> &buffer,
+                           size_t offset = 0);
+
+  ~ProcessorTraceMonitor() { Destroy(); }
+
+  void SetThreadID(lldb::tid_t tid) { m_thread_id = tid; }
+
+  lldb::tid_t GetThreadID() const { return m_thread_id; }
+
+  lldb::user_id_t GetTraceID() const { return m_traceid; }
+
+  Status GetTraceConfig(TraceOptions &config) const;
+
+  // ---------------------------------------------------------------------
+  /// Read data from a cyclic buffer
+  ///
+  /// @param[in] [out] buf
+  ///     Destination buffer, the buffer will be truncated to written size.
+  ///
+  /// @param[in] src
+  ///     Source buffer which must be a cyclic buffer.
+  ///
+  /// @param[in] src_cyc_index
+  ///     The index pointer (start of the valid data in the cyclic
+  ///     buffer).
+  ///
+  /// @param[in] offset
+  ///     The offset to begin reading the data in the cyclic buffer.
+  // ---------------------------------------------------------------------
+  static void ReadCyclicBuffer(llvm::MutableArrayRef<uint8_t> &dst,
+                               llvm::MutableArrayRef<uint8_t> src,
+                               size_t src_cyc_index, size_t offset);
+};
+} // namespace process_linux
+} // namespace lldb_private
+#endif
 No newline at end of file

source/Plugins/Process/Linux/ProcessorTrace.cpp

+//===-- ProcessorTrace.cpp ------------------------------------ -*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <algorithm>
+#include <fstream>
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/MathExtras.h"
+
+#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
+#include "ProcessorTrace.h"
+#include "lldb/Host/linux/Support.h"
+
+#include <sys/mman.h>
+#include <sys/syscall.h>
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace process_linux;
+using namespace llvm;
+
+lldb::user_id_t ProcessorTraceMonitor::m_trace_num = 1;
+
+Status ProcessorTraceMonitor::GetTraceConfig(TraceOptions &config) const {
+  Status error;
+
+  config.setType(lldb::TraceType::eTraceTypeProcessorTrace);
+  config.setMetaDataBufferSize(m_data.size());
+
+  config.setTraceBufferSize(m_aux.size());
+
+  error = GetCPUType(config);
+
+  return error;
+}
+
+Status ProcessorTraceMonitor::StartTrace(lldb::pid_t pid, lldb::tid_t tid,
+                                         const TraceOptions &config) {
+  Status error;
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  LLDB_LOG(log, "{0}", config.getThreadID());
+
+#ifndef PERF_ATTR_SIZE_VER5
+  llvm_unreachable("perf event not supported");
+#else
+
+  LLDB_LOG(log, "called thread id {0}", tid);
+  uint64_t page_size = getpagesize();
+  uint64_t bufsize = config.getTraceBufferSize();
+  uint64_t metabufsize = config.getMetaDataBufferSize();
+
+  uint64_t numpages = static_cast<uint64_t>(
+      llvm::PowerOf2Floor((bufsize + page_size - 1) / page_size));
+  numpages = std::max(1ul, numpages);
+  bufsize = page_size * numpages;
+
+  numpages = static_cast<uint64_t>(
+      llvm::PowerOf2Floor((metabufsize + page_size - 1) / page_size));
+  numpages = std::max(0ul, numpages);
+  metabufsize = page_size * numpages;
+
+  perf_event_attr attr;
+  memset(&attr, 0, sizeof(attr));
+  attr.size = sizeof(attr);
+  attr.exclude_kernel = 1;
+  attr.sample_type = PERF_SAMPLE_TIME;
+  attr.sample_id_all = 1;
+  attr.exclude_hv = 1;
+  attr.exclude_idle = 1;
+  attr.mmap = 1;
+
+  int intel_pt_type = 0;
+
+  auto ret = llvm::MemoryBuffer::getFileAsStream(
+      "/sys/bus/event_source/devices/intel_pt/type");
+  if (!ret) {
+    LLDB_LOG(log, "failed to open Config file");
+    return ret.getError();
+  }
+
+  StringRef rest = ret.get()->getBuffer();
+  if (rest.empty() || rest.trim().getAsInteger(10, intel_pt_type)) {
+    LLDB_LOG(log, "failed to read Config file");
+    error.SetErrorString("invalid file");
+    return error;
+  }
+
+  rest.trim().getAsInteger(10, intel_pt_type);
+  LLDB_LOG(log, "intel pt type {0}", intel_pt_type);
+  attr.type = intel_pt_type;
+
+  LLDB_LOG(log, "meta buffer size {0}", metabufsize);
+  LLDB_LOG(log, "buffer size {0} ", bufsize);
+
+  if (error.Fail()) {
+    LLDB_LOG(log, "Status in custom config");
+
+    return error;
+  }
+
+  errno = 0;
+  m_fd =
+      syscall(SYS_perf_event_open, &attr, static_cast<::tid_t>(tid), -1, -1, 0);
+  if (m_fd == -1) {
+    LLDB_LOG(log, "syscall error {0}", errno);
+    error.SetErrorString("perf event syscall Failed");
+    return error;
+  }
+
+  errno = 0;
+  auto base =
+      mmap(NULL, (metabufsize + page_size), PROT_WRITE, MAP_SHARED, m_fd, 0);
+  if (base == MAP_FAILED) {
+    base = nullptr;
+    LLDB_LOG(log, "mmap base error {0}", errno);
+    error.SetErrorString("Meta buffer allocation failed");
+    (void)Destroy();
+    return error;
+  }
+  m_mmap_base = reinterpret_cast<perf_event_mmap_page *>(base);
+  m_data = llvm::MutableArrayRef<uint8_t>(
+      (reinterpret_cast<uint8_t *>(m_mmap_base) + m_mmap_base->data_offset),
+      m_mmap_base->data_size);
+
+  m_mmap_base->aux_offset = m_mmap_base->data_offset + m_mmap_base->data_size;
+  m_mmap_base->aux_size = bufsize;
+
+  errno = 0;
+  void *mmap_aux = mmap(NULL, bufsize, PROT_READ, MAP_SHARED, m_fd,
+                        static_cast<long int>(m_mmap_base->aux_offset));
+  if (mmap_aux == MAP_FAILED) {
+    LLDB_LOG(log, "second mmap done {0}", errno);
+    error.SetErrorString("Trace buffer allocation failed");
+    (void)Destroy();
+    return error;
+  }
+  m_aux = llvm::MutableArrayRef<uint8_t>(reinterpret_cast<uint8_t *>(mmap_aux),
+                                         bufsize);
+#endif
+  return error;
+}
+
+Status ProcessorTraceMonitor::GetCPUType(TraceOptions &config) {
+
+  Status error;
+  uint64_t cpu_family = -1;
+  uint64_t model = -1;
+  uint64_t stepping = -1;
+  std::string vendor_id;
+
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+
+  auto BufferOrError = getProcFile("cpuinfo");
+  if (!BufferOrError)
+    return BufferOrError.getError();
+
+  LLDB_LOG(log, "GetCPUType Function");
+
+  StringRef Rest = BufferOrError.get()->getBuffer();
+  while (!Rest.empty()) {
+    StringRef Line;
+    std::tie(Line, Rest) = Rest.split('\n');
+
+    SmallVector<StringRef, 2> columns;
+    Line.split(columns, StringRef(":"), -1, false);
+
+    if (columns.size() < 2)
+      continue; // continue searching
+
+    columns[1] = columns[1].trim(" ");
+    if (columns[0].contains("cpu family") &&
+        columns[1].getAsInteger(10, cpu_family))
+      continue;
+
+    else if (columns[0].contains("model") && columns[1].getAsInteger(10, model))
+      continue;
+
+    else if (columns[0].contains("stepping") &&
+             columns[1].getAsInteger(10, stepping))
+      continue;
+
+    else if (columns[0].contains("vendor_id")) {
+      vendor_id = columns[1].str();
+      if (!vendor_id.empty())
+        continue;
+    }
+    LLDB_LOG(log, "{0}:{1}:{2}:{3}", cpu_family, model, stepping, vendor_id);
+
+    if ((cpu_family != static_cast<uint64_t>(-1)) &&
+        (model != static_cast<uint64_t>(-1)) &&
+        (stepping != static_cast<uint64_t>(-1)) && (!vendor_id.empty())) {
+      auto params_dict = std::make_shared<StructuredData::Dictionary>();
+      params_dict->AddIntegerItem("cpu_family", cpu_family);
+      params_dict->AddIntegerItem("cpu_model", model);
+      params_dict->AddIntegerItem("cpu_stepping", stepping);
+      params_dict->AddStringItem("cpu_vendor", vendor_id);
+
+      llvm::StringRef intel_custom_params_key("intel-pt");
+
+      auto intel_custom_params = std::make_shared<StructuredData::Dictionary>();
+      intel_custom_params->AddItem(
+          intel_custom_params_key,
+          StructuredData::ObjectSP(std::move(params_dict)));
+
+      config.setTraceParams(intel_custom_params);
+      return error; // we are done
+    }
+  }
+
+  error.SetErrorString("cpu info not found");
+  return error;
+}
+
+llvm::Expected<ProcessorTraceMonitorUP>
+ProcessorTraceMonitor::Create(lldb::pid_t pid, lldb::tid_t tid,
+                              const TraceOptions &config,
+                              bool useProcessSettings) {
+
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+
+  Status error;
+  if (tid == LLDB_INVALID_THREAD_ID) {
+    error.SetErrorString("thread not specified");
+    return std::move(error.ToError());
+  }
+
+  ProcessorTraceMonitorUP pt_monitor_up(new ProcessorTraceMonitor);
+
+  error = pt_monitor_up->StartTrace(pid, tid, config);
+  if (error.Fail())
+    return std::move(error.ToError());
+
+  pt_monitor_up->SetThreadID(tid);
+
+  if (useProcessSettings) {
+    pt_monitor_up->SetTraceID(0);
+  } else {
+    pt_monitor_up->SetTraceID(m_trace_num++);
+    LLDB_LOG(log, "Trace ID {0}", m_trace_num);
+  }
+  return std::move(pt_monitor_up);
+}
+
+void ProcessorTraceMonitor::Destroy() {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  int ret;
+#ifndef PERF_ATTR_SIZE_VER5
+  llvm_unreachable("perf event not supported");
+#else
+  if (!m_aux.empty() && (m_mmap_base != nullptr)) {
+    ret = munmap(m_aux.data(), m_aux.size());
+    if (ret != 0) {
+      LLDB_LOG(log, "munmap aux error {0}", errno);
+    } else
+      m_aux.drop_back(m_aux.size());
+  }
+  // Since we allocate one page extra during
+  // initialization for the perf_event_mmap_page header
+  if (m_mmap_base != nullptr) {
+    int page_size = getpagesize();
+    uint64_t data_size = m_data.size() + page_size;
+    ret = munmap(m_mmap_base, data_size);
+    if (ret != 0) {
+      LLDB_LOG(log, "munmap data error {0}", errno);
+    } else {
+      m_mmap_base = nullptr;
+      m_data.drop_back(m_data.size());
+    }
+  }
+  if (m_fd != -1) {
+    ret = close(m_fd);
+    if (ret != 0) {
+      LLDB_LOG(log, "closing file descriptor error {0}", errno);
+    } else
+      m_fd = -1;
+  }
+#endif
+}
+
+Status
+ProcessorTraceMonitor::ReadPerfTraceAux(llvm::MutableArrayRef<uint8_t> &buffer,
+                                        size_t offset) {
+
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  Status error;
+
+#ifndef PERF_ATTR_SIZE_VER5
+  llvm_unreachable("perf event not supported");
+#else
+  uint64_t head = m_mmap_base->aux_head;
+
+  LLDB_LOG(log, "Aux size -{0} , Head - {1}", m_aux.size(), head);
+
+  /**
+   * When configured as ring buffer, the aux buffer keeps wrapping around
+   * the buffer and its not possible to detect how many times the buffer
+   * wrapped. Initially the buffer is filled with zeros,as shown below
+   * so in order to get complete buffer we first copy firstpartsize, followed
+   * by any left over part from beginning to aux_head
+   *
+   * aux_offset [d,d,d,d,d,d,d,d,0,0,0,0,0,0,0,0,0,0,0] aux_size
+   *                 aux_head->||<- firstpartsize  ->|
+   *
+   * */
+
+  ReadCyclicBuffer(buffer, m_aux, static_cast<size_t>(head), offset);
+  LLDB_LOG(log, "ReadCyclic BUffer Done");
+  return error;
+#endif
+}
+
+Status
+ProcessorTraceMonitor::ReadPerfTraceData(llvm::MutableArrayRef<uint8_t> &buffer,
+                                         size_t offset) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+  uint64_t bytes_remaining = buffer.size();
+  Status error;
+#ifndef PERF_ATTR_SIZE_VER5
+  llvm_unreachable("perf event not supported");
+#else
+
+  uint64_t head = m_mmap_base->data_head;
+
+  /*
+   * The data buffer and aux buffer have different implementations
+   * with respect to their definition of head pointer. In the case
+   * of Aux data buffer the head always wraps around the aux buffer
+   * and we don't need to care about it, whereas the data_head keeps
+   * increasing and needs to be wrapped by modulus operator
+   */
+
+  LLDB_LOG(log, "bytes_remaining - {0}", bytes_remaining);
+
+  if (head > m_data.size()) {
+    head = head % m_data.size();
+    LLDB_LOG(log, "Data size -{0} Head - {1}", m_data.size(), head);
+
+    ReadCyclicBuffer(buffer, m_data, static_cast<size_t>(head), offset);
+    bytes_remaining -= buffer.size();
+  } else {
+    LLDB_LOG(log, "Head - {0}", head);
+    if (offset >= head) {
+      LLDB_LOG(log, "Invalid Offset ");
+      error.SetErrorString("invalid offset");
+      buffer = buffer.slice(buffer.size());
+      return error;
+    }
+
+    auto data = m_data.slice(offset, (head - offset));
+    auto remaining = std::copy(data.begin(), data.end(), buffer.begin());
+    bytes_remaining -= (remaining - buffer.begin());
+  }
+  buffer = buffer.drop_back(bytes_remaining);
+  return error;
+#endif
+}
+
+void ProcessorTraceMonitor::ReadCyclicBuffer(
+    llvm::MutableArrayRef<uint8_t> &dst, llvm::MutableArrayRef<uint8_t> src,
+    size_t src_cyc_index, size_t offset) {
+
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
+
+  if (dst.empty() || src.empty()) {
+    dst = dst.drop_back(dst.size());
+    return;
+  }
+
+  if (dst.data() == nullptr || src.data() == nullptr) {
+    dst = dst.drop_back(dst.size());
+    return;
+  }
+
+  if (src_cyc_index > src.size()) {
+    dst = dst.drop_back(dst.size());
+    return;
+  }
+
+  if (offset >= src.size()) {
+    LLDB_LOG(log, "Too Big offset ");
+    dst = dst.drop_back(dst.size());
+    return;
+  }
+
+  llvm::SmallVector<MutableArrayRef<uint8_t>, 2> parts = {
+      src.slice(src_cyc_index), src.drop_back(src.size() - src_cyc_index)};
+
+  if (offset > parts[0].size()) {
+    parts[1] = parts[1].slice(offset - parts[0].size());
+    parts[0] = parts[0].drop_back(parts[0].size());
+  } else if (offset == parts[0].size()) {
+    parts[0] = parts[0].drop_back(parts[0].size());
+  } else {
+    parts[0] = parts[0].slice(offset);
+  }
+  auto next = dst.begin();
+  auto bytes_left = dst.size();
+  for (auto part : parts) {
+    size_t chunk_size = std::min(part.size(), bytes_left);
+    next = std::copy_n(part.begin(), chunk_size, next);
+    bytes_left -= chunk_size;
+  }
+  dst = dst.drop_back(bytes_left);
+}

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

GDBRemoteCommunicationServerLLGS::Handle_jTraceRead(
   else if (packet.ConsumeFront("jTraceMetaRead:"))
     tracetype = MetaData;
   else {
     return SendIllFormedResponse(packet, "jTrace: Ill formed packet ");
   }
 
   lldb::user_id_t uid = LLDB_INVALID_UID;
 
-  size_t byte_count = std::numeric_limits<size_t>::max();
+  uint64_t byte_count = std::numeric_limits<uint64_t>::max();
   lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
-  size_t offset = std::numeric_limits<size_t>::max();
+  uint64_t offset = std::numeric_limits<uint64_t>::max();
 
   auto json_object = StructuredData::ParseJSON(packet.Peek());
 
   if (!json_object ||
       json_object->GetType() != lldb::eStructuredDataTypeDictionary)
     return SendIllFormedResponse(packet, "jTrace: Ill formed packet ");
 
   auto json_dict = json_object->GetAsDictionary();
GDBRemoteCommunicationServerLLGS::Handle_jTraceRead(
 
   if (tracetype == BufferData)
     error = m_debugged_process_sp->GetData(uid, tid, buf, offset);
   else if (tracetype == MetaData)
     error = m_debugged_process_sp->GetMetaData(uid, tid, buf, offset);
 
   if (error.Fail())
     return SendErrorResponse(error.GetError());
 
-  for (size_t i = 0; i < buf.size(); ++i)
-    response.PutHex8(buf[i]);
+  for (auto i : buf)
+    response.PutHex8(i);
 
   StreamGDBRemote escaped_response;
   escaped_response.PutEscapedBytes(response.GetData(), response.GetSize());
   return SendPacketNoLock(escaped_response.GetString());
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_qProcessInfo(

unittests/Process/CMakeLists.txt

 add_subdirectory(gdb-remote)
+add_subdirectory(Linux)
 add_subdirectory(minidump)

unittests/Process/Linux/CMakeLists.txt

+include_directories(${LLDB_SOURCE_DIR}/source/Plugins/Process/Linux)
+
+add_lldb_unittest(ProcessorTraceTest
+  ProcessorTraceTest.cpp
+
+  LINK_LIBS
+    lldbPluginProcessLinux
+  )
 No newline at end of file

unittests/Process/Linux/ProcessorTraceTest.cpp

+//===-- ProcessorTraceMonitorTest.cpp ------------------------- -*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "ProcessorTrace.h"
+#include "llvm/ADT/ArrayRef.h"
+// C Includes
+
+// C++ Includes
+
+using namespace lldb_private;
+using namespace process_linux;
+
+size_t ReadCylicBufferWrapper(void *buf, size_t buf_size, void *cyc_buf,
+                              size_t cyc_buf_size, size_t cyc_start,
+                              size_t offset) {
+  llvm::MutableArrayRef<uint8_t> dst(reinterpret_cast<uint8_t *>(buf),
+                                     buf_size);
+  llvm::MutableArrayRef<uint8_t> src(reinterpret_cast<uint8_t *>(cyc_buf),
+                                     cyc_buf_size);
+  ProcessorTraceMonitor::ReadCyclicBuffer(dst, src, cyc_start, offset);
+  return dst.size();
+}
+
+TEST(CyclicBuffer, EdgeCases) {
+  size_t bytes_read = 0;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  // We will always leave the last bytes untouched
+  // so that string comparisions work.
+  char bigger_buffer[10] = {};
+  char equal_size_buffer[7] = {};
+  char smaller_buffer[4] = {};
+
+  // empty buffer to read into
+  bytes_read = ReadCylicBufferWrapper(smaller_buffer, 0, cyclic_buffer,
+                                      sizeof(cyclic_buffer), 3, 0);
+  ASSERT_EQ(0, bytes_read);
+
+  // empty cyclic buffer
+  bytes_read = ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+                                      cyclic_buffer, 0, 3, 0);
+  ASSERT_EQ(0, bytes_read);
+
+  // bigger offset
+  bytes_read =
+      ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+                             cyclic_buffer, sizeof(cyclic_buffer), 3, 6);
+  ASSERT_EQ(0, bytes_read);
+
+  // wrong offset
+  bytes_read =
+      ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+                             cyclic_buffer, sizeof(cyclic_buffer), 3, 7);
+  ASSERT_EQ(0, bytes_read);
+
+  // wrong start
+  bytes_read =
+      ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+                             cyclic_buffer, sizeof(cyclic_buffer), 3, 7);
+  ASSERT_EQ(0, bytes_read);
+}
+
+TEST(CyclicBuffer, EqualSizeBuffer) {
+  size_t bytes_read = 0;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  char cyclic[] = "cyclic";
+  for (int i = 0; i < sizeof(cyclic); i++) {
+    // We will always leave the last bytes untouched
+    // so that string comparisions work.
+    char equal_size_buffer[7] = {};
+    bytes_read =
+        ReadCylicBufferWrapper(equal_size_buffer, sizeof(cyclic_buffer),
+                               cyclic_buffer, sizeof(cyclic_buffer), 3, i);
+    ASSERT_EQ((sizeof(cyclic) - i - 1), bytes_read);
+    ASSERT_STREQ(equal_size_buffer, (cyclic + i));
+  }
+}
+
+TEST(CyclicBuffer, SmallerSizeBuffer) {
+  size_t bytes_read = 0;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  // We will always leave the last bytes untouched
+  // so that string comparisions work.
+  char smaller_buffer[4] = {};
+  bytes_read =
+      ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+                             cyclic_buffer, sizeof(cyclic_buffer), 3, 0);
+  ASSERT_EQ(3, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "cyc");
+
+  bytes_read =
+      ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+                             cyclic_buffer, sizeof(cyclic_buffer), 3, 1);
+  ASSERT_EQ(3, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "ycl");
+
+  bytes_read =
+      ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+                             cyclic_buffer, sizeof(cyclic_buffer), 3, 2);
+  ASSERT_EQ(3, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "cli");
+
+  bytes_read =
+      ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+                             cyclic_buffer, sizeof(cyclic_buffer), 3, 3);
+  ASSERT_EQ(3, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "lic");
+
+  {
+    char smaller_buffer[4] = {};
+    bytes_read =
+        ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+                               cyclic_buffer, sizeof(cyclic_buffer), 3, 4);
+    ASSERT_EQ(2, bytes_read);
+    ASSERT_STREQ(smaller_buffer, "ic");
+  }
+  {
+    char smaller_buffer[4] = {};
+    bytes_read =
+        ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+                               cyclic_buffer, sizeof(cyclic_buffer), 3, 5);
+    ASSERT_EQ(1, bytes_read);
+    ASSERT_STREQ(smaller_buffer, "c");
+  }
+}
+
+TEST(CyclicBuffer, BiggerSizeBuffer) {
+  size_t bytes_read = 0;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  char cyclic[] = "cyclic";
+  for (int i = 0; i < sizeof(cyclic); i++) {
+    // We will always leave the last bytes untouched
+    // so that string comparisions work.
+    char bigger_buffer[10] = {};
+    bytes_read =
+        ReadCylicBufferWrapper(bigger_buffer, (sizeof(bigger_buffer) - 1),
+                               cyclic_buffer, sizeof(cyclic_buffer), 3, i);
+    ASSERT_EQ((sizeof(cyclic) - i - 1), bytes_read);
+    ASSERT_STREQ(bigger_buffer, (cyclic + i));
+  }
+}