Page MenuHomePhabricator

abhishek.aggarwal (Abhishek)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 5 2015, 7:27 AM (235 w, 1 h)

Recent Activity

Aug 7 2017

abhishek.aggarwal closed D33035: Tool for using Intel(R) Processor Trace hardware feature.
Aug 7 2017, 8:26 AM
abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

I was out on vacation, sorry for the delay. Looks good.

Aug 7 2017, 8:21 AM
abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Hi Greg .. Can you please review the changes? Please let me know if something needs to be changed. Thanks :)

Aug 7 2017, 2:54 AM

Jul 26 2017

abhishek.aggarwal updated the diff for D33035: Tool for using Intel(R) Processor Trace hardware feature.

Changes to make a C++ API work in python modules

  • Added a new python typemap to handle it
Jul 26 2017, 4:51 AM
abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

I am not sure how sensitive typemaps are to the names of the arguments? Maybe we can just have you use different names and then the two typemaps won't collide? You can also write some manual code to do the remapping without typemaps.

Regardless of whether you are going to change the API or not, you will need to write something to make it work in python.

const uint8_t * ptdecoder_private:: Instruction::GetRawBytes() const;

What does swig do right now when the above code is converted to python? It needs to make a python string from this and the GetRawBytesSize()...

Jul 26 2017, 4:51 AM

Jul 20 2017

abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Much better on the API with Swig. Just a few inlined questions around GetRawBytes.

Jul 20 2017, 9:13 AM

Jul 6 2017

abhishek.aggarwal updated the diff for D33035: Tool for using Intel(R) Processor Trace hardware feature.

Removed std::vector<> from public APIs

Jul 6 2017, 8:17 AM
abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

So std::vector shouldn't be used in a public API. You should make a class that wraps your objects. LLDB's public API has lldb::SBInstruction and lldb::SBInstructionList as examples. std::vector on other systems compiles differently for debug and release builds and things will crash.

Hi Greg .. Does it go for the tools built on top of LLDB (e.g. in my case where feature library is not a part of liblldb shared lib but built on top of it)? If yes, then I will proceed to remove std::vector from C++ public API of the tool and create a new class for it.

It really comes down to the issue of how stable you want this API to be. If this API is a "you are expected to rebuild any tools you make against this API every time we release a new version" then you can really do anything you want. If you want a stable API to build against, you could choose to follow the LLDB public API rules: no inheritance, fixed number of member variables that never change (which usually means a std::shared_ptr<T>, std::unique_ptr<T> or a T*), and no virtual functions. This effectively renders the C++ API into a C API, but it does force you to have your public API classes have an internal implementation (T) and then the class that you export for your API be the public facing API that everyone uses externally. The nice thing about this is that swig has a really easy time creating all of the script bindings for you if you do it this way. So it really depends on what your future use case of this API is.

If you need this via swig for internal kind of stuff, then use a typemap where you map std::vector<T> to a list() with T instances in it in python.

I want to provide a python interface for the tool's C++ public API as well so that the API can be used in python modules as well. Therefore, I think typemapping to list() will not solve the problem. Am I right?

I believe it would fix your issue for you. You write the glue code that can translate any argument (std::vector<ptdecoder::PTInstruction>) into something more python like like a list of PTInstruction python objects. We do this in a lot of places with LLDB.

I am a bit confused here as well. Are we exporting a plug-in specific python bindings for the Intel PT data? It seems like it would be nice to wrap this API into the SBTrace or other lldb interface? Am I not understanding this correctly?

Probably, I didn't understand this comment of yours. We are not exporting python binding for Intel PT data. It is a python binding for Tool's C++ API and this Tool is built on top of LLDB. Did I answer your question? Please let me know if I misunderstood your comment.

Now I understand. I didn't realize this was a stand alone tool. The main thing to figure out is how stable you want this C++ API that you are exporting to be. If that isn't an issue, feel free to use anything you want in that API. If stability is an issue you want to solve and you want to vend a C++ API, you might think about adopting LLDB's public API rules to help ensure stability.

Jul 6 2017, 8:16 AM

Jul 5 2017

abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

So std::vector shouldn't be used in a public API. You should make a class that wraps your objects. LLDB's public API has lldb::SBInstruction and lldb::SBInstructionList as examples. std::vector on other systems compiles differently for debug and release builds and things will crash.

Hi Greg .. Does it go for the tools built on top of LLDB (e.g. in my case where feature library is not a part of liblldb shared lib but built on top of it)? If yes, then I will proceed to remove std::vector from C++ public API of the tool and create a new class for it.

If you need this via swig for internal kind of stuff, then use a typemap where you map std::vector<T> to a list() with T instances in it in python.

I want to provide a python interface for the tool's C++ public API as well so that the API can be used in python modules as well. Therefore, I think typemapping to list() will not solve the problem. Am I right?

I am a bit confused here as well. Are we exporting a plug-in specific python bindings for the Intel PT data? It seems like it would be nice to wrap this API into the SBTrace or other lldb interface? Am I not understanding this correctly?

Probably, I didn't understand this comment of yours. We are not exporting python binding for Intel PT data. It is a python binding for Tool's C++ API and this Tool is built on top of LLDB. Did I answer your question? Please let me know if I misunderstood your comment.

Jul 5 2017, 4:30 AM

Jul 4 2017

abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Hi Pavel .. I could remove all exception handling code from source files. However, I am still wondering how to disable exception handling while providing python functions for C++ APIs of the features. Can you suggest something here? The whole problem is occurring because of the use of vector<> as an argument to one of the C++ APIs. To provide a python interface for these APIs, I am forced to include std_vector.i (please see tools/intel-features/intel-pt/interface/PTDecoder.i file) which is introducing exception handling code.

Hm... that's a bit of a drag. I guess the SB API never ran into this problem because it always provides it's own vector-like classes (SBModuleList, SBFileSpecList, etc.). I guess the most canonical way would be to follow that example and have your own class for a list of instructions. However, that is a bit annoying, so I do see a case for making code generated by swig as an exception to the rule.

Jul 4 2017, 7:20 AM
abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Hi Pavel .. I could remove all exception handling code from source files. However, I am still wondering how to disable exception handling while providing python functions for C++ APIs of the features. Can you suggest something here? The whole problem is occurring because of the use of vector<> as an argument to one of the C++ APIs. To provide a python interface for these APIs, I am forced to include std_vector.i (please see tools/intel-features/intel-pt/interface/PTDecoder.i file) which is introducing exception handling code.

Jul 4 2017, 6:19 AM
abhishek.aggarwal updated the diff for D33035: Tool for using Intel(R) Processor Trace hardware feature.

Removed exception handling code

Jul 4 2017, 6:13 AM

Jun 29 2017

abhishek.aggarwal added inline comments to D33035: Tool for using Intel(R) Processor Trace hardware feature.
Jun 29 2017, 10:24 AM
abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Thanks for your review Pavel. My comments are inlined. Let me know your opinion :)

Jun 29 2017, 6:15 AM

Jun 28 2017

abhishek.aggarwal updated the diff for D33035: Tool for using Intel(R) Processor Trace hardware feature.

Fixed one minor thing in CMakeFile

Jun 28 2017, 8:40 AM
abhishek.aggarwal updated the diff for D33035: Tool for using Intel(R) Processor Trace hardware feature.

Cmake files related changes

  • Using lldb's cmake functions insted of vanilla ones for cmake files
Jun 28 2017, 3:28 AM
abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Hi Pavel .. I have made the changes you suggested. My apologies for misinterpreting your previous comments but during written communications, it is sometimes difficult to interpret everything correctly. I have tried following LLDB's coding conventions and guidelines. Please let me know if I still missed things that you would have liked to see in this patch. Thanks for your patience :)

Jun 28 2017, 3:26 AM

Jun 26 2017

abhishek.aggarwal updated the diff for D33035: Tool for using Intel(R) Processor Trace hardware feature.

Changes after feedback

Jun 26 2017, 5:05 AM
abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Hi Pavel .. My comments are inlined. Please let me know if you have more concerns. I am submitting the patch with all the proposed changes.

Jun 26 2017, 5:02 AM

Jun 19 2017

abhishek.aggarwal updated subscribers of D33035: Tool for using Intel(R) Processor Trace hardware feature.
Jun 19 2017, 4:40 AM
abhishek.aggarwal updated the summary of D33035: Tool for using Intel(R) Processor Trace hardware feature.
Jun 19 2017, 4:37 AM
abhishek.aggarwal updated the summary of D33035: Tool for using Intel(R) Processor Trace hardware feature.
Jun 19 2017, 4:36 AM
abhishek.aggarwal updated the summary of D33035: Tool for using Intel(R) Processor Trace hardware feature.
Jun 19 2017, 4:36 AM
abhishek.aggarwal updated the summary of D33035: Tool for using Intel(R) Processor Trace hardware feature.
Jun 19 2017, 4:35 AM
abhishek.aggarwal updated the diff for D33035: Tool for using Intel(R) Processor Trace hardware feature.

Single feature library development for all intel specific features

Jun 19 2017, 4:26 AM

May 29 2017

abhishek.aggarwal added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

I don't really like that we are adding a public shared library for every tiny intel feature. Could we at least merge this "plugin" with the existing "intel-mpx plugin" to create one "intel support" library?

Also, adding an external dependency probably deserves a discussion on lldb-dev.

May 29 2017, 6:28 AM
abhishek.aggarwal closed D33434: Added new API to SBStructuredData class.
May 29 2017, 1:26 AM

May 26 2017

abhishek.aggarwal updated the summary of D33434: Added new API to SBStructuredData class.
May 26 2017, 2:24 AM
abhishek.aggarwal updated the diff for D33434: Added new API to SBStructuredData class.

Updating D33434: Added new API to SBStructuredData class

  • Removed inferior from test case (not required)
  • fixed enum scope
May 26 2017, 2:21 AM
abhishek.aggarwal added a comment to D33434: Added new API to SBStructuredData class.

Thanks for your suggestions. I have made changes according to feedback and submitting it here.

May 26 2017, 2:19 AM

May 24 2017

abhishek.aggarwal added a comment to D33434: Added new API to SBStructuredData class.

My comments are inlined. Please let me know if something still needs to be changed.

May 24 2017, 8:57 AM
abhishek.aggarwal updated the diff for D33434: Added new API to SBStructuredData class.

Updating D33434: Added new API to SBStructuredData class

May 24 2017, 8:56 AM

May 23 2017

abhishek.aggarwal added reviewers for D33434: Added new API to SBStructuredData class: clayborg, lldb-commits.
May 23 2017, 3:06 AM
abhishek.aggarwal created D33434: Added new API to SBStructuredData class.
May 23 2017, 3:04 AM

May 15 2017

abhishek.aggarwal added inline comments to D33035: Tool for using Intel(R) Processor Trace hardware feature.
May 15 2017, 2:10 AM

May 10 2017

abhishek.aggarwal added reviewers for D33035: Tool for using Intel(R) Processor Trace hardware feature: clayborg, jingham, lldb-commits, labath.
May 10 2017, 6:05 AM
abhishek.aggarwal abandoned D33034: Tool for using Intel(R) Processor Trace hardware feature.

Abandoning this change due to formatting problem with commit message.

May 10 2017, 6:04 AM
abhishek.aggarwal created D33035: Tool for using Intel(R) Processor Trace hardware feature.
May 10 2017, 6:03 AM
abhishek.aggarwal updated the summary of D33034: Tool for using Intel(R) Processor Trace hardware feature.
May 10 2017, 6:01 AM
abhishek.aggarwal updated the summary of D33034: Tool for using Intel(R) Processor Trace hardware feature.
May 10 2017, 6:00 AM
abhishek.aggarwal updated the summary of D33034: Tool for using Intel(R) Processor Trace hardware feature.
May 10 2017, 6:00 AM
abhishek.aggarwal updated the summary of D33034: Tool for using Intel(R) Processor Trace hardware feature.
May 10 2017, 5:58 AM
abhishek.aggarwal updated the summary of D33034: Tool for using Intel(R) Processor Trace hardware feature.
May 10 2017, 5:57 AM
abhishek.aggarwal updated the diff for D33034: Tool for using Intel(R) Processor Trace hardware feature.

#Updating D33034: Tool for using Intel(R) Processor Trace hardware feature

  1. Enter a brief description of the changes included in this update.
  2. The first line is used as subject, next lines as comment. Commit message formatting changes
May 10 2017, 5:54 AM
abhishek.aggarwal added reviewers for D33034: Tool for using Intel(R) Processor Trace hardware feature: jingham, labath.
May 10 2017, 5:42 AM
abhishek.aggarwal added reviewers for D33034: Tool for using Intel(R) Processor Trace hardware feature: clayborg, lldb-commits.
May 10 2017, 5:37 AM
abhishek.aggarwal created D33034: Tool for using Intel(R) Processor Trace hardware feature.
May 10 2017, 5:35 AM

Sep 9 2016

abhishek.aggarwal closed D24251: LLDB: API for Permission of object file's sections.

Already merged it in LLDb repo. So closing this revision.

Sep 9 2016, 1:47 AM

Sep 8 2016

abhishek.aggarwal updated the diff for D24251: LLDB: API for Permission of object file's sections.

Removed get() for shared_ptr

Sep 8 2016, 4:40 AM

Sep 7 2016

abhishek.aggarwal updated the diff for D24251: LLDB: API for Permission of object file's sections.

Changes based on review

Sep 7 2016, 2:51 AM

Sep 6 2016

abhishek.aggarwal added reviewers for D24251: LLDB: API for Permission of object file's sections: clayborg, jingham, labath, lldb-commits.
Sep 6 2016, 4:46 AM
abhishek.aggarwal retitled D24251: LLDB: API for Permission of object file's sections from to LLDB: API for Permission of object file's sections.
Sep 6 2016, 4:44 AM

Jul 29 2016

abhishek.aggarwal closed D22863: Improve code of loading plugins that provide cmnds.
Jul 29 2016, 12:54 AM

Jul 28 2016

abhishek.aggarwal updated the diff for D22863: Improve code of loading plugins that provide cmnds.

New patch according to review feedback

Jul 28 2016, 3:48 AM
abhishek.aggarwal added inline comments to D22863: Improve code of loading plugins that provide cmnds.
Jul 28 2016, 3:27 AM
abhishek.aggarwal added a comment to D22863: Improve code of loading plugins that provide cmnds.

Hi Greg

Jul 28 2016, 3:08 AM

Jul 27 2016

abhishek.aggarwal added reviewers for D22863: Improve code of loading plugins that provide cmnds: clayborg, granata.enrico, jingham.
Jul 27 2016, 7:03 AM
abhishek.aggarwal retitled D22863: Improve code of loading plugins that provide cmnds from to Improve code of loading plugins that provide cmnds.
Jul 27 2016, 7:02 AM

Feb 1 2016

abhishek.aggarwal added a comment to D16767: Fix single-stepping onto a breakpoint.

source/Plugins/Process/FreeBSD/FreeBSDThread.cpp will not compile for FreeBSD. In Line 576, bp_id is undefined. Please replace it with bp_site_sp->GetID()

Feb 1 2016, 5:35 AM
abhishek.aggarwal closed D16720: Set correct ThreadStopInfo in case of trace event.
Feb 1 2016, 1:05 AM

Jan 29 2016

abhishek.aggarwal added reviewers for D16720: Set correct ThreadStopInfo in case of trace event: clayborg, jingham, ovyalov, emaste, lldb-commits.
Jan 29 2016, 5:31 AM
abhishek.aggarwal retitled D16720: Set correct ThreadStopInfo in case of trace event from to Set correct ThreadStopInfo in case of trace event.
Jan 29 2016, 5:29 AM

Dec 2 2015

abhishek.aggarwal closed D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior.
Dec 2 2015, 1:43 AM

Dec 1 2015

abhishek.aggarwal added a comment to D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior.

Comments Inlined.

Dec 1 2015, 6:23 AM
abhishek.aggarwal added a comment to D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior.

Hi Pavel and Tamas

Dec 1 2015, 4:06 AM
abhishek.aggarwal updated the diff for D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior.

Added Loggings

Dec 1 2015, 4:03 AM
abhishek.aggarwal added a comment to D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior.

Hello Greg

Dec 1 2015, 2:09 AM

Nov 30 2015

abhishek.aggarwal updated the diff for D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior.

Removed assert if ptrace API fails

Nov 30 2015, 4:13 AM

Nov 27 2015

abhishek.aggarwal added reviewers for D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior: clayborg, ovyalov, jingham, lldb-commits.
Nov 27 2015, 6:27 AM
abhishek.aggarwal retitled D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior from to PTRACE ABI to read FXSAVE area for 32-bit inferior.
Nov 27 2015, 6:26 AM

Nov 13 2015

abhishek.aggarwal closed D14226: Fix to solve Bug 23139 & Bug 23560.
Nov 13 2015, 2:50 AM

Nov 12 2015

abhishek.aggarwal added a comment to D14226: Fix to solve Bug 23139 & Bug 23560.

Hello Jason

Nov 12 2015, 12:58 AM
abhishek.aggarwal updated the diff for D14226: Fix to solve Bug 23139 & Bug 23560.

Removed log and inserted statement terminator

Nov 12 2015, 12:56 AM
abhishek.aggarwal updated the diff for D14226: Fix to solve Bug 23139 & Bug 23560.

Used Assert instead of If condition

Nov 12 2015, 12:51 AM

Nov 2 2015

abhishek.aggarwal added reviewers for D14226: Fix to solve Bug 23139 & Bug 23560: clayborg, jingham, jasonmolenda.
Nov 2 2015, 5:07 AM
abhishek.aggarwal retitled D14226: Fix to solve Bug 23139 & Bug 23560 from to Fix to solve Bug 23139 & Bug 23560.
Nov 2 2015, 5:02 AM

Oct 12 2015

abhishek.aggarwal closed D13587: X86: Change FTAG register size in FXSAVE structure.
Oct 12 2015, 2:58 AM

Oct 9 2015

abhishek.aggarwal added reviewers for D13587: X86: Change FTAG register size in FXSAVE structure: clayborg, ovyalov, jingham.
Oct 9 2015, 5:19 AM
abhishek.aggarwal retitled D13587: X86: Change FTAG register size in FXSAVE structure from to X86: Change FTAG register size in FXSAVE structure.
Oct 9 2015, 5:18 AM

Oct 6 2015

abhishek.aggarwal added a comment to D13434: Bug 25050: X87 FPU Special Purpose Registers.

I thought that the patch can be landed even if one of the reviewers accepts it. Is it so that the patch requires approval from all the reviewers? If that is the case then I am extremely sorry to land it without your approval.

Oct 6 2015, 7:35 AM
abhishek.aggarwal closed D13434: Bug 25050: X87 FPU Special Purpose Registers.
Oct 6 2015, 12:05 AM
abhishek.aggarwal added a comment to D13434: Bug 25050: X87 FPU Special Purpose Registers.

A test was laready present where registers were loaded from lldb command terminal with specific values and then read to compare the values. However, the test still passed because lldb wrote the register values at wrong offsets, but then it read the register values from the same wrong offsets also.

Oct 6 2015, 12:04 AM

Oct 5 2015

abhishek.aggarwal added reviewers for D13434: Bug 25050: X87 FPU Special Purpose Registers: emaste, mikesart, clayborg.
Oct 5 2015, 7:22 AM
abhishek.aggarwal retitled D13434: Bug 25050: X87 FPU Special Purpose Registers from to Bug 25050: X87 FPU Special Purpose Registers.
Oct 5 2015, 7:19 AM

Sep 9 2015

abhishek.aggarwal added a comment to D12677: Bug 24733: TestRegisters.py for Clang inferiors.

I reverted the CL because it was causing TestRegisters.test_fp_special_purpose_register_read to fail on OSX:

  • stop reason = EXC_BREAKPOINT
  • "register read ftag" yields 0x80 instead of expected 0x8000
Sep 9 2015, 3:53 AM

Sep 8 2015

abhishek.aggarwal closed D12677: Bug 24733: TestRegisters.py for Clang inferiors.

Couldn't land this patch by arc due to merge conflicts. Hence merging it manually and closing this revision.

Sep 8 2015, 3:24 AM

Sep 7 2015

abhishek.aggarwal updated the diff for D12677: Bug 24733: TestRegisters.py for Clang inferiors.

Clang/GCC generate different assembly for same inferior.

Sep 7 2015, 8:44 AM
abhishek.aggarwal added inline comments to D12677: Bug 24733: TestRegisters.py for Clang inferiors.
Sep 7 2015, 8:25 AM
abhishek.aggarwal added a reviewer for D12677: Bug 24733: TestRegisters.py for Clang inferiors: labath.
Sep 7 2015, 7:17 AM
abhishek.aggarwal retitled D12677: Bug 24733: TestRegisters.py for Clang inferiors from to Bug 24733: TestRegisters.py for Clang inferiors.
Sep 7 2015, 7:16 AM

Sep 4 2015

abhishek.aggarwal added a comment to D12592: Bug 24457 - X87 FPU Special Purpose Registers.

Overall LGTM. I'd put the offset calculation into an inline function.

It sounds like you gave it a test on a 32-bit process running against a 64-bit OS? (I'm not sure if we're still using the x86_64 register contexts on 32-bit processes running on a 64-bit host these days, but it would be a good check).

Sep 4 2015, 2:32 AM
abhishek.aggarwal updated the diff for D12592: Bug 24457 - X87 FPU Special Purpose Registers.

Created a data member in the class to store byte offset of fctrl

Sep 4 2015, 2:27 AM
abhishek.aggarwal closed D12595: SysV ABI for i386 Architecture.
Sep 4 2015, 12:45 AM

Sep 3 2015

abhishek.aggarwal added reviewers for D12595: SysV ABI for i386 Architecture: clayborg, jingham.
Sep 3 2015, 4:27 AM
abhishek.aggarwal retitled D12595: SysV ABI for i386 Architecture from to SysV ABI for i386 Architecture.
Sep 3 2015, 4:25 AM
abhishek.aggarwal added reviewers for D12592: Bug 24457 - X87 FPU Special Purpose Registers: clayborg, tfiala, ashok.thirumurthi, granata.enrico.
Sep 3 2015, 2:55 AM
abhishek.aggarwal retitled D12592: Bug 24457 - X87 FPU Special Purpose Registers from to Bug 24457 - X87 FPU Special Purpose Registers.
Sep 3 2015, 2:53 AM

Jul 6 2015

abhishek.aggarwal closed D10308: Use both OS and Architecture to choose correct ABI.
Jul 6 2015, 5:50 AM

Jun 29 2015

abhishek.aggarwal updated the diff for D10308: Use both OS and Architecture to choose correct ABI.

Followed up on Jason's comments

Jun 29 2015, 2:00 AM

Jun 26 2015

abhishek.aggarwal added a comment to D10308: Use both OS and Architecture to choose correct ABI.

If I understood you correctly, you want to add arch.GetTriple().isiOS() also along with arch.GetTriple().isMacOSX(). Am I right ? If yes then I will do it.

Jun 26 2015, 12:51 AM