[Lldb-commits] [PATCH] D108050: [wip] [trace] Hierarchical Trace Representation (HTR) redesign and "call stack trace" visualization

2021-08-13 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision.
Herald added a subscriber: mgorny.
jj10306 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Creating this patch so all the work from my internship is public and so I can 
continue to work on HTR if I'm able to get a hold of a machine supports Intel 
PT (-:

- Refactor/redesign the first iteration HTR implementation from 
https://reviews.llvm.org/D105741
- Add HTR support for TSC and add ability to see approximate timestamps in the 
trace visualization
- Change BasicSuperBlockPass to PatternMerge and the pass now is aware of 
functions

Here's an example of the "call stack trace" visualization:
F18512493: Screen Shot 2021-08-12 at 9.29.13 PM.png 


The arrows in the image above point from a function call in the program to the 
corresponding block in the trace. Looking at the visualization on the right, 
you'll notice that the first two `push_back` blocks are significantly smaller 
than the third `push_back` block. 
Well, if we look at the code on the left, this is exactly what we would expect; 
the third call to `push_back` on line 9 will require a reallocation since we 
defined the capacity of the vector to be 2 on line 6!
Although contrived, this example illustrates how the "call stack trace" 
visualization allows you to gain deeper insight about the behavior of a program.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108050

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/CMakeLists.txt
  lldb/source/Plugins/TraceExporter/common/LayerHTR.cpp
  lldb/source/Plugins/TraceExporter/common/LayerHTR.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Plugins/TraceExporter/common/TraceHTR.h
  lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
  lldb/test/API/commands/trace/TestTraceExport.py

Index: lldb/test/API/commands/trace/TestTraceExport.py
===
--- lldb/test/API/commands/trace/TestTraceExport.py
+++ lldb/test/API/commands/trace/TestTraceExport.py
@@ -35,16 +35,18 @@
 error=True)
 
 def testExportCreatesFile(self):
-self.expect("trace load -v " +
-os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
-substrs=["intel-pt"])
-
-ctf_test_file = self.getBuildArtifact("ctf-test.json")
-
-if os.path.exists(ctf_test_file):
-remove_file(ctf_test_file)
-self.expect(f"thread trace export ctf --file {ctf_test_file}")
-self.assertTrue(os.path.exists(ctf_test_file))
+# TODO: Update this test
+pass
+#self.expect("trace load -v " +
+#os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+#substrs=["intel-pt"])
+#
+#ctf_test_file = self.getBuildArtifact("ctf-test.json")
+#
+#if os.path.exists(ctf_test_file):
+#remove_file(ctf_test_file)
+#self.expect(f"thread trace export ctf --file {ctf_test_file}")
+#self.assertTrue(os.path.exists(ctf_test_file))
 
 
 def testHtrBasicSuperBlockPass(self):
@@ -55,55 +57,60 @@
 trace of this program and load it like the other tests instead of
 manually executing the commands to trace the program.
 '''
-self.expect(f"target create {os.path.join(self.getSourceDir(), 'intelpt-trace', 'export_ctf_test_program.out')}")
-self.expect("b main")
-self.expect("r")
-self.expect("b exit")
-self.expect("thread trace start")
-self.expect("c")
-
-ctf_test_file = self.getBuildArtifact("ctf-test.json")
-
-if os.path.exists(ctf_test_file):
-remove_file(ctf_test_file)
-self.expect(f"thread trace export ctf --file {ctf_test_file}")
-self.assertTrue(os.path.exists(ctf_test_file))
-
-
-with open(ctf_test_file) as f:
-data = json.load(f)
-
-num_units_by_layer = defaultdict(int)
-index_of_first_layer_1_block = None
-for i, event in enumerate(data):
-layer_id = event.get('pid')
-if layer_id == 1 and index_of_first_layer_1_block is None:
-index_of_first_layer_1_block = i
-if layer_id is not None and event['ph'] == 'B':
-num_units_by_layer[layer_id] += 1
-
-# Check that there are two layers
-self.assertTrue(0 in num_units_by_layer and 1 in num_units_by_layer)
-# Check that each layer has the correct total number of blocks
-self.assertTrue(num_units_by_layer[0] == 1630)
-self.assertTrue(num_units_by_layer[1] == 383)
-
-
-exp

[Lldb-commits] [PATCH] D108053: [lldb] skip host build for lldb_tblgen with LLDB_TABLEGEN_EXE set

2021-08-13 Thread Manoj Gupta via Phabricator via lldb-commits
manojgupta created this revision.
manojgupta added reviewers: JDevlieghere, mstorsjo.
Herald added a subscriber: mgorny.
manojgupta requested review of this revision.
Herald added a project: LLDB.

When cross compiling lldb-server, do not create a host build
for building lldb-tblgeb when LLDB_TABLEGEN_EXE is already
provided. This avoids an expensive and time-consuming build step
if lldb-tblgen was already built previously for host.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108053

Files:
  lldb/CMakeLists.txt


Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -64,7 +64,7 @@
   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 endif()
 
-if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE)
+if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE AND NOT LLDB_TABLEGEN_EXE)
   set(LLVM_USE_HOST_TOOLS ON)
   include(CrossCompile)
   if (NOT NATIVE_LLVM_DIR OR NOT NATIVE_Clang_DIR)


Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -64,7 +64,7 @@
   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 endif()
 
-if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE)
+if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE AND NOT LLDB_TABLEGEN_EXE)
   set(LLVM_USE_HOST_TOOLS ON)
   include(CrossCompile)
   if (NOT NATIVE_LLVM_DIR OR NOT NATIVE_Clang_DIR)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D108053: [lldb] skip host build for lldb_tblgen with LLDB_TABLEGEN_EXE set

2021-08-13 Thread Manoj Gupta via Phabricator via lldb-commits
manojgupta added a comment.

This is a local patch we have been carrying in Chrome OS.
What we do to cross compile lldb-server:

1. Create host build to build tools like llvm-tblgen, clang-tblgen, lldb-tblgen
2. Cross compile llvm and clang libraries using the just build *tblgen.
3. Cross compile lldb by setting the path to pre-cross compiled libraries and 
*tblgen binaries from step 1.

Without this patch, cross compiling lldb-server start yet another host build 
just to build lldb-tblgen which we want to avoid.
Also see 
https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/2851114/12/dev-util/lldb-server/lldb-server-12.0_pre416183.ebuild


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108053

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D108053: [lldb] skip host build for lldb_tblgen with LLDB_TABLEGEN_EXE set

2021-08-13 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Looks reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108053

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 1f7b25e - [lldb] skip host build for lldb_tblgen with LLDB_TABLEGEN_EXE set

2021-08-13 Thread Manoj Gupta via lldb-commits

Author: Manoj Gupta
Date: 2021-08-13T14:18:03-07:00
New Revision: 1f7b25ea76a925aca690da28de9d78db7ca99d0c

URL: 
https://github.com/llvm/llvm-project/commit/1f7b25ea76a925aca690da28de9d78db7ca99d0c
DIFF: 
https://github.com/llvm/llvm-project/commit/1f7b25ea76a925aca690da28de9d78db7ca99d0c.diff

LOG: [lldb] skip host build for lldb_tblgen with LLDB_TABLEGEN_EXE set

When cross compiling lldb-server, do not create a host build
for building lldb-tblgeb when LLDB_TABLEGEN_EXE is already
provided. This avoids an expensive and time-consuming build step
if lldb-tblgen was already built previously for host.

Reviewed By: JDevlieghere

Differential Revision: https://reviews.llvm.org/D108053

Added: 


Modified: 
lldb/CMakeLists.txt

Removed: 




diff  --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
index 2bb05c1e220b3..594c769141b43 100644
--- a/lldb/CMakeLists.txt
+++ b/lldb/CMakeLists.txt
@@ -64,7 +64,7 @@ if(LLVM_ENABLE_MODULES)
   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 endif()
 
-if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE)
+if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE AND NOT LLDB_TABLEGEN_EXE)
   set(LLVM_USE_HOST_TOOLS ON)
   include(CrossCompile)
   if (NOT NATIVE_LLVM_DIR OR NOT NATIVE_Clang_DIR)



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D108053: [lldb] skip host build for lldb_tblgen with LLDB_TABLEGEN_EXE set

2021-08-13 Thread Manoj Gupta via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f7b25ea76a9: [lldb] skip host build for lldb_tblgen with 
LLDB_TABLEGEN_EXE set (authored by manojgupta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108053

Files:
  lldb/CMakeLists.txt


Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -64,7 +64,7 @@
   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 endif()
 
-if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE)
+if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE AND NOT LLDB_TABLEGEN_EXE)
   set(LLVM_USE_HOST_TOOLS ON)
   include(CrossCompile)
   if (NOT NATIVE_LLVM_DIR OR NOT NATIVE_Clang_DIR)


Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -64,7 +64,7 @@
   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 endif()
 
-if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE)
+if(CMAKE_CROSSCOMPILING AND LLDB_BUILT_STANDALONE AND NOT LLDB_TABLEGEN_EXE)
   set(LLVM_USE_HOST_TOOLS ON)
   include(CrossCompile)
   if (NOT NATIVE_LLVM_DIR OR NOT NATIVE_Clang_DIR)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-13 Thread Rumeet Dhindsa via Phabricator via lldb-commits
rdhindsa created this revision.
rdhindsa added reviewers: labath, clayborg.
Herald added a subscriber: emaste.
rdhindsa requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: LLDB.

Add support for shared library load when executable called through ld.

Currently, when an executable is called like: ld-linux --library-path  
executable, lldb is not able to load the shared libraries needed by the 
executable provided by library-path. lldb is not able to set Rendezvous 
breakpoint in this case.

This patch adds the support to enable setting rendezvous breakpoint when called 
using ld-*.so. It enables it to hit the breakpoint and extract the address of 
the rendezvous structure, which is how lldb is now able to load the shared 
libraries.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108061

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/test/Shell/ObjectFile/ELF/Inputs/main.cpp
  lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.cpp
  lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.h
  lldb/test/Shell/ObjectFile/ELF/ld_test.test

Index: lldb/test/Shell/ObjectFile/ELF/ld_test.test
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/ld_test.test
@@ -0,0 +1,20 @@
+# REQUIRES: x86
+# REQUIRES: system-linux
+#
+# RUN: %clang -target x86_64-pc-linux -o %t1.o -c %S/Inputs/signal_file.cpp
+# RUN: %clang -target x86_64-pc-linux -o %t2.o -c %S/Inputs/main.cpp
+# RUN: %clang -target x86_64-pc-linux -shared %t1.o -o %t3.so
+# RUN: %clang -o %tmain %t2.o %t3.so -L. -Wl,-rpath,%t
+#
+# RUN: echo '-n' > %t.in
+# RUN: echo 'run' >> %t.in
+# RUN: echo 'bt' >> %t.in
+#
+# RUN: %lldb -b -s %t.in -- %tmain 2>&1 | FileCheck %s
+#
+# RUN: %lldb -b -s %t.in -- /lib64/ld-linux-x86-64.so.2 --library-path %t  %tmain 2>&1 | FileCheck %s 
+#
+# bt
+# CHECK: (lldb) bt
+# CHECK: ld_test.test.tmp3.so`get_signal_crash()
+#
Index: lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.h
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.h
@@ -0,0 +1 @@
+int get_signal_crash(void);
Index: lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.cpp
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.cpp
@@ -0,0 +1,7 @@
+#include "signal_file.h"
+#include 
+
+int get_signal_crash(void) {
+  raise(SIGSEGV);
+  return 0;
+}
Index: lldb/test/Shell/ObjectFile/ELF/Inputs/main.cpp
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/Inputs/main.cpp
@@ -0,0 +1,3 @@
+#include "signal_file.h"
+
+int main() { return get_signal_crash(); }
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -131,6 +131,9 @@
   lldb_private::Address
   GetImageInfoAddress(lldb_private::Target *target) override;
 
+  lldb_private::Address
+  GetRendezvousStructureAddress(lldb_private::Target *target) override;
+
   lldb_private::Address GetEntryPointAddress() override;
 
   lldb_private::Address GetBaseAddress() override;
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -882,6 +882,21 @@
   return Address();
 }
 
+Address ObjectFileELF::GetRendezvousStructureAddress(Target *target) {
+  ModuleSP module_sp(GetModule());
+  if (module_sp) {
+SymbolContextList contexts;
+SymbolContext context;
+module_sp->FindSymbolsWithNameAndType(ConstString("_r_debug"),
+  eSymbolTypeAny, contexts);
+if (contexts.GetSize()) {
+  if (contexts.GetContextAtIndex(0, context))
+return context.symbol->GetAddress();
+}
+  }
+  return Address();
+}
+
 lldb_private::Address ObjectFileELF::GetEntryPointAddress() {
   if (m_entry_point_address.IsValid())
 return m_entry_point_address;
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -333,28 +333,48 @@
 LLDB_LOG(log, "Rendezvous structure is not set up yet. "
   "Trying to locate rendezvous breakpoint in the interpreter "
   "

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

2021-08-13 Thread walter erquinigo via Phabricator via lldb-commits
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

Almost there. Many cosmetic changes but the logic is all good.




Comment at: lldb/include/lldb/Target/Trace.h:178
+  /// \param[in] tid
   /// The thread in question.
   ///

The id of the thread in question



Comment at: lldb/source/Commands/Options.td:739
 
+
+let Command = "process trace save" in {

remove this line



Comment at: lldb/source/Commands/Options.td:748
+
+
 let Command = "script import" in {

remove this line



Comment at: lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp:28-29
+  oss << "0x" << std::hex << module.load_address.value;
+  std::string str(oss.str());
+  result["loadAddress"] = str;
+  if (module.uuid)

merge into one single line



Comment at: lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp:45-47
+  for (JSONThread e : process.threads) {
+threads_arr.push_back(toJSON(e));
+  }

delete braces



Comment at: lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp:51-53
+  for (JSONModule e : process.modules) {
+modules_arr.push_back(toJSON(e));
+  }

same here



Comment at: lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp:61-63
+  for (JSONProcess e : session_base.processes) {
+arr.push_back(toJSON(e));
+  }

ditto




Comment at: lldb/source/Plugins/Trace/common/TraceJSONStructs.h:28-33
+  JSONModule() = default;
+
+  JSONModule(std::string system_path, llvm::Optional file,
+ JSONAddress load_address, llvm::Optional uuid)
+  : system_path(system_path), file(file), load_address(load_address),
+uuid(uuid) {}

you can omit these constructors because they don't have anything custom



Comment at: lldb/source/Plugins/Trace/common/TraceJSONStructs.h:42-46
+  JSONThread() = default;
+
+  JSONThread(int64_t tid, std::string trace_file)
+  : tid(tid), trace_file(trace_file) {}
+

same here, just delete these lines



Comment at: lldb/source/Plugins/Trace/common/TraceJSONStructs.h:52-57
+  JSONProcess() = default;
+
+  JSONProcess(int64_t pid, std::string triple, std::vector threads,
+  std::vector modules)
+  : pid(pid), triple(triple), threads(threads), modules(modules) {}
+

similarly, delete these



Comment at: lldb/source/Plugins/Trace/common/TraceJSONStructs.h:69-72
+  JSONTraceSessionBase() = default;
+
+  JSONTraceSessionBase(std::vector processes)
+  : processes(processes) {}

delete



Comment at: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp:27
+llvm::Error
+TraceSessionSaver::WriteSessionToFile(const llvm::json::Value 
&trace_session_jo,
+  FileSpec directory) {

trace_session_description



Comment at: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp:34
+  std::ofstream os(trace_path.GetPath());
+  os << out;
+  os.close();

move line 30 here to avoid that unnecessary out variable



Comment at: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp:50
+
+  JSONTraceSessionBase result;
+  Expected> json_threads =

json_session_description



Comment at: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp:58-60
+  if (!json_modules) {
+return json_modules.takeError();
+  }

delete braces



Comment at: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp:77
+FileSpec directory) {
+  std::vector result;
+  for (ThreadSP thread_sp : live_process.Threads()) {

rename this to json_threads



Comment at: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp:86
+   ".trace");
+json_thread.trace_file = raw_trace_path.GetPath().c_str();
+result.push_back(json_thread);

avoid this. Instead of creating first an empty json_thread with invalid values, 
wait until this moment to create a valid JSONThread

  json_threads.push_back(JSONThread { thread_sp->GetID(), 
raw_trace_path.GetPath().c_str() });

that way all the code that you write is valid at all times



Comment at: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp:94-96
+if (!raw_trace.get()) {
+  continue;
+}

delete braces



Comment at: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp:114
+   FileSpec directory) {
+  std::vector result;
+  ModuleList module_list = live_process.GetTarget().GetImages();

json_modules



Comment at: lldb/source/Plugins/Trac