[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

As I stated before we should be printing the disassembly for each instruction on the output lines. We should probably also normalize the address hex value to it doesn't change widths. Something like:

  [ 0] 0x000000000040065f: pushq  %rbp
  [ 1] 0x000000000040065a: movq   %rsp, %rbp
  [ 2] 0x0000000000400657: movl   $0x0, -0x1c(%rbp)
  [ 3] 0x0000000000400654: callq  0x100000f82               ; symbol stub for: printf
  [ 4] 0x00007ffff7df1950: movl   %edi, -0x20(%rbp)

And possibly in this patch or in another patch, we should print out when the source file and line changes

  a.out`main @ main.cpp:12
    [ 0] 0x000000000040065f: pushq  %rbp
    [ 1] 0x000000000040065a: movq   %rsp, %rbp
    [ 2] 0x0000000000400657: movl   $0x0, -0x1c(%rbp)
  a.out`main @ main.cpp:13
    [ 3] 0x0000000000400654: callq  0x00007ffff7df1950               ; symbol stub for: printf
  libc.so`printf
    [ 4] 0x00007ffff7df1950: movl   %edi, -0x20(%rbp)



================
Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:159-160
+            substrs=['''thread #1: tid = 3842849, total instructions = 2
+  [0] no memory mapped at this address: 0x400518
+  [1] no memory mapped at this address: 0x400511'''])
+
----------------
These lines should start with the address like all other lines. Then the question is what the output should look like. Do we really need to tell the user that there is no memory mapped here? Can we just print "<???>" or nothing if we have no information like:

```
[0] 0x400518: <???>
[1] 0x400511: <???>
```



================
Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:180
+  [ 3] 0x400654
+  [ 4] no memory mapped at this address: 0x7ffff7df1950
+  [ 5] 0x400516
----------------
```
[ 4] 0x7ffff7df1950 <???>
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89283/new/

https://reviews.llvm.org/D89283