wallace updated this revision to Diff 301128.
wallace added a comment.

Followed @vsk's suggestion. I didn't do exactly what he said, but I ended up 
creating my own IntelPTError to represent better the different kinds of errors 
and make sure we can create a correct llvm::Error when traversing the 
instructions.
The IntelPTInstruction code is much simpler now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Symbol/SymbolContext.h
  lldb/include/lldb/Target/Trace.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSessionFileParser.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/intelpt-trace-multi-file/a.out
  lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.h
  lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.h
  lldb/test/API/commands/trace/intelpt-trace-multi-file/libbar.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/libfoo.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/main.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file.trace
  lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
  lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
@@ -0,0 +1,31 @@
+{
+  "trace": {
+    "type": "intel-pt",
+    "pt_cpu": {
+      "vendor": "intel",
+      "family": 2123123,
+      "model": 12123123,
+      "stepping": 1231231
+    }
+  },
+  "processes": [
+    {
+      "pid": 1234,
+      "triple": "x86_64-*-linux",
+      "threads": [
+        {
+          "tid": 3842849,
+          "traceFile": "3842849.trace"
+        }
+      ],
+      "modules": [
+        {
+          "file": "a.out",
+          "systemPath": "a.out",
+          "loadAddress": "0x0000000000400000",
+          "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+        }
+      ]
+    }
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
@@ -0,0 +1,31 @@
+{
+  "trace": {
+    "type": "intel-pt",
+    "pt_cpu": {
+      "vendor": "intel",
+      "family": 6,
+      "model": 79,
+      "stepping": 1
+    }
+  },
+  "processes": [
+    {
+      "pid": 1234,
+      "triple": "x86_64-*-linux",
+      "threads": [
+        {
+          "tid": 3842849,
+          "traceFile": "3842849.trace"
+        }
+      ],
+      "modules": [
+        {
+          "file": "a.out",
+          "systemPath": "a.out",
+          "loadAddress": "0x0000000000FFFFF0",
+          "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+        }
+      ]
+    }
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
@@ -0,0 +1,43 @@
+{
+  "trace": {
+    "type": "intel-pt",
+    "pt_cpu": {
+      "vendor": "intel",
+      "family": 6,
+      "model": 79,
+      "stepping": 1
+    }
+  },
+  "processes": [
+    {
+      "pid": 815455,
+      "triple": "x86_64-*-linux",
+      "threads": [
+        {
+          "tid": 815455,
+          "traceFile": "multi-file.trace"
+        }
+      ],
+      "modules": [
+        {
+          "file": "a.out",
+          "systemPath": "a.out",
+          "loadAddress": "0x0000000000400000",
+          "uuid": "D2414468-7112-B7C5-408D-FF07E30D5B17-A5BFD2C4"
+        },
+        {
+          "file": "libfoo.so",
+          "systemPath": "libfoo.so",
+          "loadAddress": "0x00007ffff7bd9000",
+          "uuid": "B30FFEDA-8BB2-3D08-4580-C5937ED11E2B-21BE778C"
+        },
+        {
+          "file": "libbar.so",
+          "systemPath": "libbar.so",
+          "loadAddress": "0x00007ffff79d7000",
+          "uuid": "6633B038-EA73-D1A6-FF9A-7D0C0EDF733D-95FEA2CC"
+        }
+      ]
+    }
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace-multi-file/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace-multi-file/main.cpp
@@ -0,0 +1,19 @@
+#include "foo.h"
+
+int __attribute__((always_inline)) inline_function() {
+  int z = 0;
+  z++;
+  return z;
+}
+
+int main() {
+  int res = foo();
+
+  res++;
+
+  res += inline_function();
+
+  res += foo();
+
+  return res;
+}
Index: lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.h
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.h
@@ -0,0 +1 @@
+int foo();
Index: lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp
@@ -0,0 +1,7 @@
+#include "bar.h"
+
+int foo() {
+  int y = bar();
+  y++;
+  return y;
+}
Index: lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.h
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.h
@@ -0,0 +1 @@
+int bar();
Index: lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.cpp
@@ -0,0 +1,5 @@
+int bar() {
+  int x = 1;
+  x++;
+  return x;
+}
Index: lldb/test/API/commands/trace/TestTraceDumpInstructions.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceDumpInstructions.py
+++ lldb/test/API/commands/trace/TestTraceDumpInstructions.py
@@ -20,7 +20,8 @@
             error=True)
 
         # We now check the output when there's a non-running target
-        self.expect("target create " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+        self.expect("target create " + 
+            os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
 
         self.expect("thread trace dump instructions",
             substrs=["error: invalid process"],
@@ -34,59 +35,225 @@
             substrs=["error: this thread is not being traced"],
             error=True)
 
-    def testDumpInstructions(self):
-        self.expect("trace load -v " + os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+    def testRawDumpInstructions(self):
+        self.expect("trace load -v " +
+            os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
             substrs=["intel-pt"])
 
-        self.expect("thread trace dump instructions",
-            substrs=['thread #1: tid = 3842849, total instructions = 1000',
-                     'would print 20 instructions from position 0'])
+        self.expect("thread trace dump instructions --raw",
+            substrs=['''thread #1: tid = 3842849, total instructions = 21
+    [ 1] 0x0000000000400518    
+    [ 2] 0x000000000040051f    
+    [ 3] 0x0000000000400529    
+    [ 4] 0x000000000040052d    
+    [ 5] 0x0000000000400521    
+    [ 6] 0x0000000000400525    
+    [ 7] 0x0000000000400529    
+    [ 8] 0x000000000040052d    
+    [ 9] 0x0000000000400521    
+    [10] 0x0000000000400525    
+    [11] 0x0000000000400529    
+    [12] 0x000000000040052d    
+    [13] 0x0000000000400521    
+    [14] 0x0000000000400525    
+    [15] 0x0000000000400529    
+    [16] 0x000000000040052d    
+    [17] 0x0000000000400521    
+    [18] 0x0000000000400525    
+    [19] 0x0000000000400529    
+    [20] 0x000000000040052d'''])
 
-        # We check if we can pass count and offset
-        self.expect("thread trace dump instructions --count 5 --start-position 10",
-            substrs=['thread #1: tid = 3842849, total instructions = 1000',
-                     'would print 5 instructions from position 10'])
+        # We check if we can pass count and position
+        self.expect("thread trace dump instructions --count 5 --position 10 --raw",
+            substrs=['''thread #1: tid = 3842849, total instructions = 21
+    [ 6] 0x0000000000400525    
+    [ 7] 0x0000000000400529    
+    [ 8] 0x000000000040052d    
+    [ 9] 0x0000000000400521    
+    [10] 0x0000000000400525'''])
 
         # We check if we can access the thread by index id
-        self.expect("thread trace dump instructions 1",
-            substrs=['thread #1: tid = 3842849, total instructions = 1000',
-                     'would print 20 instructions from position 0'])
+        self.expect("thread trace dump instructions 1 --raw",
+            substrs=['''thread #1: tid = 3842849, total instructions = 21
+    [ 1] 0x0000000000400518'''])
 
         # We check that we get an error when using an invalid thread index id
         self.expect("thread trace dump instructions 10", error=True,
             substrs=['error: no thread with index: "10"'])
 
-    def testDumpInstructionsWithMultipleThreads(self):
+    def testDumpFullInstructionsWithMultipleThreads(self):
         # We load a trace with two threads
-        self.expect("trace load -v " + os.path.join(self.getSourceDir(), "intelpt-trace", "trace_2threads.json"))
+        self.expect("trace load -v " + 
+            os.path.join(self.getSourceDir(), "intelpt-trace", "trace_2threads.json"))
 
         # We print the instructions of two threads simultaneously
-        self.expect("thread trace dump instructions 1 2",
-            substrs=['''thread #1: tid = 3842849, total instructions = 1000
-  would print 20 instructions from position 0
-thread #2: tid = 3842850, total instructions = 1000
-  would print 20 instructions from position 0'''])
-
-        # We use custom --count and --start-position, saving the command to history for later
-        ci = self.dbg.GetCommandInterpreter()
-
-        result = lldb.SBCommandReturnObject()
-        ci.HandleCommand("thread trace dump instructions 1 2 --count 12 --start-position 5", result, True)
-        self.assertIn('''thread #1: tid = 3842849, total instructions = 1000
-  would print 12 instructions from position 5
-thread #2: tid = 3842850, total instructions = 1000
-  would print 12 instructions from position 5''', result.GetOutput())
-
-        # We use a repeat command and ensure the previous count is used and the start-position has moved to the next position
-        result = lldb.SBCommandReturnObject()
-        ci.HandleCommand("", result)
-        self.assertIn('''thread #1: tid = 3842849, total instructions = 1000
-  would print 12 instructions from position 17
-thread #2: tid = 3842850, total instructions = 1000
-  would print 12 instructions from position 17''', result.GetOutput())
-
-        ci.HandleCommand("", result)
-        self.assertIn('''thread #1: tid = 3842849, total instructions = 1000
-  would print 12 instructions from position 29
-thread #2: tid = 3842850, total instructions = 1000
-  would print 12 instructions from position 29''', result.GetOutput())
+        self.expect("thread trace dump instructions 1 2 --count 2",
+            substrs=['''thread #1: tid = 3842849, total instructions = 21
+  a.out`main + 28 at main.cpp:4
+    [19] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
+    [20] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+thread #2: tid = 3842850, total instructions = 21
+  a.out`main + 28 at main.cpp:4
+    [19] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
+    [20] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5'''])
+
+        # We use custom --count and --position, saving the command to history for later
+        self.expect("thread trace dump instructions 1 2 --count 2 --position 20", inHistory=True,
+            substrs=['''thread #1: tid = 3842849, total instructions = 21
+  a.out`main + 28 at main.cpp:4
+    [19] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
+    [20] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+thread #2: tid = 3842850, total instructions = 21
+  a.out`main + 28 at main.cpp:4
+    [19] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
+    [20] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5'''])
+
+        # We use a repeat command twice and ensure the previous count is used and the
+        # start position moves with each command.
+        self.expect("", inHistory=True,
+            substrs=['''thread #1: tid = 3842849, total instructions = 21
+  a.out`main + 20 at main.cpp:5
+    [17] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)
+  a.out`main + 24 at main.cpp:4
+    [18] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
+thread #2: tid = 3842850, total instructions = 21
+  a.out`main + 20 at main.cpp:5
+    [17] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)
+  a.out`main + 24 at main.cpp:4
+    [18] 0x0000000000400525    addl   $0x1, -0x8(%rbp)'''])
+
+        self.expect("", inHistory=True,
+            substrs=['''thread #1: tid = 3842849, total instructions = 21
+  a.out`main + 28 at main.cpp:4
+    [15] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
+    [16] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+thread #2: tid = 3842850, total instructions = 21
+  a.out`main + 28 at main.cpp:4
+    [15] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
+    [16] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5'''])
+
+    def testInvalidBounds(self):
+        self.expect("trace load -v " +
+            os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
+
+        # The output should be work when too many instructions are asked
+        self.expect("thread trace dump instructions --count 20 --position 2",
+            substrs=['''thread #1: tid = 3842849, total instructions = 21
+  a.out`main + 4 at main.cpp:2
+    [0] 0x0000000000400511    movl   $0x0, -0x4(%rbp)
+  a.out`main + 11 at main.cpp:4
+    [1] 0x0000000000400518    movl   $0x0, -0x8(%rbp)
+    [2] 0x000000000040051f    jmp    0x400529                  ; <+28> at main.cpp:4'''])
+
+        # Should print no instructions if the position is out of bounds
+        self.expect("thread trace dump instructions --position 23",
+            endstr='thread #1: tid = 3842849, total instructions = 21\n')
+
+        # Should fail with negative bounds
+        self.expect("thread trace dump instructions --position -1", error=True)
+        self.expect("thread trace dump instructions --count -1", error=True)
+
+    def testWrongImage(self):
+        self.expect("trace load " +
+            os.path.join(self.getSourceDir(), "intelpt-trace", "trace_bad_image.json"))
+        self.expect("thread trace dump instructions",
+            substrs=['''thread #1: tid = 3842849, total instructions = 2
+    [0] 0x0000000000400511    error: no memory mapped at this address
+    [1] 0x0000000000400518    error: no memory mapped at this address'''])
+
+    def testWrongCPU(self):
+        self.expect("trace load " +
+            os.path.join(self.getSourceDir(), "intelpt-trace", "trace_wrong_cpu.json"))
+        self.expect("thread trace dump instructions",
+            substrs=['''thread #1: tid = 3842849, total instructions = 1
+    [0] error: unknown cpu'''])
+
+    def testMultiFileTraceWithMissingModule(self):
+        self.expect("trace load " +
+            os.path.join(self.getSourceDir(), "intelpt-trace-multi-file", "multi-file-no-ld.json"))
+
+        # This instructions in this test covers the following flow:
+        #
+        # - The trace starts with a call to libfoo, which triggers the dynamic
+        #   linker, but the dynamic linker is not included in the JSON file,
+        #   thus the trace reports a set of missing instructions after
+        #   instruction [6].
+        # - Then, the dump continues in the next synchronization point showing
+        #   a call to an inlined function, which is displayed as [inlined].
+        # - Finally, a call to libfoo is performed, which invokes libbar inside.
+        # 
+        # Whenever there's a line or symbol change, including the inline case, a
+        # line is printed showing the symbol context change.
+        #
+        # Finally, the instruction disassembly is included in the dump.
+        self.expect("thread trace dump instructions --count 50",
+            substrs=['''thread #1: tid = 815455, total instructions = 46
+  a.out`main + 15 at main.cpp:10
+    [ 0] 0x000000000040066f    callq  0x400540                  ; symbol stub for: foo()
+  a.out`symbol stub for: foo()
+    [ 1] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
+    [ 2] 0x0000000000400546    pushq  $0x2
+    [ 3] 0x000000000040054b    jmp    0x400510
+  a.out`(none)
+    [ 4] 0x0000000000400510    pushq  0x200af2(%rip)            ; _GLOBAL_OFFSET_TABLE_ + 8
+    [ 5] 0x0000000000400516    jmpq   *0x200af4(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 16
+    [ 6] 0x00007ffff7df1950    error: no memory mapped at this address
+    ...missing instructions
+  a.out`main + 20 at main.cpp:10
+    [ 7] 0x0000000000400674    movl   %eax, -0xc(%rbp)
+  a.out`main + 23 at main.cpp:12
+    [ 8] 0x0000000000400677    movl   -0xc(%rbp), %eax
+    [ 9] 0x000000000040067a    addl   $0x1, %eax
+    [10] 0x000000000040067f    movl   %eax, -0xc(%rbp)
+  a.out`main + 34 [inlined] inline_function() at main.cpp:4
+    [11] 0x0000000000400682    movl   $0x0, -0x4(%rbp)
+  a.out`main + 41 [inlined] inline_function() + 7 at main.cpp:5
+    [12] 0x0000000000400689    movl   -0x4(%rbp), %eax
+    [13] 0x000000000040068c    addl   $0x1, %eax
+    [14] 0x0000000000400691    movl   %eax, -0x4(%rbp)
+  a.out`main + 52 [inlined] inline_function() + 18 at main.cpp:6
+    [15] 0x0000000000400694    movl   -0x4(%rbp), %eax
+  a.out`main + 55 at main.cpp:14
+    [16] 0x0000000000400697    movl   -0xc(%rbp), %ecx
+    [17] 0x000000000040069a    addl   %eax, %ecx
+    [18] 0x000000000040069c    movl   %ecx, -0xc(%rbp)
+  a.out`main + 63 at main.cpp:16
+    [19] 0x000000000040069f    callq  0x400540                  ; symbol stub for: foo()
+  a.out`symbol stub for: foo()
+    [20] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
+  libfoo.so`foo() at foo.cpp:3
+    [21] 0x00007ffff7bd96e0    pushq  %rbp
+    [22] 0x00007ffff7bd96e1    movq   %rsp, %rbp
+  libfoo.so`foo() + 4 at foo.cpp:4
+    [23] 0x00007ffff7bd96e4    subq   $0x10, %rsp
+    [24] 0x00007ffff7bd96e8    callq  0x7ffff7bd95d0            ; symbol stub for: bar()
+  libfoo.so`symbol stub for: bar()
+    [25] 0x00007ffff7bd95d0    jmpq   *0x200a4a(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 32
+  libbar.so`bar() at bar.cpp:1
+    [26] 0x00007ffff79d7690    pushq  %rbp
+    [27] 0x00007ffff79d7691    movq   %rsp, %rbp
+  libbar.so`bar() + 4 at bar.cpp:2
+    [28] 0x00007ffff79d7694    movl   $0x1, -0x4(%rbp)
+  libbar.so`bar() + 11 at bar.cpp:3
+    [29] 0x00007ffff79d769b    movl   -0x4(%rbp), %eax
+    [30] 0x00007ffff79d769e    addl   $0x1, %eax
+    [31] 0x00007ffff79d76a3    movl   %eax, -0x4(%rbp)
+  libbar.so`bar() + 22 at bar.cpp:4
+    [32] 0x00007ffff79d76a6    movl   -0x4(%rbp), %eax
+    [33] 0x00007ffff79d76a9    popq   %rbp
+    [34] 0x00007ffff79d76aa    retq   
+  libfoo.so`foo() + 13 at foo.cpp:4
+    [35] 0x00007ffff7bd96ed    movl   %eax, -0x4(%rbp)
+  libfoo.so`foo() + 16 at foo.cpp:5
+    [36] 0x00007ffff7bd96f0    movl   -0x4(%rbp), %eax
+    [37] 0x00007ffff7bd96f3    addl   $0x1, %eax
+    [38] 0x00007ffff7bd96f8    movl   %eax, -0x4(%rbp)
+  libfoo.so`foo() + 27 at foo.cpp:6
+    [39] 0x00007ffff7bd96fb    movl   -0x4(%rbp), %eax
+    [40] 0x00007ffff7bd96fe    addq   $0x10, %rsp
+    [41] 0x00007ffff7bd9702    popq   %rbp
+    [42] 0x00007ffff7bd9703    retq   
+  a.out`main + 68 at main.cpp:16
+    [43] 0x00000000004006a4    movl   -0xc(%rbp), %ecx
+    [44] 0x00000000004006a7    addl   %eax, %ecx
+    [45] 0x00000000004006a9    movl   %ecx, -0xc(%rbp)'''])
Index: lldb/source/Target/TraceSessionFileParser.cpp
===================================================================
--- lldb/source/Target/TraceSessionFileParser.cpp
+++ lldb/source/Target/TraceSessionFileParser.cpp
@@ -37,7 +37,6 @@
   ModuleSpec module_spec;
   module_spec.GetFileSpec() = local_file_spec;
   module_spec.GetPlatformFileSpec() = system_file_spec;
-  module_spec.SetObjectOffset(module.load_address.value);
 
   if (module.uuid.hasValue())
     module_spec.GetUUID().SetFromStringRef(*module.uuid);
@@ -45,7 +44,14 @@
   Status error;
   ModuleSP module_sp =
       target_sp->GetOrCreateModule(module_spec, /*notify*/ false, &error);
-  return error.ToError();
+
+  if (error.Fail())
+    return error.ToError();
+
+  bool load_addr_changed = false;
+  module_sp->SetLoadAddress(*target_sp, module.load_address.value, false,
+                            load_addr_changed);
+  return llvm::Error::success();
 }
 
 Error TraceSessionFileParser::CreateJSONError(json::Path::Root &root,
Index: lldb/source/Target/Trace.cpp
===================================================================
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -8,11 +8,13 @@
 
 #include "lldb/Target/Trace.h"
 
-#include <sstream>
-
 #include "llvm/Support/Format.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Symbol/Function.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/Stream.h"
 
@@ -79,10 +81,188 @@
   return createInvalidPlugInError(name);
 }
 
+static int GetNumberOfDigits(size_t num) {
+  return num == 0 ? 1 : static_cast<int>(log10(num)) + 1;
+}
+
+/// Dump the symbol context of the given instruction address if it's different
+/// from the symbol context of the previous instruction in the trace.
+///
+/// \param[in] prev_sc
+///     The symbol context of the previous instruction in the trace.
+///
+/// \param[in] address
+///     The address whose symbol information will be dumped.
+///
+/// \return
+///     The symbol context of the current address, which might differ from the
+///     previous one.
+static SymbolContext DumpSymbolContext(Stream &s, const SymbolContext &prev_sc,
+                                       Target &target, const Address &address) {
+  AddressRange range;
+  if (prev_sc.GetAddressRange(eSymbolContextEverything, 0,
+                              /*inline_block_range*/ false, range) &&
+      range.ContainsFileAddress(address))
+    return prev_sc;
+
+  SymbolContext sc;
+  address.CalculateSymbolContext(&sc, eSymbolContextEverything);
+
+  if (!prev_sc.module_sp && !sc.module_sp)
+    return sc;
+  if (prev_sc.module_sp == sc.module_sp && !sc.function && !sc.symbol &&
+      !prev_sc.function && !prev_sc.symbol)
+    return sc;
+
+  s.Printf("  ");
+
+  if (!sc.module_sp)
+    s.Printf("(none)");
+  else if (!sc.function && !sc.symbol)
+    s.Printf("%s`(none)",
+             sc.module_sp->GetFileSpec().GetFilename().AsCString());
+  else
+    sc.DumpStopContext(&s, &target, address, /*show_fullpath*/ false,
+                       /*show_module*/ true, /*show_inlined_frames*/ false,
+                       /*show_function_arguments*/ true,
+                       /*show_function_name*/ true,
+                       /*show_inline_callsite_line_info*/ false);
+  s.Printf("\n");
+  return sc;
+}
+
+/// Dump an instruction given by its address using a given disassembler, unless
+/// the instruction is not present in the disassembler.
+///
+/// \param[in] disassembler
+///     A disassembler containing a certain instruction list.
+///
+/// \param[in] address
+///     The address of the instruction to dump.
+///
+/// \return
+///     \b true if the information could be dumped, \b false otherwise.
+static bool TryDumpInstructionInfo(Stream &s,
+                                   const DisassemblerSP &disassembler,
+                                   const ExecutionContext &exe_ctx,
+                                   const Address &address) {
+  if (!disassembler)
+    return false;
+
+  if (InstructionSP instruction =
+          disassembler->GetInstructionList().GetInstructionAtAddress(address)) {
+    instruction->Dump(&s, /*show_address*/ false, /*show_bytes*/ false,
+                      /*max_opcode_byte_size*/ 0, &exe_ctx,
+                      /*sym_ctx*/ nullptr, /*prev_sym_ctx*/ nullptr,
+                      /*disassembly_addr_format*/ nullptr,
+                      /*max_address_text_size*/ 0);
+    return true;
+  }
+
+  return false;
+}
+
+/// Dump an instruction instruction given by its address.
+///
+/// \param[in] prev_disassembler
+///     The disassembler that was used to dump the previous instruction in the
+///     trace. It is useful to avoid recomputations.
+///
+/// \param[in] address
+///     The address of the instruction to dump.
+///
+/// \return
+///     A disassembler that contains the given instruction, which might differ
+///     from the previous disassembler.
+static DisassemblerSP
+DumpInstructionInfo(Stream &s, const SymbolContext &sc,
+                    const DisassemblerSP &prev_disassembler,
+                    ExecutionContext &exe_ctx, const Address &address) {
+  // We first try to use the previous disassembler
+  if (TryDumpInstructionInfo(s, prev_disassembler, exe_ctx, address))
+    return prev_disassembler;
+
+  // Now we try using the current function's disassembler
+  if (sc.function) {
+    DisassemblerSP disassembler =
+        sc.function->GetInstructions(exe_ctx, nullptr, true);
+    if (TryDumpInstructionInfo(s, disassembler, exe_ctx, address))
+      return disassembler;
+  }
+
+  // We fallback to disassembly one instruction
+  Target &target = exe_ctx.GetTargetRef();
+  const ArchSpec &arch = target.GetArchitecture();
+  AddressRange range(address, arch.GetMaximumOpcodeByteSize() * 1);
+  DisassemblerSP disassembler = Disassembler::DisassembleRange(
+      arch, /*plugin_name*/ nullptr,
+      /*flavor*/ nullptr, target, range, /*prefer_file_cache*/ true);
+  if (TryDumpInstructionInfo(s, disassembler, exe_ctx, address))
+    return disassembler;
+  return nullptr;
+}
+
 void Trace::DumpTraceInstructions(Thread &thread, Stream &s, size_t count,
-                                  size_t start_position) const {
-  s.Printf("thread #%u: tid = %" PRIu64 ", total instructions = 1000\n",
-           thread.GetIndexID(), thread.GetID());
-  s.Printf("  would print %zu instructions from position %zu\n", count,
-           start_position);
+                                  size_t end_position, bool raw) {
+  size_t instructions_count = GetInstructionCount(thread);
+  s.Printf("thread #%u: tid = %" PRIu64 ", total instructions = %zu\n",
+           thread.GetIndexID(), thread.GetID(), instructions_count);
+
+  if (count == 0 || end_position >= instructions_count)
+    return;
+
+  size_t start_position =
+      end_position + 1 < count ? 0 : end_position + 1 - count;
+
+  int digits_count = GetNumberOfDigits(end_position);
+  auto printInstructionIndex = [&](size_t index) {
+    s.Printf("    [%*zu] ", digits_count, index);
+  };
+
+  bool was_prev_instruction_an_error = false;
+  Target &target = thread.GetProcess()->GetTarget();
+
+  SymbolContext sc;
+  DisassemblerSP disassembler;
+  ExecutionContext exe_ctx;
+  target.CalculateExecutionContext(exe_ctx);
+
+  TraverseInstructions(
+      thread, start_position, TraceDirection::Forwards,
+      [&](size_t index, Expected<lldb::addr_t> load_address) -> bool {
+        if (load_address) {
+          // We print an empty line after a sequence of errors to show more
+          // clearly that there's a gap in the trace
+          if (was_prev_instruction_an_error)
+            s.Printf("    ...missing instructions\n");
+
+          Address address;
+          if (!raw) {
+            target.GetSectionLoadList().ResolveLoadAddress(*load_address,
+                                                           address);
+
+            sc = DumpSymbolContext(s, sc, target, address);
+          }
+
+          printInstructionIndex(index);
+          s.Printf("0x%016" PRIx64 "    ", *load_address);
+
+          if (!raw) {
+            disassembler =
+                DumpInstructionInfo(s, sc, disassembler, exe_ctx, address);
+          }
+
+          was_prev_instruction_an_error = false;
+        } else {
+          printInstructionIndex(index);
+          s << toString(load_address.takeError());
+          was_prev_instruction_an_error = true;
+          if (!raw)
+            sc = SymbolContext();
+        }
+
+        s.Printf("\n");
+
+        return index < end_position;
+      });
 }
Index: lldb/source/Target/ProcessTrace.cpp
===================================================================
--- lldb/source/Target/ProcessTrace.cpp
+++ lldb/source/Target/ProcessTrace.cpp
@@ -12,6 +12,8 @@
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 
 using namespace lldb;
@@ -121,5 +123,9 @@
 
 size_t ProcessTrace::DoReadMemory(addr_t addr, void *buf, size_t size,
                                   Status &error) {
-  return 0;
+  Address resolved_address;
+  GetTarget().GetSectionLoadList().ResolveLoadAddress(addr, resolved_address);
+
+  return GetTarget().ReadMemoryFromFileCache(resolved_address, buf, size,
+                                             error);
 }
Index: lldb/source/Symbol/SymbolContext.cpp
===================================================================
--- lldb/source/Symbol/SymbolContext.cpp
+++ lldb/source/Symbol/SymbolContext.cpp
@@ -71,7 +71,8 @@
                                     const Address &addr, bool show_fullpaths,
                                     bool show_module, bool show_inlined_frames,
                                     bool show_function_arguments,
-                                    bool show_function_name) const {
+                                    bool show_function_name,
+                                    bool show_inline_callsite_line_info) const {
   bool dumped_something = false;
   if (show_module && module_sp) {
     if (show_fullpaths)
@@ -127,11 +128,17 @@
           s->Printf(" + %" PRIu64, inlined_function_offset);
         }
       }
-      const Declaration &call_site = inlined_block_info->GetCallSite();
-      if (call_site.IsValid()) {
+      if (show_inline_callsite_line_info) {
+        const Declaration &call_site = inlined_block_info->GetCallSite();
+        if (call_site.IsValid()) {
+          s->PutCString(" at ");
+          call_site.DumpStopContext(s, show_fullpaths);
+        }
+      } else if (line_entry.IsValid()) {
         s->PutCString(" at ");
-        call_site.DumpStopContext(s, show_fullpaths);
+        line_entry.DumpStopContext(s, show_fullpaths);
       }
+
       if (show_inlined_frames) {
         s->EOL();
         s->Indent();
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -9,12 +9,8 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 
-#include "intel-pt.h"
-#include "llvm/ADT/Optional.h"
-
+#include "IntelPTDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
-#include "lldb/Target/Trace.h"
-#include "lldb/lldb-private.h"
 
 namespace lldb_private {
 namespace trace_intel_pt {
@@ -59,6 +55,15 @@
 
   llvm::StringRef GetSchema() override;
 
+  void TraverseInstructions(
+      const Thread &thread, size_t position, TraceDirection direction,
+      std::function<bool(size_t index, llvm::Expected<lldb::addr_t> load_addr)>
+          callback) override;
+
+  size_t GetInstructionCount(const Thread &thread) override;
+
+  size_t GetCursorPosition(const Thread &thread) override;
+
 private:
   friend class TraceIntelPTSessionFileParser;
 
@@ -68,8 +73,20 @@
   TraceIntelPT(const pt_cpu &pt_cpu,
                const std::vector<std::shared_ptr<ThreadTrace>> &traced_threads);
 
+  /// Decode the trace of the given thread that, i.e. recontruct the traced
+  /// instructions. That trace must be managed by this class.
+  ///
+  /// \param[in] thread
+  ///     If \a thread is a \a ThreadTrace, then its internal trace file will be
+  ///     decoded. Live threads are not currently supported.
+  ///
+  /// \return
+  ///     A \a DecodedThread instance if decoding was successful, or a \b
+  ///     nullptr if the thread's trace is not managed by this class.
+  const DecodedThread *Decode(const Thread &thread);
+
   pt_cpu m_pt_cpu;
-  std::map<std::pair<lldb::pid_t, lldb::tid_t>, std::shared_ptr<ThreadTrace>>
+  std::map<std::pair<lldb::pid_t, lldb::tid_t>, ThreadTraceDecoder>
       m_trace_threads;
 };
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -64,5 +64,46 @@
     : m_pt_cpu(pt_cpu) {
   for (const std::shared_ptr<ThreadTrace> &thread : traced_threads)
     m_trace_threads.emplace(
-        std::make_pair(thread->GetProcess()->GetID(), thread->GetID()), thread);
+        std::piecewise_construct,
+        std::forward_as_tuple(thread->GetProcess()->GetID(), thread->GetID()),
+        std::forward_as_tuple(thread, pt_cpu));
+}
+
+const DecodedThread *TraceIntelPT::Decode(const Thread &thread) {
+  auto it = m_trace_threads.find(
+      std::make_pair(thread.GetProcess()->GetID(), thread.GetID()));
+  if (it == m_trace_threads.end())
+    return nullptr;
+  return &it->second.Decode();
+}
+
+size_t TraceIntelPT::GetCursorPosition(const Thread &thread) {
+  const DecodedThread *decoded_thread = Decode(thread);
+  if (!decoded_thread)
+    return 0;
+  return decoded_thread->GetCursorPosition();
+}
+
+void TraceIntelPT::TraverseInstructions(
+    const Thread &thread, size_t position, TraceDirection direction,
+    std::function<bool(size_t index, Expected<lldb::addr_t> load_addr)>
+        callback) {
+  const DecodedThread *decoded_thread = Decode(thread);
+  if (!decoded_thread)
+    return;
+
+  ArrayRef<IntelPTInstruction> instructions = decoded_thread->GetInstructions();
+
+  ssize_t delta = direction == TraceDirection::Forwards ? 1 : -1;
+  for (ssize_t i = position; i < (ssize_t)instructions.size() && i >= 0;
+       i += delta)
+    if (!callback(i, instructions[i].GetLoadAddress()))
+      break;
+}
+
+size_t TraceIntelPT::GetInstructionCount(const Thread &thread) {
+  if (const DecodedThread *decoded_thread = Decode(thread))
+    return decoded_thread->GetInstructions().size();
+  else
+    return 0;
 }
Index: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
===================================================================
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
@@ -0,0 +1,52 @@
+//===-- IntelPTDecoder.h --======--------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+
+#include "intel-pt.h"
+
+#include "DecodedThread.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Utility/FileSpec.h"
+
+namespace lldb_private {
+namespace trace_intel_pt {
+
+/// \a lldb_private::ThreadTrace decoder that stores the output from decoding,
+/// avoiding recomputations, as decoding is expensive.
+class ThreadTraceDecoder {
+public:
+  /// \param[in] trace_thread
+  ///     The thread whose trace file will be decoded.
+  ///
+  /// \param[in] pt_cpu
+  ///     The libipt cpu used when recording the trace.
+  ThreadTraceDecoder(const std::shared_ptr<ThreadTrace> &trace_thread,
+                     const pt_cpu &pt_cpu)
+      : m_trace_thread(trace_thread), m_pt_cpu(pt_cpu), m_decoded_thread() {}
+
+  /// Decode the thread and store the result internally.
+  ///
+  /// \return
+  ///     A \a DecodedThread instance.
+  const DecodedThread &Decode();
+
+private:
+  ThreadTraceDecoder(const ThreadTraceDecoder &other) = delete;
+  ThreadTraceDecoder &operator=(const ThreadTraceDecoder &other) = delete;
+
+  std::shared_ptr<ThreadTrace> m_trace_thread;
+  pt_cpu m_pt_cpu;
+  llvm::Optional<DecodedThread> m_decoded_thread;
+};
+
+} // namespace trace_intel_pt
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
Index: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
===================================================================
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
@@ -0,0 +1,209 @@
+//===-- IntelPTDecoder.cpp --------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "IntelPTDecoder.h"
+
+#include "llvm/Support/MemoryBuffer.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/ThreadTrace.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::trace_intel_pt;
+using namespace llvm;
+
+/// Move the decoder forward to the next synchronization point (i.e. next PSB
+/// packet).
+///
+/// Once the decoder is at that sync. point, it can start decoding instructions.
+///
+/// \return
+///   A negative number with the libipt error if we couldn't synchronize.
+///   Otherwise, a positive number with the synchronization status will be
+///   returned.
+static int FindNextSynchronizationPoint(pt_insn_decoder &decoder) {
+  // Try to sync the decoder. If it fails, then get
+  // the decoder_offset and try to sync again from
+  // the next synchronization point. If the
+  // new_decoder_offset is same as decoder_offset
+  // then we can't move to the next synchronization
+  // point. Otherwise, keep resyncing until either
+  // end of trace stream (eos) is reached or
+  // pt_insn_sync_forward() passes.
+  int errcode = pt_insn_sync_forward(&decoder);
+
+  if (errcode != -pte_eos && errcode < 0) {
+    uint64_t decoder_offset = 0;
+    int errcode_off = pt_insn_get_offset(&decoder, &decoder_offset);
+    if (errcode_off >= 0) { // we could get the offset
+      while (true) {
+        errcode = pt_insn_sync_forward(&decoder);
+        if (errcode >= 0 || errcode == -pte_eos)
+          break;
+
+        uint64_t new_decoder_offset = 0;
+        errcode_off = pt_insn_get_offset(&decoder, &new_decoder_offset);
+        if (errcode_off < 0)
+          break; // We can't further synchronize.
+        else if (new_decoder_offset <= decoder_offset) {
+          // We tried resyncing the decoder and
+          // decoder didn't make any progress because
+          // the offset didn't change. We will not
+          // make any progress further. Hence,
+          // stopping in this situation.
+          break;
+        }
+        // We'll try again starting from a new offset.
+        decoder_offset = new_decoder_offset;
+      }
+    }
+  }
+
+  return errcode;
+}
+
+/// Before querying instructions, we need to query the events associated that
+/// instruction e.g. timing events like ptev_tick, or paging events like
+/// ptev_paging.
+///
+/// \return
+///   0 if there were no errors processing the events, or a negative libipt
+///   error code in case of errors.
+static int ProcessPTEvents(pt_insn_decoder &decoder, int errcode) {
+  while (errcode & pts_event_pending) {
+    pt_event event;
+    errcode = pt_insn_event(&decoder, &event, sizeof(event));
+    if (errcode < 0)
+      return errcode;
+  }
+  return 0;
+};
+
+/// Decode all the instructions from a configured decoder.
+/// The decoding flow is based on
+/// https://github.com/intel/libipt/blob/master/doc/howto_libipt.md#the-instruction-flow-decode-loop
+/// but with some relaxation to allow for gaps in the trace.
+///
+/// Error codes returned by libipt while decoding are:
+/// - negative: actual errors
+/// - positive or zero: not an error, but a list of bits signaling the status of
+/// the decoder
+///
+/// \param[in] decoder
+///   A configured libipt \a pt_insn_decoder.
+///
+/// \return
+///   The decoded instructions.
+static std::vector<IntelPTInstruction>
+DecodeInstructions(pt_insn_decoder &decoder) {
+  std::vector<IntelPTInstruction> instructions;
+
+  while (true) {
+    int errcode = FindNextSynchronizationPoint(decoder);
+    if (errcode == -pte_eos)
+      break;
+
+    if (errcode < 0) {
+      instructions.emplace_back(errcode);
+      break;
+    }
+
+    // We have synchronized, so we can start decoding
+    // instructions and events.
+    while (true) {
+      errcode = ProcessPTEvents(decoder, errcode);
+      if (errcode < 0) {
+        instructions.emplace_back(errcode);
+        break;
+      }
+      pt_insn insn;
+
+      errcode = pt_insn_next(&decoder, &insn, sizeof(insn));
+      if (errcode == -pte_eos)
+        break;
+
+      if (errcode < 0) {
+        instructions.emplace_back(insn, errcode);
+        break;
+      }
+
+      instructions.emplace_back(insn);
+    }
+  }
+
+  return instructions;
+}
+
+/// Callback used by libipt for reading the process memory.
+///
+/// More information can be found in
+/// https://github.com/intel/libipt/blob/master/doc/man/pt_image_set_callback.3.md
+static int ReadProcessMemory(uint8_t *buffer, size_t size,
+                             const pt_asid * /* unused */, uint64_t pc,
+                             void *context) {
+  Process *process = static_cast<Process *>(context);
+
+  Status error;
+  int bytes_read = process->ReadMemory(pc, buffer, size, error);
+  if (error.Fail())
+    return -pte_nomap;
+  return bytes_read;
+}
+
+static std::vector<IntelPTInstruction>
+CreateDecoderAndDecode(Process &process, const pt_cpu &pt_cpu,
+                       const FileSpec &trace_file) {
+  ErrorOr<std::unique_ptr<MemoryBuffer>> trace_or_error =
+      MemoryBuffer::getFile(trace_file.GetPath());
+  if (std::error_code err = trace_or_error.getError())
+    return {IntelPTInstruction(errorCodeToError(err))};
+
+  MemoryBuffer &trace = **trace_or_error;
+
+  pt_config config;
+  pt_config_init(&config);
+  config.cpu = pt_cpu;
+
+  if (int errcode = pt_cpu_errata(&config.errata, &config.cpu))
+    return {IntelPTInstruction(errcode)};
+
+  // The libipt library does not modify the trace buffer, hence the following
+  // cast is safe.
+  config.begin =
+      reinterpret_cast<uint8_t *>(const_cast<char *>(trace.getBufferStart()));
+  config.end =
+      reinterpret_cast<uint8_t *>(const_cast<char *>(trace.getBufferEnd()));
+
+  pt_insn_decoder *decoder = pt_insn_alloc_decoder(&config);
+  if (!decoder)
+    return {IntelPTInstruction(-pte_nomem)};
+
+  pt_image *image = pt_insn_get_image(decoder);
+
+  int errcode = pt_image_set_callback(image, ReadProcessMemory, &process);
+  assert(errcode == 0);
+  (void)errcode;
+
+  std::vector<IntelPTInstruction> instructions = DecodeInstructions(*decoder);
+
+  pt_insn_free_decoder(decoder);
+  return instructions;
+}
+
+const DecodedThread &ThreadTraceDecoder::Decode() {
+  if (!m_decoded_thread.hasValue()) {
+    m_decoded_thread = DecodedThread(
+        CreateDecoderAndDecode(*m_trace_thread->GetProcess(), m_pt_cpu,
+                               m_trace_thread->GetTraceFile()));
+  }
+
+  return *m_decoded_thread;
+}
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===================================================================
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -0,0 +1,165 @@
+//===-- DecodedThread.h -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
+#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
+
+#include <vector>
+
+#include "llvm/Support/Error.h"
+
+#include "lldb/Target/Trace.h"
+
+#include "intel-pt.h"
+
+namespace lldb_private {
+namespace trace_intel_pt {
+
+/// Base class for representing an IntelPT decoding error.
+class IntelPTError {
+public:
+  IntelPTError() = default;
+
+  virtual ~IntelPTError() = default;
+
+  IntelPTError(const IntelPTError &other) = delete;
+
+  /// \return
+  ///     A \a llvm::Error object representing this error.
+  virtual llvm::Error ToError() const = 0;
+};
+
+/// Class representing an error while using libipt's decoder.
+///
+class IntelPTLibiptError : public IntelPTError {
+public:
+  /// \param[in] libipt_error_code
+  ///     Negative number returned by libipt when decoding the trace.
+  ///
+  /// \param[in] address
+  ///     Optional instruction address. When decoding an individual instruction,
+  ///     its address might be available in the \a pt_insn object, and should be
+  ///     passed to this constructor. Other errors don't have an associated
+  ///     address.
+  IntelPTLibiptError(int libipt_error_code,
+                     lldb::addr_t address = LLDB_INVALID_ADDRESS);
+
+  llvm::Error ToError() const override;
+
+private:
+  int m_libipt_error_code;
+  lldb::addr_t m_address;
+};
+
+/// Class representing any error while decoding, but not due to libipt.
+class IntelPTCustomError : public IntelPTError {
+public:
+  IntelPTCustomError(llvm::Error &error);
+
+  llvm::Error ToError() const override;
+
+private:
+  std::string m_error_message;
+};
+
+/// \class IntelPTInstruction
+/// An instruction obtained from decoding a trace. It is either an actual
+/// instruction or an error indicating a gap in the trace.
+///
+/// Gaps in the trace can come in a few flavors:
+///   - tracing gaps (e.g. tracing was paused and then resumed)
+///   - tracing errors (e.g. buffer overflow)
+///   - decoding errors (e.g. some memory region couldn't be decoded)
+/// As mentioned, any gap is represented as an error in this class.
+class IntelPTInstruction {
+public:
+  IntelPTInstruction(const pt_insn &pt_insn) : m_pt_insn(pt_insn) {}
+
+  /// Error constructors
+  ///
+  /// \a libipt_error_code must be a negative number returned by libipt.
+  ///
+  /// \{
+  IntelPTInstruction(int libipt_error_code)
+      : m_error(std::make_shared<IntelPTLibiptError>(libipt_error_code)) {}
+
+  IntelPTInstruction(const pt_insn &pt_insn, int libipt_error_code)
+      : m_error(std::make_shared<IntelPTLibiptError>(libipt_error_code,
+                                                     pt_insn.ip)) {}
+
+  IntelPTInstruction(llvm::Error &&error)
+      : m_error(std::make_shared<IntelPTCustomError>(error)) {}
+  /// \}
+
+  /// Check if this object represents an error (i.e. a gap).
+  ///
+  /// \return
+  ///     Whether this object represents an error.
+  bool IsError() const;
+
+  /// \return
+  ///     The instruction pointer address, or an \a llvm::Error if it is an
+  ///     error.
+  llvm::Expected<lldb::addr_t> GetLoadAddress() const;
+
+  /// \return
+  ///     An \a llvm::Error object if this class corresponds to an Error, or an
+  ///     \a llvm::Error::success otherwise.
+  llvm::Error GetError() const;
+
+private:
+  pt_insn m_pt_insn;
+  std::shared_ptr<IntelPTError> m_error;
+};
+
+/// \class DecodedThread
+/// Class holding the instructions and function call hierarchy obtained from
+/// decoding a trace, as well as a position cursor used when reverse debugging
+/// the trace.
+///
+/// Each decoded thread contains a cursor to the current position the user is
+/// stopped at. See \a Trace::GetCursorPosition for more information.
+class DecodedThread {
+public:
+  DecodedThread(std::vector<IntelPTInstruction> &&instructions)
+      : m_instructions(std::move(instructions)), m_position(GetLastPosition()) {
+  }
+
+  /// Get the instructions from the decoded trace. Some of them might indicate
+  /// errors (i.e. gaps) in the trace.
+  ///
+  /// \return
+  ///   The instructions of the trace.
+  llvm::ArrayRef<IntelPTInstruction> GetInstructions() const;
+
+  /// \return
+  ///   The current position of the cursor of this trace, or 0 if there are no
+  ///   instructions.
+  size_t GetCursorPosition() const;
+
+  /// Change the position of the cursor of this trace. If this value is to high,
+  /// the new position will be set as the last instruction of the trace.
+  ///
+  /// \return
+  ///     The effective new position.
+  size_t SetCursorPosition(size_t new_position);
+  /// \}
+
+private:
+  /// \return
+  ///     The index of the last element of the trace, or 0 if empty.
+  size_t GetLastPosition() const;
+
+  std::vector<IntelPTInstruction> m_instructions;
+  size_t m_position;
+};
+
+} // namespace trace_intel_pt
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
===================================================================
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -0,0 +1,68 @@
+//===-- DecodedThread.cpp -------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DecodedThread.h"
+
+#include "lldb/Utility/StreamString.h"
+
+using namespace lldb_private;
+using namespace lldb_private::trace_intel_pt;
+using namespace llvm;
+
+IntelPTLibiptError::IntelPTLibiptError(int libipt_error_code,
+                                       lldb::addr_t address)
+    : m_libipt_error_code(libipt_error_code), m_address(address) {
+  assert(libipt_error_code < 0);
+}
+
+Error IntelPTLibiptError::ToError() const {
+  const char *libipt_error_message = pt_errstr(pt_errcode(m_libipt_error_code));
+  if (m_address != LLDB_INVALID_ADDRESS && m_address > 0)
+    return createStringError(inconvertibleErrorCode(),
+                             "0x%016" PRIx64 "    error: %s", m_address,
+                             libipt_error_message);
+  return createStringError(inconvertibleErrorCode(), "error: %s",
+                           libipt_error_message);
+}
+
+IntelPTCustomError::IntelPTCustomError(Error &error)
+    : m_error_message(toString(std::move(error))) {}
+
+Error IntelPTCustomError::ToError() const {
+  return createStringError(inconvertibleErrorCode(), "error: %s",
+                           m_error_message.c_str());
+}
+
+bool IntelPTInstruction::IsError() const { return (bool)m_error; }
+
+Expected<lldb::addr_t> IntelPTInstruction::GetLoadAddress() const {
+  if (IsError())
+    return GetError();
+  return m_pt_insn.ip;
+}
+
+Error IntelPTInstruction::GetError() const {
+  if (IsError())
+    return m_error->ToError();
+  return Error::success();
+}
+
+size_t DecodedThread::GetLastPosition() const {
+  return m_instructions.empty() ? 0 : m_instructions.size() - 1;
+}
+
+ArrayRef<IntelPTInstruction> DecodedThread::GetInstructions() const {
+  return makeArrayRef(m_instructions);
+}
+
+size_t DecodedThread::GetCursorPosition() const { return m_position; }
+
+size_t DecodedThread::SetCursorPosition(size_t new_position) {
+  m_position = std::min(new_position, GetLastPosition());
+  return m_position;
+}
Index: lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
+++ lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
@@ -10,6 +10,8 @@
 find_library(LIBIPT_LIBRARY ipt PATHS ${LIBIPT_LIBRARY_PATH} REQUIRED)
 
 add_lldb_library(lldbPluginTraceIntelPT PLUGIN
+  DecodedThread.cpp
+  IntelPTDecoder.cpp
   TraceIntelPT.cpp
   TraceIntelPTSessionFileParser.cpp
 
Index: lldb/source/Core/Disassembler.cpp
===================================================================
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -952,6 +952,13 @@
   return inst_sp;
 }
 
+InstructionSP InstructionList::GetInstructionAtAddress(const Address &address) {
+  uint32_t index = GetIndexOfInstructionAtAddress(address);
+  if (index != UINT32_MAX)
+    return GetInstructionAtIndex(index);
+  return nullptr;
+}
+
 void InstructionList::Dump(Stream *s, bool show_address, bool show_bytes,
                            const ExecutionContext *exe_ctx) {
   const uint32_t max_opcode_byte_size = GetMaxOpcocdeByteSize();
Index: lldb/source/Commands/Options.td
===================================================================
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -1008,16 +1008,14 @@
 let Command = "thread trace dump instructions" in {
   def thread_trace_dump_instructions_count : Option<"count", "c">, Group<1>,
     Arg<"Count">,
-    Desc<"The number of instructions to display starting at the current "
-    "position in reverse order chronologically.">;
-  def thread_trace_dump_instructions_start_position:
-    Option<"start-position", "s">,
+    Desc<"The number of instructions to display ending at the current position.">;
+  def thread_trace_dump_instructions_position : Option<"position", "p">,
     Group<1>,
     Arg<"Index">,
-    Desc<"The position of the first instruction to print. Defaults to the "
-    "current position, i.e. where the thread is stopped. The instructions are "
-    "indexed in reverse order, which means that a start position of 0 refers "
-    "to the last instruction chronologically.">;
+    Desc<"The position to use instead of the current position of the trace.">;
+  def thread_trace_dump_instructions_raw : Option<"raw", "r">,
+    Group<1>,
+    Desc<"Dump only instruction address without disassembly nor symbol information.">;
 }
 
 let Command = "type summary add" in {
Index: lldb/source/Commands/CommandObjectThread.cpp
===================================================================
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -2200,15 +2200,19 @@
           m_count = count;
         break;
       }
-      case 's': {
-        int32_t start_position;
-        if (option_arg.empty() || option_arg.getAsInteger(0, start_position) ||
-            start_position < 0)
+      case 'p': {
+        int32_t position;
+        if (option_arg.empty() || option_arg.getAsInteger(0, position) ||
+            position < 0)
           error.SetErrorStringWithFormat(
               "invalid integer value for option '%s'",
               option_arg.str().c_str());
         else
-          m_start_position = start_position;
+          m_position = position;
+        break;
+      }
+      case 'r': {
+        m_raw = true;
         break;
       }
       default:
@@ -2219,19 +2223,20 @@
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_count = kDefaultCount;
-      m_start_position = kDefaultStartPosition;
+      m_position = llvm::None;
+      m_raw = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_thread_trace_dump_instructions_options);
     }
 
-    static const uint32_t kDefaultCount = 20;
-    static const uint32_t kDefaultStartPosition = 0;
+    static const size_t kDefaultCount = 20;
 
     // Instance variables to hold the values for command options.
-    uint32_t m_count;
-    uint32_t m_start_position;
+    size_t m_count;
+    llvm::Optional<ssize_t> m_position;
+    bool m_raw;
   };
 
   CommandObjectTraceDumpInstructions(CommandInterpreter &interpreter)
@@ -2253,30 +2258,24 @@
                                uint32_t index) override {
     current_command_args.GetCommandString(m_repeat_command);
     m_create_repeat_command_just_invoked = true;
+    m_consecutive_repetitions = 0;
     return m_repeat_command.c_str();
   }
 
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
+    if (IsRepeatCommand())
+      m_consecutive_repetitions++;
     bool status = CommandObjectIterateOverThreads::DoExecute(args, result);
-    PrepareRepeatArguments();
-    return status;
-  }
 
-  void PrepareRepeatArguments() {
-    m_repeat_start_position = m_options.m_count + GetStartPosition();
     m_create_repeat_command_just_invoked = false;
+    return status;
   }
 
   bool IsRepeatCommand() {
     return !m_repeat_command.empty() && !m_create_repeat_command_just_invoked;
   }
 
-  uint32_t GetStartPosition() {
-    return IsRepeatCommand() ? m_repeat_start_position
-                             : m_options.m_start_position;
-  }
-
   bool HandleOneThread(lldb::tid_t tid, CommandReturnObject &result) override {
     const TraceSP &trace_sp = m_exe_ctx.GetTargetSP()->GetTrace();
     if (!trace_sp) {
@@ -2287,8 +2286,15 @@
     ThreadSP thread_sp =
         m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid);
 
-    trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
-                                    m_options.m_count, GetStartPosition());
+    size_t count = m_options.m_count;
+    ssize_t position = m_options.m_position.getValueOr(
+                           trace_sp->GetCursorPosition(*thread_sp)) -
+                       m_consecutive_repetitions * count;
+    if (position < 0)
+      result.SetError("error: no more data");
+    else
+      trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
+                                      count, position, m_options.m_raw);
     return true;
   }
 
@@ -2297,7 +2303,7 @@
   // Repeat command helpers
   std::string m_repeat_command;
   bool m_create_repeat_command_just_invoked;
-  uint32_t m_repeat_start_position;
+  size_t m_consecutive_repetitions = 0;
 };
 
 // CommandObjectMultiwordTraceDump
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2108,7 +2108,7 @@
         return status.
         """
         # Fail fast if 'cmd' is not meaningful.
-        if not cmd or len(cmd) == 0:
+        if cmd is None:
             raise Exception("Bad 'cmd' parameter encountered")
 
         trace = (True if traceAlways else trace)
Index: lldb/include/lldb/Target/Trace.h
===================================================================
--- lldb/include/lldb/Target/Trace.h
+++ lldb/include/lldb/Target/Trace.h
@@ -35,6 +35,11 @@
 class Trace : public PluginInterface,
               public std::enable_shared_from_this<Trace> {
 public:
+  enum class TraceDirection {
+    Forwards = 0,
+    Backwards,
+  };
+
   /// Dump the trace data that this plug-in has access to.
   ///
   /// This function will dump all of the trace data for all threads in a user
@@ -98,12 +103,24 @@
   ///     The JSON schema of this Trace plug-in.
   virtual llvm::StringRef GetSchema() = 0;
 
-  /// Dump \a count instructions of the given thread's \a Trace starting at the
-  /// \a start_position position in reverse order.
+  /// Each decoded thread contains a cursor to the current position the user is
+  /// stopped at. When reverse debugging, each operation like reverse-next or
+  /// reverse-continue will move this cursor, which is then picked by any
+  /// subsequent dump or reverse operation.
+  ///
+  /// The initial position for this cursor is the last element of the thread,
+  /// which is the most recent chronologically.
+  ///
+  /// \return
+  ///     The current position of the thread's trace or \b 0 if empty.
+  virtual size_t GetCursorPosition(const Thread &thread) = 0;
+
+  /// Dump \a count instructions of the given thread's trace ending at the
+  /// given \a end_position position.
   ///
-  /// The instructions are indexed in reverse order, which means that the \a
-  /// start_position 0 represents the last instruction of the trace
-  /// chronologically.
+  /// The instructions are printed along with their indices or positions, which
+  /// are increasing chronologically. This means that the \a index 0 represents
+  /// the oldest instruction of the trace chronologically.
   ///
   /// \param[in] thread
   ///     The thread whose trace will be dumped.
@@ -114,10 +131,54 @@
   /// \param[in] count
   ///     The number of instructions to print.
   ///
-  /// \param[in] start_position
-  ///     The position of the first instruction to print.
+  /// \param[in] end_position
+  ///     The position of the last instruction to print.
+  ///
+  /// \param[in] raw
+  ///     Dump only instruction addresses without disassembly nor symbol
+  ///     information.
   void DumpTraceInstructions(Thread &thread, Stream &s, size_t count,
-                             size_t start_position) const;
+                             size_t end_position, bool raw);
+
+  /// Run the provided callback on the instructions of the trace of the given
+  /// thread.
+  ///
+  /// The instructions will be traversed starting at the given \a position
+  /// sequentially until the callback returns \b false, in which case no more
+  /// instructions are inspected.
+  ///
+  /// The purpose of this method is to allow inspecting traced instructions
+  /// without exposing the internal representation of how they are stored on
+  /// memory.
+  ///
+  /// \param[in] thread
+  ///     The thread whose trace will be traversed.
+  ///
+  /// \param[in] position
+  ///     The instruction position to start iterating on.
+  ///
+  /// \param[in] direction
+  ///     If \b TraceDirection::Forwards, then then instructions will be
+  ///     traversed forwards chronologically, i.e. with incrementing indices. If
+  ///     \b TraceDirection::Backwards, the traversal is done backwards
+  ///     chronologically, i.e. with decrementing indices.
+  ///
+  /// \param[in] callback
+  ///     The callback to execute on each instruction. If it returns \b false,
+  ///     the iteration stops.
+  virtual void TraverseInstructions(
+      const Thread &thread, size_t position, TraceDirection direction,
+      std::function<bool(size_t index, llvm::Expected<lldb::addr_t> load_addr)>
+          callback) = 0;
+
+  /// Get the number of available instructions in the trace of the given thread.
+  ///
+  /// \param[in] thread
+  ///     The thread whose trace will be inspected.
+  ///
+  /// \return
+  ///     The total number of instructions in the trace.
+  virtual size_t GetInstructionCount(const Thread &thread) = 0;
 };
 
 } // namespace lldb_private
Index: lldb/include/lldb/Symbol/SymbolContext.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolContext.h
+++ lldb/include/lldb/Symbol/SymbolContext.h
@@ -139,11 +139,19 @@
   ///     be printed.  In disassembly formatting, where we want a format
   ///     like "<*+36>", this should be false and "*" will be printed
   ///     instead.
+  ///
+  /// \param[in] show_inline_callsite_line_info
+  ///     When processing an inline block, the line info of the callsite
+  ///     is dumped if this flag is \b true, otherwise the line info
+  ///     of the actual inlined function is dumped.
+  ///
+  /// \return
+  ///     \b true if some text was dumped, \b false otherwise.
   bool DumpStopContext(Stream *s, ExecutionContextScope *exe_scope,
                        const Address &so_addr, bool show_fullpaths,
                        bool show_module, bool show_inlined_frames,
-                       bool show_function_arguments,
-                       bool show_function_name) const;
+                       bool show_function_arguments, bool show_function_name,
+                       bool show_inline_callsite_line_info = true) const;
 
   /// Get the address range contained within a symbol context.
   ///
Index: lldb/include/lldb/Core/Disassembler.h
===================================================================
--- lldb/include/lldb/Core/Disassembler.h
+++ lldb/include/lldb/Core/Disassembler.h
@@ -271,6 +271,13 @@
 
   lldb::InstructionSP GetInstructionAtIndex(size_t idx) const;
 
+  /// Get the instruction at the given address.
+  ///
+  /// \return
+  ///    A valid \a InstructionSP if the address could be found, or null
+  ///    otherwise.
+  lldb::InstructionSP GetInstructionAtAddress(const Address &addr);
+
   //------------------------------------------------------------------
   /// Get the index of the next branch instruction.
   ///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to