[Lldb-commits] [PATCH] D123015: handle Ctrl+C to stop a running process

[Luboš Luňák via Phabricator via lldb-commits]

llunak created this revision.
llunak added a reviewer: clayborg.
llunak added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
llunak requested review of this revision.
Herald added a subscriber: lldb-commits.

This is essentially the same as IOHandlerEditline::Interrupt(), minus the 
editline stuff.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123015

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Core/IOHandlerCursesGUI.cpp


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -7715,7 +7715,9 @@
 
 void IOHandlerCursesGUI::Cancel() {}
 
-bool IOHandlerCursesGUI::Interrupt() { return false; }
+bool IOHandlerCursesGUI::Interrupt() {
+  return m_debugger.GetCommandInterpreter().IOHandlerInterrupt(*this);
+}
 
 void IOHandlerCursesGUI::GotEOF() {}
 
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -610,6 +610,8 @@
 
   bool IsInteractive();
 
+  bool IOHandlerInterrupt(IOHandler &io_handler) override;
+
 protected:
   friend class Debugger;
 
@@ -623,8 +625,6 @@
 return ConstString();
   }
 
-  bool IOHandlerInterrupt(IOHandler &io_handler) override;
-
   void GetProcessOutput();
 
   bool DidProcessStopAbnormally() const;


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -7715,7 +7715,9 @@
 
 void IOHandlerCursesGUI::Cancel() {}
 
-bool IOHandlerCursesGUI::Interrupt() { return false; }
+bool IOHandlerCursesGUI::Interrupt() {
+  return m_debugger.GetCommandInterpreter().IOHandlerInterrupt(*this);
+}
 
 void IOHandlerCursesGUI::GotEOF() {}
 
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -610,6 +610,8 @@
 
   bool IsInteractive();
 
+  bool IOHandlerInterrupt(IOHandler &io_handler) override;
+
 protected:
   friend class Debugger;
 
@@ -623,8 +625,6 @@
 return ConstString();
   }
 
-  bool IOHandlerInterrupt(IOHandler &io_handler) override;
-
   void GetProcessOutput();
 
   bool DidProcessStopAbnormally() const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D121999: [lldb][AArch64] Update disassembler feature list and add tests for all extensions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> It sounds like we should figure out a way to enable all extensions with a 
> single stroke.

Totally. llvm-objdump could do with a `-mcpu=max` equivalent too.

I think it's possible to get all extension names right now, but not the newest 
arch version. I'll look into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121999

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Luboš Luňák via Phabricator via lldb-commits]

llunak created this revision.
llunak added a reviewer: clayborg.
llunak added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
llunak requested review of this revision.
Herald added a subscriber: lldb-commits.

Valgrind makes everything run much slower, so don't time out too soon.

BTW, does it make sense to get even things like this reviewed, or is it ok if I 
push these directly if I'm reasonably certain I know what I'm doing? I feel 
like I'm spamming you by now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123020

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -34,6 +34,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/Valgrind.h"
 
 #include "ProcessGDBRemoteLog.h"
 
@@ -1244,6 +1245,8 @@
 GDBRemoteCommunication::ScopedTimeout::ScopedTimeout(
 GDBRemoteCommunication &gdb_comm, std::chrono::seconds timeout)
 : m_gdb_comm(gdb_comm), m_timeout_modified(false) {
+  if (llvm::sys::RunningOnValgrind())
+timeout *= 100; // Valgrind makes things take much longer.
   auto curr_timeout = gdb_comm.GetPacketTimeout();
   // Only update the timeout if the timeout is greater than the current
   // timeout. If the current timeout is larger, then just use that.


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -34,6 +34,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/Valgrind.h"
 
 #include "ProcessGDBRemoteLog.h"
 
@@ -1244,6 +1245,8 @@
 GDBRemoteCommunication::ScopedTimeout::ScopedTimeout(
 GDBRemoteCommunication &gdb_comm, std::chrono::seconds timeout)
 : m_gdb_comm(gdb_comm), m_timeout_modified(false) {
+  if (llvm::sys::RunningOnValgrind())
+timeout *= 100; // Valgrind makes things take much longer.
   auto curr_timeout = gdb_comm.GetPacketTimeout();
   // Only update the timeout if the timeout is greater than the current
   // timeout. If the current timeout is larger, then just use that.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 434b545 - [lldb][AArch64] Update disassembler feature list and add tests for all extensions

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-04-04T11:21:01Z
New Revision: 434b545d4fc7824cf03976a8844020e33040855e

URL: 
https://github.com/llvm/llvm-project/commit/434b545d4fc7824cf03976a8844020e33040855e
DIFF: 
https://github.com/llvm/llvm-project/commit/434b545d4fc7824cf03976a8844020e33040855e.diff

LOG: [lldb][AArch64] Update disassembler feature list and add tests for all 
extensions

This updates the disassembler to enable every optional extension.
Previously we had added things that we added "support" for in lldb.
(where support means significant work like new registers, fault types, etc.)

Something like TME (transactional memory) wasn't added because
there are no new lldb features for it. However we should still be
disassembling the instructions.

So I went through the AArch64 extensions and added all the missing
ones. The new test won't prevent us missing a new extension but it
does at least document our current settings.

Reviewed By: labath

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

Added: 
lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s

Modified: 
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp 
b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
index 2777f77eed7ee..930520ad6090b 100644
--- a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -1180,7 +1180,9 @@ DisassemblerLLVMC::DisassemblerLLVMC(const ArchSpec &arch,
   // If any AArch64 variant, enable latest ISA with any optional
   // extensions like MTE.
   if (triple.isAArch64()) {
-features_str += "+v9.3a,+mte";
+features_str += "+v9.3a,+mte,+sm4,+sha2,+sha3,+aes,+fp16fml,+sve2-aes,+"
+"sve2-sm4,+sve2-sha3,+sve2-bitperm,+f32mm,+f64mm,+tme,+"
+"ls64,+sme,+sme-f64,+sme-i64,+spe,+rand,+brbe";
 
 if (triple.getVendor() == llvm::Triple::Apple)
   cpu = "apple-latest";

diff  --git a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s 
b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
new file mode 100644
index 0..ede6fdb3471ba
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
@@ -0,0 +1,104 @@
+# REQUIRES: aarch64
+
+# This checks that lldb's disassembler enables every extension that an AArch64
+# target could have.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-linux-gnueabihf %s -o %t \
+# RUN: --mattr=+tme,+mte,+crc,+lse,+rdm,+sm4,+sha3,+aes,+dotprod,+fullfp16 \
+# RUN: 
--mattr=+fp16fml,+sve,+sve2,+sve2-aes,+sve2-sm4,+sve2-sha3,+sve2-bitperm \
+# RUN: --mattr=+spe,+rcpc,+ssbs,+sb,+predres,+bf16,+mops,+hbc,+sme,+sme-i64 \
+# RUN: --mattr=+sme-f64,+flagm,+pauth,+brbe,+ls64,+f64mm,+f32mm,+i8mm,+rand
+# RUN: %lldb %t -o "disassemble -n fn" -o exit 2>&1 | FileCheck %s
+
+.globl  fn
+.type   fn, @function
+fn:
+  // These are in the same order as 
llvm/include/llvm/Support/AArch64TargetParser.def
+  crc32b w0, w0, w0   // CRC
+  ldaddab w0, w0, [sp]// LSE
+  sqrdmlah v0.4h, v1.4h, v2.4h// RDM
+  // CRYPTO enables a combination of other features
+  sm4e v0.4s, v0.4s   // SM4
+  bcax v0.16b, v0.16b, v0.16b, v0.16b // SHA3
+  sha256h q0, q0, v0.4s   // SHA256
+  aesd v0.16b, v0.16b // AES
+  sdot v0.2s, v1.8b, v2.8b// DOTPROD
+  fcvt d0, s0 // FP
+  addp v0.4s, v0.4s, v0.4s// SIMD (neon)
+  fabs h1, h2 // FP16
+  fmlal v0.2s, v1.2h, v2.2h   // FP16FML
+  psb csync   // PROFILE/SPE
+  msr erxpfgctl_el1, x0   // RAS
+  abs z31.h, p7/m, z31.h  // SVE
+  sqdmlslbt z0.d, z1.s, z31.s // SVE2
+  aesd z0.b, z0.b, z31.b  // SVE2AES
+  sm4e z0.s, z0.s, z0.s   // SVE2SM4
+  rax1 z0.d, z0.d, z0.d   // SVE2SHA3
+  bdep z0.b, z1.b, z31.b  // SVE2BITPERM
+  ldaprb w0, [x0, #0] // RCPC
+  mrs x0, rndr// RAND
+  irg x0, x0  // MTE
+  mrs x2, ssbs// SSBS
+  sb  // SB
+  cfp rctx, x0// PREDRES
+  bfdot v2.2s, v3.4h, v4.4h   // BF16
+  smmla v1.4s, v16.16b, v31.16b   // I8MM
+  fmmla z0.s, z1.s, z2.s  // F32MM
+  fmmla z0.d, z1.d, z2.d  // F64MM
+  tcommit // TME
+  ld64b x0, [x13] // LS64
+  brb iall// BRBE
+  pacia x0, x1// PAUTH
+  cfinv   // FLAGM
+  addha za0.s, p0/m, p0/m, z0.s   // SME
+  fmopa za0.d, p0/m, p0/m, z0.d, z0.d // SMEF64
+  addha za0.d

[Lldb-commits] [PATCH] D121999: [lldb][AArch64] Update disassembler feature list and add tests for all extensions

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG434b545d4fc7: [lldb][AArch64] Update disassembler feature 
list and add tests for all… (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121999

Files:
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s

Index: lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s
@@ -0,0 +1,104 @@
+# REQUIRES: aarch64
+
+# This checks that lldb's disassembler enables every extension that an AArch64
+# target could have.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-linux-gnueabihf %s -o %t \
+# RUN: --mattr=+tme,+mte,+crc,+lse,+rdm,+sm4,+sha3,+aes,+dotprod,+fullfp16 \
+# RUN: --mattr=+fp16fml,+sve,+sve2,+sve2-aes,+sve2-sm4,+sve2-sha3,+sve2-bitperm \
+# RUN: --mattr=+spe,+rcpc,+ssbs,+sb,+predres,+bf16,+mops,+hbc,+sme,+sme-i64 \
+# RUN: --mattr=+sme-f64,+flagm,+pauth,+brbe,+ls64,+f64mm,+f32mm,+i8mm,+rand
+# RUN: %lldb %t -o "disassemble -n fn" -o exit 2>&1 | FileCheck %s
+
+.globl  fn
+.type   fn, @function
+fn:
+  // These are in the same order as llvm/include/llvm/Support/AArch64TargetParser.def
+  crc32b w0, w0, w0   // CRC
+  ldaddab w0, w0, [sp]// LSE
+  sqrdmlah v0.4h, v1.4h, v2.4h// RDM
+  // CRYPTO enables a combination of other features
+  sm4e v0.4s, v0.4s   // SM4
+  bcax v0.16b, v0.16b, v0.16b, v0.16b // SHA3
+  sha256h q0, q0, v0.4s   // SHA256
+  aesd v0.16b, v0.16b // AES
+  sdot v0.2s, v1.8b, v2.8b// DOTPROD
+  fcvt d0, s0 // FP
+  addp v0.4s, v0.4s, v0.4s// SIMD (neon)
+  fabs h1, h2 // FP16
+  fmlal v0.2s, v1.2h, v2.2h   // FP16FML
+  psb csync   // PROFILE/SPE
+  msr erxpfgctl_el1, x0   // RAS
+  abs z31.h, p7/m, z31.h  // SVE
+  sqdmlslbt z0.d, z1.s, z31.s // SVE2
+  aesd z0.b, z0.b, z31.b  // SVE2AES
+  sm4e z0.s, z0.s, z0.s   // SVE2SM4
+  rax1 z0.d, z0.d, z0.d   // SVE2SHA3
+  bdep z0.b, z1.b, z31.b  // SVE2BITPERM
+  ldaprb w0, [x0, #0] // RCPC
+  mrs x0, rndr// RAND
+  irg x0, x0  // MTE
+  mrs x2, ssbs// SSBS
+  sb  // SB
+  cfp rctx, x0// PREDRES
+  bfdot v2.2s, v3.4h, v4.4h   // BF16
+  smmla v1.4s, v16.16b, v31.16b   // I8MM
+  fmmla z0.s, z1.s, z2.s  // F32MM
+  fmmla z0.d, z1.d, z2.d  // F64MM
+  tcommit // TME
+  ld64b x0, [x13] // LS64
+  brb iall// BRBE
+  pacia x0, x1// PAUTH
+  cfinv   // FLAGM
+  addha za0.s, p0/m, p0/m, z0.s   // SME
+  fmopa za0.d, p0/m, p0/m, z0.d, z0.d // SMEF64
+  addha za0.d, p0/m, p0/m, z0.d   // SMEI64
+lbl:
+  bc.eq lbl   // HBC
+  cpyfp [x0]!, [x1]!, x2! // MOPS
+  mrs x0, pmccntr_el0 // PERFMON
+.fn_end:
+  .size   fn, .fn_end-fn
+
+# CHECK: command-disassemble-aarch64-extensions.s.tmp`fn:
+# CHECK: crc32b w0, w0, w0
+# CHECK: ldaddab w0, w0, [sp]
+# CHECK: sqrdmlah v0.4h, v1.4h, v2.4h
+# CHECK: sm4e   v0.4s, v0.4s
+# CHECK: bcax   v0.16b, v0.16b, v0.16b, v0.16b
+# CHECK: sha256h q0, q0, v0.4s
+# CHECK: aesd   v0.16b, v0.16b
+# CHECK: sdot   v0.2s, v1.8b, v2.8b
+# CHECK: fcvt   d0, s0
+# CHECK: addp   v0.4s, v0.4s, v0.4s
+# CHECK: fabs   h1, h2
+# CHECK: fmlal  v0.2s, v1.2h, v2.2h
+# CHECK: psbcsync
+# CHECK: msrERXPFGCTL_EL1, x0
+# CHECK: absz31.h, p7/m, z31.h
+# CHECK: sqdmlslbt z0.d, z1.s, z31.s
+# CHECK: aesd   z0.b, z0.b, z31.b
+# CHECK: sm4e   z0.s, z0.s, z0.s
+# CHECK: rax1   z0.d, z0.d, z0.d
+# CHECK: bdep   z0.b, z1.b, z31.b
+# CHECK: ldaprb w0, [x0]
+# CHECK: mrsx0, RNDR
+# CHECK: irgx0, x0
+# CHECK: mrsx2, SSBS
+# CHECK: sb
+# CHECK: cfprctx, x0
+# CHECK: bfdot  v2.2s, v3.4h, v4.4h
+# CHECK: smmla  v1.4s, v16.16b, v31.16b
+# CHECK: fmmla  z0.s, z1.s, z2.s
+# CHECK: fmmla  z0.d, z1.d, z2.d
+# CHECK: tcommit
+# CHECK: ld64b  x0, [x13]
+# CHECK: brbiall
+# CHECK: pacia  x0, x1
+# CHECK: cfinv
+# CHECK: addha  za0.s, p0/m, p0/m, z0.s
+# CHECK: fmopa  za0.d, p0/m, p0/m, z0.d, z0.d
+# CHECK: addha  za0.d, p0/m, p0/m, z0.d
+# CHECK: bc.eq  0x98
+# CHECK: cpyfp  [x0]!, [x1]!, x2!
+# CHECK: mrsx0, PMCCNTR_EL0
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/sourc

[Lldb-commits] [PATCH] D123025: [lldb-vscode] Implement stderr/stdout on win32 and redirect lldb log to VSCode

[Michael Hept via Phabricator via lldb-commits]

nidefawl created this revision.
nidefawl added a reviewer: wallace.
nidefawl added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
nidefawl requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch implements stderr/stdout forwarding on windows.
This was previously not implemented in D99974 .
I added separate callbacks so the output can be sent to the different channels 
VSCode provides (OutputType::Stdout, OutputType::Stderr, OutputType::Console).

This patch also passes a handler a log callback to SBDebugger::Create to be 
able to see logging output when it is enabled.

Since the output is now redirect on early startup I removed the calls to 
SetOutputFileHandle/SetErrorFileHandle, which set them to /dev/null.

I send the output of stderr/stdout/lldb log to OutputType::Console


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123025

Files:
  lldb/tools/lldb-vscode/OutputRedirector.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -65,11 +65,6 @@
 #define PATH_MAX MAX_PATH
 #endif
 typedef int socklen_t;
-constexpr const char *dev_null_path = "nul";
-
-#else
-constexpr const char *dev_null_path = "/dev/null";
-
 #endif
 
 using namespace lldb_vscode;
@@ -1446,23 +1441,13 @@
 //   }]
 // }
 void request_initialize(const llvm::json::Object &request) {
-  g_vsc.debugger = lldb::SBDebugger::Create(true /*source_init_files*/);
+  auto log_cb = [](const char *buf, void *baton) -> void {
+g_vsc.SendOutput(OutputType::Console, llvm::StringRef{buf});
+  };
+  g_vsc.debugger =
+  lldb::SBDebugger::Create(true /*source_init_files*/, log_cb, nullptr);
   g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction);
 
-  // Create an empty target right away since we might get breakpoint requests
-  // before we are given an executable to launch in a "launch" request, or a
-  // executable when attaching to a process by process ID in a "attach"
-  // request.
-  FILE *out = llvm::sys::RetryAfterSignal(nullptr, fopen, dev_null_path, "w");
-  if (out) {
-// Set the output and error file handles to redirect into nothing otherwise
-// if any code in LLDB prints to the debugger file handles, the output and
-// error file handles are initialized to STDOUT and STDERR and any output
-// will kill our debug session.
-g_vsc.debugger.SetOutputFileHandle(out, true);
-g_vsc.debugger.SetErrorFileHandle(out, false);
-  }
-
   // Start our event thread so we can receive events from the debugger, target,
   // process and more.
   g_vsc.event_thread = std::thread(EventThreadFunction);
@@ -3147,18 +3132,25 @@
 /// \return
 /// A fd pointing to the original stdout.
 int SetupStdoutStderrRedirection() {
-  int new_stdout_fd = dup(fileno(stdout));
-  auto stdout_err_redirector_callback = [&](llvm::StringRef data) {
-g_vsc.SendOutput(OutputType::Console, data);
+  int stdoutfd = fileno(stdout);
+  int new_stdout_fd = dup(stdoutfd);
+  auto output_callback_stderr = [](llvm::StringRef data) {
+g_vsc.SendOutput(OutputType::Stderr, data);
   };
-
-  for (int fd : {fileno(stdout), fileno(stderr)}) {
-if (llvm::Error err = RedirectFd(fd, stdout_err_redirector_callback)) {
-  std::string error_message = llvm::toString(std::move(err));
-  if (g_vsc.log)
-*g_vsc.log << error_message << std::endl;
-  stdout_err_redirector_callback(error_message);
-}
+  auto output_callback_stdout = [](llvm::StringRef data) {
+g_vsc.SendOutput(OutputType::Stdout, data);
+  };
+  if (llvm::Error err = RedirectFd(stdoutfd, output_callback_stdout)) {
+std::string error_message = llvm::toString(std::move(err));
+if (g_vsc.log)
+  *g_vsc.log << error_message << std::endl;
+output_callback_stderr(error_message);
+  }
+  if (llvm::Error err = RedirectFd(fileno(stderr), output_callback_stderr)) {
+std::string error_message = llvm::toString(std::move(err));
+if (g_vsc.log)
+  *g_vsc.log << error_message << std::endl;
+output_callback_stderr(error_message);
   }
 
   /// used only by TestVSCode_redirection_to_console.py
Index: lldb/tools/lldb-vscode/OutputRedirector.cpp
===
--- lldb/tools/lldb-vscode/OutputRedirector.cpp
+++ lldb/tools/lldb-vscode/OutputRedirector.cpp
@@ -8,18 +8,25 @@
 
 #if !defined(_WIN32)
 #include 
+#else
+#include 
+#include 
 #endif
 
 #include "OutputRedirector.h"
+#include "llvm/ADT/StringRef.h"
 
 using namespace llvm;
 
 namespace lldb_vscode {
 
 Error RedirectFd(int fd, std::function callback) {
-#if !defined(_WIN32)
   int new_fd[2];
+#if defined(_WIN32)
+  if (_pipe(new_fd, 4096, O_TEXT) == -1) {
+#else
   if (pipe(new_fd) == -1) {
+#endif
 int error 

[Lldb-commits] [PATCH] D123025: [lldb-vscode] Implement stderr/stdout on win32 and redirect lldb log to VSCode

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a reviewer: labath.
mstorsjo added inline comments.



Comment at: lldb/tools/lldb-vscode/OutputRedirector.cpp:12
+#else
+#include 
+#include 

Minor style issue - I guess it'd be less of double negation, if we'd change the 
ifdef to `#if defined(_WIN32) .. #else`



Comment at: lldb/tools/lldb-vscode/OutputRedirector.cpp:55
   }
-  callback(StringRef(buffer, bytes_count).str());
+  callback(StringRef(buffer, bytes_count));
 }

This change looks unrelated (although I'm not familiar with this piece of 
code), although it's probably correct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123025

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

[Lldb-commits] [PATCH] D122943: [LLDB][NativePDB] Fix a crash when S_DEFRANGE_SUBFIELD_REGISTER descirbes a simple type

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Seems reasonable, but I don't know much about pdb's. @rnk, do you want to take 
a look?




Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp:54
 struct FindMembersSize : public TypeVisitorCallbacks {
-  FindMembersSize(std::vector> &members_info,
+  FindMembersSize(llvm::SmallVector> 
&members_info,
   TpiStream &tpi)

I think `SmallVectorImpl` is still the official way to take SmallVector 
references.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122943

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

[Lldb-commits] [PATCH] D122944: [lldb] Prevent object file plugins from messing with the data buffer if they don't match

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yes, that's the general idea, but you forgot the `CreateMemoryInstance` 
callback -- we should fix that too.


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

https://reviews.llvm.org/D122944

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

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: dblaikie, labath.
labath added a reviewer: dblaikie.
labath added a comment.

In principle, I like this a lot, though I'm not sure if this is the best way to 
integrate with with the StringMap class. Adding @dblaikie for thoughts on that.

Some background:
LLDB uses a string pool, and during (parallel) dwarf indexing, this string pool 
is the biggest bottleneck. To reduce contention, lldb actually uses a set of 
string pools, where it first computes a hash of the string (I believe this is 
the one-byte hash that Jonas is referring to) to determine which pool (and 
which mutex) to use, and then performs the actual container operation. This 
approach has resulted in considerable speedup, but it means the string is 
getting hashed more than once. Luboš is trying to remove the redundant hashes 
and speed it up even further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122974

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

[Lldb-commits] [PATCH] D122980: make ConstStringTable use DenseMap rather than std::map

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

In D122980#3424636 , @JDevlieghere 
wrote:

> I feel like this came up in the past and there was a reason an unordered map 
> couldn't work? Maybe I'm confusing this with something else. Added Pavel as 
> he would certainly know.

This is pretty new code, so I think you must be thinking of something else.

I don't see any reason why this would have to be a std::map. I think the code 
just hasn't been optimized yet.


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

https://reviews.llvm.org/D122980

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

> BTW, does it make sense to get even things like this reviewed, or is it ok if 
> I push these directly if I'm reasonably certain I know what I'm doing? I feel 
> like I'm spamming you by now.

Generally, I would say yes. I'm not even sure that some of your other patches 
should have been submitted without a pre-commit review (*all* llvm patches are 
expected to be reviewed).

In principle this idea seems fine, but the implementation is somewhat odd. The 
caller of ScopedTimeout is passing a specific timeout value, but then the class 
sets the timeout to something completely different. And it means you're not 
increasing the timeout for regular (fast) packets, so they still get to run 
with the default timeout of 1 second.

I would say that the entire ScopedTimeout class should be multiplier-based. 
Instead of passing a fix value, the user would say "I know this packed is slow, 
so I want to use X-times the usual timeout value". Then, we could support 
valgrind (and various *SANs) by just setting the initial timeout value to a 
reasonably high value, and the multipliers would take care of the rest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D123001: make 'step out' step out of the selected frame

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This should definitely have a test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123001

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

[Lldb-commits] [PATCH] D122944: [lldb] Prevent object file plugins from messing with the data buffer if they don't match

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 420193.
JDevlieghere added a comment.

Update `CreateMemoryInstance`


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

https://reviews.llvm.org/D122944

Files:
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h

Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -30,12 +30,12 @@
   }
 
   static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
  lldb::offset_t data_offset, const FileSpec *file,
  lldb::offset_t file_offset, lldb::offset_t length);
 
   static ObjectFile *CreateMemoryInstance(const lldb::ModuleSP &module_sp,
-  lldb::DataBufferSP &data_sp,
+  lldb::DataBufferSP data_sp,
   const lldb::ProcessSP &process_sp,
   lldb::addr_t header_addr);
 
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
@@ -88,7 +88,7 @@
 }
 
 ObjectFile *
-ObjectFileWasm::CreateInstance(const ModuleSP &module_sp, DataBufferSP &data_sp,
+ObjectFileWasm::CreateInstance(const ModuleSP &module_sp, DataBufferSP data_sp,
offset_t data_offset, const FileSpec *file,
offset_t file_offset, offset_t length) {
   Log *log = GetLog(LLDBLog::Object);
@@ -141,7 +141,7 @@
 }
 
 ObjectFile *ObjectFileWasm::CreateMemoryInstance(const ModuleSP &module_sp,
- DataBufferSP &data_sp,
+ DataBufferSP data_sp,
  const ProcessSP &process_sp,
  addr_t header_addr) {
   if (!ValidateModuleHeader(data_sp))
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -62,7 +62,7 @@
   static llvm::StringRef GetPluginDescriptionStatic();
 
   static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
  lldb::offset_t data_offset, const lldb_private::FileSpec *file,
  lldb::offset_t offset, lldb::offset_t length);
 
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -77,12 +77,10 @@
  "(32 and 64 bit)";
 }
 
-ObjectFile *ObjectFilePECOFF::CreateInstance(const lldb::ModuleSP &module_sp,
- DataBufferSP &data_sp,
- lldb::offset_t data_offset,
- const lldb_private::FileSpec *file_p,
- lldb::offset_t file_offset,
- lldb::offset_t length) {
+ObjectFile *ObjectFilePECOFF::CreateInstance(
+const lldb::ModuleSP &module_sp, DataBufferSP data_sp,
+lldb::offset_t data_offset, const lldb_private::FileSpec *file_p,
+lldb::offset_t file_offset, lldb::offset_t length) {
   FileSpec file = file_p ? *file_p : FileSpec();
   if (!data_sp) {
 data_sp = MapFileData(file, len

[Lldb-commits] [PATCH] D122980: make ConstStringTable use DenseMap rather than std::map

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

In D122980#3426196 , @labath wrote:

> In D122980#3424636 , @JDevlieghere 
> wrote:
>
>> I feel like this came up in the past and there was a reason an unordered map 
>> couldn't work? Maybe I'm confusing this with something else. Added Pavel as 
>> he would certainly know.
>
> This is pretty new code, so I think you must be thinking of something else.
>
> I don't see any reason why this would have to be a std::map. I think the code 
> just hasn't been optimized yet.

Cool, thanks for confirming. LGTM!


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

https://reviews.llvm.org/D122980

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

[Lldb-commits] [lldb] cf3e401 - Prevent GetAugmentedArchSpec() from attaching "unknown" environments

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2022-04-04T08:56:58-07:00
New Revision: cf3e4011b57b07d88e3e577295c3327b07c15922

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

LOG: Prevent GetAugmentedArchSpec() from attaching "unknown" environments

Environments are optional and a missing environment is distinct from
the default "unknown" environment enumerator.  The test is negative,
because the function uses the host triple and is unpredictable.

rdar://91007207

https://reviews.llvm.org/D122946

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

Added: 


Modified: 
lldb/source/Host/common/HostInfoBase.cpp
lldb/unittests/Host/HostInfoTest.cpp

Removed: 




diff  --git a/lldb/source/Host/common/HostInfoBase.cpp 
b/lldb/source/Host/common/HostInfoBase.cpp
index 6a11dd5f0c187..22c0403006e9d 100644
--- a/lldb/source/Host/common/HostInfoBase.cpp
+++ b/lldb/source/Host/common/HostInfoBase.cpp
@@ -209,7 +209,8 @@ ArchSpec HostInfoBase::GetAugmentedArchSpec(llvm::StringRef 
triple) {
 normalized_triple.setVendor(host_triple.getVendor());
   if (normalized_triple.getOSName().empty())
 normalized_triple.setOS(host_triple.getOS());
-  if (normalized_triple.getEnvironmentName().empty())
+  if (normalized_triple.getEnvironmentName().empty() &&
+  !host_triple.getEnvironmentName().empty())
 normalized_triple.setEnvironment(host_triple.getEnvironment());
   return ArchSpec(normalized_triple);
 }

diff  --git a/lldb/unittests/Host/HostInfoTest.cpp 
b/lldb/unittests/Host/HostInfoTest.cpp
index 0accdd8dbcdbf..daec8b46e5425 100644
--- a/lldb/unittests/Host/HostInfoTest.cpp
+++ b/lldb/unittests/Host/HostInfoTest.cpp
@@ -43,6 +43,9 @@ TEST_F(HostInfoTest, GetAugmentedArchSpec) {
   // Test LLDB_ARCH_DEFAULT
   EXPECT_EQ(HostInfo::GetAugmentedArchSpec(LLDB_ARCH_DEFAULT).GetTriple(),
 HostInfo::GetArchitecture(HostInfo::eArchKindDefault).GetTriple());
+  EXPECT_NE(
+  
HostInfo::GetAugmentedArchSpec("armv7k").GetTriple().getEnvironmentName(),
+  "unknown");
 }
 
 TEST_F(HostInfoTest, GetHostname) {



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

[Lldb-commits] [PATCH] D122946: Prevent HostInfoBase::GetAugmentedArchSpec() from attaching "unknown" environments

[Adrian Prantl via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf3e4011b57b: Prevent GetAugmentedArchSpec() from attaching 
"unknown" environments (authored by aprantl).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122946

Files:
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/unittests/Host/HostInfoTest.cpp


Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -43,6 +43,9 @@
   // Test LLDB_ARCH_DEFAULT
   EXPECT_EQ(HostInfo::GetAugmentedArchSpec(LLDB_ARCH_DEFAULT).GetTriple(),
 HostInfo::GetArchitecture(HostInfo::eArchKindDefault).GetTriple());
+  EXPECT_NE(
+  
HostInfo::GetAugmentedArchSpec("armv7k").GetTriple().getEnvironmentName(),
+  "unknown");
 }
 
 TEST_F(HostInfoTest, GetHostname) {
Index: lldb/source/Host/common/HostInfoBase.cpp
===
--- lldb/source/Host/common/HostInfoBase.cpp
+++ lldb/source/Host/common/HostInfoBase.cpp
@@ -209,7 +209,8 @@
 normalized_triple.setVendor(host_triple.getVendor());
   if (normalized_triple.getOSName().empty())
 normalized_triple.setOS(host_triple.getOS());
-  if (normalized_triple.getEnvironmentName().empty())
+  if (normalized_triple.getEnvironmentName().empty() &&
+  !host_triple.getEnvironmentName().empty())
 normalized_triple.setEnvironment(host_triple.getEnvironment());
   return ArchSpec(normalized_triple);
 }


Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -43,6 +43,9 @@
   // Test LLDB_ARCH_DEFAULT
   EXPECT_EQ(HostInfo::GetAugmentedArchSpec(LLDB_ARCH_DEFAULT).GetTriple(),
 HostInfo::GetArchitecture(HostInfo::eArchKindDefault).GetTriple());
+  EXPECT_NE(
+  HostInfo::GetAugmentedArchSpec("armv7k").GetTriple().getEnvironmentName(),
+  "unknown");
 }
 
 TEST_F(HostInfoTest, GetHostname) {
Index: lldb/source/Host/common/HostInfoBase.cpp
===
--- lldb/source/Host/common/HostInfoBase.cpp
+++ lldb/source/Host/common/HostInfoBase.cpp
@@ -209,7 +209,8 @@
 normalized_triple.setVendor(host_triple.getVendor());
   if (normalized_triple.getOSName().empty())
 normalized_triple.setOS(host_triple.getOS());
-  if (normalized_triple.getEnvironmentName().empty())
+  if (normalized_triple.getEnvironmentName().empty() &&
+  !host_triple.getEnvironmentName().empty())
 normalized_triple.setEnvironment(host_triple.getEnvironment());
   return ArchSpec(normalized_triple);
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Is this really something we want to hardcode at all? It seems like this should 
be a setting that is configured by the test harness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D122975: parallelize module loading in DynamicLoaderPOSIXDYLD()

[Pavel Labath via Phabricator via lldb-commits]

labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.

This is an interesting area for optimisation, but I don't think it's going to 
be that easy. A couple of years ago, I experimented with a similar approach, 
but I could not come up with a satisfactory solution in a reasonable amount of 
time. This is how the `PrefetchModuleSpecs` call came into being. I'm not 
entirely proud of it, but it was sufficient for my use case, and it is single 
threaded (which is always nice).

To move forward with this we'd basically need to resolve all of the issues that 
you mention in the patch description:

- threadpool-within-threadpool is a real problem. We shouldn't have n^2 threads 
running in the system. That will cause thrashing, possible OOMs, and just make 
it generally hard to figure out what's going on. I'm not sure what did you mean 
by the semaphore idea, but I don't think it would be sufficient to grab a 
semaphore in some thread before starting some work -- that'll still leave us 
with n^2 threads. We should prevent this many threads from being spawned in the 
first place.
- the thread safety claim is extremely bold. There's a big difference between 
running something in one thread (it doesn't matter that it's not the 
applications main thread -- this is running on a thread which is dedicated to 
serving one particular process), and spawning a bunch of threads to do 
something in parallel. It took us several years to fix all the corner cases 
relating to the dwarf index pooling, and that code was much more self-contained 
than this. The `Target::GetOrCreateModule` function (the core of this 
functionality) is pretty complex, consist of multiple levels of retries and 
lookups, and I don't think it would produce reasonable results if it was 
executed concurrently. I believe that in the normal case (all of the modules 
are distinct) it would do the right thing, but I'm pretty sure that it could do 
something strange and unexpected if the list contained (e.g.) the same module 
twice. The obvious fix (add a lock guard the manipulation of the target module 
list) would most likely defeat the main purpose of this patch. So, I think we 
would need to implement this function differently, or at least, provide some 
proof that current implementation is correct.

But before you go and try to do all of that (if this wasn't enough to 
discourage you :P), I'd like to understand what is the precise thing that 
you're trying to optimise. If there is like a single hot piece of code that we 
want to optimise, then we might be able to come up with an different approach 
(a'la PrefetchModuleSpecs) to achieve the same thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122975

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

If it's just a matter of setting the default timeout value, then I don't think 
it would be that bad -- we already set a different timeout for debug and 
release builds, so it we could conceivably put this there. But yes, my "setting 
the initial timeout value to a reasonably high value" comment was going in the 
direction that this could be then set from inside the test harness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[Andrew Turner via Phabricator via lldb-commits]

andrew updated this revision to Diff 420205.
andrew added a comment.

Add support for the PAC test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c

Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
@@ -4,8 +4,26 @@
 // To enable PAC return address signing compile with following clang arguments:
 // -march=armv8.3-a -mbranch-protection=pac-ret+leaf
 
+#include 
 #include 
 
+#if defined(__FreeBSD__)
+#include 
+
+static bool pac_supported(void) {
+  unsigned long hwcap;
+
+  if (elf_aux_info(AT_HWCAP, &hwcap, sizeof(hwcap)) != 0)
+return false;
+
+  return (hwcap & HWCAP_PACA) != 0;
+}
+#else
+static bool pac_supported(void) {
+  return true;
+}
+#endif
+
 static void __attribute__((noinline)) func_c(void) {
   exit(0); // Frame func_c
 }
@@ -19,6 +37,9 @@
 }
 
 int main(int argc, char *argv[]) {
+  if (!pac_supported())
+return 1;
+
   func_a(); // Frame main
   return 0;
 }
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -6,28 +6,26 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import re
 
 
 class AArch64UnwindPAC(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIf(archs=no_match(["aarch64"]))
-@skipIf(oslist=no_match(['linux']))
-def test(self):
-"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
-if not self.isAArch64PAuth():
-self.skipTest('Target must support Pointer Authentication.')
-
+def common(self, backtrace_tail):
 self.build()
 
 self.line = line_number('main.c', '// Frame func_c')
 
 exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
-
 lldbutil.run_break_set_by_file_and_line(
 self, "main.c", self.line, num_expected_locations=1)
 self.runCmd("run", RUN_SUCCEEDED)
+if re.match('Process .* exited with status = 1[^0-9]', self.res.GetOutput()):
+# No PAC support
+self.skipTest('Target must support Pointer Authentication.')
+
 self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
 substrs=["stop reason = breakpoint 1."])
 
@@ -35,13 +33,32 @@
 process = target.GetProcess()
 thread = process.GetThreadAtIndex(0)
 
-backtrace = ["func_c", "func_b", "func_a", "main", "__libc_start_main", "_start"]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
+backtrace = ["func_c", "func_b", "func_a", "main"] + backtrace_tail
+# This doesn't work as we get a ___lldb_unnamed_symbol on FreeBSD
+#self.assertEqual(thread.GetNumFrames(), len(backtrace))
 for frame_idx, frame in enumerate(thread.frames):
 frame = thread.GetFrameAtIndex(frame_idx)
+lldb_unnamed = "___lldb_unnamed_symbol"
+if frame.GetFunctionName()[:len(lldb_unnamed)] == lldb_unnamed:
+break
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
 			# Check line number for functions in main.c
 if (frame_idx < 4):
 self.assertEqual(frame.GetLineEntry().GetLine(),
  line_number("main.c", "Frame " + backtrace[frame_idx]))
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['freebsd']))
+def test_freebsd(self):
+"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
+self.common(["__start"])
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test_linux(self):
+"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
+if not self.isAArch64PAuth():
+self.skipTest('Target must support Pointer Authentication.')
+
+self.common(["__libc_start_main", "_start"])
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
==

[Lldb-commits] [PATCH] D122944: [lldb] Prevent object file plugins from messing with the data buffer if they don't match

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc69307e5eeb5: [lldb] Prevent object file plugins from 
changing the data buffer (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D122944?vs=420193&id=420209#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122944

Files:
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h

Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -30,12 +30,12 @@
   }
 
   static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
  lldb::offset_t data_offset, const FileSpec *file,
  lldb::offset_t file_offset, lldb::offset_t length);
 
   static ObjectFile *CreateMemoryInstance(const lldb::ModuleSP &module_sp,
-  lldb::DataBufferSP &data_sp,
+  lldb::DataBufferSP data_sp,
   const lldb::ProcessSP &process_sp,
   lldb::addr_t header_addr);
 
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
@@ -88,7 +88,7 @@
 }
 
 ObjectFile *
-ObjectFileWasm::CreateInstance(const ModuleSP &module_sp, DataBufferSP &data_sp,
+ObjectFileWasm::CreateInstance(const ModuleSP &module_sp, DataBufferSP data_sp,
offset_t data_offset, const FileSpec *file,
offset_t file_offset, offset_t length) {
   Log *log = GetLog(LLDBLog::Object);
@@ -141,7 +141,7 @@
 }
 
 ObjectFile *ObjectFileWasm::CreateMemoryInstance(const ModuleSP &module_sp,
- DataBufferSP &data_sp,
+ DataBufferSP data_sp,
  const ProcessSP &process_sp,
  addr_t header_addr) {
   if (!ValidateModuleHeader(data_sp))
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -62,12 +62,12 @@
   static llvm::StringRef GetPluginDescriptionStatic();
 
   static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
  lldb::offset_t data_offset, const lldb_private::FileSpec *file,
  lldb::offset_t offset, lldb::offset_t length);
 
   static lldb_private::ObjectFile *CreateMemoryInstance(
-  const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+  const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
   const lldb::ProcessSP &process_sp, lldb::addr_t header_addr);
 
   static size_t GetModuleSpecifications(const lldb_private::FileSpec &file,
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -77,12 +77,10 @@
  "(32 and 64 bit)";
 }
 
-ObjectFile *ObjectFilePECOFF::CreateInstance(const lldb::ModuleSP &module_sp,
- DataBufferSP &data_sp,
- lldb::offse

[Lldb-commits] [lldb] c69307e - [lldb] Prevent object file plugins from changing the data buffer

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-04-04T09:24:24-07:00
New Revision: c69307e5eeb585203a68b24f020d17ad75821c8a

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

LOG: [lldb] Prevent object file plugins from changing the data buffer

The current design allows that the object file contents could be mapped
by one object file plugin and then used by another. Presumably the idea
here was to avoid mapping the same file twice.

This becomes an issue when one object file plugin wants to map the file
differently from the others. For example, ObjectFileELF needs to map its
memory as writable while others likeObjectFileMachO needs it to be
mapped read-only.

This patch prevents plugins from changing the buffer by passing them is
by value rather than by reference.

Differential revision: https://reviews.llvm.org/D122944

Added: 


Modified: 
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h

Removed: 




diff  --git a/lldb/include/lldb/lldb-private-interfaces.h 
b/lldb/include/lldb/lldb-private-interfaces.h
index 2ed083ec8ae95..33b902b378f18 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -45,13 +45,13 @@ typedef size_t (*ObjectFileGetModuleSpecifications)(
 lldb::offset_t data_offset, lldb::offset_t file_offset,
 lldb::offset_t length, ModuleSpecList &module_specs);
 typedef ObjectFile *(*ObjectFileCreateInstance)(const lldb::ModuleSP 
&module_sp,
-lldb::DataBufferSP &data_sp,
+lldb::DataBufferSP data_sp,
 lldb::offset_t data_offset,
 const FileSpec *file,
 lldb::offset_t file_offset,
 lldb::offset_t length);
 typedef ObjectFile *(*ObjectFileCreateMemoryInstance)(
-const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
 const lldb::ProcessSP &process_sp, lldb::addr_t offset);
 typedef bool (*ObjectFileSaveCore)(const lldb::ProcessSP &process_sp,
const FileSpec &outfile,

diff  --git a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp 
b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
index ce701fd823fdf..6b208c220221b 100644
--- a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
+++ b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
@@ -57,7 +57,7 @@ void ObjectFileBreakpad::Terminate() {
 }
 
 ObjectFile *ObjectFileBreakpad::CreateInstance(
-const ModuleSP &module_sp, DataBufferSP &data_sp, offset_t data_offset,
+const ModuleSP &module_sp, DataBufferSP data_sp, offset_t data_offset,
 const FileSpec *file, offset_t file_offset, offset_t length) {
   if (!data_sp) {
 data_sp = MapFileData(*file, length, file_offset);
@@ -84,7 +84,7 @@ ObjectFile *ObjectFileBreakpad::CreateInstance(
 }
 
 ObjectFile *ObjectFileBreakpad::CreateMemoryInstance(
-const ModuleSP &module_sp, DataBufferSP &data_sp,
+const ModuleSP &module_sp, DataBufferSP data_sp,
 const ProcessSP &process_sp, addr_t header_addr) {
   return nullptr;
 }

diff  --git a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h 
b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
index f04e0b4dd7a7a..155ad9631be32 100644
--- a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
+++ b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
@@ -27,12 +27,12 @@ class ObjectFileBreakpad : public ObjectFile {
   }
 
   static ObjectFile *
-  CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+  Cre

[Lldb-commits] [PATCH] D122975: parallelize module loading in DynamicLoaderPOSIXDYLD()

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D122975#3426575 , @labath wrote:

> I'd like to understand what is the precise thing that you're trying to 
> optimise. If there is like a single hot piece of code that we want to 
> optimise, then we might be able to come up with an different approach (a'la 
> PrefetchModuleSpecs) to achieve the same thing.

F22668124: lldb2.png 

The scenario is a repeated lldb start with index cache enabled (and so 
everything necessary cached), the debugged app is debug build of LibreOffice 
Calc (~100 medium-sized C++ libraries). 90% of the CPU time is spent in 
LoadFromCache() calls, because 70% of the CPU time is spent in ConstString (and 
that's after my ConstString optimizations). I don't think there's any other way 
to make it faster other than parallelizing it, although parallelizing only 
LoadFromCache() should be enough and I'd expect that to be limited enough to be 
safe if you expect the lldb codebase is not generally thread-safe. In this 
patch I parallelized higher because it was simple to do it up there, and I 
don't know how to easily get at all the LoadFromCache() calls (especially the 
one called from SymbolFileDWARFDebugMap is relatively deep). If you know how to 
parallelize only that, that works for me too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122975

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

[Lldb-commits] [lldb] 9a6a0df - [lldb] make ConstStringTable use DenseMap rather than std::map

[Luboš Luňák via lldb-commits]

Author: Luboš Luňák
Date: 2022-04-04T18:46:22+02:00
New Revision: 9a6a0dfa06a5acd01f6b55f8a1ba9f0f5109e02c

URL: 
https://github.com/llvm/llvm-project/commit/9a6a0dfa06a5acd01f6b55f8a1ba9f0f5109e02c
DIFF: 
https://github.com/llvm/llvm-project/commit/9a6a0dfa06a5acd01f6b55f8a1ba9f0f5109e02c.diff

LOG: [lldb] make ConstStringTable use DenseMap rather than std::map

The ordering is not needed, and DenseMap is faster. I can measure
time spent in the SaveToCache() calls reduced to ~40% during LLDB
startup (and the total startup cost reduced to ~70%).

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

Added: 


Modified: 
lldb/include/lldb/Core/DataFileCache.h

Removed: 




diff  --git a/lldb/include/lldb/Core/DataFileCache.h 
b/lldb/include/lldb/Core/DataFileCache.h
index 3016c531f6746..6f7de679f8679 100644
--- a/lldb/include/lldb/Core/DataFileCache.h
+++ b/lldb/include/lldb/Core/DataFileCache.h
@@ -13,6 +13,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/UUID.h"
 #include "lldb/lldb-forward.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Caching.h"
 #include 
 
@@ -190,7 +191,7 @@ class ConstStringTable {
 
 private:
   std::vector m_strings;
-  std::map m_string_to_offset;
+  llvm::DenseMap m_string_to_offset;
   /// Skip one byte to start the string table off with an empty string.
   uint32_t m_next_offset = 1;
 };



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

[Lldb-commits] [PATCH] D122980: make ConstStringTable use DenseMap rather than std::map

[Luboš Luňák via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a6a0dfa06a5: [lldb] make ConstStringTable use DenseMap 
rather than std::map (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122980

Files:
  lldb/include/lldb/Core/DataFileCache.h


Index: lldb/include/lldb/Core/DataFileCache.h
===
--- lldb/include/lldb/Core/DataFileCache.h
+++ lldb/include/lldb/Core/DataFileCache.h
@@ -13,6 +13,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/UUID.h"
 #include "lldb/lldb-forward.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Caching.h"
 #include 
 
@@ -190,7 +191,7 @@
 
 private:
   std::vector m_strings;
-  std::map m_string_to_offset;
+  llvm::DenseMap m_string_to_offset;
   /// Skip one byte to start the string table off with an empty string.
   uint32_t m_next_offset = 1;
 };


Index: lldb/include/lldb/Core/DataFileCache.h
===
--- lldb/include/lldb/Core/DataFileCache.h
+++ lldb/include/lldb/Core/DataFileCache.h
@@ -13,6 +13,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/UUID.h"
 #include "lldb/lldb-forward.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Caching.h"
 #include 
 
@@ -190,7 +191,7 @@
 
 private:
   std::vector m_strings;
-  std::map m_string_to_offset;
+  llvm::DenseMap m_string_to_offset;
   /// Skip one byte to start the string table off with an empty string.
   uint32_t m_next_offset = 1;
 };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D123020#3426252 , @labath wrote:

>> BTW, does it make sense to get even things like this reviewed, or is it ok 
>> if I push these directly if I'm reasonably certain I know what I'm doing? I 
>> feel like I'm spamming you by now.
>
> Generally, I would say yes. I'm not even sure that some of your other patches 
> should have been submitted without a pre-commit review (*all* llvm patches 
> are expected to be reviewed).

I see. I haven't contributed anything for a while and git history shows a 
number of commits to do not refer to reviews.llvm.org, so I assumed simple 
patches were fine. I'll get even the simple ones reviewed if that's expected.

> I would say that the entire ScopedTimeout class should be multiplier-based. 
> Instead of passing a fix value, the user would say "I know this packed is 
> slow, so I want to use X-times the usual timeout value". Then, we could 
> support valgrind (and various *SANs) by just setting the initial timeout 
> value to a reasonably high value, and the multipliers would take care of the 
> rest.

For the Valgrind case it doesn't really matter what the timeout is, as long as 
it's long enough to not time out on normal calls. Even multiplying is just 
taking a guess at it, Valgrind doesn't slow things down linearly.

In D123020#3426572 , @JDevlieghere 
wrote:

> Is this really something we want to hardcode at all? It seems like this 
> should be a setting that is configured by the test harness.

This is not about tests. I had a double-free abort, so I ran lldb normally in 
valgrind and it aborted attach because of the 6-second timeout in 
GDBRemoteCommunicationClient::QueryNoAckModeSupported(). That's too short for 
anything in Valgrind, and I thought this change would be useful in general, as 
otherwise lldb is presumably unusable for Valgrind runs. But if you think this 
requires getting complicated as such rewritting an entire class for it, then 
I'm abandoning the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D123020#3426839 , @llunak wrote:

> In D123020#3426252 , @labath wrote:
>
>>> BTW, does it make sense to get even things like this reviewed, or is it ok 
>>> if I push these directly if I'm reasonably certain I know what I'm doing? I 
>>> feel like I'm spamming you by now.
>>
>> Generally, I would say yes. I'm not even sure that some of your other 
>> patches should have been submitted without a pre-commit review (*all* llvm 
>> patches are expected to be reviewed).
>
> I see. I haven't contributed anything for a while and git history shows a 
> number of commits to do not refer to reviews.llvm.org, so I assumed simple 
> patches were fine. I'll get even the simple ones reviewed if that's expected.

FWIW the official policy is outlined here: https://llvm.org/docs/CodeReview.html

>> I would say that the entire ScopedTimeout class should be multiplier-based. 
>> Instead of passing a fix value, the user would say "I know this packed is 
>> slow, so I want to use X-times the usual timeout value". Then, we could 
>> support valgrind (and various *SANs) by just setting the initial timeout 
>> value to a reasonably high value, and the multipliers would take care of the 
>> rest.
>
> For the Valgrind case it doesn't really matter what the timeout is, as long 
> as it's long enough to not time out on normal calls. Even multiplying is just 
> taking a guess at it, Valgrind doesn't slow things down linearly.
>
> In D123020#3426572 , @JDevlieghere 
> wrote:
>
>> Is this really something we want to hardcode at all? It seems like this 
>> should be a setting that is configured by the test harness.
>
> This is not about tests. I had a double-free abort, so I ran lldb normally in 
> valgrind and it aborted attach because of the 6-second timeout in 
> GDBRemoteCommunicationClient::QueryNoAckModeSupported(). That's too short for 
> anything in Valgrind, and I thought this change would be useful in general, 
> as otherwise lldb is presumably unusable for Valgrind runs.

My suggestion was that this timeout should be configurable through a setting. 
If it is, then you can increase it when running under Valgrind (or anywhere 
else that slows down lldb, like the sanitizers). I wouldn't mind having a bit 
of code that checks `llvm::sys::RunningOnValgrind()` and increases the default 
timeouts so that you don't even have to change your setting. But what I don't 
think is a good idea is to sprinkle checks for whether we're running under 
Vanguard throughout the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D122975: parallelize module loading in DynamicLoaderPOSIXDYLD()

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I had tried something similar with the thread pools when trying to parallelize 
similar stuff. The solution I made was to have a global thread pool for the 
entire LLDB process, but then the LLVM thread pool stuff needed to be modified 
to handle different groups of threads where work could be added to a queue and 
then users can wait on the queue. The queues then need to be managed by the 
thread pool code. Queues could also be serial queues or concurrent queues. I 
never completed the patch, but just wanted to pass along the ideas I had used. 
So instead of adding everything to a separate pool, the main thread pool could 
take queues. The code for your code above would look something like:

  llvm::ThreadPool::Queue queue(llvm::ThreadPool::PoolType::Concurrent);
  for (size_t i = 0; i < infos.size(); ++i)
queue.push(load_module_fn, i);
  Debugger::GetThreadPool().wait(queue);

We could have a static function on Debugger, or just make a static function 
inside of LLDB to grab the thread pool, and queue up the work for the 
individual queues. Then we can have one central location for the thread pool 
and anyone can throw work onto the pool with individual queues that can be 
waited on separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122975

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

[Lldb-commits] [PATCH] D122991: [lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn updated this revision to Diff 420245.
zrthxn added a comment.

Change in memory use is appreciable.

  thread #1: tid = 40371
Raw trace size: 4 KiB
Total number of instructions: 145011
Total approximate memory usage: 1840.96 KiB
Average memory usage per instruction: 13.00 bytes
  
Number of TSC decoding errors: 0

Tests still need to be updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122991

Files:
  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/TraceIntelPT.cpp

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
@@ -112,7 +112,7 @@
   }
   s << "\n";
   DecodedThreadSP decoded_trace_sp = Decode(thread);
-  size_t insn_len = decoded_trace_sp->GetInstructions().size();
+  size_t insn_len = decoded_trace_sp->GetInstructionPointers().size();
   size_t mem_used = decoded_trace_sp->CalculateApproximateMemoryUsage();
 
   s.Format("  Raw trace size: {0} KiB\n", *raw_size / 1024);
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -20,14 +20,14 @@
 TraceCursorIntelPT::TraceCursorIntelPT(ThreadSP thread_sp,
DecodedThreadSP decoded_thread_sp)
 : TraceCursor(thread_sp), m_decoded_thread_sp(decoded_thread_sp) {
-  assert(!m_decoded_thread_sp->GetInstructions().empty() &&
+  assert(!m_decoded_thread_sp->GetInstructionPointers().empty() &&
  "a trace should have at least one instruction or error");
-  m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_pos = m_decoded_thread_sp->GetInstructionPointers().size() - 1;
   m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
-  return m_decoded_thread_sp->GetInstructions().size();
+  return m_decoded_thread_sp->GetInstructionPointers().size();
 }
 
 bool TraceCursorIntelPT::Next() {
@@ -78,8 +78,8 @@
   return std::abs(dist);
 }
   };
-  
-	int64_t dist = FindDistanceAndSetPos();
+
+  int64_t dist = FindDistanceAndSetPos();
   m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
   return dist;
 }
@@ -93,7 +93,7 @@
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
+  return m_decoded_thread_sp->GetInstructionLoadAddress(m_pos);
 }
 
 Optional
@@ -111,8 +111,8 @@
 TraceCursorIntelPT::GetInstructionControlFlowType() {
   lldb::addr_t next_load_address =
   m_pos + 1 < GetInternalInstructionSize()
-  ? m_decoded_thread_sp->GetInstructions()[m_pos + 1].GetLoadAddress()
+  ? m_decoded_thread_sp->GetInstructionLoadAddress(m_pos + 1)
   : LLDB_INVALID_ADDRESS;
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetControlFlowType(
-  next_load_address);
+  return m_decoded_thread_sp->GetInstructionControlFlowType(m_pos,
+next_load_address);
 }
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -51,61 +51,6 @@
   lldb::addr_t m_address;
 };
 
-/// \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), m_is_error(false) {}
-
-  /// Error constructor
-  IntelPTInstruction();
-
-  /// 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 \a LLDB_INVALID_ADDRESS if it is
-  /// an error.
-  lldb::addr_t GetLoadAddress() const;
-
-  /// Get the size in bytes of an instance of this class
-  static size_t GetMemoryUsage();
-
-  /// Get the \a lldb::TraceInstructionControlFlowType categories of the
-  /// instruction.
-  ///
-  /// \param[

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Utility/DataBuffer.h:58
+  /// if the object contains no bytes.
+  virtual const uint8_t *GetBytesImpl() const = 0;
+

Are you sure this should be public?



Comment at: lldb/include/lldb/Utility/DataBuffer.h:98-103
   /// Get a const pointer to the data.
   ///
   /// \return
   /// A const pointer to the bytes owned by this object, or NULL
   /// if the object contains no bytes.
+  const uint8_t *GetBytes() const { return GetBytesImpl(); }

Replace with `using DataBuffer::GetBytes` ?



Comment at: lldb/include/lldb/Utility/DataBuffer.h:109-111
   llvm::ArrayRef GetData() const {
 return llvm::ArrayRef(GetBytes(), GetByteSize());
   }

ditto



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:361
   // Update the data to contain the entire file if it doesn't already
   if (data_sp->GetByteSize() < length) {
+data_sp = MapFileDataWritable(*file, length, file_offset);

I guess this should be done unconditionally now.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:382
 ObjectFile *ObjectFileELF::CreateMemoryInstance(
 const lldb::ModuleSP &module_sp, DataBufferSP &data_sp,
 const lldb::ProcessSP &process_sp, lldb::addr_t header_addr) {

I am assuming this will always point to a writable kind of a data buffer. Could 
we change the prototype to reflect that?



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2624
   DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+  WritableDataBuffer *data_buffer =
+  static_cast(data_buffer_sp.get());

add `// ObjectFileELF creates a WritableDataBuffer in CreateInstance`


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

https://reviews.llvm.org/D122856

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

[Lldb-commits] [PATCH] D122991: [lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn updated this revision to Diff 420256.
zrthxn added a comment.

Updated tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122991

Files:
  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/TraceIntelPT.cpp
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -38,8 +38,10 @@
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 0.98 KiB
-  Average memory usage per instruction: 48.00 bytes'''])
+  Total approximate memory usage: 0.27 KiB
+  Average memory usage per instruction: 13.00 bytes
+
+  Number of TSC decoding errors: 0'''])
 
 def testLoadInvalidTraces(self):
 src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -40,7 +40,7 @@
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 0.98 KiB
-  Average memory usage per instruction: 48.00 bytes
+  Total approximate memory usage: 0.27 KiB
+  Average memory usage per instruction: 13.00 bytes
 
   Number of TSC decoding errors: 0'''])
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
@@ -112,7 +112,7 @@
   }
   s << "\n";
   DecodedThreadSP decoded_trace_sp = Decode(thread);
-  size_t insn_len = decoded_trace_sp->GetInstructions().size();
+  size_t insn_len = decoded_trace_sp->GetInstructionPointers().size();
   size_t mem_used = decoded_trace_sp->CalculateApproximateMemoryUsage();
 
   s.Format("  Raw trace size: {0} KiB\n", *raw_size / 1024);
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -20,14 +20,14 @@
 TraceCursorIntelPT::TraceCursorIntelPT(ThreadSP thread_sp,
DecodedThreadSP decoded_thread_sp)
 : TraceCursor(thread_sp), m_decoded_thread_sp(decoded_thread_sp) {
-  assert(!m_decoded_thread_sp->GetInstructions().empty() &&
+  assert(!m_decoded_thread_sp->GetInstructionPointers().empty() &&
  "a trace should have at least one instruction or error");
-  m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_pos = m_decoded_thread_sp->GetInstructionPointers().size() - 1;
   m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
-  return m_decoded_thread_sp->GetInstructions().size();
+  return m_decoded_thread_sp->GetInstructionPointers().size();
 }
 
 bool TraceCursorIntelPT::Next() {
@@ -78,8 +78,8 @@
   return std::abs(dist);
 }
   };
-  
-	int64_t dist = FindDistanceAndSetPos();
+
+  int64_t dist = FindDistanceAndSetPos();
   m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
   return dist;
 }
@@ -93,7 +93,7 @@
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
+  return m_decoded_thread_sp->GetInstructionLoadAddress(m_pos);
 }
 
 Optional
@@ -111,8 +111,8 @@
 TraceCursorIntelPT::GetInstructionControlFlowType() {
   lldb::addr_t next_load_address =
   m_pos + 1 < GetInternalInstructionSize()
-  ? m_decoded_thread_sp->GetInstructions()[m_pos + 1].GetLoadAddress()
+  ? m_decoded_thread_sp->GetInstructionLoadAddress(m_pos + 1)
   : LLDB_INVALID_ADDRESS;
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetControlFlowType(
-  next_load_address);
+  return m_decoded_thread_sp->GetInstructionControlFlowType(m_pos,
+next_load_address);
 }
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -51,61 +51,6 @@
   lldb::addr_t m_address;
 };
 
-/// \class IntelPTInstruction
-/// An instruction obtained from decoding a trace. It is either an actual
-/// instruction or an error indicating a gap in the 

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 420262.
JDevlieghere marked 7 inline comments as done.

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

https://reviews.llvm.org/D122856

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Host/FileSystem.h
  lldb/include/lldb/Symbol/CompactUnwindInfo.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Target/ProcessStructReader.h
  lldb/include/lldb/Target/RegisterCheckpoint.h
  lldb/include/lldb/Target/RegisterContext.h
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/include/lldb/Utility/DataBuffer.h
  lldb/include/lldb/Utility/DataBufferHeap.h
  lldb/include/lldb/Utility/DataBufferLLVM.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/DataFormatters/TypeFormat.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/common/Host.cpp
  lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
  lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm.h
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_i386.h
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.h
  lldb/source/Plugins/Process/Utility/RegisterContextDummy.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDummy.h
  lldb/source/Plugins/Process/Utility/RegisterContextHistory.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextHistory.h
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
  lldb/source/Plugins/Process/Utility/RegisterContextThreadMemory.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextThreadMemory.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_mips64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_mips64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_powerpc.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_powerpc.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_s390x.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_s390x.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_32.cpp
  lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
  lldb/source/Symbol/ObjectFile.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Utility/DataBufferHeap.cpp
  lldb/source/Utility/DataBufferLLVM.cpp

Index: lldb/source/Utility/DataBufferLLVM.cpp
===
--- lldb/source/Utility/DataBufferLLVM.cpp
+++ lldb/source/Utility/DataBufferLLVM.cpp
@@ -14,8 +14,7 @@
 
 using namespace lldb_private;
 
-Dat

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:361
   // Update the data to contain the entire file if it doesn't already
   if (data_sp->GetByteSize() < length) {
+data_sp = MapFileDataWritable(*file, length, file_offset);

labath wrote:
> I guess this should be done unconditionally now.
No, I tried doing that actually and it broke a bunch of unit tests. I can take 
a look at it later this week if you think this is important. 



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:382
 ObjectFile *ObjectFileELF::CreateMemoryInstance(
 const lldb::ModuleSP &module_sp, DataBufferSP &data_sp,
 const lldb::ProcessSP &process_sp, lldb::addr_t header_addr) {

labath wrote:
> I am assuming this will always point to a writable kind of a data buffer. 
> Could we change the prototype to reflect that?
Are you okay with making that a separate patch? 


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

https://reviews.llvm.org/D122856

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

[Lldb-commits] [PATCH] D122975: parallelize module loading in DynamicLoaderPOSIXDYLD()

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D122975#3427008 , @clayborg wrote:

> I had tried something similar with the thread pools when trying to 
> parallelize similar stuff. The solution I made was to have a global thread 
> pool for the entire LLDB process, but then the LLVM thread pool stuff needed 
> to be modified to handle different groups of threads where work could be 
> added to a queue and then users can wait on the queue. The queues then need 
> to be managed by the thread pool code. Queues could also be serial queues or 
> concurrent queues. I never completed the patch, but just wanted to pass along 
> the ideas I had used. So instead of adding everything to a separate pool, the 
> main thread pool could take queues. The code for your code above would look 
> something like:

I got a similar idea too, but I think it wouldn't work here in this 
threadpool-within-threadpool situation. If you limit the total number of 
threads, then the "outer" tasks could allocate all the threads and would 
deadlock on "inner" tasks waiting for threads (and if you don't limit threads 
then there's no need to bother). That's why I came up with the semaphore idea, 
as that would require threads to acquire slots and "outer" tasks could 
temporarily release them when spawning "inner" tasks (I didn't consider pending 
threads to be a problem, but if it is, then the supposed ThreadPool queues 
presumably could do that somehow too).

But if lldb code is not considered thread-safe to parallelize the module 
loading at this level, then this is all irrelevant. If it's required to 
parallelize only cache loading, then that removes the 
threadpool-within-threadpool situation. I don't know if I understand the code 
enough to try that though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122975

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

[Lldb-commits] [PATCH] D123073: [lldb] Change CreateMemoryInstance to take a WritableDataBuffer

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
Herald added subscribers: pmatos, asb, sbc100, emaste.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added subscribers: MaskRay, aheejin.

Change CreateMemoryInstance to take a WritableDataBuffer


https://reviews.llvm.org/D123073

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
  lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
  lldb/source/Symbol/ObjectFile.cpp

Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -35,7 +35,7 @@
 static ObjectFileSP
 CreateObjectFromContainer(const lldb::ModuleSP &module_sp, const FileSpec *file,
   lldb::offset_t file_offset, lldb::offset_t file_size,
-  DataBufferSP &data_sp, lldb::offset_t &data_offset) {
+  DataBufferSP data_sp, lldb::offset_t &data_offset) {
   ObjectContainerCreateInstance callback;
   for (uint32_t idx = 0;
(callback = PluginManager::GetObjectContainerCreateCallbackAtIndex(
@@ -152,7 +152,7 @@
 ObjectFileSP ObjectFile::FindPlugin(const lldb::ModuleSP &module_sp,
 const ProcessSP &process_sp,
 lldb::addr_t header_addr,
-DataBufferSP &data_sp) {
+WritableDataBufferSP data_sp) {
   ObjectFileSP object_file_sp;
 
   if (module_sp) {
@@ -241,7 +241,7 @@
 ObjectFile::ObjectFile(const lldb::ModuleSP &module_sp,
const FileSpec *file_spec_ptr,
lldb::offset_t file_offset, lldb::offset_t length,
-   const lldb::DataBufferSP &data_sp,
+   lldb::DataBufferSP data_sp,
lldb::offset_t data_offset)
 : ModuleChild(module_sp),
   m_file(), // This file could be different from the original module's file
@@ -265,7 +265,7 @@
 
 ObjectFile::ObjectFile(const lldb::ModuleSP &module_sp,
const ProcessSP &process_sp, lldb::addr_t header_addr,
-   DataBufferSP &header_data_sp)
+   DataBufferSP header_data_sp)
 : ModuleChild(module_sp), m_file(), m_type(eTypeInvalid),
   m_strata(eStrataInvalid), m_file_offset(0), m_length(0), m_data(),
   m_process_wp(process_sp), m_memory_addr(header_addr), m_sections_up(),
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
===
--- lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -35,7 +35,7 @@
  lldb::offset_t file_offset, lldb::offset_t length);
 
   static ObjectFile *CreateMemoryInstance(const lldb::ModuleSP &module_sp,
-  lldb::DataBufferSP data_sp,
+  lldb::WritableDataBufferSP data_sp,
   const lldb::ProcessSP &process_sp,
   lldb::addr_t header_addr);
 
@@ -111,11 +111,11 @@
   llvm::Optional GetExternalDebugInfoFileSpec();
 
 private:
-  ObjectFileWasm(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
+  ObjectFileWasm(const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
  lldb::offset_t data_offset, const FileSpec *file,
  lldb::offset_t offset, lldb::offset_t length);
   ObjectFileWasm(const lldb::ModuleSP &module_sp,
- lldb::DataBufferSP &header_data_sp,
+ lldb::WritableDataBufferSP header_data_sp,
  const lldb::ProcessSP &process_sp, lldb::addr_t header_addr);
 
   /// Wasm section decoding routines.
Index: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
===
--- 

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:382
 ObjectFile *ObjectFileELF::CreateMemoryInstance(
 const lldb::ModuleSP &module_sp, DataBufferSP &data_sp,
 const lldb::ProcessSP &process_sp, lldb::addr_t header_addr) {

JDevlieghere wrote:
> labath wrote:
> > I am assuming this will always point to a writable kind of a data buffer. 
> > Could we change the prototype to reflect that?
> Are you okay with making that a separate patch? 
https://reviews.llvm.org/D123073


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

https://reviews.llvm.org/D122856

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

[Lldb-commits] [PATCH] D122660: [lldb] Avoid duplicate vdso modules when opening core files

[Michał Górny via Phabricator via lldb-commits]

mgorny accepted this revision.
mgorny added a comment.
This revision is now accepted and ready to land.

Well, I don't see anything wrong with it and it doesn't seem to cause any 
regressions on FreeBSD. I wouldn't call myself an expert on this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122660

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

[Lldb-commits] [PATCH] D122716: [lldb/linux] Handle main thread exits

[Michał Górny via Phabricator via lldb-commits]

mgorny accepted this revision.
mgorny added a comment.
This revision is now accepted and ready to land.

Makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122716

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

[Lldb-commits] [PATCH] D123001: make 'step out' step out of the selected frame

[Luboš Luňák via Phabricator via lldb-commits]

llunak updated this revision to Diff 420295.
llunak added a comment.

Added a test.


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

https://reviews.llvm.org/D123001

Files:
  lldb/include/lldb/Target/Thread.h
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Target/Thread.cpp
  lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
  lldb/test/API/commands/gui/basicdebug/func.c
  lldb/test/API/commands/gui/basicdebug/main.c

Index: lldb/test/API/commands/gui/basicdebug/main.c
===
--- lldb/test/API/commands/gui/basicdebug/main.c
+++ lldb/test/API/commands/gui/basicdebug/main.c
@@ -1,7 +1,14 @@
 extern int func();
+extern void func_up();
 
 int main(int argc, char **argv) {
-  func(); // Break here
-  func(); // Second
+  int dummy;
+  func();// Break here
+  func();// Second
+  dummy = 1; // Dummy command 1
+
+  func_up(); // First func1 call
+  dummy = 2; // Dummy command 2
+
   return 0;
 }
Index: lldb/test/API/commands/gui/basicdebug/func.c
===
--- lldb/test/API/commands/gui/basicdebug/func.c
+++ lldb/test/API/commands/gui/basicdebug/func.c
@@ -1,3 +1,12 @@
 int func() {
   return 1; // In function
 }
+
+void func_down() {
+  int dummy = 1; // In func_down
+  (void)dummy;
+}
+
+void func_up() {
+  func_down(); // In func_up
+}
Index: lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
===
--- lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
+++ lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -32,7 +32,7 @@
 self.child.send("s") # step
 self.child.expect("return 1; // In function[^\r\n]+<<< Thread 1: step in")
 self.child.send("u") # up
-self.child.expect_exact("func(); // Break here")
+self.child.expect_exact("func();// Break here")
 self.child.send("d") # down
 self.child.expect_exact("return 1; // In function")
 self.child.send("f") # finish
@@ -40,7 +40,19 @@
 self.child.send("s") # move onto the second one
 self.child.expect("<<< Thread 1: step in")
 self.child.send("n") # step over
-self.child.expect("<<< Thread 1: step over")
+self.child.expect("// Dummy command 1[^\r\n]+<<< Thread 1: step over")
+self.child.send("n")
+
+# Test that 'up' + 'step out' steps out of the selected function.
+self.child.send("s") # move into func_up()
+self.child.expect("// In func_up")
+self.child.send("s") # move into func_down()
+self.child.expect("// In func_down")
+self.child.send("u") # up
+self.child.expect("// In func_up")
+self.child.send("f") # finish
+self.child.expect("// Dummy command 2[^\r\n]+<<< Thread 1: step out")
+self.child.send("n")
 
 # Press escape to quit the gui
 self.child.send(escape_key)
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1953,7 +1953,7 @@
   return error;
 }
 
-Status Thread::StepOut() {
+Status Thread::StepOut(uint32_t frame_idx) {
   Status error;
   Process *process = GetProcess().get();
   if (StateIsStoppedState(process->GetState(), true)) {
@@ -1963,7 +1963,7 @@
 
 ThreadPlanSP new_plan_sp(QueueThreadPlanForStepOut(
 abort_other_plans, nullptr, first_instruction, stop_other_threads,
-eVoteYes, eVoteNoOpinion, 0, error));
+eVoteYes, eVoteNoOpinion, frame_idx, error));
 
 new_plan_sp->SetIsControllingPlan(true);
 new_plan_sp->SetOkayToDiscard(false);
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -6402,8 +6402,11 @@
   if (exe_ctx.HasThreadScope()) {
 Process *process = exe_ctx.GetProcessPtr();
 if (process && process->IsAlive() &&
-StateIsStoppedState(process->GetState(), true))
-  exe_ctx.GetThreadRef().StepOut();
+StateIsStoppedState(process->GetState(), true)) {
+  Thread *thread = exe_ctx.GetThreadPtr();
+  uint32_t frame_idx = thread->GetSelectedFrameIndex();
+  exe_ctx.GetThreadRef().StepOut(frame_idx);
+}
   }
 }
   return MenuActionResult::Handled;
@@ -7361,7 +7364,9 @@
 m_debugger.GetCommandInterpreter().GetExecutionContext();
 if (exe_ctx.HasThreadScope() &&
 StateIsStoppedState(exe_ctx.GetProcessRef().GetState(), true)) {
-  exe_ctx.GetThreadRef().StepOut();
+  Thread *thread = exe_ctx.GetThreadPtr();
+  uint32_t frame_idx = thread->GetSelectedFrameIndex();
+  exe_ctx.GetThreadRef().StepOut(frame_idx);
 }
   }
   

[Lldb-commits] [PATCH] D122882: Add JSONGenerator::DumpBinaryEscaped method in debugserver, update most callers to use this, skip escaping step

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 420303.
jasonmolenda added a comment.

Update patch to remove unnecessary temporary std::string when dumping a string 
into its JSON representation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122882

Files:
  lldb/tools/debugserver/source/JSONGenerator.h
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -582,9 +582,8 @@
   // of these coming out at the wrong time (i.e. when the remote side
   // is not waiting for a process control completion response).
   stream << "JSON-async:";
-  dictionary.Dump(stream);
-  const std::string payload = binary_encode_string(stream.str());
-  return SendPacket(payload);
+  dictionary.DumpBinaryEscaped(stream);
+  return SendPacket(stream.str());
 }
 
 // Given a std::string packet contents to send, possibly encode/compress it.
@@ -2793,6 +2792,7 @@
   ostrm << std::hex << "jstopinfo:";
   std::ostringstream json_strm;
   threads_info_sp->Dump(json_strm);
+  threads_info_sp->Clear();
   append_hexified_string(ostrm, json_strm.str());
   ostrm << ';';
 }
@@ -5556,11 +5556,10 @@
 
 if (threads_info_sp) {
   std::ostringstream strm;
-  threads_info_sp->Dump(strm);
+  threads_info_sp->DumpBinaryEscaped(strm);
   threads_info_sp->Clear();
-  std::string binary_packet = binary_encode_string(strm.str());
-  if (!binary_packet.empty())
-return SendPacket(binary_packet);
+  if (strm.str().size() > 0)
+return SendPacket(strm.str());
 }
   }
   return SendPacket("E85");
@@ -5881,11 +5880,10 @@
 
 if (json_sp.get()) {
   std::ostringstream json_str;
-  json_sp->Dump(json_str);
+  json_sp->DumpBinaryEscaped(json_str);
   json_sp->Clear();
   if (json_str.str().size() > 0) {
-std::string json_str_quoted = binary_encode_string(json_str.str());
-return SendPacket(json_str_quoted);
+return SendPacket(json_str.str());
   } else {
 SendPacket("E84");
   }
@@ -5915,11 +5913,10 @@
 
 if (json_sp.get()) {
   std::ostringstream json_str;
-  json_sp->Dump(json_str);
+  json_sp->DumpBinaryEscaped(json_str);
   json_sp->Clear();
   if (json_str.str().size() > 0) {
-std::string json_str_quoted = binary_encode_string(json_str.str());
-return SendPacket(json_str_quoted);
+return SendPacket(json_str.str());
   } else {
 SendPacket("E86");
   }
Index: lldb/tools/debugserver/source/JSONGenerator.h
===
--- lldb/tools/debugserver/source/JSONGenerator.h
+++ lldb/tools/debugserver/source/JSONGenerator.h
@@ -113,6 +113,8 @@
 
 virtual void Dump(std::ostream &s) const = 0;
 
+virtual void DumpBinaryEscaped(std::ostream &s) const = 0;
+
   private:
 Type m_type;
   };
@@ -136,6 +138,17 @@
   s << "]";
 }
 
+void DumpBinaryEscaped(std::ostream &s) const override {
+  s << "[";
+  const size_t arrsize = m_items.size();
+  for (size_t i = 0; i < arrsize; ++i) {
+m_items[i]->DumpBinaryEscaped(s);
+if (i + 1 < arrsize)
+  s << ",";
+  }
+  s << "]";
+}
+
   protected:
 typedef std::vector collection;
 collection m_items;
@@ -151,6 +164,8 @@
 
 void Dump(std::ostream &s) const override { s << m_value; }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 uint64_t m_value;
   };
@@ -165,6 +180,8 @@
 
 void Dump(std::ostream &s) const override { s << m_value; }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 double m_value;
   };
@@ -184,6 +201,8 @@
 s << "false";
 }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 bool m_value;
   };
@@ -199,15 +218,33 @@
 void SetValue(const std::string &string) { m_value = string; }
 
 void Dump(std::ostream &s) const override {
-  std::string quoted;
+  s << '"';
+  const size_t strsize = m_value.size();
+  for (size_t i = 0; i < strsize; ++i) {
+char ch = m_value[i];
+if (ch == '"')
+  s << '\\';
+s << ch;
+  }
+  s << '"';
+}
+
+void DumpBinaryEscaped(std::ostream &s) const override {
+  s << '"';
   const size_t strsize = m_value.size();
   for (size_t i = 0; i < strsize; ++i) {
 char ch = m_value[i];
 if (ch == '"')
-  quoted.push_back('\\');
-quoted.push_back(ch);
+  s << '\\';
+// gdb remote serial protocol binary escaping
+if (ch == '#' || ch == '$' || ch == '}' || c

[Lldb-commits] [lldb] 7ebcd88 - Add DumpBinaryEscaped method to JSONGenerator, avoid extra copy

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-04-04T14:14:02-07:00
New Revision: 7ebcd8891a7acc265cee4996d55a287a035f8771

URL: 
https://github.com/llvm/llvm-project/commit/7ebcd8891a7acc265cee4996d55a287a035f8771
DIFF: 
https://github.com/llvm/llvm-project/commit/7ebcd8891a7acc265cee4996d55a287a035f8771.diff

LOG: Add DumpBinaryEscaped method to JSONGenerator, avoid extra copy

All uses of JSONGenerator in debugserver would create a JSON text
dump of the object collection, then copy that string into a
binary-escaped string, then send it up to the lldb side or
make a compressed version and send that.

This adds a DumpBinaryEscaped method to JSONGenerator which
does the gdb remote serial protocol binary escaping directly,
and removes the need to pass over the string and have an
additional copy in memory.

Differential Revision: https://reviews.llvm.org/D122882
rdar://91117456

Added: 


Modified: 
lldb/tools/debugserver/source/JSONGenerator.h
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/JSONGenerator.h 
b/lldb/tools/debugserver/source/JSONGenerator.h
index fff84946eda27..1818250df5281 100644
--- a/lldb/tools/debugserver/source/JSONGenerator.h
+++ b/lldb/tools/debugserver/source/JSONGenerator.h
@@ -113,6 +113,8 @@ class JSONGenerator {
 
 virtual void Dump(std::ostream &s) const = 0;
 
+virtual void DumpBinaryEscaped(std::ostream &s) const = 0;
+
   private:
 Type m_type;
   };
@@ -136,6 +138,17 @@ class JSONGenerator {
   s << "]";
 }
 
+void DumpBinaryEscaped(std::ostream &s) const override {
+  s << "[";
+  const size_t arrsize = m_items.size();
+  for (size_t i = 0; i < arrsize; ++i) {
+m_items[i]->DumpBinaryEscaped(s);
+if (i + 1 < arrsize)
+  s << ",";
+  }
+  s << "]";
+}
+
   protected:
 typedef std::vector collection;
 collection m_items;
@@ -151,6 +164,8 @@ class JSONGenerator {
 
 void Dump(std::ostream &s) const override { s << m_value; }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 uint64_t m_value;
   };
@@ -165,6 +180,8 @@ class JSONGenerator {
 
 void Dump(std::ostream &s) const override { s << m_value; }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 double m_value;
   };
@@ -184,6 +201,8 @@ class JSONGenerator {
 s << "false";
 }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 bool m_value;
   };
@@ -199,15 +218,33 @@ class JSONGenerator {
 void SetValue(const std::string &string) { m_value = string; }
 
 void Dump(std::ostream &s) const override {
-  std::string quoted;
+  s << '"';
+  const size_t strsize = m_value.size();
+  for (size_t i = 0; i < strsize; ++i) {
+char ch = m_value[i];
+if (ch == '"')
+  s << '\\';
+s << ch;
+  }
+  s << '"';
+}
+
+void DumpBinaryEscaped(std::ostream &s) const override {
+  s << '"';
   const size_t strsize = m_value.size();
   for (size_t i = 0; i < strsize; ++i) {
 char ch = m_value[i];
 if (ch == '"')
-  quoted.push_back('\\');
-quoted.push_back(ch);
+  s << '\\';
+// gdb remote serial protocol binary escaping
+if (ch == '#' || ch == '$' || ch == '}' || ch == '*') {
+  s << '}'; // 0x7d next character is escaped
+  s << static_cast(ch ^ 0x20);
+} else {
+  s << ch;
+}
   }
-  s << '"' << quoted.c_str() << '"';
+  s << '"';
 }
 
   protected:
@@ -269,7 +306,43 @@ class JSONGenerator {
   s << "}";
 }
 
+void DumpBinaryEscaped(std::ostream &s) const override {
+  bool have_printed_one_elem = false;
+  s << "{";
+  for (collection::const_iterator iter = m_dict.begin();
+   iter != m_dict.end(); ++iter) {
+if (!have_printed_one_elem) {
+  have_printed_one_elem = true;
+} else {
+  s << ",";
+}
+s << "\"" << binary_encode_string(iter->first) << "\":";
+iter->second->DumpBinaryEscaped(s);
+  }
+  // '}' must be escaped for the gdb remote serial
+  // protocol.
+  s << "}";
+  s << static_cast('}' ^ 0x20);
+}
+
   protected:
+std::string binary_encode_string(const std::string &s) const {
+  std::string output;
+  const size_t s_size = s.size();
+  const char *s_chars = s.c_str();
+
+  for (size_t i = 0; i < s_size; i++) {
+unsigned char ch = *(s_chars + i);
+if (ch == '#' || ch == '$' || ch == '}' || ch == '*') {
+  output.push_back('}'); // 0x7d
+  output.push_back(ch ^ 0x20);
+} else {
+  output.push_back(ch);
+}
+  }
+  return output;
+}
+
 // Keep the dictionary as a vector so th

[Lldb-commits] [PATCH] D122882: Add JSONGenerator::DumpBinaryEscaped method in debugserver, update most callers to use this, skip escaping step

[Jason Molenda via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ebcd8891a7a: Add DumpBinaryEscaped method to JSONGenerator, 
avoid extra copy (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122882

Files:
  lldb/tools/debugserver/source/JSONGenerator.h
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -582,9 +582,8 @@
   // of these coming out at the wrong time (i.e. when the remote side
   // is not waiting for a process control completion response).
   stream << "JSON-async:";
-  dictionary.Dump(stream);
-  const std::string payload = binary_encode_string(stream.str());
-  return SendPacket(payload);
+  dictionary.DumpBinaryEscaped(stream);
+  return SendPacket(stream.str());
 }
 
 // Given a std::string packet contents to send, possibly encode/compress it.
@@ -2793,6 +2792,7 @@
   ostrm << std::hex << "jstopinfo:";
   std::ostringstream json_strm;
   threads_info_sp->Dump(json_strm);
+  threads_info_sp->Clear();
   append_hexified_string(ostrm, json_strm.str());
   ostrm << ';';
 }
@@ -5556,11 +5556,10 @@
 
 if (threads_info_sp) {
   std::ostringstream strm;
-  threads_info_sp->Dump(strm);
+  threads_info_sp->DumpBinaryEscaped(strm);
   threads_info_sp->Clear();
-  std::string binary_packet = binary_encode_string(strm.str());
-  if (!binary_packet.empty())
-return SendPacket(binary_packet);
+  if (strm.str().size() > 0)
+return SendPacket(strm.str());
 }
   }
   return SendPacket("E85");
@@ -5881,11 +5880,10 @@
 
 if (json_sp.get()) {
   std::ostringstream json_str;
-  json_sp->Dump(json_str);
+  json_sp->DumpBinaryEscaped(json_str);
   json_sp->Clear();
   if (json_str.str().size() > 0) {
-std::string json_str_quoted = binary_encode_string(json_str.str());
-return SendPacket(json_str_quoted);
+return SendPacket(json_str.str());
   } else {
 SendPacket("E84");
   }
@@ -5915,11 +5913,10 @@
 
 if (json_sp.get()) {
   std::ostringstream json_str;
-  json_sp->Dump(json_str);
+  json_sp->DumpBinaryEscaped(json_str);
   json_sp->Clear();
   if (json_str.str().size() > 0) {
-std::string json_str_quoted = binary_encode_string(json_str.str());
-return SendPacket(json_str_quoted);
+return SendPacket(json_str.str());
   } else {
 SendPacket("E86");
   }
Index: lldb/tools/debugserver/source/JSONGenerator.h
===
--- lldb/tools/debugserver/source/JSONGenerator.h
+++ lldb/tools/debugserver/source/JSONGenerator.h
@@ -113,6 +113,8 @@
 
 virtual void Dump(std::ostream &s) const = 0;
 
+virtual void DumpBinaryEscaped(std::ostream &s) const = 0;
+
   private:
 Type m_type;
   };
@@ -136,6 +138,17 @@
   s << "]";
 }
 
+void DumpBinaryEscaped(std::ostream &s) const override {
+  s << "[";
+  const size_t arrsize = m_items.size();
+  for (size_t i = 0; i < arrsize; ++i) {
+m_items[i]->DumpBinaryEscaped(s);
+if (i + 1 < arrsize)
+  s << ",";
+  }
+  s << "]";
+}
+
   protected:
 typedef std::vector collection;
 collection m_items;
@@ -151,6 +164,8 @@
 
 void Dump(std::ostream &s) const override { s << m_value; }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 uint64_t m_value;
   };
@@ -165,6 +180,8 @@
 
 void Dump(std::ostream &s) const override { s << m_value; }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 double m_value;
   };
@@ -184,6 +201,8 @@
 s << "false";
 }
 
+void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
   protected:
 bool m_value;
   };
@@ -199,15 +218,33 @@
 void SetValue(const std::string &string) { m_value = string; }
 
 void Dump(std::ostream &s) const override {
-  std::string quoted;
+  s << '"';
+  const size_t strsize = m_value.size();
+  for (size_t i = 0; i < strsize; ++i) {
+char ch = m_value[i];
+if (ch == '"')
+  s << '\\';
+s << ch;
+  }
+  s << '"';
+}
+
+void DumpBinaryEscaped(std::ostream &s) const override {
+  s << '"';
   const size_t strsize = m_value.size();
   for (size_t i = 0; i < strsize; ++i) {
 char ch = m_value[i];
 if (ch == '"')
-  quoted.push_back('\\');
-quoted.push_back(ch);
+  s << '\\';
+// gdb re

[Lldb-commits] [PATCH] D122660: [lldb] Avoid duplicate vdso modules when opening core files

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

We have seen this issue as well in linux, so thanks for fixing it. I never did 
any of the work on the posix dyld loader so I had been avoiding trying to fix 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122660

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

[Lldb-commits] [PATCH] D123092: [LLDB][NativePDB] Fix inline line info in line table

[Zequan Wu via Phabricator via lldb-commits]

zequanwu created this revision.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

It fixes the following case:

  0602  line 1 (+1)
  0315  code 0x15 (+0x15)
  0B2B  code 0x20 (+0xB) line 2 (+1)
  0602  line 3 (+1)
  0311  code 0x31 (+0x11)
  ...

Inline ranges should have following mapping:
`[0x15, 0x20) -> line 1`
`[0x20, 0x31) -> line 2`
Inline line entries:
`0x15, line 1`, `0x20, line 2`, `0x31, line 3`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123092

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites_live.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp

STAMPS
actor(@zequanwu) application(Differential) author(@zequanwu) herald(H343) 
herald(H425) herald(H576) herald(H864) monogram(D123092) object-type(DREV) 
phid(PHID-DREV-7wvqlerpq324xpa26viv) revision-repository(rG) 
revision-status(needs-review) subscriber(@lldb-commits) tag(#all) tag(#lldb) 
via(conduit)

Index: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
@@ -1,7 +1,9 @@
 // clang-format off
 // REQUIRES: system-windows
 
-// RUN: %build -o %t.exe -- %s
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full %t.obj -base:0x40 -out:%t.exe -pdb:%t.pdb
+
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/inline_sites_live.lldbinit 2>&1 | FileCheck %s
 
@@ -22,6 +24,32 @@
   foo(argc);
 }
 
+// CHECK:  (lldb) image dump line-table inline_sites_live.cpp -v
+// CHECK-NEXT: Line table
+// CHECK-NEXT: {{.*}}401000:{{.*}}:10
+// CHECK-NEXT: {{.*}}401007:{{.*}}:10, is_terminal_entry = TRUE
+// CHECK-NEXT: {{.*}}401010:{{.*}}:12
+// CHECK-NEXT: {{.*}}401018:{{.*}}:13
+// CHECK-NEXT: {{.*}}401021:{{.*}}:14
+// CHECK-NEXT: {{.*}}401027:{{.*}}:14, is_terminal_entry = TRUE
+// CHECK-NEXT: {{.*}}401030:{{.*}}:16
+// CHECK-NEXT: {{.*}}401038:{{.*}}:17
+// CHECK-NEXT: {{.*}}401043:{{.*}}:18
+// CHECK-NEXT: {{.*}}40104b:{{.*}}:13, is_start_of_statement = TRUE, is_prologue_end = TRUE
+// CHECK-NEXT: {{.*}}401054:{{.*}}:19
+// CHECK-NEXT: {{.*}}40105d:{{.*}}:20
+// CHECK-NEXT: {{.*}}401066:{{.*}}:21
+// CHECK-NEXT: {{.*}}40106c:{{.*}}:21, is_terminal_entry = TRUE
+// CHECK-NEXT: {{.*}}401070:{{.*}}:23
+// CHECK-NEXT: {{.*}}40107d:{{.*}}:24
+// CHECK-NEXT: {{.*}}401085:{{.*}}:17, is_start_of_statement = TRUE, is_prologue_end = TRUE
+// CHECK-NEXT: {{.*}}401090:{{.*}}:18
+// CHECK-NEXT: {{.*}}401098:{{.*}}:13, is_start_of_statement = TRUE, is_prologue_end = TRUE
+// CHECK-NEXT: {{.*}}4010a1:{{.*}}:19, is_start_of_statement = TRUE
+// CHECK-NEXT: {{.*}}4010aa:{{.*}}:20
+// CHECK-NEXT: {{.*}}4010b3:{{.*}}:25
+// CHECK-NEXT: {{.*}}4010ba:{{.*}}:25, is_terminal_entry = TRUE
+
 // CHECK:  * thread #1, stop reason = breakpoint 1
 // CHECK-NEXT:frame #0: {{.*}}`main [inlined] bar(param=2)
 // CHECK:  (lldb) p param
Index: lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites_live.lldbinit
===
--- lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites_live.lldbinit
+++ lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites_live.lldbinit
@@ -1,3 +1,4 @@
+image dump line-table inline_sites_live.cpp -v
 br set -p BP_bar -f inline_sites_live.cpp
 br set -p BP_foo -f inline_sites_live.cpp
 run
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1311,7 +1311,10 @@
 
   // Parse range and line info.
   uint32_t code_offset = 0;
+  uint32_t last_line_code_offset = 0;
   int32_t line_offset = 0;
+  // Only used for inline range.
+  int32_t prev_line_offset = -1;
   bool has_base = false;
   bool is_new_line_offset = false;
 
@@ -1321,10 +1324,13 @@
 
   auto change_code_offset = [&](uint32_t code_delta) {
 if (has_base) {
-  inline_site_sp->ranges.Append(RangeSourceLineVector::Entry(
-  code_offset, code_delta, decl_line + line_offset));
+  uint32_t source_line =
+  decl_line + (prev_line_offset == -1 ? line_offset : prev_line_offset);
+  inline_site_sp->ranges.Append(
+  RangeSourceLineVector::Entry(code_offset, code_delta, source_line));
   is_prologue_end = false;
   is_start_of_statement = false;
+  prev_line_offset = -1;
 } else {
   is_start_of_statement = true;
 }
@@ -1337,25 +1343,35 @@
   true, false, is_prologue_end, false, false);
   

[Lldb-commits] [PATCH] D123098: [lldb] Fix undefined behavior: left shift of negative value

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, jingham, jasonmolenda.
Herald added a project: All.
JDevlieghere requested review of this revision.

Fix undefined behavior in AppleObjCRuntimeV2 where we were left shifting a 
signed value.

rdar://91242879


https://reviews.llvm.org/D123098

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2765,7 +2765,7 @@
   (((uint64_t)unobfuscated << m_objc_debug_taggedpointer_payload_lshift) >>
m_objc_debug_taggedpointer_payload_rshift);
   int64_t data_payload_signed =
-  ((int64_t)((int64_t)unobfuscated
+  ((int64_t)((uint64_t)unobfuscated
  << m_objc_debug_taggedpointer_payload_lshift) >>
m_objc_debug_taggedpointer_payload_rshift);
   return ClassDescriptorSP(new ClassDescriptorV2Tagged(


STAMPS
actor(@JDevlieghere) application(Differential) author(@JDevlieghere) 
herald(H576) herald(H864) monogram(D123098) object-type(DREV) 
phid(PHID-DREV-vn5s2fbg65fw42dy4msc) reviewer(@aprantl) reviewer(@jasonmolenda) 
reviewer(@jingham) revision-status(needs-review) subscriber(@lldb-commits) 
tag(#all) via(web)

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2765,7 +2765,7 @@
   (((uint64_t)unobfuscated << m_objc_debug_taggedpointer_payload_lshift) >>
m_objc_debug_taggedpointer_payload_rshift);
   int64_t data_payload_signed =
-  ((int64_t)((int64_t)unobfuscated
+  ((int64_t)((uint64_t)unobfuscated
  << m_objc_debug_taggedpointer_payload_lshift) >>
m_objc_debug_taggedpointer_payload_rshift);
   return ClassDescriptorSP(new ClassDescriptorV2Tagged(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122943: [LLDB][NativePDB] Fix a crash when S_DEFRANGE_SUBFIELD_REGISTER descirbes a simple type

[Alexandre Ganea via Phabricator via lldb-commits]

aganea added a comment.

There's a typo in the title, `s/descirbes/describes/`




Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp:54
 struct FindMembersSize : public TypeVisitorCallbacks {
-  FindMembersSize(std::vector> &members_info,
+  FindMembersSize(llvm::SmallVector> 
&members_info,
   TpiStream &tpi)

labath wrote:
> I think `SmallVectorImpl` is still the official way to take SmallVector 
> references.
`ArrayRef` would be even better! :) ie. `llvm::ArrayRef> members_info;`



Comment at: 
lldb/test/Shell/SymbolFile/NativePDB/subfield_register_simple_type.s:27
+# CHECK:  LineEntry: [0x00401026-0x00401039): C:\src\a.cpp:3
+# CHECK-NEXT: Variable: id = {{.*}}, name = "x", type = "int64_t", valid 
ranges = [0x0040102f-0x00401036), location = DW_OP_reg0 EAX, DW_OP_piece 0x4, 
DW_OP_reg2 EDX, DW_OP_piece 0x4, decl =
+

nitpick: Is anything after (and including) ", valid ranges" necessary to cover 
this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122943

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

[Lldb-commits] [PATCH] D123098: [lldb] Fix undefined behavior: left shift of negative value

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Note, in C++20 this was made well-defined see godbolt 
 the paper that did this was P0907R4 Signed 
Integers are Two’s Complement 



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

https://reviews.llvm.org/D123098

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

[Lldb-commits] [PATCH] D122991: [lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`

[walter erquinigo via Phabricator via lldb-commits]

wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

only mostly cosmetic changes needed. Thanks for this. I'm glad that we are 
bringing the usage down




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:46-47
+
+TraceInstructionControlFlowType DecodedThread::GetInstructionControlFlowType(
+size_t insn_index, lldb::addr_t next_load_address) const {
+  if (IsInstructionAnError(insn_index))





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:54-55
 
-  switch (m_pt_insn.iclass) {
+  lldb::addr_t ip = m_instruction_ips[insn_index];
+  uint8_t size = m_instruction_sizes[insn_index];
+  pt_insn_class iclass = m_instruction_classes[insn_index];

let's give better names to these variables



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:62
 mask |= eTraceInstructionControlFlowTypeBranch;
-if (m_pt_insn.ip + m_pt_insn.size != next_load_address)
+if (ip + size != next_load_address)
   mask |= eTraceInstructionControlFlowTypeTakenBranch;

we can stop using next_load_address because we now have access to the entire 
trace



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:176
+ sizeof(pt_insn::iclass) * m_instruction_classes.size() +
  m_errors.getMemorySize();
 }

you also need to include `m_instruction_timestamps`, probably 
`m_instruction_timestamps.size() * (sizeof(size_t) + sizeof(uint64_t))`



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:127
+  /// Append a decoding error with a corresponding TSC.
+  void AppendError(llvm::Error &&error, uint64_t TSC);
+

lowercase tsc because it's a variable



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:129-136
+  /// Get the instruction pointers from the decoded trace. Some of them might
+  /// indicate errors (i.e. gaps) in the trace. For an instruction error, you
+  /// can access its underlying error message with the \a
+  /// GetErrorByInstructionIndex() method.
   ///
   /// \return
   ///   The instructions of the trace.

I regret having created this method because it forces us to have a certain 
interface for the storage of the instructions. Let's just delete this method 
and create one new method
  size_t GetInstructionsCount() const;
which should be able to replace all the external calls to this method



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:139
+  /// \return
+  /// The instruction pointer address, or \a LLDB_INVALID_ADDRESS if it is
+  /// an error.





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:143-154
+  /// Get the \a lldb::TraceInstructionControlFlowType categories of the
+  /// instruction.
+  ///
+  /// \param[in] next_load_address
+  /// The address of the next instruction in the trace or \b
+  /// LLDB_INVALID_ADDRESS if not available.
+  ///

Now that DecodedThread knows everything, we can simplify this method a bit. 
First, we can remove the parameter `next_load_address`, because it can easily 
be gotten by checking the instruction at index `insn_index + 1`



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:193-199
+  /// Record an error decoding a TSC timestamp.
+  ///
+  /// See \a GetTscErrors() for more documentation.
+  ///
+  /// \param[in] libipt_error_code
+  ///   An error returned by the libipt library.
+  void RecordTscError(int libipt_error_code);

+1



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:218
+  std::vector m_instruction_ips;
+  /// The low level storage of all instruction sizes.
+  std::vector m_instruction_sizes;





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:220
+  std::vector m_instruction_sizes;
+  /// The low level storage of all instruction classes.
+  std::vector m_instruction_classes;





Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:23-30
+  assert(!m_decoded_thread_sp->GetInstructionPointers().empty() &&
  "a trace should have at least one instruction or error");
-  m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_pos = m_decoded_thread_sp->GetInstructionPointers().size() - 1;
   m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122991

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

[Lldb-commits] [PATCH] D123098: [lldb] Fix undefined behavior: left shift of negative value

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

LGTM but `unobfuscated` is a uint64_t, does this cast do anything now?


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

https://reviews.llvm.org/D123098

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

[Lldb-commits] [PATCH] D123025: [lldb-vscode] Implement stderr/stdout on win32 and redirect lldb log to VSCode

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/tools/lldb-vscode/OutputRedirector.cpp:12
+#else
+#include 
+#include 

mstorsjo wrote:
> Minor style issue - I guess it'd be less of double negation, if we'd change 
> the ifdef to `#if defined(_WIN32) .. #else`
feel free to refactor that part



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1444-1449
+  auto log_cb = [](const char *buf, void *baton) -> void {
+g_vsc.SendOutput(OutputType::Console, llvm::StringRef{buf});
+  };
+  g_vsc.debugger =
+  lldb::SBDebugger::Create(true /*source_init_files*/, log_cb, nullptr);
   g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction);

could you test by creating a .lldbinit file with garbage so that the debugger 
prints error messages during initialization, and then make sure that they are 
properly redirected to the console of the IDE?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123025

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

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: jj10306, zrthxn.
Herald added a subscriber: mgorny.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

As we soon will need to decode multiple raw traces for the same thread,
having a class that encapsulates the decoding of a single raw trace is
a stepping stone that will make the coming features easier to implement.

So, I'm creating a LibiptDecoder class with that purpose. I refactored
the code and it's now much more readable. Besides that, more comments
were added. With this new structure, it's also easier to implement unit
tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123106

Files:
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h

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,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 
-#include "IntelPTDecoder.h"
+#include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-types.h"
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -9,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
 
-#include "IntelPTDecoder.h"
+#include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 
 namespace lldb_private {
Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
===
--- lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
+++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
@@ -1,4 +1,4 @@
-//===-- IntelPTDecoder.h --==*- C++ -*-===//
+//===-- ThreadDecoder.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.
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
-#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
+#define LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
 
 #include "intel-pt.h"
 
@@ -84,4 +84,4 @@
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#endif // LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
@@ -0,0 +1,80 @@
+//===-- ThreadDecoder.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 "ThreadDecoder.h"
+
+#include "llvm/Support/MemoryBuffer.h"
+
+#include "../common/ThreadPostMortemTrace.h"
+#include "LibiptDecoder.h"
+#include "TraceIntelPT.h"
+
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::trace_intel_pt;
+using namespace llvm;
+
+// ThreadDecoder 
+
+DecodedThreadSP ThreadDecoder::Decode() {
+  if (!m_decoded_thread.hasValue())
+m_decoded_thread = DoDecode();
+  return *m_decoded_thread;
+}
+
+// LiveThreadDecoder 
+
+LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace)
+: m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
+
+DecodedThreadSP LiveThreadDecoder::DoDecode() {
+  DecodedThreadSP decoded_thread_sp =
+  std::make_shared(m_thread_sp);
+
+  Expected> buffer =
+  m_trace.GetLiveThreadBuffer(m_thread_sp->GetID());
+  if (!buffer) {
+decoded_th

[Lldb-commits] [lldb] aaca2ac - [lldb][gui] do not show the help window on first gui startup

[Luboš Luňák via lldb-commits]

Author: Luboš Luňák
Date: 2022-04-05T08:29:13+02:00
New Revision: aaca2acd5f3434ccd1d568596161628ce53d9066

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

LOG: [lldb][gui] do not show the help window on first gui startup

It's rather annoying if it's there after every startup,
and that 'Help (F6)' at the top should be enough to help people
who don't know.

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

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/test/API/commands/gui/basic/TestGuiBasic.py
lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index bb15f4d55936e..8577f4b282974 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -7667,14 +7667,6 @@ void IOHandlerCursesGUI::Activate() {
 status_window_sp->SetDelegate(
 WindowDelegateSP(new StatusBarWindowDelegate(m_debugger)));
 
-// Show the main help window once the first time the curses GUI is
-// launched
-static bool g_showed_help = false;
-if (!g_showed_help) {
-  g_showed_help = true;
-  main_window_sp->CreateHelpSubwindow();
-}
-
 // All colors with black background.
 init_pair(1, COLOR_BLACK, COLOR_BLACK);
 init_pair(2, COLOR_RED, COLOR_BLACK);

diff  --git a/lldb/test/API/commands/gui/basic/TestGuiBasic.py 
b/lldb/test/API/commands/gui/basic/TestGuiBasic.py
index 4e1a0f990a683..a28754473ded0 100644
--- a/lldb/test/API/commands/gui/basic/TestGuiBasic.py
+++ b/lldb/test/API/commands/gui/basic/TestGuiBasic.py
@@ -26,18 +26,9 @@ def test_gui(self):
 
 escape_key = chr(27).encode()
 
-# Start the GUI for the first time and check for the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.expect_exact("Welcome to the LLDB curses GUI.")
 
-# Press escape to quit the welcome screen
-self.child.send(escape_key)
-# Press escape again to quit the gui
-self.child.send(escape_key)
-self.expect_prompt()
-
-# Start the GUI a second time, this time we should have the normal GUI.
-self.child.sendline("gui")
 # Check for GUI elements in the menu bar.
 self.child.expect_exact("Target")
 self.child.expect_exact("Process")

diff  --git a/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py 
b/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
index a7df72509d58f..98c8ff5db08b7 100644
--- a/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
+++ b/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -25,10 +25,8 @@ def test_gui(self):
 
 escape_key = chr(27).encode()
 
-# Start the GUI and close the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.expect("Welcome to the LLDB curses GUI.")
-self.child.send(escape_key)
 
 # Simulate a simple debugging session.
 self.child.send("s") # step

diff  --git a/lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py 
b/lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
index 70729594fde4e..627fa7a9aed9d 100644
--- a/lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
+++ b/lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
@@ -29,9 +29,8 @@ def test_gui(self):
 escape_key = chr(27).encode()
 down_key = chr(27)+'OB' # for vt100 terminal (lldbexpect sets 
TERM=vt100)
 
-# Start the GUI and close the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.send(escape_key)
 self.child.expect_exact("Sources") # wait for gui
 
 # Go to next line, set a breakpoint.

diff  --git a/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py 
b/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
index b1be950dd2006..633aa1f570088 100644
--- a/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
+++ b/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
@@ -31,9 +31,8 @@ def test_gui(self):
 right_key = chr(27)+'OC'
 ctrl_l = chr(12)
 
-# Start the GUI and close the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.send(escape_key)
 
 # Check the sources window.
 self.child.expect_exact("Sources")



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

[Lldb-commits] [lldb] f90fa55 - [lldb][gui] use just '#2' instead of 'frame #2' in the threads/frame view

[Luboš Luňák via lldb-commits]

Author: Luboš Luňák
Date: 2022-04-05T08:29:13+02:00
New Revision: f90fa55569fc8fc3b44512823ed9e2a0d1243826

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

LOG: [lldb][gui] use just '#2' instead of 'frame #2' in the threads/frame view

Since the threads/frame view is taking only a small part on the right side
of the screen, only a part of the function name of each frame is visible.
It seems rather wasteful to spell out 'frame' there when it's obvious
that it is a frame, it's better to use the space for more of the function
name.

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

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index 8577f4b282974..7fc23292983fa 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -5016,8 +5016,7 @@ class FrameTreeDelegate : public TreeDelegate {
 public:
   FrameTreeDelegate() : TreeDelegate() {
 FormatEntity::Parse(
-"frame #${frame.index}: {${function.name}${function.pc-offset}}}",
-m_format);
+"#${frame.index}: {${function.name}${function.pc-offset}}}", m_format);
   }
 
   ~FrameTreeDelegate() override = default;

diff  --git 
a/lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py 
b/lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
index 9eed5b0ef6c39..34a6bf3643534 100644
--- a/lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
+++ b/lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
@@ -33,7 +33,7 @@ def test_gui(self):
 self.child.expect_exact("Threads")
 
 # The thread running thread_start_routine should be expanded.
-self.child.expect_exact("frame #0: break_here")
+self.child.expect_exact("#0: break_here")
 
 # Exit GUI.
 self.child.send(escape_key)
@@ -47,7 +47,7 @@ def test_gui(self):
 self.child.expect_exact("Threads")
 
 # The main thread should be expanded.
-self.child.expect("frame #\d+: main")
+self.child.expect("#\d+: main")
 
 # Quit the GUI
 self.child.send(escape_key)



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

[Lldb-commits] [lldb] 76bc772 - [lldb][gui] make 'step out' step out of the selected frame

[Luboš Luňák via lldb-commits]

Author: Luboš Luňák
Date: 2022-04-05T08:29:13+02:00
New Revision: 76bc7729208976e6ff6bc28b08e460d97cab5bb3

URL: 
https://github.com/llvm/llvm-project/commit/76bc7729208976e6ff6bc28b08e460d97cab5bb3
DIFF: 
https://github.com/llvm/llvm-project/commit/76bc7729208976e6ff6bc28b08e460d97cab5bb3.diff

LOG: [lldb][gui] make 'step out' step out of the selected frame

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

Added: 


Modified: 
lldb/include/lldb/Target/Thread.h
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Target/Thread.cpp
lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
lldb/test/API/commands/gui/basicdebug/func.c
lldb/test/API/commands/gui/basicdebug/main.c

Removed: 




diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 2fd7d8859f525..f5f024434c8ee 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -539,9 +539,12 @@ class Thread : public std::enable_shared_from_this,
   /// This function is designed to be used by commands where the
   /// process is publicly stopped.
   ///
+  /// \param[in] frame_idx
+  /// The frame index to step out of.
+  ///
   /// \return
   /// An error that describes anything that went wrong
-  virtual Status StepOut();
+  virtual Status StepOut(uint32_t frame_idx = 0);
 
   /// Retrieves the per-thread data area.
   /// Most OSs maintain a per-thread pointer (e.g. the FS register on
@@ -836,7 +839,7 @@ class Thread : public std::enable_shared_from_this,
   ///See standard meanings for the stop & run votes in ThreadPlan.h.
   ///
   /// \param[in] frame_idx
-  /// The fame index.
+  /// The frame index.
   ///
   /// \param[out] status
   /// A status with an error if queuing failed.

diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index 7fc23292983fa..caf88c7fdf376 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -6402,8 +6402,11 @@ class ApplicationDelegate : public WindowDelegate, 
public MenuDelegate {
   if (exe_ctx.HasThreadScope()) {
 Process *process = exe_ctx.GetProcessPtr();
 if (process && process->IsAlive() &&
-StateIsStoppedState(process->GetState(), true))
-  exe_ctx.GetThreadRef().StepOut();
+StateIsStoppedState(process->GetState(), true)) {
+  Thread *thread = exe_ctx.GetThreadPtr();
+  uint32_t frame_idx = thread->GetSelectedFrameIndex();
+  exe_ctx.GetThreadRef().StepOut(frame_idx);
+}
   }
 }
   return MenuActionResult::Handled;
@@ -7361,7 +7364,9 @@ class SourceFileWindowDelegate : public WindowDelegate {
 m_debugger.GetCommandInterpreter().GetExecutionContext();
 if (exe_ctx.HasThreadScope() &&
 StateIsStoppedState(exe_ctx.GetProcessRef().GetState(), true)) {
-  exe_ctx.GetThreadRef().StepOut();
+  Thread *thread = exe_ctx.GetThreadPtr();
+  uint32_t frame_idx = thread->GetSelectedFrameIndex();
+  exe_ctx.GetThreadRef().StepOut(frame_idx);
 }
   }
   return eKeyHandled;

diff  --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 332e03bedbf17..3803748be2971 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1953,7 +1953,7 @@ Status Thread::StepOver(bool source_step,
   return error;
 }
 
-Status Thread::StepOut() {
+Status Thread::StepOut(uint32_t frame_idx) {
   Status error;
   Process *process = GetProcess().get();
   if (StateIsStoppedState(process->GetState(), true)) {
@@ -1963,7 +1963,7 @@ Status Thread::StepOut() {
 
 ThreadPlanSP new_plan_sp(QueueThreadPlanForStepOut(
 abort_other_plans, nullptr, first_instruction, stop_other_threads,
-eVoteYes, eVoteNoOpinion, 0, error));
+eVoteYes, eVoteNoOpinion, frame_idx, error));
 
 new_plan_sp->SetIsControllingPlan(true);
 new_plan_sp->SetOkayToDiscard(false);

diff  --git a/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py 
b/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
index 98c8ff5db08b7..ee5a74c1f1798 100644
--- a/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
+++ b/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -32,7 +32,7 @@ def test_gui(self):
 self.child.send("s") # step
 self.child.expect("return 1; // In function[^\r\n]+<<< Thread 1: step 
in")
 self.child.send("u") # up
-self.child.expect_exact("func(); // Break here")
+self.child.expect_exact("func();// Break here")
 self.child.send("d") # down
 self.child.expect_exact("return 1; // In function")
 self.child.send("f") # finish
@@ -40,7 +40,19 @@ def test_gui(self):
 self.child.send("s") # move onto the second one
 sel

[Lldb-commits] [PATCH] D122997: do not show the help window on first gui startup

[Luboš Luňák via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaaca2acd5f34: [lldb][gui] do not show the help window on 
first gui startup (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122997

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/test/API/commands/gui/basic/TestGuiBasic.py
  lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
  lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
  lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py


Index: lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
===
--- lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
+++ lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
@@ -31,9 +31,8 @@
 right_key = chr(27)+'OC'
 ctrl_l = chr(12)
 
-# Start the GUI and close the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.send(escape_key)
 
 # Check the sources window.
 self.child.expect_exact("Sources")
Index: lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
===
--- lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
+++ lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
@@ -29,9 +29,8 @@
 escape_key = chr(27).encode()
 down_key = chr(27)+'OB' # for vt100 terminal (lldbexpect sets 
TERM=vt100)
 
-# Start the GUI and close the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.send(escape_key)
 self.child.expect_exact("Sources") # wait for gui
 
 # Go to next line, set a breakpoint.
Index: lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
===
--- lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
+++ lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -25,10 +25,8 @@
 
 escape_key = chr(27).encode()
 
-# Start the GUI and close the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.expect("Welcome to the LLDB curses GUI.")
-self.child.send(escape_key)
 
 # Simulate a simple debugging session.
 self.child.send("s") # step
Index: lldb/test/API/commands/gui/basic/TestGuiBasic.py
===
--- lldb/test/API/commands/gui/basic/TestGuiBasic.py
+++ lldb/test/API/commands/gui/basic/TestGuiBasic.py
@@ -26,18 +26,9 @@
 
 escape_key = chr(27).encode()
 
-# Start the GUI for the first time and check for the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.expect_exact("Welcome to the LLDB curses GUI.")
 
-# Press escape to quit the welcome screen
-self.child.send(escape_key)
-# Press escape again to quit the gui
-self.child.send(escape_key)
-self.expect_prompt()
-
-# Start the GUI a second time, this time we should have the normal GUI.
-self.child.sendline("gui")
 # Check for GUI elements in the menu bar.
 self.child.expect_exact("Target")
 self.child.expect_exact("Process")
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -7667,14 +7667,6 @@
 status_window_sp->SetDelegate(
 WindowDelegateSP(new StatusBarWindowDelegate(m_debugger)));
 
-// Show the main help window once the first time the curses GUI is
-// launched
-static bool g_showed_help = false;
-if (!g_showed_help) {
-  g_showed_help = true;
-  main_window_sp->CreateHelpSubwindow();
-}
-
 // All colors with black background.
 init_pair(1, COLOR_BLACK, COLOR_BLACK);
 init_pair(2, COLOR_RED, COLOR_BLACK);


Index: lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
===
--- lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
+++ lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
@@ -31,9 +31,8 @@
 right_key = chr(27)+'OC'
 ctrl_l = chr(12)
 
-# Start the GUI and close the welcome window.
+# Start the GUI.
 self.child.sendline("gui")
-self.child.send(escape_key)
 
 # Check the sources window.
 self.child.expect_exact("Sources")
Index: lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
===
--- lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
+++ lldb/test/API/commands/gui/breakpoints/TestGuiBreakpoints.py
@@ -29,9 +29,8

[Lldb-commits] [PATCH] D122998: use just '#' instead of 'frame #2' in the threads/frame view

[Luboš Luňák via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf90fa55569fc: [lldb][gui] use just '#2' instead of 
'frame #2' in the threads/frame view (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122998

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py


Index: 
lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
===
--- lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
+++ lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
@@ -33,7 +33,7 @@
 self.child.expect_exact("Threads")
 
 # The thread running thread_start_routine should be expanded.
-self.child.expect_exact("frame #0: break_here")
+self.child.expect_exact("#0: break_here")
 
 # Exit GUI.
 self.child.send(escape_key)
@@ -47,7 +47,7 @@
 self.child.expect_exact("Threads")
 
 # The main thread should be expanded.
-self.child.expect("frame #\d+: main")
+self.child.expect("#\d+: main")
 
 # Quit the GUI
 self.child.send(escape_key)
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -5016,8 +5016,7 @@
 public:
   FrameTreeDelegate() : TreeDelegate() {
 FormatEntity::Parse(
-"frame #${frame.index}: {${function.name}${function.pc-offset}}}",
-m_format);
+"#${frame.index}: {${function.name}${function.pc-offset}}}", m_format);
   }
 
   ~FrameTreeDelegate() override = default;


Index: lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
===
--- lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
+++ lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
@@ -33,7 +33,7 @@
 self.child.expect_exact("Threads")
 
 # The thread running thread_start_routine should be expanded.
-self.child.expect_exact("frame #0: break_here")
+self.child.expect_exact("#0: break_here")
 
 # Exit GUI.
 self.child.send(escape_key)
@@ -47,7 +47,7 @@
 self.child.expect_exact("Threads")
 
 # The main thread should be expanded.
-self.child.expect("frame #\d+: main")
+self.child.expect("#\d+: main")
 
 # Quit the GUI
 self.child.send(escape_key)
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -5016,8 +5016,7 @@
 public:
   FrameTreeDelegate() : TreeDelegate() {
 FormatEntity::Parse(
-"frame #${frame.index}: {${function.name}${function.pc-offset}}}",
-m_format);
+"#${frame.index}: {${function.name}${function.pc-offset}}}", m_format);
   }
 
   ~FrameTreeDelegate() override = default;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123001: make 'step out' step out of the selected frame

[Luboš Luňák via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG76bc77292089: [lldb][gui] make 'step out' step out 
of the selected frame (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123001

Files:
  lldb/include/lldb/Target/Thread.h
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Target/Thread.cpp
  lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
  lldb/test/API/commands/gui/basicdebug/func.c
  lldb/test/API/commands/gui/basicdebug/main.c

Index: lldb/test/API/commands/gui/basicdebug/main.c
===
--- lldb/test/API/commands/gui/basicdebug/main.c
+++ lldb/test/API/commands/gui/basicdebug/main.c
@@ -1,7 +1,14 @@
 extern int func();
+extern void func_up();
 
 int main(int argc, char **argv) {
-  func(); // Break here
-  func(); // Second
+  int dummy;
+  func();// Break here
+  func();// Second
+  dummy = 1; // Dummy command 1
+
+  func_up(); // First func1 call
+  dummy = 2; // Dummy command 2
+
   return 0;
 }
Index: lldb/test/API/commands/gui/basicdebug/func.c
===
--- lldb/test/API/commands/gui/basicdebug/func.c
+++ lldb/test/API/commands/gui/basicdebug/func.c
@@ -1,3 +1,12 @@
 int func() {
   return 1; // In function
 }
+
+void func_down() {
+  int dummy = 1; // In func_down
+  (void)dummy;
+}
+
+void func_up() {
+  func_down(); // In func_up
+}
Index: lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
===
--- lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
+++ lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -32,7 +32,7 @@
 self.child.send("s") # step
 self.child.expect("return 1; // In function[^\r\n]+<<< Thread 1: step in")
 self.child.send("u") # up
-self.child.expect_exact("func(); // Break here")
+self.child.expect_exact("func();// Break here")
 self.child.send("d") # down
 self.child.expect_exact("return 1; // In function")
 self.child.send("f") # finish
@@ -40,7 +40,19 @@
 self.child.send("s") # move onto the second one
 self.child.expect("<<< Thread 1: step in")
 self.child.send("n") # step over
-self.child.expect("<<< Thread 1: step over")
+self.child.expect("// Dummy command 1[^\r\n]+<<< Thread 1: step over")
+self.child.send("n")
+
+# Test that 'up' + 'step out' steps out of the selected function.
+self.child.send("s") # move into func_up()
+self.child.expect("// In func_up")
+self.child.send("s") # move into func_down()
+self.child.expect("// In func_down")
+self.child.send("u") # up
+self.child.expect("// In func_up")
+self.child.send("f") # finish
+self.child.expect("// Dummy command 2[^\r\n]+<<< Thread 1: step out")
+self.child.send("n")
 
 # Press escape to quit the gui
 self.child.send(escape_key)
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1953,7 +1953,7 @@
   return error;
 }
 
-Status Thread::StepOut() {
+Status Thread::StepOut(uint32_t frame_idx) {
   Status error;
   Process *process = GetProcess().get();
   if (StateIsStoppedState(process->GetState(), true)) {
@@ -1963,7 +1963,7 @@
 
 ThreadPlanSP new_plan_sp(QueueThreadPlanForStepOut(
 abort_other_plans, nullptr, first_instruction, stop_other_threads,
-eVoteYes, eVoteNoOpinion, 0, error));
+eVoteYes, eVoteNoOpinion, frame_idx, error));
 
 new_plan_sp->SetIsControllingPlan(true);
 new_plan_sp->SetOkayToDiscard(false);
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -6402,8 +6402,11 @@
   if (exe_ctx.HasThreadScope()) {
 Process *process = exe_ctx.GetProcessPtr();
 if (process && process->IsAlive() &&
-StateIsStoppedState(process->GetState(), true))
-  exe_ctx.GetThreadRef().StepOut();
+StateIsStoppedState(process->GetState(), true)) {
+  Thread *thread = exe_ctx.GetThreadPtr();
+  uint32_t frame_idx = thread->GetSelectedFrameIndex();
+  exe_ctx.GetThreadRef().StepOut(frame_idx);
+}
   }
 }
   return MenuActionResult::Handled;
@@ -7361,7 +7364,9 @@
 m_debugger.GetCommandInterpreter().GetExecutionContext();
 if (exe_ctx.HasThreadScope() &&
 StateIsStoppedState(exe_ctx.GetProcessRef().GetState(), true)) {
-  exe_ctx.GetThreadRef().StepOut();
+  Thread *thread = exe_ctx.GetThreadPtr();

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

[walter erquinigo via Phabricator via lldb-commits]

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

nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123106

Files:
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h

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,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 
-#include "IntelPTDecoder.h"
+#include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-types.h"
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -9,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
 
-#include "IntelPTDecoder.h"
+#include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 
 namespace lldb_private {
Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
===
--- lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
+++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
@@ -1,4 +1,4 @@
-//===-- IntelPTDecoder.h --==*- C++ -*-===//
+//===-- ThreadDecoder.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.
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
-#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
+#define LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
 
 #include "intel-pt.h"
 
@@ -84,4 +84,4 @@
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#endif // LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
@@ -0,0 +1,80 @@
+//===-- ThreadDecoder.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 "ThreadDecoder.h"
+
+#include "llvm/Support/MemoryBuffer.h"
+
+#include "../common/ThreadPostMortemTrace.h"
+#include "LibiptDecoder.h"
+#include "TraceIntelPT.h"
+
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::trace_intel_pt;
+using namespace llvm;
+
+// ThreadDecoder 
+
+DecodedThreadSP ThreadDecoder::Decode() {
+  if (!m_decoded_thread.hasValue())
+m_decoded_thread = DoDecode();
+  return *m_decoded_thread;
+}
+
+// LiveThreadDecoder 
+
+LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace)
+: m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
+
+DecodedThreadSP LiveThreadDecoder::DoDecode() {
+  DecodedThreadSP decoded_thread_sp =
+  std::make_shared(m_thread_sp);
+
+  Expected> buffer =
+  m_trace.GetLiveThreadBuffer(m_thread_sp->GetID());
+  if (!buffer) {
+decoded_thread_sp->AppendError(buffer.takeError());
+return decoded_thread_sp;
+  }
+
+  decoded_thread_sp->SetRawTraceSize(buffer->size());
+  DecodeInMemoryTrace(*decoded_thread_sp, m_trace,
+  MutableArrayRef(*buffer));
+  return decoded_thread_sp;
+}
+
+// PostMortemThreadDecoder ===
+
+PostMortemThreadDecoder::PostMortemThreadDecoder(
+const lldb::ThreadPostMortemTraceSP &trace_thread, TraceIntelPT &trace)
+: m_trace_thread(trace_thread), m_trace(trace) {}
+
+DecodedThreadSP PostMortemThreadDe