Page MenuHomePhabricator

ravitheja (Ravitheja Addepally)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 22 2015, 4:34 AM (228 w, 3 d)

Recent Activity

Jul 12 2017

ravitheja added a comment to D34945: Adding Support for Error Strings in Remote Packets.

Uploaded SVN Revision Number - 307773 to fix Android Builder.

Jul 12 2017, 11:27 PM
ravitheja committed rL307773: Fixing Android builder.
Fixing Android builder
Jul 12 2017, 4:54 AM
ravitheja committed rL307768: Adding Support for Error Strings in Remote Packets.
Adding Support for Error Strings in Remote Packets
Jul 12 2017, 4:16 AM
ravitheja closed D34945: Adding Support for Error Strings in Remote Packets.
Jul 12 2017, 4:15 AM

Jul 11 2017

ravitheja added inline comments to D34945: Adding Support for Error Strings in Remote Packets.
Jul 11 2017, 12:30 AM

Jul 6 2017

ravitheja updated the diff for D34945: Adding Support for Error Strings in Remote Packets.

Correcting mistakes.

Jul 6 2017, 4:24 AM
ravitheja updated the diff for D34945: Adding Support for Error Strings in Remote Packets.

Support for Hex encoded strings and more error checking.

Jul 6 2017, 1:04 AM

Jul 5 2017

ravitheja updated the diff for D34945: Adding Support for Error Strings in Remote Packets.

Forgot this in the doc.

Jul 5 2017, 8:06 AM
ravitheja updated the diff for D34945: Adding Support for Error Strings in Remote Packets.

Updating Doc and changing feature to be enabled by client.

Jul 5 2017, 8:03 AM

Jul 4 2017

ravitheja added a comment to D34945: Adding Support for Error Strings in Remote Packets.

Yeah that would be broken, as long as we make the error packet more than 3 bytes, then it won't work with the older lldb clients.

Jul 4 2017, 4:45 AM
ravitheja added a comment to D34945: Adding Support for Error Strings in Remote Packets.

With this patch, I see the extra packet to query from server is probably unnecessary. I mean the error code is still there and it should be sufficient for the client to detect an error. As long as it does not mistake it for something else it should not be a problem ? .

Jul 4 2017, 4:18 AM
ravitheja added inline comments to D34945: Adding Support for Error Strings in Remote Packets.
Jul 4 2017, 4:16 AM
ravitheja updated the diff for D34945: Adding Support for Error Strings in Remote Packets.

changes from feedback.

Jul 4 2017, 4:14 AM

Jul 3 2017

ravitheja committed rL307030: Fixing warnings for unused variables and copy ellision.
Fixing warnings for unused variables and copy ellision
Jul 3 2017, 8:08 AM
ravitheja closed D34946: Fixing warnings for unused variables and copy ellision.
Jul 3 2017, 8:07 AM
ravitheja created D34946: Fixing warnings for unused variables and copy ellision.
Jul 3 2017, 7:34 AM
ravitheja added reviewers for D34946: Fixing warnings for unused variables and copy ellision: labath, ted.
Jul 3 2017, 7:34 AM
ravitheja added reviewers for D34945: Adding Support for Error Strings in Remote Packets: labath, jingham, sas, lldb-commits, clayborg.
Jul 3 2017, 7:34 AM
ravitheja created D34945: Adding Support for Error Strings in Remote Packets.
Jul 3 2017, 7:34 AM

Jun 29 2017

ravitheja closed D33595: Fixing broken builds.
Jun 29 2017, 4:57 AM

Jun 28 2017

ravitheja committed rL306520: Linux unit tests should only run on.
Linux unit tests should only run on
Jun 28 2017, 2:01 AM
ravitheja committed rL306516: Implementation of Intel(R) Processor Trace support for Linux.
Implementation of Intel(R) Processor Trace support for Linux
Jun 28 2017, 12:59 AM
ravitheja closed D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 28 2017, 12:58 AM

Jun 27 2017

ravitheja updated the diff for D33674: Implementation of Intel(R) Processor Trace support for Linux.

Removing Destroy() function.

Jun 27 2017, 7:20 AM

Jun 26 2017

ravitheja added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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.

Jun 26 2017, 8:42 AM
ravitheja added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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 )

Jun 26 2017, 8:39 AM
ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 26 2017, 8:01 AM
ravitheja added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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.

Jun 26 2017, 7:59 AM
ravitheja updated the diff for D33674: Implementation of Intel(R) Processor Trace support for Linux.

More changes from past feedback.

Jun 26 2017, 7:15 AM

Jun 22 2017

ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 22 2017, 8:20 AM
ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 22 2017, 8:11 AM

Jun 21 2017

ravitheja updated the diff for D33674: Implementation of Intel(R) Processor Trace support for Linux.

Changes for last feedback.

Jun 21 2017, 4:50 AM

Jun 19 2017

ravitheja added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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.

Jun 19 2017, 6:56 AM
ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 19 2017, 4:53 AM
ravitheja added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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

Jun 19 2017, 4:18 AM
ravitheja added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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.

Jun 19 2017, 4:16 AM

Jun 16 2017

ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 16 2017, 6:37 AM
ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 16 2017, 6:30 AM
ravitheja added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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

Jun 16 2017, 6:27 AM
ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 16 2017, 12:28 AM
ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 16 2017, 12:26 AM

Jun 14 2017

ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 14 2017, 6:49 AM
ravitheja updated the diff for D33674: Implementation of Intel(R) Processor Trace support for Linux.

Changes suggested in previous round of feedback.

Jun 14 2017, 6:48 AM

Jun 7 2017

ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 7 2017, 7:51 AM
ravitheja added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Jun 7 2017, 7:44 AM

May 30 2017

ravitheja updated the summary of D33674: Implementation of Intel(R) Processor Trace support for Linux.
May 30 2017, 5:37 AM
ravitheja updated the summary of D33674: Implementation of Intel(R) Processor Trace support for Linux.
May 30 2017, 5:36 AM
ravitheja created D33674: Implementation of Intel(R) Processor Trace support for Linux.
May 30 2017, 5:33 AM

May 29 2017

ravitheja added a comment to D32585: Implementation of remote packets for Trace data..

Hello, the reason I switched to allocating with new (std::nothrow) uint8_t[byte_count]; was because the byte_count is actually user defined and I do want to catch cases where lldb is not able to allocate the requested buffer size.

May 29 2017, 12:22 AM

May 26 2017

ravitheja added a comment to D33595: Fixing broken builds.

@labath Thanx for fixing the 32 bit builds, I am sorry for the trouble caused.

May 26 2017, 7:31 AM
ravitheja updated the diff for D33595: Fixing broken builds.

Using uint64_t everywhere.

May 26 2017, 7:31 AM
ravitheja committed rL303991: Fixing Memory Leak.
Fixing Memory Leak
May 26 2017, 7:26 AM
ravitheja added reviewers for D33595: Fixing broken builds: labath, clayborg.
May 26 2017, 6:54 AM
ravitheja created D33595: Fixing broken builds.
May 26 2017, 6:52 AM
ravitheja committed rL303972: Implementation of remote packets for Trace data..
Implementation of remote packets for Trace data.
May 26 2017, 4:51 AM
ravitheja closed D32585: Implementation of remote packets for Trace data..
May 26 2017, 4:46 AM

May 24 2017

ravitheja updated the diff for D32585: Implementation of remote packets for Trace data..

Running clang-format.

May 24 2017, 1:45 AM

May 23 2017

ravitheja updated the diff for D32585: Implementation of remote packets for Trace data..

Modifying string literals in Unit tests.

May 23 2017, 5:51 AM
ravitheja added inline comments to D32585: Implementation of remote packets for Trace data..
May 23 2017, 3:17 AM
ravitheja updated the diff for D32585: Implementation of remote packets for Trace data..

Updates from last review.

May 23 2017, 2:58 AM
ravitheja added inline comments to D32585: Implementation of remote packets for Trace data..
May 23 2017, 1:56 AM

May 22 2017

ravitheja added inline comments to D32585: Implementation of remote packets for Trace data..
May 22 2017, 7:26 AM
ravitheja added a comment to D32585: Implementation of remote packets for Trace data..

Sorry I had to update, since the Error class has been changed to Status.

May 22 2017, 12:23 AM
ravitheja updated the diff for D32585: Implementation of remote packets for Trace data..

Adding unit tests and making the packets fully JSON.

May 22 2017, 12:21 AM

May 18 2017

ravitheja added a comment to D32585: Implementation of remote packets for Trace data..

Well nothings preventing me from doing so, I have the changes for that were suggested to me for this revision. I thought I would first upload them, so that people can look at the parallel while I upload the linux server code and Unit tests.

May 18 2017, 5:01 AM
ravitheja added a comment to D32585: Implementation of remote packets for Trace data..
TraceOptions opt;
opt.setType(...);
opt.setBufferSize(...);
opt.setMetaBufferSize(); // with an appropriate TraceOptions constructor, this could be a one-liner
  std::future<user_id_t> result = std::async(std::launch::async, [&] {
    return client.StartTrace(opt, error);
  });
HandlePacket(server, "JTrace:start:type:dead:buffersize:beef:metabuffersize:baad:threadid:f00d:jparams:...", "47");
ASSERT_EQ(47, result.get());
ASSERT_TRUE(error.Success());

This depends on the packet code being in GdbRemoteCommunicationClient and not ProcessGdbRemote as it is now, but moving it there is a good idea anyway -- ProcessGdbRemote should do higher level stuff, packet parsing should be left in the GDBRCClient.

The server side code unfortunately cannot be tested this way, but I believe I will get around to enabling that soon as well.

May 18 2017, 2:31 AM

May 11 2017

ravitheja added a comment to D32585: Implementation of remote packets for Trace data..

I quite like that you have added just the packet plumbing code without an concrete implementation. However, that is still a significant amount of parsing code that should be accompanied by a test. The test suite for the client side of the protocol is ready (TestGdbRemoteCommunicationClient), so I'd like to see at least that.

May 11 2017, 4:47 AM
ravitheja added inline comments to D32585: Implementation of remote packets for Trace data..
May 11 2017, 2:25 AM
ravitheja updated the diff for D32585: Implementation of remote packets for Trace data..

Changes for the feedback recieved in first round of review.

May 11 2017, 2:14 AM

Apr 28 2017

ravitheja added inline comments to D32585: Implementation of remote packets for Trace data..
Apr 28 2017, 12:44 AM

Apr 27 2017

ravitheja added reviewers for D32585: Implementation of remote packets for Trace data.: clayborg, lldb-commits, tberghammer, labath.
Apr 27 2017, 2:07 AM
ravitheja created D32585: Implementation of remote packets for Trace data..
Apr 27 2017, 1:59 AM

Apr 26 2017

ravitheja committed rL301389: Initial implementation of SB APIs for Tracing support..
Initial implementation of SB APIs for Tracing support.
Apr 26 2017, 2:02 AM
ravitheja closed D29581: Initial implementation of SB APIs for Tracing support..
Apr 26 2017, 2:01 AM
ravitheja updated the diff for D29581: Initial implementation of SB APIs for Tracing support..

Fixing few header file inclusions.

Apr 26 2017, 1:59 AM

Apr 20 2017

ravitheja added inline comments to D29581: Initial implementation of SB APIs for Tracing support..
Apr 20 2017, 4:24 AM
ravitheja updated the diff for D29581: Initial implementation of SB APIs for Tracing support..

New diff with requested changes.

Apr 20 2017, 4:22 AM

Apr 3 2017

ravitheja updated the diff for D29581: Initial implementation of SB APIs for Tracing support..

Changes for review.

Apr 3 2017, 6:09 AM

Mar 10 2017

ravitheja added inline comments to D29581: Initial implementation of SB APIs for Tracing support..
Mar 10 2017, 2:27 AM

Mar 7 2017

ravitheja added inline comments to D29581: Initial implementation of SB APIs for Tracing support..
Mar 7 2017, 6:17 AM

Mar 3 2017

ravitheja added inline comments to D29581: Initial implementation of SB APIs for Tracing support..
Mar 3 2017, 5:24 AM
ravitheja updated the diff for D29581: Initial implementation of SB APIs for Tracing support..

Changes according to the comments.

Mar 3 2017, 4:57 AM

Feb 17 2017

ravitheja added a reviewer for D29581: Initial implementation of SB APIs for Tracing support.: lldb-commits.
Feb 17 2017, 2:39 AM

Feb 6 2017

ravitheja added a comment to D29581: Initial implementation of SB APIs for Tracing support..

Hello,

I am working to enable support for Processor Trace in LLDB. Last year I had received recommendations on the lldb mailing list on how this should/could be done. To summarize, the functionality is as follows ->
  • The actual trace gathering and OS specific trace handling is implemented in the server.
  • We have a generic trace abstraction at the SB API and remote packet level.
  • The lldb-server will conform to the trace abstraction so that trace technology and OS specific details can be contained in the lldb-server.
  • The actual trace interpretation will be left to tools existing outside lldb which will use the SB API's to obtain raw trace data and interpret it.
  • The trace architecture provides the possibility of adding support for multiple trace types without lldb having to interpret it (this also prevents lldb code base to bloat up due to support of several trace technologies).
Feb 6 2017, 2:43 AM
ravitheja added reviewers for D29581: Initial implementation of SB APIs for Tracing support.: clayborg, k8stone.
Feb 6 2017, 2:28 AM
ravitheja created D29581: Initial implementation of SB APIs for Tracing support..
Feb 6 2017, 2:26 AM

Nov 3 2016

ravitheja committed rL285885: Test for YMMRegisters..
Test for YMMRegisters.
Nov 3 2016, 1:45 AM
ravitheja closed D26242: Test for YMMRegisters. by committing rL285885: Test for YMMRegisters..
Nov 3 2016, 1:45 AM

Nov 2 2016

ravitheja added reviewers for D26242: Test for YMMRegisters.: tberghammer, clayborg, zturner.
Nov 2 2016, 6:06 AM
ravitheja retitled D26242: Test for YMMRegisters. from to Test for YMMRegisters..
Nov 2 2016, 6:03 AM

Jul 7 2016

ravitheja committed rL274750: Fix for PrintStackTraces.
Fix for PrintStackTraces
Jul 7 2016, 6:07 AM
ravitheja closed D21221: Fix for PrintStackTraces.
Jul 7 2016, 6:07 AM

Jul 6 2016

ravitheja updated the diff for D21221: Fix for PrintStackTraces.

Removing other files.

Jul 6 2016, 1:28 AM

Jun 27 2016

ravitheja updated the diff for D21221: Fix for PrintStackTraces.

Renaming testcase

Jun 27 2016, 12:36 AM

Jun 23 2016

ravitheja updated the diff for D21221: Fix for PrintStackTraces.

Adding testcase

Jun 23 2016, 5:48 AM

Jun 15 2016

ravitheja added a comment to D21221: Fix for PrintStackTraces.

@jasonmolenda The approach suggested seems promising, I look forward to it, if u want I can test it at my end ? once I see it. But I must say this patch also fixes the row offset calculation, so the unwindplan specified in the eh_frame plan is specified for the complete PLT section and not w.r.t the function symbol, so the offset used for picking the row from the unwindplan should be calculated taking into consideration this point.

Jun 15 2016, 7:45 AM

Jun 14 2016

ravitheja added a comment to D21221: Fix for PrintStackTraces.

@labath In order to reproduce this situation without the help of standard library, I would have to write handwritten assembly and the CFI directives for that, is that fine ?

Jun 14 2016, 2:43 AM
ravitheja added a comment to D21221: Fix for PrintStackTraces.

so regarding this particular situation I want to give little more insight ->
It starts out from here

0x40143a <+346>: movabsq $0x403e32, %rdi           ; imm = 0x403E32 
0x401444 <+356>: movb   $0x0, %al
0x401446 <+358>: callq  0x400d30                  ; symbol stub for: printf
0x40144b <+363>: movq   0x6071c0, %rdi
0x401453 <+371>: movl   %eax, -0xdc(%rbp)
0x401459 <+377>: callq  0x400ed0                  ; symbol stub for: fflush  (jump to next disassembly block 1)
0x40145e <+382>: movl   $0x40, %esi
0x401463 <+387>: leaq   -0xb0(%rbp), %rdi
0x40146a <+394>: movq   0x607158, %rdx
0x401472 <+402>: movl   %eax, -0xe0(%rbp)
Jun 14 2016, 1:59 AM
ravitheja added inline comments to D21221: Fix for PrintStackTraces.
Jun 14 2016, 1:44 AM

Jun 13 2016

ravitheja updated the diff for D21221: Fix for PrintStackTraces.

Checks for nullptr

Jun 13 2016, 8:55 AM