[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature

Abhishek via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Jun 19 04:40:07 PDT 2017

abhishek.aggarwal added a subscriber: ravitheja.
abhishek.aggarwal added inline comments.

Comment at: tools/intel-pt/CMakeLists.txt:53
+  target_link_libraries(lldbIntelPT PRIVATE
labath wrote:
> All of this needs to go away. I think you only needed it because you are plucking NativeProcessLinux internals, so fixing that should fix this too.
Fixing this.

Comment at: tools/intel-pt/Decoder.cpp:27
+                              std::string &result) {
+  uint32_t error_code = sberror.GetError();
+  switch (error_code) {
abhishek.aggarwal wrote:
> clayborg wrote:
> > We really shouldn't be interpreting integer error codes here. The string in the "sberror" should be what you use. Modify the code that creates these errors in the first place to also put a string values you have here into the lldb_private::Error to begin with and remove this function.
> Removing this function.
Currently, the codes are generated by lldb server and remote packets don't support sending error strings from lldb server to lldb client.
Now, I can think of 2 ways:

1.  modify remote packets to send error strings as well (in addition to error codes).
2.  or decode error codes and generate error strings at lldb client side

Which one do you prefer? @labath prefers 1st one (in discussions of https://reviews.llvm.org/D33674). If you also prefers 1st one then I (or @ravitheja) can work on modifying packets accordingly and submit a separate patch for that.

My idea is to keep error codes and its decoding logic here for now. Once we submit patch of modified packets and that patch gets approved, I can remove this decoding function from here all together. Please let me know what you prefer.


More information about the lldb-commits mailing list