[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks good to me. Pavel?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/test/Shell/Log/TestHandlers.test:1-2
+# RUN: %lldb -o 'log enable -h os -f /tmp/foo  gdb-remote packets' 2>&1 | 
FileCheck %s  --check-prefix UNSUPPORTED-FILE
+# RUN: %lldb -o 'log enable -h os -b 10  gdb-remote packets' 2>&1 | FileCheck 
%s  --check-prefix UNSUPPORTED-BUFFER
+

Can we use the long option names for readability here?


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

https://reviews.llvm.org/D143623

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

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

[Alex Langford via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D143623

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

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 496011.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

Update error message


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

https://reviews.llvm.org/D143623

Files:
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/test/Shell/Log/TestHandlers.test


Index: lldb/test/Shell/Log/TestHandlers.test
===
--- /dev/null
+++ lldb/test/Shell/Log/TestHandlers.test
@@ -0,0 +1,5 @@
+# RUN: %lldb -o 'log enable -h os -f /tmp/foo  gdb-remote packets' 2>&1 | 
FileCheck %s  --check-prefix UNSUPPORTED-FILE
+# RUN: %lldb -o 'log enable -h os -b 10  gdb-remote packets' 2>&1 | FileCheck 
%s  --check-prefix UNSUPPORTED-BUFFER
+
+# UNSUPPORTED-FILE: a file name can only be specified for the stream handler
+# UNSUPPORTED-BUFFER:  a buffer size can only be specified for the circular 
and stream buffer handler
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -177,6 +177,21 @@
   return false;
 }
 
+if ((m_options.handler != eLogHandlerCircular &&
+ m_options.handler != eLogHandlerStream) &&
+m_options.buffer_size.GetCurrentValue() != 0) {
+  result.AppendError(
+  "a buffer size can only be specified for the circular and stream"
+  "buffer handler.\n");
+  return false;
+}
+
+if (m_options.handler != eLogHandlerStream && m_options.log_file) {
+  result.AppendError(
+  "a file name can only be specified for the stream handler.\n");
+  return false;
+}
+
 // Store into a std::string since we're about to shift the channel off.
 const std::string channel = std::string(args[0].ref());
 args.Shift(); // Shift off the channel


Index: lldb/test/Shell/Log/TestHandlers.test
===
--- /dev/null
+++ lldb/test/Shell/Log/TestHandlers.test
@@ -0,0 +1,5 @@
+# RUN: %lldb -o 'log enable -h os -f /tmp/foo  gdb-remote packets' 2>&1 | FileCheck %s  --check-prefix UNSUPPORTED-FILE
+# RUN: %lldb -o 'log enable -h os -b 10  gdb-remote packets' 2>&1 | FileCheck %s  --check-prefix UNSUPPORTED-BUFFER
+
+# UNSUPPORTED-FILE: a file name can only be specified for the stream handler
+# UNSUPPORTED-BUFFER:  a buffer size can only be specified for the circular and stream buffer handler
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -177,6 +177,21 @@
   return false;
 }
 
+if ((m_options.handler != eLogHandlerCircular &&
+ m_options.handler != eLogHandlerStream) &&
+m_options.buffer_size.GetCurrentValue() != 0) {
+  result.AppendError(
+  "a buffer size can only be specified for the circular and stream"
+  "buffer handler.\n");
+  return false;
+}
+
+if (m_options.handler != eLogHandlerStream && m_options.log_file) {
+  result.AppendError(
+  "a file name can only be specified for the stream handler.\n");
+  return false;
+}
+
 // Store into a std::string since we're about to shift the channel off.
 const std::string channel = std::string(args[0].ref());
 args.Shift(); // Shift off the channel
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

[Alex Langford via Phabricator via lldb-commits]

bulbazord requested changes to this revision.
bulbazord added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/source/Commands/CommandObjectLog.cpp:180-184
+if ((m_options.handler != eLogHandlerCircular &&
+ m_options.handler != eLogHandlerStream) &&
+m_options.buffer_size.GetCurrentValue() != 0) {
+  result.AppendError("a buffer size can only be specified for the circular 
"
+ "buffer handler.\n");





Comment at: lldb/test/Shell/Log/TestHandlers.test:5
+# UNSUPPORTED-FILE: a file name can only be specified for the stream handler
+# UNSUPPORTED-BUFFER:  a buffer size can only be specified for the circular 
buffer handler




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

https://reviews.llvm.org/D143623

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

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, clayborg, mib, bulbazord.
Herald added a project: All.
JDevlieghere requested review of this revision.

Print an error for unsupported combinations of log handlers and log options. 
Only the stream log handler takes a file and only the circular and stream 
handler take a buffer size. This cannot be dealt with through option groups 
because the option combinations depend on the requested handler.


https://reviews.llvm.org/D143623

Files:
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/test/Shell/Log/TestHandlers.test


Index: lldb/test/Shell/Log/TestHandlers.test
===
--- /dev/null
+++ lldb/test/Shell/Log/TestHandlers.test
@@ -0,0 +1,5 @@
+# RUN: %lldb -o 'log enable -h os -f /tmp/foo  gdb-remote packets' 2>&1 | 
FileCheck %s  --check-prefix UNSUPPORTED-FILE
+# RUN: %lldb -o 'log enable -h os -b 10  gdb-remote packets' 2>&1 | FileCheck 
%s  --check-prefix UNSUPPORTED-BUFFER
+
+# UNSUPPORTED-FILE: a file name can only be specified for the stream handler
+# UNSUPPORTED-BUFFER:  a buffer size can only be specified for the circular 
buffer handler
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -177,6 +177,20 @@
   return false;
 }
 
+if ((m_options.handler != eLogHandlerCircular &&
+ m_options.handler != eLogHandlerStream) &&
+m_options.buffer_size.GetCurrentValue() != 0) {
+  result.AppendError("a buffer size can only be specified for the circular 
"
+ "buffer handler.\n");
+  return false;
+}
+
+if (m_options.handler != eLogHandlerStream && m_options.log_file) {
+  result.AppendError(
+  "a file name can only be specified for the stream handler.\n");
+  return false;
+}
+
 // Store into a std::string since we're about to shift the channel off.
 const std::string channel = std::string(args[0].ref());
 args.Shift(); // Shift off the channel


Index: lldb/test/Shell/Log/TestHandlers.test
===
--- /dev/null
+++ lldb/test/Shell/Log/TestHandlers.test
@@ -0,0 +1,5 @@
+# RUN: %lldb -o 'log enable -h os -f /tmp/foo  gdb-remote packets' 2>&1 | FileCheck %s  --check-prefix UNSUPPORTED-FILE
+# RUN: %lldb -o 'log enable -h os -b 10  gdb-remote packets' 2>&1 | FileCheck %s  --check-prefix UNSUPPORTED-BUFFER
+
+# UNSUPPORTED-FILE: a file name can only be specified for the stream handler
+# UNSUPPORTED-BUFFER:  a buffer size can only be specified for the circular buffer handler
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -177,6 +177,20 @@
   return false;
 }
 
+if ((m_options.handler != eLogHandlerCircular &&
+ m_options.handler != eLogHandlerStream) &&
+m_options.buffer_size.GetCurrentValue() != 0) {
+  result.AppendError("a buffer size can only be specified for the circular "
+ "buffer handler.\n");
+  return false;
+}
+
+if (m_options.handler != eLogHandlerStream && m_options.log_file) {
+  result.AppendError(
+  "a file name can only be specified for the stream handler.\n");
+  return false;
+}
+
 // Store into a std::string since we're about to shift the channel off.
 const std::string channel = std::string(args[0].ref());
 args.Shift(); // Shift off the channel
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd160873c03ae: [lldb] Add --gdb-format flag to dwim-print 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141425

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.h
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -27,22 +27,36 @@
 before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
 return re.escape(before) + r"\$\d+" + re.escape(after)
 
-def _expect_cmd(self, expr: str, base_cmd: str) -> None:
+def _expect_cmd(
+self,
+dwim_cmd: str,
+actual_cmd: str,
+) -> None:
 """Run dwim-print and verify the output against the expected command."""
-cmd = f"{base_cmd} {expr}"
-cmd_output = self._run_cmd(cmd)
+# Resolve the dwim-print command to either `expression` or `frame variable`.
+substitute_cmd = dwim_cmd.replace("dwim-print", actual_cmd, 1)
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+interp.ResolveCommand(substitute_cmd, result)
+self.assertTrue(result.Succeeded(), result.GetError())
+
+resolved_cmd = result.GetOutput()
+if actual_cmd == "frame variable":
+resolved_cmd = resolved_cmd.replace(" -- ", " ", 1)
+
+expected_output = self._run_cmd(resolved_cmd)
 
 # Verify dwim-print chose the expected command.
 self.runCmd("settings set dwim-print-verbosity full")
-substrs = [f"note: ran `{cmd}`"]
+substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if base_cmd == "expression --" and self.PERSISTENT_VAR.search(cmd_output):
-patterns.append(self._mask_persistent_var(cmd_output))
+if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+patterns.append(self._mask_persistent_var(expected_output))
 else:
-substrs.append(cmd_output)
+substrs.append(expected_output)
 
-self.expect(f"dwim-print {expr}", substrs=substrs, patterns=patterns)
+self.expect(dwim_cmd, substrs=substrs, patterns=patterns)
 
 def test_variables(self):
 """Test dwim-print with variables."""
@@ -50,7 +64,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 vars = ("argc", "argv")
 for var in vars:
-self._expect_cmd(var, "frame variable")
+self._expect_cmd(f"dwim-print {var}", "frame variable")
 
 def test_variable_paths(self):
 """Test dwim-print with variable path expressions."""
@@ -58,7 +72,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("", "*argv", "argv[0]")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_expressions(self):
 """Test dwim-print with expressions."""
@@ -66,8 +80,26 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("argc + 1", "(void)argc", "(int)abs(argc)")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_dummy_target_expressions(self):
 """Test dwim-print's ability to evaluate expressions without a target."""
-self._expect_cmd("1 + 2", "expression --")
+self._expect_cmd("dwim-print 1 + 2", "expression")
+
+def test_gdb_format(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print/x argc", "frame variable")
+self._expect_cmd(f"dwim-print/x argc + 1", "expression")
+
+def test_format_flags(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -fx -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -fx -- argc + 1", "expression")
+
+def test_display_flags(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -T -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -T -- argc + 1", "expression")
Index: lldb/source/Commands/CommandObjectDWIMPrint.h
===
--- lldb/source/Commands/CommandObjectDWIMPrint.h
+++ lldb/source/Commands/CommandObjectDWIMPrint.h
@@ -10,6 +10,9 @@
 #define LLDB_SOURCE_COMMANDS_COMMANDOBJECTDWIMPRINT_H
 
 #include "lldb/Interpreter/CommandObject.h"
+#include 

[Lldb-commits] [lldb] d160873 - [lldb] Add --gdb-format flag to dwim-print

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-02-08T19:16:20-08:00
New Revision: d160873c03aedfcd201851829aa423cc10ef593a

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

LOG: [lldb] Add --gdb-format flag to dwim-print

Add support for the `--gdb-format`/`-G` flag to `dwim-print`.

The gdb-format flag allows users to alias `p` to `dwim-print`.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/source/Commands/CommandObjectDWIMPrint.h
lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index e15e723de5880..81da150aa2ddd 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -9,13 +9,19 @@
 #include "CommandObjectDWIMPrint.h"
 
 #include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Interpreter/OptionGroupFormat.h"
+#include "lldb/Interpreter/OptionGroupValueObjectDisplay.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
 
 using namespace llvm;
 using namespace lldb;
@@ -26,28 +32,49 @@ 
CommandObjectDWIMPrint::CommandObjectDWIMPrint(CommandInterpreter )
"Print a variable or expression.",
"dwim-print [ | ]",
eCommandProcessMustBePaused | eCommandTryTargetAPILock) 
{
+  m_option_group.Append(_format_options,
+OptionGroupFormat::OPTION_GROUP_FORMAT |
+OptionGroupFormat::OPTION_GROUP_GDB_FMT,
+LLDB_OPT_SET_1);
+  m_option_group.Append(_varobj_options, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
+  m_option_group.Finalize();
 }
 
-bool CommandObjectDWIMPrint::DoExecute(StringRef expr,
+Options *CommandObjectDWIMPrint::GetOptions() { return _option_group; }
+
+bool CommandObjectDWIMPrint::DoExecute(StringRef command,
CommandReturnObject ) {
-  // Ignore leading and trailing whitespace.
-  expr = expr.trim();
+  m_option_group.NotifyOptionParsingStarting(_exe_ctx);
+  OptionsWithRaw args{command};
+  StringRef expr = args.GetRawPart();
 
-  if (expr.empty()) {
+  if (args.HasArgs()) {
+if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group,
+   m_exe_ctx))
+  return false;
+  } else if (command.empty()) {
 result.AppendErrorWithFormatv("'{0}' takes a variable or expression",
   m_cmd_name);
 return false;
   }
-
   auto verbosity = GetDebugger().GetDWIMPrintVerbosity();
 
+  DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
+  eLanguageRuntimeDescriptionDisplayVerbosityFull,
+  m_format_options.GetFormat());
+
   // First, try `expr` as the name of a frame variable.
   if (StackFrame *frame = m_exe_ctx.GetFramePtr()) {
 auto valobj_sp = frame->FindVariable(ConstString(expr));
 if (valobj_sp && valobj_sp->GetError().Success()) {
-  if (verbosity == eDWIMPrintVerbosityFull)
-result.AppendMessageWithFormatv("note: ran `frame variable {0}`", 
expr);
-  valobj_sp->Dump(result.GetOutputStream());
+  if (verbosity == eDWIMPrintVerbosityFull) {
+StringRef flags;
+if (args.HasArgs())
+  flags = args.GetArgString();
+result.AppendMessageWithFormatv("note: ran `frame variable {0}{1}`",
+flags, expr);
+  }
+  valobj_sp->Dump(result.GetOutputStream(), dump_options);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return true;
 }
@@ -63,9 +90,14 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef expr,
 ValueObjectSP valobj_sp;
 if (target.EvaluateExpression(expr, exe_scope, valobj_sp) ==
 eExpressionCompleted) {
-  if (verbosity != eDWIMPrintVerbosityNone)
-result.AppendMessageWithFormatv("note: ran `expression -- {0}`", expr);
-  valobj_sp->Dump(result.GetOutputStream());
+  if (verbosity != eDWIMPrintVerbosityNone) {
+StringRef flags;
+if (args.HasArgs())
+  flags = args.GetArgStringWithDelimiter();
+result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
+expr);
+  }
+  

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

[Adrian Vogelsgesang via Phabricator via lldb-commits]

avogelsgesang added a comment.

@labath @aprantl Thanks for your reviews! I would like to backport this fix 
together with the fix from https://reviews.llvm.org/D132815 to the 16.0 release 
branch now. For that I would need your approval on 
https://github.com/llvm/llvm-project-release-prs/pull/254


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143127

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

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141425

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

[Lldb-commits] [PATCH] D143215: lldb can know architecture-dependent watchpoint behaviors, instead of depending on the remote stub

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 495976.
jasonmolenda retitled this revision from "Separate 
Process::GetWatchpointSupportInfo into two methods to get the two separate 
pieces of information" to "lldb can know architecture-dependent watchpoint 
behaviors, instead of depending on the remote stub".
jasonmolenda edited the summary of this revision.
jasonmolenda added a comment.
Herald added subscribers: pcwang-thead, s.egerton, simoncook.

Update to fix David's second round of comments, rewrite the Title/Summary to 
more accurately reflect what I'm doing in this patch.  I think we're about done 
here, but open to any comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/API/SBProcess.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
  lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -790,19 +790,18 @@
 }
 
 static bool CheckIfWatchpointsSupported(Target *target, Status ) {
-  uint32_t num_supported_hardware_watchpoints;
-  Status rc = target->GetProcessSP()->GetWatchpointSupportInfo(
-  num_supported_hardware_watchpoints);
+  std::optional num_supported_hardware_watchpoints =
+  target->GetProcessSP()->GetWatchpointSlotCount();
 
   // If unable to determine the # of watchpoints available,
   // assume they are supported.
-  if (rc.Fail())
+  if (!num_supported_hardware_watchpoints)
 return true;
 
   if (num_supported_hardware_watchpoints == 0) {
 error.SetErrorStringWithFormat(
 "Target supports (%u) hardware watchpoint slots.\n",
-num_supported_hardware_watchpoints);
+*num_supported_hardware_watchpoints);
 return false;
   }
   return true;
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -823,16 +823,8 @@
 // stop
 
 ProcessSP process_sp = exe_ctx.GetProcessSP();
-uint32_t num;
-bool wp_triggers_after;
+bool wp_triggers_after = process_sp->GetWatchpointReportedAfter();
 
-if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
-.Success()) {
-  m_should_stop_is_valid = true;
-  m_should_stop = true;
-  return m_should_stop;
-}
-
 if (!wp_triggers_after) {
   // We have to step over the watchpoint before we know what to do:   
   StopInfoWatchpointSP me_as_siwp_sp 
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2355,6 +2355,24 @@
   return error;
 }
 
+bool Process::GetWatchpointReportedAfter() {
+  if (std::optional subclass_override = DoGetWatchpointReportedAfter())
+return *subclass_override;
+
+  bool reported_after = true;
+  const ArchSpec  = GetTarget().GetArchitecture();
+  if (!arch.IsValid())
+return reported_after;
+  llvm::Triple triple = arch.GetTriple();
+
+  if (triple.isMIPS() || triple.isPPC64() || triple.isRISCV())
+reported_after = false;
+  if (triple.isAArch64() || triple.isArmMClass() || triple.isARM())
+reported_after = false;
+
+  return reported_after;
+}
+
 ModuleSP Process::ReadModuleFromMemory(const FileSpec _spec,
lldb::addr_t header_addr,
size_t size_to_read) {
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -159,7 +159,7 @@
 
   Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
 
-  Status GetWatchpointSupportInfo(uint32_t ) override;
+  std::optional GetWatchpointSlotCount() override;
 
   llvm::Expected TraceSupported() override;
 
@@ -172,7 +172,7 @@
   llvm::Expected>
   TraceGetBinaryData(const TraceGetBinaryDataRequest ) override;
 
-  Status GetWatchpointSupportInfo(uint32_t , bool ) override;
+  std::optional DoGetWatchpointReportedAfter() override;
 
   bool StartNoticingNewThreads() override;
 
Index: 

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

Not sure how useful it would be but I recorded the full list of methods get 
added with this change. Take a look and let me know if there are any that you 
think shouldn't be added.

  SBAttachInfo::~SBAttachInfo()
  SBBroadcaster::operator<(const lldb::SBBroadcaster &) const;
  
SBCommandInterpreter::SourceInitFileInHomeDirectory(lldb::SBCommandReturnObject 
&, bool);
  SBCommandInterpreterRunOptions::SBCommandInterpreterRunOptions(const 
SBCommandInterpreterRunOptions &);
  SBCommandInterpreterRunOptions::GetEchoCOmmentCommands() const;
  SBCommandInterpreterRunOptions::SetEchoCommentCommands(bool);
  SBCommandInterpreterRunOptions::GetAutoHandleEvents() const;
  SBCommandInterpreterRunOptions::SetAutoHandleEvents(bool);
  SBCommandInterpreterRunOptions::GetSpawnThread() const;
  SBCommandInterpreterRunOptions::SetSpawnThread(bool);
  SBDebugger::GetBroadcasterClass();
  SBDebugger::PrintDiagnosticsOnError();
  SBDebugger::SkipAppInitFiles(bool);
  SBDebugger::SaveInputTerminalState();
  SBDebugger::RestoreInputTerminalState();
  SBDebugger::SetUseSourceCache(bool);
  SBDebugger::GetUseSourceCache() const;
  SBDebugger::GetREPLLanguage() const;
  SBDebugger::SetREPLLanguage(lldb::LanguageType);
  SBEvent::GetDescription)(lldb::SBStream &);
  SBLaunchInfo::~SBLaunchInfo();
  SBLaunchInfo::SBLaunchInfo(const SBLaunchInfo &);
  SBModule::GetUUIDBytes() const;
  SBPlatform::SBPlatform(const SBPlatform &);
  SBProcess::Detach(bool);
  SBProcess::GetStopEventForStopID(uint32_t);
  SBProcess::GetBroadcasterClass();
  SBProcess::LoadImage(const lldb::SBFileSpec &, const lldb::SBFileSpec &, 
lldb::SBError &);
  SBQueue::SBQueue(const SBQueue &);
  SBReproducer::Capture();
  SBReproducer::Replay(const char *);
  SBReproducer::Replay(const char *, bool);
  SBReproducer::Finalize(const char *);
  SBReproducer::GetPath();
  SBReproducer::Generate();
  SBSourceManager::SBSourceManager(const SBDebugger &);
  SBSourceManager::SBSourceManager(const SBTarget &);
  SBStringList::GetStringAtIndex(size_t) const;
  SBTarget::BreakpointCreateByName(const char *, const SBFileSpecList &, const 
SBFileSpecList &);
  SBTarget::BreakpointCreateByRegex(const char *, const SBFileSpecList &, const 
SBFileSpecList &);
  SBThreadPlan::SBThreadPlan(lldb::SBThread &, const char *, 
lldb::SBStructuredData &);
  SBThreadPlan::QueueThreadPlanForStepOverRange(SBAddress &, lldb::addr_t, 
SBError &);
  SBThreadPlan::QueueThreadPlanForStepInRange(SBAddress &, lldb::addr_t, 
SBError &);
  SBThreadPlan::QueueThreadPlanForStepOut(uint32_t, bool, SBError &);
  SBThreadPlan::QueueThreadPlanForRunToAddress(SBAddress, SBError &);
  SBTrace::LoadTraceFromFile(SBError &, SBDebugger &, const SBFileSpec &);
  SBTypeMember::GetDescription(lldb::SBStream &, lldb::DescriptionLevel);
  SBType::GetDescription(lldb::SBStream &, lldb::DescriptionLevel);
  SBTypeList::SBTypeList(const lldb::SBTypeList &);
  SBTypeCategory::operator==(lldb::SBTypeCategory &);
  SBTypeCategory::operator!=(lldb::SBTypeCategory &);
  SBTypeSummary::DoesPrintValue(lldb::SBValue);
  SBTypeSynthetic::IsClassName();
  SBValue::Watch(bool, bool, bool);
  SBWatchpoint::Clear();


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D142926#4113600 , @clayborg wrote:

> In D142926#4111717 , @bulbazord 
> wrote:
>
>> Certainly interesting:
>>
>> - SBListener::StopListeningForEventClass return type conflicts (ABI break?)
>
> No, probably a copy paste error when originally checked into .i file?

Likely. This method is already tested in TestListener.py. Previously it returns 
an uint32_t (which turns into a python integer), now it returns a bool.

>> - SBProcess::GetQueueAtIndex parameter types conflict (ABI break?)
>
> No, probably a copy paste error when originally checked into .i file?

Almost certainly. This method is well tested, I don't have many concerns here.

>> - SBTarget::BreakpointCreateByNames first parameter is different: `const 
>> char **symbol_name` vs `const char *symbol_name[]`. I'm not sure if this is 
>> going to be an issue.
>
> Maybe SWIG didn't like the "const char **" or it didn't do the right thing? 
> Test and verify if we switch to using the header file we have no issues 
> please?

I'm not 100% sure why there was some divergence. For now I've done some 
`ifdef/else/endif` to separate this. I don't think there's a problem if we pick 
one over the other but this method is completely untested so I am punting it a 
bit. :)

In D142926#4113615 , @clayborg wrote:

> In D142926#4111717 , @bulbazord 
> wrote:
>
>> Potentially interesting:
>>
>> - SBData::GetDescription base_addr parameter has default value now
>> - SBInstructionList::GetInstructionsCount canSetBreakpoint has default value 
>> now
>> - SBMemoryRegionInfo::SBMemoryRegionInfo 3rd constructor parameter 
>> stack_memory has default value now
>
> Make sure it does work. I seem to remember people sometimes making two 
> functions in the .i file for default parameters. Can't remember if SWIG does 
> the right thing with default params or not, but please test to ensure both 
> work (with and without).

I went through all 3 of them myself. Their result of having a default argument 
now is that there is a version of the function exposed that has 1 less 
argument. For example, `SBInstructionList::GetInstructionsCount` has 3 
arguments where the last one now has a default value. You can still do 
`GetInstructionsCount(arg1, arg2, arg3)` where arg3 can be `True` or `False`. 
This is the same behavior as before. The new thing that got added was 
`GetInstructionsCount(arg1, arg2)` where arg3 will default to `False` without 
you specifying it. This is true for `SBData::GetDescription` and 
`SBMemoryRegionInfo`'s 3rd constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 495978.
bulbazord added a comment.

- Removed a bunch of header guards effectively adding methods to the python 
bindings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

Files:
  lldb/bindings/CMakeLists.txt
  lldb/bindings/interface/SBAddress.i
  lldb/bindings/interface/SBAddressDocstrings.i
  lldb/bindings/interface/SBAddressExtensions.i
  lldb/bindings/interface/SBAttachInfo.i
  lldb/bindings/interface/SBAttachInfoDocstrings.i
  lldb/bindings/interface/SBBlock.i
  lldb/bindings/interface/SBBlockDocstrings.i
  lldb/bindings/interface/SBBlockExtensions.i
  lldb/bindings/interface/SBBreakpoint.i
  lldb/bindings/interface/SBBreakpointDocstrings.i
  lldb/bindings/interface/SBBreakpointExtensions.i
  lldb/bindings/interface/SBBreakpointLocation.i
  lldb/bindings/interface/SBBreakpointLocationDocstrings.i
  lldb/bindings/interface/SBBreakpointLocationExtensions.i
  lldb/bindings/interface/SBBreakpointName.i
  lldb/bindings/interface/SBBreakpointNameDocstrings.i
  lldb/bindings/interface/SBBreakpointNameExtensions.i
  lldb/bindings/interface/SBBroadcaster.i
  lldb/bindings/interface/SBBroadcasterDocstrings.i
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/bindings/interface/SBCommandInterpreterDocstrings.i
  lldb/bindings/interface/SBCommandInterpreterRunOptions.i
  lldb/bindings/interface/SBCommandInterpreterRunOptionsDocstrings.i
  lldb/bindings/interface/SBCommandReturnObject.i
  lldb/bindings/interface/SBCommandReturnObjectDocstrings.i
  lldb/bindings/interface/SBCommandReturnObjectExtensions.i
  lldb/bindings/interface/SBCommunication.i
  lldb/bindings/interface/SBCommunicationDocstrings.i
  lldb/bindings/interface/SBCompileUnit.i
  lldb/bindings/interface/SBCompileUnitDocstrings.i
  lldb/bindings/interface/SBCompileUnitExtensions.i
  lldb/bindings/interface/SBData.i
  lldb/bindings/interface/SBDataDocstrings.i
  lldb/bindings/interface/SBDataExtensions.i
  lldb/bindings/interface/SBDebugger.i
  lldb/bindings/interface/SBDebuggerDocstrings.i
  lldb/bindings/interface/SBDebuggerExtensions.i
  lldb/bindings/interface/SBDeclaration.i
  lldb/bindings/interface/SBDeclarationDocstrings.i
  lldb/bindings/interface/SBDeclarationExtensions.i
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBEnvironmentDocstrings.i
  lldb/bindings/interface/SBError.i
  lldb/bindings/interface/SBErrorDocstrings.i
  lldb/bindings/interface/SBErrorExtensions.i
  lldb/bindings/interface/SBEvent.i
  lldb/bindings/interface/SBEventDocstrings.i
  lldb/bindings/interface/SBExecutionContext.i
  lldb/bindings/interface/SBExecutionContextDocstrings.i
  lldb/bindings/interface/SBExecutionContextExtensions.i
  lldb/bindings/interface/SBExpressionOptions.i
  lldb/bindings/interface/SBExpressionOptionsDocstrings.i
  lldb/bindings/interface/SBFile.i
  lldb/bindings/interface/SBFileDocstrings.i
  lldb/bindings/interface/SBFileExtensions.i
  lldb/bindings/interface/SBFileSpec.i
  lldb/bindings/interface/SBFileSpecDocstrings.i
  lldb/bindings/interface/SBFileSpecExtensions.i
  lldb/bindings/interface/SBFileSpecList.i
  lldb/bindings/interface/SBFileSpecListDocstrings.i
  lldb/bindings/interface/SBFrame.i
  lldb/bindings/interface/SBFrameDocstrings.i
  lldb/bindings/interface/SBFrameExtensions.i
  lldb/bindings/interface/SBFunction.i
  lldb/bindings/interface/SBFunctionDocstrings.i
  lldb/bindings/interface/SBFunctionExtensions.i
  lldb/bindings/interface/SBHostOS.i
  lldb/bindings/interface/SBHostOSDocstrings.i
  lldb/bindings/interface/SBInstruction.i
  lldb/bindings/interface/SBInstructionDocstrings.i
  lldb/bindings/interface/SBInstructionExtensions.i
  lldb/bindings/interface/SBInstructionList.i
  lldb/bindings/interface/SBInstructionListDocstrings.i
  lldb/bindings/interface/SBInstructionListExtensions.i
  lldb/bindings/interface/SBLanguageRuntime.i
  lldb/bindings/interface/SBLanguageRuntimeDocstrings.i
  lldb/bindings/interface/SBLaunchInfo.i
  lldb/bindings/interface/SBLaunchInfoDocstrings.i
  lldb/bindings/interface/SBLineEntry.i
  lldb/bindings/interface/SBLineEntryDocstrings.i
  lldb/bindings/interface/SBLineEntryExtensions.i
  lldb/bindings/interface/SBListener.i
  lldb/bindings/interface/SBListenerDocstrings.i
  lldb/bindings/interface/SBMemoryRegionInfo.i
  lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i
  lldb/bindings/interface/SBMemoryRegionInfoExtensions.i
  lldb/bindings/interface/SBMemoryRegionInfoList.i
  lldb/bindings/interface/SBMemoryRegionInfoListDocstrings.i
  lldb/bindings/interface/SBModule.i
  lldb/bindings/interface/SBModuleDocstrings.i
  lldb/bindings/interface/SBModuleExtensions.i
  lldb/bindings/interface/SBModuleSpec.i
  lldb/bindings/interface/SBModuleSpecDocstrings.i
  lldb/bindings/interface/SBModuleSpecExtensions.i
  lldb/bindings/interface/SBPlatform.i
  lldb/bindings/interface/SBPlatformDocstrings.i
  lldb/bindings/interface/SBProcess.i
  

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/source/Target/Process.cpp:2373
+reported_after = false;
+  return reported_after;
+}

Emmmer wrote:
> DavidSpickett wrote:
> > DavidSpickett wrote:
> > > Would this be any clearer with multiple returns? Or one giant return, but 
> > > the logic is a bit cryptic then.
> > > 
> > > ```
> > >   const ArchSpec  = GetTarget().GetArchitecture();
> > >   if (!arch.IsValid())
> > > return true;
> > > 
> > >   llvm::Triple triple = arch.GetTriple();
> > >   return !(triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || 
> > > triple.isArmMClass() || triple.isARM());
> > > ```
> > > 
> > > Better as:
> > > ```
> > >   const ArchSpec  = GetTarget().GetArchitecture();
> > >   if (!arch.IsValid())
> > > return false;
> > > 
> > >   llvm::Triple triple = arch.GetTriple();
> > >   if (triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || 
> > > triple.isArmMClass() || triple.isARM())
> > > return false;
> > > 
> > >   return true;
> > > ```
> > > 
> > > Also, do we know what RISC-V does?
> > @Emmmer any idea?
> It seems standard RISC-V uses the `before` mode, that is 
> > The action for this trigger will be taken just before the instruction that 
> > triggered it is committed, but after all preceding instructions are 
> > committed.
> 
> If I understand this code correctly (correct me if I was wrong), we should 
> return `false` for RISC-V. 
> 
> But in the debug-spec [1], RISC-V seems to allow both `before` and `after` 
> modes, which can be detected through CSRs. When debug-spec lands, we may 
> change the code accordingly.
> 
> [1]: 
> https://github.com/riscv/riscv-debug-spec/blob/6309972901417a0fa72d812d2ffe89e432f00dff/xml/hwbp_registers.xml#L365
Thanks!  I'll add isRISCV() to the architectures we default to "watchpoints 
received before instruction executes".  This can be overridden by a gdb remote 
stub which adds the lldb-extension key to `qHostInfo` response.  although it 
sounds like it could technically be per *process*??? in which case the key 
should really be sent in `qProcessInfo`.  The remote stub should be able to 
send `qHostInfo` before it has attached to any specific process.



Comment at: lldb/source/Target/StopInfo.cpp:833
-  m_should_stop = true;
-  return m_should_stop;
-}

DavidSpickett wrote:
> This is no longer needed because `GetWatchpointReportedAfter` will fall back 
> to the process' defaults if the GDB layer does not/can not provide a value, 
> correct?
Previously, this would fetch the number of watchpoints available (if the 
lldb-extended packet was provided) and it would fetch whether watchpoints are 
received before or after (if the qHostInfo includes the lldb-extended key), and 
you would get an error if the former was unavailable.  (I think the latter 
would default to "watchpoints are after").  This `m_should_stop = true` is the 
behavior when watchpoints are received after the instruction has executed; this 
is the crux of the bug I was trying to fix, where lldb would not instruction 
step over the instruction and re-add it, when the packet declaring the number 
of watchpoints was unimplemented.  



Comment at: lldb/source/Target/Target.cpp:804
 "Target supports (%u) hardware watchpoint slots.\n",
-num_supported_hardware_watchpoints);
+*num_supported_hardware_watchpoints);
 return false;

DavidSpickett wrote:
> This should just append "Target supports 0 hardware...".
> 
> In theory a compiler could figure that out but also it's a bit confusing 
> because it makes the reader wonder what value other than 0 is supposed to end 
> up here.
It is a bit confusing.  The layers that return the `optional` will 
return the actual number of hardware watchpoints available, including 0 if that 
is the correct value.  Or a `nullopt` to indicate that it could not be 
retrieved. So in this method, we got back the number of watchpoints available, 
and it was said to be 0.  lldb won't actually check against this number if you 
try to set one, so even if we got a bogus value here, you could try to set a 
watchpoint and see if it works.  
`SBProcess::GetNumSupportedHardwareWatchpoints()` takes an SBError out arg & 
returns a uint32_t.  It can return 0 to mean "could not get number of 
watchpoints" or "there are zero watchpoints" but the SBError tells you whether 
it was the failure case or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

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

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 495971.
mib marked an inline comment as done.
mib edited the summary of this revision.
mib added a comment.

Addressed @JDevlieghere comments.


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

https://reviews.llvm.org/D143104

Files:
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/Host/ProcessLaunchInfo.h
  lldb/include/lldb/Interpreter/ScriptedMetadata.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Target/Target.cpp
  lldb/source/Utility/ProcessInfo.cpp
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -137,8 +137,14 @@
 
 target_1 = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
 self.assertTrue(target_1, VALID_TARGET)
+
+# We still need to specify a PID when attaching even for scripted processes
+attach_info = lldb.SBAttachInfo(42)
+attach_info.SetProcessPluginName("ScriptedProcess")
+attach_info.SetScriptedProcessClassName("dummy_scripted_process.DummyScriptedProcess")
+
 error = lldb.SBError()
-process_1 = target_1.Launch(launch_info, error)
+process_1 = target_1.Attach(attach_info, error)
 self.assertTrue(process_1 and process_1.IsValid(), PROCESS_IS_VALID)
 self.assertEqual(process_1.GetProcessID(), 42)
 self.assertEqual(process_1.GetNumThreads(), 1)
Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Utility/ProcessInfo.h"
 
+#include "lldb/Interpreter/ScriptedMetadata.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
@@ -36,6 +37,7 @@
   m_gid = UINT32_MAX;
   m_arch.Clear();
   m_pid = LLDB_INVALID_PROCESS_ID;
+  m_scripted_metadata_sp.reset();
 }
 
 const char *ProcessInfo::GetName() const {
@@ -109,6 +111,10 @@
   }
 }
 
+bool ProcessInfo::IsScriptedProcess() const {
+  return m_scripted_metadata_sp && *m_scripted_metadata_sp;
+}
+
 void ProcessInstanceInfo::Dump(Stream , UserIDResolver ) const {
   if (m_pid != LLDB_INVALID_PROCESS_ID)
 s.Printf("pid = %" PRIu64 "\n", m_pid);
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3080,6 +3080,17 @@
 
 void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); }
 
+void Target::SaveScriptedLaunchInfo(lldb_private::ProcessInfo _info) {
+  if (process_info.IsScriptedProcess()) {
+// Only copy scripted process launch options.
+ProcessLaunchInfo _launch_info = const_cast(
+GetGlobalProperties().GetProcessLaunchInfo());
+default_launch_info.SetProcessPluginName("ScriptedProcess");
+default_launch_info.SetScriptedMetadata(process_info.GetScriptedMetadata());
+SetProcessLaunchInfo(default_launch_info);
+  }
+}
+
 Status Target::Launch(ProcessLaunchInfo _info, Stream *stream) {
   m_stats.SetLaunchOrAttachTime();
   Status error;
@@ -3109,19 +3120,7 @@
 
   launch_info.GetFlags().Set(eLaunchFlagDebug);
 
-  if (launch_info.IsScriptedProcess()) {
-// Only copy scripted process launch options.
-ProcessLaunchInfo _launch_info = const_cast(
-GetGlobalProperties().GetProcessLaunchInfo());
-
-default_launch_info.SetProcessPluginName("ScriptedProcess");
-default_launch_info.SetScriptedProcessClassName(
-launch_info.GetScriptedProcessClassName());
-default_launch_info.SetScriptedProcessDictionarySP(
-launch_info.GetScriptedProcessDictionarySP());
-
-SetProcessLaunchInfo(launch_info);
-  }
+  SaveScriptedLaunchInfo(launch_info);
 
   // Get the value of synchronous execution here.  If you wait till after you
   // have started to run, then you could have hit a breakpoint, whose command
@@ -3334,11 +,12 @@
 
   Status error;

[Lldb-commits] [PATCH] D143548: [lldb] Add the ability to remove diagnostic callbacks

[Alex Langford via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D143548

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo updated this revision to Diff 495952.
ayermolo marked 2 inline comments as done.
ayermolo added a comment.

addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
@@ -45,6 +45,26 @@
   EncodeDecode(DIERef(200, DIERef::Section::DebugTypes, 0x11223344));
 }
 
+TEST(DWARFIndexCachingTest, DIERefEncodeDecodeMax) {
+  // Tests DIERef::Encode(...) and DIERef::Decode(...)
+  EncodeDecode(DIERef(std::nullopt, DIERef::Section::DebugInfo,
+  DIERef::k_die_offset_mask - 1));
+  EncodeDecode(DIERef(std::nullopt, DIERef::Section::DebugTypes,
+  DIERef::k_die_offset_mask - 1));
+  EncodeDecode(
+  DIERef(100, DIERef::Section::DebugInfo, DIERef::k_die_offset_mask - 1));
+  EncodeDecode(
+  DIERef(200, DIERef::Section::DebugTypes, DIERef::k_die_offset_mask - 1));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugInfo,
+  DIERef::k_file_index_mask));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugTypes,
+  DIERef::k_file_index_mask));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugInfo,
+  0x11223344));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugTypes,
+  0x11223344));
+}
+
 static void EncodeDecode(const NameToDIE , ByteOrder byte_order) {
   const uint8_t addr_size = 8;
   DataEncoder encoder(byte_order, addr_size);
Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -713,7 +713,7 @@
   //   Entries:
   // - AbbrCode:0x1
   //   Values:
-  // - Value:   0x01020304
+  // - Value:   0x0120304
   // - AbbrCode:0x0
   const char *dwo_yamldata = R"(
 --- !ELF
@@ -750,7 +750,7 @@
   auto dwo_module_sp = std::make_shared(dwo_file->moduleSpec());
   SymbolFileDWARFDwo dwo_symfile(
   skeleton_symfile, dwo_module_sp->GetObjectFile()->shared_from_this(),
-  0x01020304);
+  0x0120304);
   auto *dwo_dwarf_unit = dwo_symfile.DebugInfo().GetUnitAtIndex(0);
 
   testExpressionVendorExtensions(dwo_module_sp, *dwo_dwarf_unit);
Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
@@ -4,9 +4,9 @@
 # RUN:   -o exit | FileCheck %s
 
 # CHECK-LABEL: image lookup -v -s lookup_rnglists
-# CHECK:  Function: id = {0x4028}, name = "rnglists", range = [0x-0x0003)
-# CHECK:Blocks: id = {0x4028}, range = [0x-0x0003)
-# CHECK-NEXT:   id = {0x4037}, range = [0x0001-0x0002)
+# CHECK:  Function: id = {0x2028}, name = "rnglists", range = [0x-0x0003)
+# CHECK:Blocks: id = {0x2028}, range = [0x-0x0003)
+# CHECK-NEXT:   id = {0x2037}, range = 

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo marked 16 inline comments as done.
ayermolo added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:32
   enum Section : uint8_t { DebugInfo, DebugTypes };
-
-  DIERef(std::optional dwo_num, Section section,
+  // Making constructors protected to limit where DIERef is used directly.
+  DIERef(std::optional file_index, Section section,

clayborg wrote:
> labath wrote:
> > they're not actually protected
> There were for a bit to control access to this, but in reality we ended up 
> friending too many classes and then ran into issues with the DIERefTest 
> stuff, so we decided to make them public again. We can remove this comment.
Oops, forgot to remove. For one of the internal revisions I experimented with 
making them protected.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:198
   "attach the file at the start of this error message",
-  m_offset, (unsigned)form);
+  (uint64_t)m_offset, (unsigned)form);
   *offset_ptr = m_offset;

clayborg wrote:
> Needed? Same as above
m_offset is is bit field now, so without it clang produces error.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1682
 SymbolFileDWARF::GetDIE(const DIERef _ref) {
-  if (die_ref.dwo_num()) {
-SymbolFileDWARF *dwarf = *die_ref.dwo_num() == 0x3fff
- ? m_dwp_symfile.get()
- : this->DebugInfo()
-   .GetUnitAtIndex(*die_ref.dwo_num())
-   ->GetDwoSymbolFile();
-return dwarf->DebugInfo().GetDIE(die_ref);
-  }
-
-  return DebugInfo().GetDIE(die_ref);
+  return GetDIE(die_ref.get_id());
 }

clayborg wrote:
> labath wrote:
> > clayborg wrote:
> > > Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be 
> > > the one source of truth when finding a DIE. We could make 
> > > "SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then 
> > > have "SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and 
> > > then call "SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.
> > +1
> Ok. So lets do this - change "DWARFDIE 
> SymbolFileDWARF::GetDIE(lldb::user_id_t uid)" to just be:
> 
> ```
> DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t uid) {
>   return GetDIE(DIERef(uid));
> }
> ```
> And then change the current "DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t 
> uid)" to be the one that does all of the work:
> 
> ```
> DWARFDIE SymbolFileDWARF::GetDIE(DIERef die_ref) {
>   std::optional file_index = die_ref.file_index();
>   if (file_index) {
> if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
>   symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO 
> case
> else if (*file_index == DIERef::k_file_index_mask)
>   symbol_file = m_dwp_symfile.get(); // DWP case
> else
>   symbol_file = this->DebugInfo()
> .GetUnitAtIndex(*die_ref.file_index())
> ->GetDwoSymbolFile(); // DWO case
>   } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
> symbol_file = nullptr;
>   } else {
> symbol_file = this;
>   }
> 
>   if (symbol_file)
> return symbol_file->GetDIE(die_ref);
> 
>   return DWARFDIE();
> }
> ```
> 
ah, yes, great suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 495939.
mib edited the summary of this revision.

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

https://reviews.llvm.org/D141658

Files:
  lldb/examples/python/crashlog.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
@@ -1,7 +1,7 @@
 # RUN: %clang_host -g %S/Inputs/test.c -o %t.out
 # RUN: cp %S/Inputs/a.out.crash %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}'
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode batch %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -3,7 +3,7 @@
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -b -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -s -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a -s --mode interactive -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o 'command source -s 0 %s' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
@@ -3,7 +3,7 @@
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a --mode interactive -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o "thread list" -o "bt all" 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
@@ -2,7 +2,7 @@
 
 # RUN: cp %S/Inputs/no_threadState.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode batch %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -2,11 +2,11 @@
 
 # RUN: cp %S/Inputs/a.out.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode batch %t.crash' 2>&1 | FileCheck %s
 
 # RUN: 

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 495938.
mib added a comment.

Address @JDevlieghere feedback: rename `legacy` loading mode into `batch`.


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

https://reviews.llvm.org/D141658

Files:
  lldb/examples/python/crashlog.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
@@ -1,7 +1,7 @@
 # RUN: %clang_host -g %S/Inputs/test.c -o %t.out
 # RUN: cp %S/Inputs/a.out.crash %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}'
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode legacy %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -3,7 +3,7 @@
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -b -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -s -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a -s --mode interactive -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o 'command source -s 0 %s' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
@@ -3,7 +3,7 @@
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a --mode interactive -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o "thread list" -o "bt all" 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
@@ -2,7 +2,7 @@
 
 # RUN: cp %S/Inputs/no_threadState.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog --mode batch %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -2,11 +2,11 @@
 
 # RUN: cp %S/Inputs/a.out.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 495933.
mib added a comment.

Fixed module loading issue.


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

https://reviews.llvm.org/D141702

Files:
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -19,12 +19,15 @@
 #include "lldb/Interpreter/OptionGroupBoolean.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Interpreter/ScriptedMetadata.h"
+#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Queue.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/State.h"
 
+#include "llvm/Support/ThreadPool.h"
+
 #include 
 
 LLDB_PLUGIN_DEFINE(ScriptedProcess)
@@ -383,19 +386,31 @@
 return ScriptedInterface::ErrorWithMessage(
 LLVM_PRETTY_FUNCTION, "No loaded images.", error);
 
+  size_t num_images_to_load = loaded_images_sp->GetSize();
   ModuleList module_list;
   Target  = GetTarget();
 
-  auto reload_image = [, _list, _with_message](
-  StructuredData::Object *obj) -> bool {
-StructuredData::Dictionary *dict = obj->GetAsDictionary();
-
+  StructuredData::DictionarySP capabilities_sp =
+  GetInterface().GetCapabilities();
+  if (!capabilities_sp)
+ScriptedInterface::ErrorWithMessage(
+LLVM_PRETTY_FUNCTION,
+"Couldn't fetch scripted process capabilities.\nContinuing loading "
+"dynamic libraries.",
+error);
+
+  bool force_lookup = false;
+  capabilities_sp->GetValueForKeyAsBoolean("force_symbol_lookup", force_lookup);
+
+  std::mutex m;
+  std::vector module_specs(num_images_to_load);
+  Progress progress("Fetching external dependencies", num_images_to_load);
+  auto fetch_symbols =
+  [_with_message, , _lookup, , _specs,
+   ](StructuredData::Dictionary *dict, size_t index) -> bool {
 if (!dict)
   return error_with_message("Couldn't cast image object into dictionary.");
 
-ModuleSpec module_spec;
-llvm::StringRef value;
-
 bool has_path = dict->HasKey("path");
 bool has_uuid = dict->HasKey("uuid");
 if (!has_path && !has_uuid)
@@ -403,6 +418,8 @@
 if (!dict->HasKey("load_addr"))
   return error_with_message("Dictionary is missing key 'load_addr'");
 
+ModuleSpec module_spec;
+llvm::StringRef value;
 if (has_path) {
   dict->GetValueForKeyAsString("path", value);
   module_spec.GetFileSpec().SetPath(value);
@@ -414,12 +431,43 @@
 }
 module_spec.GetArchitecture() = target.GetArchitecture();
 
+Status error;
+bool try_download_object_and_symbols =
+Symbols::DownloadObjectAndSymbolFile(module_spec, error, force_lookup);
+bool has_download_succeeded =
+FileSystem::Instance().Exists(module_spec.GetFileSpec());
+bool try_fetching_symbols = (try_download_object_and_symbols ||
+ error.Success() || has_download_succeeded);
+
+{
+  std::lock_guard lock(m);
+  // We need to increment progress and append the module spec to the vector
+  // even if symbol fetching failed.
+  progress.Increment();
+  module_specs[index] = module_spec;
+}
+
+if (!try_fetching_symbols)
+  return error_with_message(error.AsCString());
+
+return true;
+  };
+
+  auto load_modules = [, _list,
+   _with_message](StructuredData::Dictionary *dict,
+ModuleSpec _spec) -> bool {
+if (!dict)
+  return error_with_message("Structured data object is not a dictionary.");
+
 ModuleSP module_sp =
-target.GetOrCreateModule(module_spec, true /* notify */);
+target.GetOrCreateModule(module_spec, true /*=notify*/);
 
 if (!module_sp)
   return error_with_message("Couldn't create or get module.");
 
+Debugger::ReportSymbolChange(module_spec);
+
+llvm::StringRef value;
 lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
 lldb::addr_t slide = LLDB_INVALID_OFFSET;
 dict->GetValueForKeyAsInteger("load_addr", load_addr);
@@ -438,16 +486,30 @@
 if (!changed && !module_sp->GetObjectFile())
   return error_with_message("Couldn't set the load address for module.");
 
-dict->GetValueForKeyAsString("path", value);
-FileSpec objfile(value);
-module_sp->SetFileSpecAndObjectName(objfile, objfile.GetFilename());

[Lldb-commits] [PATCH] D142715: [LLDB] Apply FixCodeAddress to all forms of address arguments

[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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142715

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

[Lldb-commits] [PATCH] D143548: [lldb] Add the ability to remove diagnostic callbacks

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D143548#4113687 , @mib wrote:

> LGTM! Is there a simple way to test this ?

I considered a unit test, but that would require making everything protected 
and would basically boil down to testing the implementation of the underlying 
container, but I can add it if you think it's worthwhile. I think a better way 
to test this is through an integration test that exercises this code path. 
D143548  would indirectly test it (it doesn't 
crash but I assume ASAN would have caught it).


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

https://reviews.llvm.org/D143548

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

[Lldb-commits] [PATCH] D143548: [lldb] Add the ability to remove diagnostic callbacks

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

LGTM! Is there a simple way to test this ?




Comment at: lldb/source/Utility/Diagnostics.cpp:55-58
+  m_callbacks.erase(
+  std::remove_if(m_callbacks.begin(), m_callbacks.end(),
+ [id](const CallbackEntry ) { return e.id == id; }),
+  m_callbacks.end());

nit: may be use `llvm::remove_if` instead ?


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

https://reviews.llvm.org/D143548

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

[Lldb-commits] [lldb] 785009e - [lldb][test] Fix function references to function calls (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-02-08T12:12:33-08:00
New Revision: 785009e19fbcbeaca206f7e5a786cb034a374e19

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

LOG: [lldb][test] Fix function references to function calls (NFC)

Added: 


Modified: 
lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
lldb/test/API/functionalities/step_scripted/TestStepScripted.py

Removed: 




diff  --git a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py 
b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
index a2c9397f1a79b..ad188a32c0b97 100644
--- a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
+++ b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
@@ -83,7 +83,7 @@ def do_test_auto_continue(self, return_true):
   command = "target stop-hook add -G 1 -P stop_hook.stop_handler -k 
increment -v 5 -n step_out_of_me"
 
 self.interp.HandleCommand(command, result)
-self.assertTrue(result.Succeeded, "Set the target stop hook")
+self.assertTrue(result.Succeeded(), "Set the target stop hook")
 
 # First run to main.  If we go straight to the first stop hook hit,
 # run_to_source_breakpoint will fail because we aren't at original 
breakpoint
@@ -131,7 +131,7 @@ def stop_hooks_scripted(self, g_var_value, specifier = 
None):
 command += specifier
 
 self.interp.HandleCommand(command, result)
-self.assertTrue(result.Succeeded, "Set the target stop hook")
+self.assertTrue(result.Succeeded(), "Set the target stop hook")
 (target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
"Set a breakpoint here", 
self.main_source_file)
 # At this point we've hit our stop hook so we should have run our 
expression,

diff  --git a/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py 
b/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
index 38bb17423350b..da2781574577e 100644
--- a/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
+++ b/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
@@ -39,7 +39,7 @@ def step_out_test(self):
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 interp.HandleCommand("target stop-hook add -o 'expr g_var++'", result)
-self.assertTrue(result.Succeeded, "Set the target stop hook")
+self.assertTrue(result.Succeeded(), "Set the target stop hook")
 thread.StepOut()
 var = target.FindFirstGlobalVariable("g_var")
 self.assertTrue(var.IsValid())
@@ -49,7 +49,7 @@ def after_expr_test(self):
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 interp.HandleCommand("target stop-hook add -o 'expr g_var++'", result)
-self.assertTrue(result.Succeeded, "Set the target stop hook")
+self.assertTrue(result.Succeeded(), "Set the target stop hook")
 
 (target, process, thread, first_bkpt) = 
lldbutil.run_to_source_breakpoint(self,
"Set a breakpoint here", 
self.main_source_file)

diff  --git a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py 
b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
index 189c792edf5e9..b02c5ebfd90ca 100644
--- a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
+++ b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
@@ -148,6 +148,6 @@ def do_test_stop_others(self):
 result = lldb.SBCommandReturnObject()
 
 interp.HandleCommand("settings set target.process.run-all-threads 
true", result)
-self.assertTrue(result.Succeeded, "setting run-all-threads works.")
+self.assertTrue(result.Succeeded(), "setting run-all-threads works.")
 
 self.run_step(False, None, thread_id)



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

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D142926#4111717 , @bulbazord wrote:

> Potentially interesting:
>
> - SBData::GetDescription base_addr parameter has default value now
> - SBInstructionList::GetInstructionsCount canSetBreakpoint has default value 
> now
> - SBMemoryRegionInfo::SBMemoryRegionInfo 3rd constructor parameter 
> stack_memory has default value now

Make sure it does work. I seem to remember people sometimes making two 
functions in the .i file for default parameters. Can't remember if SWIG does 
the right thing with default params or not, but please test to ensure both work 
(with and without).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D142926#4111717 , @bulbazord wrote:

> In addition to what I wrote above, I also fixed several documentation bugs 
> (putting docstrings on the wrong method).
>
> I manually audited the generated code before and after this change. Here are 
> my notes:
>
> Not too interesting:
>
> - Some parameter names changed
> - Fixed lots of documentation bugs
> - Some documentation changed because of a re-ordering of function declarations
>
> Potentially interesting:
>
> - SBData::GetDescription base_addr parameter has default value now
> - SBInstructionList::GetInstructionsCount canSetBreakpoint has default value 
> now
> - SBMemoryRegionInfo::SBMemoryRegionInfo 3rd constructor parameter 
> stack_memory has default value now
>
> Certainly interesting:
>
> - SBListener::StopListeningForEventClass return type conflicts (ABI break?)

No, probably a copy paste error when originally checked into .i file?

> - SBProcess::GetQueueAtIndex parameter types conflict (ABI break?)

No, probably a copy paste error when originally checked into .i file?

> - SBTarget::BreakpointCreateByNames first parameter is different: `const char 
> **symbol_name` vs `const char *symbol_name[]`. I'm not sure if this is going 
> to be an issue.

Maybe SWIG didn't like the "const char **" or it didn't do the right thing? 
Test and verify if we switch to using the header file we have no issues please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:22-24
+/// - file_index: identifies the dwo file in the Module. If this field is not
+/// set,
+///   the DIERef references the main, dwo or .o file.





Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:32
   enum Section : uint8_t { DebugInfo, DebugTypes };
-
-  DIERef(std::optional dwo_num, Section section,
+  // Making constructors protected to limit where DIERef is used directly.
+  DIERef(std::optional file_index, Section section,

labath wrote:
> they're not actually protected
There were for a bit to control access to this, but in reality we ended up 
friending too many classes and then ran into issues with the DIERefTest stuff, 
so we decided to make them public again. We can remove this comment.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp:75
+  if (ref)
+return DIERef(*GetDWARF(), *ref).get_id();
+

labath wrote:
> Is this the only call site of the `DIERef(SymbolFileDWARF&)` constructor?
> 
> If so, and if we make it such that `DWARFBaseDIE::GetDIERef` returns the 
> fully filled in DIERef, then this function can just call get_id() on the 
> result, and we can delete that constructor.
This line doesn't make sense. If we got a valid DIERef back from GetDIERef(), 
then we just return that as it would have used the SymbolFileDWARF to fill 
everything in already. So we might not need that extra constructor if this is 
the only place as Pavel suggested.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1682
 SymbolFileDWARF::GetDIE(const DIERef _ref) {
-  if (die_ref.dwo_num()) {
-SymbolFileDWARF *dwarf = *die_ref.dwo_num() == 0x3fff
- ? m_dwp_symfile.get()
- : this->DebugInfo()
-   .GetUnitAtIndex(*die_ref.dwo_num())
-   ->GetDwoSymbolFile();
-return dwarf->DebugInfo().GetDIE(die_ref);
-  }
-
-  return DebugInfo().GetDIE(die_ref);
+  return GetDIE(die_ref.get_id());
 }

labath wrote:
> clayborg wrote:
> > Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be the 
> > one source of truth when finding a DIE. We could make 
> > "SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then 
> > have "SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and 
> > then call "SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.
> +1
Ok. So lets do this - change "DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t 
uid)" to just be:

```
DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t uid) {
  return GetDIE(DIERef(uid));
}
```
And then change the current "DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t 
uid)" to be the one that does all of the work:

```
DWARFDIE SymbolFileDWARF::GetDIE(DIERef die_ref) {
  std::optional file_index = die_ref.file_index();
  if (file_index) {
if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
  symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO case
else if (*file_index == DIERef::k_file_index_mask)
  symbol_file = m_dwp_symfile.get(); // DWP case
else
  symbol_file = this->DebugInfo()
.GetUnitAtIndex(*die_ref.file_index())
->GetDwoSymbolFile(); // DWO case
  } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
symbol_file = nullptr;
  } else {
symbol_file = this;
  }

  if (symbol_file)
return symbol_file->GetDIE(die_ref);

  return DWARFDIE();
}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D142715: [LLDB] Apply FixCodeAddress to all forms of address arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I left a few nits here and there but overall this LGTM




Comment at: lldb/include/lldb/Interpreter/OptionArgParser.h:19-20
 struct OptionArgParser {
+  // Try to parse an address. If it succeeds return the address with the
+  // non-address bits removed.
   static lldb::addr_t ToAddress(const ExecutionContext *exe_ctx,





Comment at: lldb/include/lldb/Interpreter/OptionArgParser.h:25
+
+  // As for ToAddress but do not remove non-address bits from the result.
+  static lldb::addr_t ToRawAddress(const ExecutionContext *exe_ctx,





Comment at: lldb/source/Interpreter/OptionArgParser.cpp:161-162
+
+  Process *process = exe_ctx->GetProcessPtr();
+  if (process)
+if (ABISP abi_sp = process->GetABI())




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142715

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

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 495899.
kastiglione added a comment.

replace raise with assertTrue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141425

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.h
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -27,22 +27,36 @@
 before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
 return re.escape(before) + r"\$\d+" + re.escape(after)
 
-def _expect_cmd(self, expr: str, base_cmd: str) -> None:
+def _expect_cmd(
+self,
+dwim_cmd: str,
+actual_cmd: str,
+) -> None:
 """Run dwim-print and verify the output against the expected command."""
-cmd = f"{base_cmd} {expr}"
-cmd_output = self._run_cmd(cmd)
+# Resolve the dwim-print command to either `expression` or `frame variable`.
+substitute_cmd = dwim_cmd.replace("dwim-print", actual_cmd, 1)
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+interp.ResolveCommand(substitute_cmd, result)
+self.assertTrue(result.Succeeded(), result.GetError())
+
+resolved_cmd = result.GetOutput()
+if actual_cmd == "frame variable":
+resolved_cmd = resolved_cmd.replace(" -- ", " ", 1)
+
+expected_output = self._run_cmd(resolved_cmd)
 
 # Verify dwim-print chose the expected command.
 self.runCmd("settings set dwim-print-verbosity full")
-substrs = [f"note: ran `{cmd}`"]
+substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if base_cmd == "expression --" and self.PERSISTENT_VAR.search(cmd_output):
-patterns.append(self._mask_persistent_var(cmd_output))
+if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+patterns.append(self._mask_persistent_var(expected_output))
 else:
-substrs.append(cmd_output)
+substrs.append(expected_output)
 
-self.expect(f"dwim-print {expr}", substrs=substrs, patterns=patterns)
+self.expect(dwim_cmd, substrs=substrs, patterns=patterns)
 
 def test_variables(self):
 """Test dwim-print with variables."""
@@ -50,7 +64,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 vars = ("argc", "argv")
 for var in vars:
-self._expect_cmd(var, "frame variable")
+self._expect_cmd(f"dwim-print {var}", "frame variable")
 
 def test_variable_paths(self):
 """Test dwim-print with variable path expressions."""
@@ -58,7 +72,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("", "*argv", "argv[0]")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_expressions(self):
 """Test dwim-print with expressions."""
@@ -66,8 +80,26 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("argc + 1", "(void)argc", "(int)abs(argc)")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_dummy_target_expressions(self):
 """Test dwim-print's ability to evaluate expressions without a target."""
-self._expect_cmd("1 + 2", "expression --")
+self._expect_cmd("dwim-print 1 + 2", "expression")
+
+def test_gdb_format(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print/x argc", "frame variable")
+self._expect_cmd(f"dwim-print/x argc + 1", "expression")
+
+def test_format_flags(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -fx -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -fx -- argc + 1", "expression")
+
+def test_display_flags(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -T -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -T -- argc + 1", "expression")
Index: lldb/source/Commands/CommandObjectDWIMPrint.h
===
--- lldb/source/Commands/CommandObjectDWIMPrint.h
+++ lldb/source/Commands/CommandObjectDWIMPrint.h
@@ -10,6 +10,9 @@
 #define LLDB_SOURCE_COMMANDS_COMMANDOBJECTDWIMPRINT_H
 
 #include "lldb/Interpreter/CommandObject.h"
+#include "lldb/Interpreter/OptionGroupFormat.h"
+#include 

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 495895.
kastiglione added a comment.

Add a test for display options too (-T)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141425

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.h
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -27,22 +27,37 @@
 before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
 return re.escape(before) + r"\$\d+" + re.escape(after)
 
-def _expect_cmd(self, expr: str, base_cmd: str) -> None:
+def _expect_cmd(
+self,
+dwim_cmd: str,
+actual_cmd: str,
+) -> None:
 """Run dwim-print and verify the output against the expected command."""
-cmd = f"{base_cmd} {expr}"
-cmd_output = self._run_cmd(cmd)
+# Resolve the dwim-print command to either `expression` or `frame variable`.
+substitute_cmd = dwim_cmd.replace("dwim-print", actual_cmd, 1)
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+interp.ResolveCommand(substitute_cmd, result)
+if not result.Succeeded():
+raise RuntimeError(result.GetError())
+
+resolved_cmd = result.GetOutput()
+if actual_cmd == "frame variable":
+resolved_cmd = resolved_cmd.replace(" -- ", " ", 1)
+
+expected_output = self._run_cmd(resolved_cmd)
 
 # Verify dwim-print chose the expected command.
 self.runCmd("settings set dwim-print-verbosity full")
-substrs = [f"note: ran `{cmd}`"]
+substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if base_cmd == "expression --" and self.PERSISTENT_VAR.search(cmd_output):
-patterns.append(self._mask_persistent_var(cmd_output))
+if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+patterns.append(self._mask_persistent_var(expected_output))
 else:
-substrs.append(cmd_output)
+substrs.append(expected_output)
 
-self.expect(f"dwim-print {expr}", substrs=substrs, patterns=patterns)
+self.expect(dwim_cmd, substrs=substrs, patterns=patterns)
 
 def test_variables(self):
 """Test dwim-print with variables."""
@@ -50,7 +65,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 vars = ("argc", "argv")
 for var in vars:
-self._expect_cmd(var, "frame variable")
+self._expect_cmd(f"dwim-print {var}", "frame variable")
 
 def test_variable_paths(self):
 """Test dwim-print with variable path expressions."""
@@ -58,7 +73,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("", "*argv", "argv[0]")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_expressions(self):
 """Test dwim-print with expressions."""
@@ -66,8 +81,26 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("argc + 1", "(void)argc", "(int)abs(argc)")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_dummy_target_expressions(self):
 """Test dwim-print's ability to evaluate expressions without a target."""
-self._expect_cmd("1 + 2", "expression --")
+self._expect_cmd("dwim-print 1 + 2", "expression")
+
+def test_gdb_format(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print/x argc", "frame variable")
+self._expect_cmd(f"dwim-print/x argc + 1", "expression")
+
+def test_format_flags(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -fx -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -fx -- argc + 1", "expression")
+
+def test_display_flags(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -T -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -T -- argc + 1", "expression")
Index: lldb/source/Commands/CommandObjectDWIMPrint.h
===
--- lldb/source/Commands/CommandObjectDWIMPrint.h
+++ lldb/source/Commands/CommandObjectDWIMPrint.h
@@ -10,6 +10,9 @@
 #define LLDB_SOURCE_COMMANDS_COMMANDOBJECTDWIMPRINT_H
 
 #include "lldb/Interpreter/CommandObject.h"
+#include "lldb/Interpreter/OptionGroupFormat.h"

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 495887.
kastiglione added a comment.

remove unnecessary header addition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141425

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.h
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -27,22 +27,37 @@
 before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
 return re.escape(before) + r"\$\d+" + re.escape(after)
 
-def _expect_cmd(self, expr: str, base_cmd: str) -> None:
+def _expect_cmd(
+self,
+dwim_cmd: str,
+actual_cmd: str,
+) -> None:
 """Run dwim-print and verify the output against the expected command."""
-cmd = f"{base_cmd} {expr}"
-cmd_output = self._run_cmd(cmd)
+# Resolve the dwim-print command to either `expression` or `frame variable`.
+substitute_cmd = dwim_cmd.replace("dwim-print", actual_cmd, 1)
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+interp.ResolveCommand(substitute_cmd, result)
+if not result.Succeeded():
+raise RuntimeError(result.GetError())
+
+resolved_cmd = result.GetOutput()
+if actual_cmd == "frame variable":
+resolved_cmd = resolved_cmd.replace(" -- ", " ", 1)
+
+expected_output = self._run_cmd(resolved_cmd)
 
 # Verify dwim-print chose the expected command.
 self.runCmd("settings set dwim-print-verbosity full")
-substrs = [f"note: ran `{cmd}`"]
+substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if base_cmd == "expression --" and self.PERSISTENT_VAR.search(cmd_output):
-patterns.append(self._mask_persistent_var(cmd_output))
+if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+patterns.append(self._mask_persistent_var(expected_output))
 else:
-substrs.append(cmd_output)
+substrs.append(expected_output)
 
-self.expect(f"dwim-print {expr}", substrs=substrs, patterns=patterns)
+self.expect(dwim_cmd, substrs=substrs, patterns=patterns)
 
 def test_variables(self):
 """Test dwim-print with variables."""
@@ -50,7 +65,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 vars = ("argc", "argv")
 for var in vars:
-self._expect_cmd(var, "frame variable")
+self._expect_cmd(f"dwim-print {var}", "frame variable")
 
 def test_variable_paths(self):
 """Test dwim-print with variable path expressions."""
@@ -58,7 +73,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("", "*argv", "argv[0]")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_expressions(self):
 """Test dwim-print with expressions."""
@@ -66,8 +81,20 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("argc + 1", "(void)argc", "(int)abs(argc)")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_dummy_target_expressions(self):
 """Test dwim-print's ability to evaluate expressions without a target."""
-self._expect_cmd("1 + 2", "expression --")
+self._expect_cmd("dwim-print 1 + 2", "expression")
+
+def test_gdb_format(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print/x argc", "frame variable")
+self._expect_cmd(f"dwim-print/x argc + 1", "expression")
+
+def test_flags(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -fx -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -fx -- argc + 1", "expression")
Index: lldb/source/Commands/CommandObjectDWIMPrint.h
===
--- lldb/source/Commands/CommandObjectDWIMPrint.h
+++ lldb/source/Commands/CommandObjectDWIMPrint.h
@@ -10,6 +10,9 @@
 #define LLDB_SOURCE_COMMANDS_COMMANDOBJECTDWIMPRINT_H
 
 #include "lldb/Interpreter/CommandObject.h"
+#include "lldb/Interpreter/OptionGroupFormat.h"
+#include "lldb/Interpreter/OptionGroupValueObjectDisplay.h"
+#include "lldb/Interpreter/OptionValueFormat.h"
 
 namespace lldb_private {
 
@@ -31,8 +34,14 @@
 
   ~CommandObjectDWIMPrint() override = default;
 
+  Options *GetOptions() override;
+
 private:
   bool 

[Lldb-commits] [PATCH] D143282: [lldb] Accept negative indexes in __getitem__

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ff636729d06: [lldb] Accept negative indexes in __getitem__ 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143282

Files:
  lldb/bindings/interface/SBBreakpoint.i
  lldb/bindings/interface/SBInstructionList.i
  lldb/bindings/interface/SBModule.i
  lldb/bindings/interface/SBProcess.i
  lldb/bindings/interface/SBSymbolContextList.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interface/SBThread.i
  lldb/bindings/interface/SBTypeCategory.i
  lldb/bindings/interface/SBTypeEnumMember.i
  lldb/bindings/interface/SBValue.i
  lldb/bindings/interface/SBValueList.i
  lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
  lldb/test/API/python_api/thread/TestThreadAPI.py

Index: lldb/test/API/python_api/thread/TestThreadAPI.py
===
--- lldb/test/API/python_api/thread/TestThreadAPI.py
+++ lldb/test/API/python_api/thread/TestThreadAPI.py
@@ -48,6 +48,11 @@
 self.setTearDownCleanup(dictionary=d)
 self.step_over_3_times(self.exe_name)
 
+def test_negative_indexing(self):
+"""Test SBThread.frame with negative indexes."""
+self.build()
+self.validate_negative_indexing()
+
 def setUp(self):
 # Call super's setUp().
 TestBase.setUp(self)
@@ -269,3 +274,29 @@
 thread.RunToAddress(start_addr)
 self.runCmd("process status")
 #self.runCmd("thread backtrace")
+
+def validate_negative_indexing(self):
+exe = self.getBuildArtifact("a.out")
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+breakpoint = target.BreakpointCreateByLocation(
+"main.cpp", self.break_line)
+self.assertTrue(breakpoint, VALID_BREAKPOINT)
+self.runCmd("breakpoint list")
+
+# Launch the process, and do not stop at the entry point.
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+
+thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint")
+self.runCmd("process status")
+
+pos_range = range(thread.num_frames)
+neg_range = range(thread.num_frames, 0, -1)
+for pos, neg in zip(pos_range, neg_range):
+self.assertEqual(thread.frame[pos].idx, thread.frame[-neg].idx)
Index: lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
===
--- lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
+++ lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
@@ -62,6 +62,9 @@
 location = breakpoint.GetLocationAtIndex(0)
 self.assertTrue(location.IsValid())
 
+# Test negative index access.
+self.assertTrue(breakpoint.location[-1].IsValid())
+
 # Make sure the breakpoint's target is right:
 self.assertEqual(target, breakpoint.GetTarget(), "Breakpoint reports its target correctly")
 
Index: lldb/bindings/interface/SBValueList.i
===
--- lldb/bindings/interface/SBValueList.i
+++ lldb/bindings/interface/SBValueList.i
@@ -146,7 +146,8 @@
 # Access with "int" to get Nth item in the list
 #
 if type(key) is int:
-if key < count:
+if -count <= key < count:
+key %= count
 return self.GetValueAtIndex(key)
 #
 # Access with "str" to get values by name
Index: lldb/bindings/interface/SBValue.i
===
--- lldb/bindings/interface/SBValue.i
+++ lldb/bindings/interface/SBValue.i
@@ -459,8 +459,11 @@
 return 0
 
 def __getitem__(self, key):
-if type(key) is int and key < len(self):
-return self.sbvalue.GetChildAtIndex(key)
+if isinstance(key, int):
+count = len(self)
+if -count <= key < count:
+key %= count
+return self.sbvalue.GetChildAtIndex(key)
 return None
 
 def get_child_access_object(self):
Index: lldb/bindings/interface/SBTypeEnumMember.i
===
--- lldb/bindings/interface/SBTypeEnumMember.i
+++ lldb/bindings/interface/SBTypeEnumMember.i
@@ -121,7 +121,8 @@
 def __getitem__(self, key):
   num_elements = self.GetSize()
   if 

[Lldb-commits] [lldb] 3ff6367 - [lldb] Accept negative indexes in __getitem__

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-02-08T10:46:26-08:00
New Revision: 3ff636729d067801039b3a37618f6ce0dd1c3d24

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

LOG: [lldb] Accept negative indexes in __getitem__

To the Python bindings, add support for Python-like negative indexes.

While was using `script`, I tried to access a thread's bottom frame with
`thread.frame[-1]`, but that failed. This change updates the `__getitem__`
implementations to support negative indexes as one would expect in Python.

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

Added: 


Modified: 
lldb/bindings/interface/SBBreakpoint.i
lldb/bindings/interface/SBInstructionList.i
lldb/bindings/interface/SBModule.i
lldb/bindings/interface/SBProcess.i
lldb/bindings/interface/SBSymbolContextList.i
lldb/bindings/interface/SBTarget.i
lldb/bindings/interface/SBThread.i
lldb/bindings/interface/SBTypeCategory.i
lldb/bindings/interface/SBTypeEnumMember.i
lldb/bindings/interface/SBValue.i
lldb/bindings/interface/SBValueList.i
lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
lldb/test/API/python_api/thread/TestThreadAPI.py

Removed: 




diff  --git a/lldb/bindings/interface/SBBreakpoint.i 
b/lldb/bindings/interface/SBBreakpoint.i
index a7048309edd9f..a61874d8ebd81 100644
--- a/lldb/bindings/interface/SBBreakpoint.i
+++ b/lldb/bindings/interface/SBBreakpoint.i
@@ -273,8 +273,11 @@ public:
 return 0
 
 def __getitem__(self, key):
-if type(key) is int and key < len(self):
-return self.sbbreakpoint.GetLocationAtIndex(key)
+if isinstance(key, int):
+count = len(self)
+if -count <= key < count:
+key %= count
+return self.sbbreakpoint.GetLocationAtIndex(key)
 return None
 
 def get_locations_access_object(self):

diff  --git a/lldb/bindings/interface/SBInstructionList.i 
b/lldb/bindings/interface/SBInstructionList.i
index b51c0374c3adb..e80452e3bed52 100644
--- a/lldb/bindings/interface/SBInstructionList.i
+++ b/lldb/bindings/interface/SBInstructionList.i
@@ -83,7 +83,9 @@ public:
 '''Access instructions by integer index for array access or by 
lldb.SBAddress to find an instruction that matches a section offset address 
object.'''
 if type(key) is int:
 # Find an instruction by index
-if key < len(self):
+count = len(self)
+if -count <= key < count:
+key %= count
 return self.GetInstructionAtIndex(key)
 elif type(key) is SBAddress:
 # Find an instruction using a lldb.SBAddress object

diff  --git a/lldb/bindings/interface/SBModule.i 
b/lldb/bindings/interface/SBModule.i
index de476f706261c..f181d96a55f99 100644
--- a/lldb/bindings/interface/SBModule.i
+++ b/lldb/bindings/interface/SBModule.i
@@ -415,7 +415,8 @@ public:
 def __getitem__(self, key):
 count = len(self)
 if type(key) is int:
-if key < count:
+if -count <= key < count:
+key %= count
 return self.sbmodule.GetSymbolAtIndex(key)
 elif type(key) is str:
 matches = []
@@ -476,7 +477,8 @@ public:
 def __getitem__(self, key):
 count = len(self)
 if type(key) is int:
-if key < count:
+if -count <= key < count:
+key %= count
 return self.sbmodule.GetSectionAtIndex(key)
 elif type(key) is str:
 for idx in range(count):
@@ -511,7 +513,8 @@ public:
 def __getitem__(self, key):
 count = len(self)
 if type(key) is int:
-if key < count:
+if -count <= key < count:
+key %= count
 return self.sbmodule.GetCompileUnitAtIndex(key)
 elif type(key) is str:
 is_full_path = key[0] == '/'

diff  --git a/lldb/bindings/interface/SBProcess.i 
b/lldb/bindings/interface/SBProcess.i
index 0ef558459e84f..01da4274ed20a 100644
--- a/lldb/bindings/interface/SBProcess.i
+++ b/lldb/bindings/interface/SBProcess.i
@@ -487,8 +487,11 @@ public:
 return 0
 
 def __getitem__(self, key):
-if type(key) is int and key < len(self):
-return self.sbprocess.GetThreadAtIndex(key)
+if isinstance(key, int):
+count = 

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:24
+/// set,
+///   the DIERef references the main, dwo or .o file.
 /// - section: identifies the section of the debug info entry in the given 
file:

I don't understand this sentence.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:32
   enum Section : uint8_t { DebugInfo, DebugTypes };
-
-  DIERef(std::optional dwo_num, Section section,
+  // Making constructors protected to limit where DIERef is used directly.
+  DIERef(std::optional file_index, Section section,

they're not actually protected



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp:75
+  if (ref)
+return DIERef(*GetDWARF(), *ref).get_id();
+

Is this the only call site of the `DIERef(SymbolFileDWARF&)` constructor?

If so, and if we make it such that `DWARFBaseDIE::GetDIERef` returns the fully 
filled in DIERef, then this function can just call get_id() on the result, and 
we can delete that constructor.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1682
 SymbolFileDWARF::GetDIE(const DIERef _ref) {
-  if (die_ref.dwo_num()) {
-SymbolFileDWARF *dwarf = *die_ref.dwo_num() == 0x3fff
- ? m_dwp_symfile.get()
- : this->DebugInfo()
-   .GetUnitAtIndex(*die_ref.dwo_num())
-   ->GetDwoSymbolFile();
-return dwarf->DebugInfo().GetDIE(die_ref);
-  }
-
-  return DebugInfo().GetDIE(die_ref);
+  return GetDIE(die_ref.get_id());
 }

clayborg wrote:
> Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be the 
> one source of truth when finding a DIE. We could make 
> "SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then have 
> "SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and then call 
> "SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D143501: [WIP][clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D143501#4113347 , @aprantl wrote:

> Nice! Does `expr -- std::basic_string s` still work after this change? 
> Not that anyone would want to type this over `std::string` ...

Yup that still works. We would still emit it as `basic_string` if that was 
typed out in the source and that's how LLDB would show it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143501

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

[Lldb-commits] [PATCH] D143501: [WIP][clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: clang/test/CodeGen/debug-info-preferred-names.cpp:1
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - 
-debugger-tuning=lldb | FileCheck --check-prefixes=COMMON,LLDB %s
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - 
-debugger-tuning=gdb | FileCheck --check-prefixes=COMMON,GDB %s

aprantl wrote:
> Is `-debug-info-kind=limited` needed here? Using it here is odd since LLDB 
> doesn't really support it and favors `=standalone` instead.
Not needed, just leftover from different test. Will change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143501

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

[Lldb-commits] [PATCH] D143501: [WIP][clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py:61
 
 if self.expectedCompilerVersion(['>', '16.0']):
+string_type = "std::string"

aprantl wrote:
> Out of curiosity: How does this function work when building the testsuite 
> with a compiler that isn't Clang?
> I vaguely recall there was someone in the community running a bot that built 
> the LLDB testsuite against GCC.
Good point, this should also check for the compiler really. I assume because 
GCC isn't at that version yet it just works on that GCC buildbot

Will change in a different patch since there's other tests with this condition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143501

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

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

[Adrian Vogelsgesang via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG54d4a2550d31: [LLDB] Fix assertion failure by removing 
`CopyType` in `std::coroutine_handle`… (authored by avogelsgesang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143127

Files:
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.h


Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
@@ -16,8 +16,6 @@
 
 namespace lldb_private {
 
-class ClangASTImporter;
-
 namespace formatters {
 
 /// Summary provider for `std::coroutine_handle` from  libc++, libstdc++ and
@@ -50,7 +48,6 @@
   lldb::ValueObjectSP m_resume_ptr_sp;
   lldb::ValueObjectSP m_destroy_ptr_sp;
   lldb::ValueObjectSP m_promise_ptr_sp;
-  std::unique_ptr m_ast_importer;
 };
 
 SyntheticChildrenFrontEnd *
Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -8,7 +8,6 @@
 
 #include "Coroutines.h"
 
-#include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/VariableList.h"
@@ -97,8 +96,7 @@
 
 lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
 StdlibCoroutineHandleSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp),
-  m_ast_importer(std::make_unique()) {
+: SyntheticChildrenFrontEnd(*valobj_sp) {
   if (valobj_sp)
 Update();
 }
@@ -174,8 +172,7 @@
 if (Function *destroy_func =
 ExtractDestroyFunction(target_sp, frame_ptr_addr)) {
   if (CompilerType inferred_type = InferPromiseType(*destroy_func)) {
-// Copy the type over to the correct `TypeSystemClang` instance
-promise_type = m_ast_importer->CopyType(*ast_ctx, inferred_type);
+promise_type = inferred_type;
   }
 }
   }


Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
@@ -16,8 +16,6 @@
 
 namespace lldb_private {
 
-class ClangASTImporter;
-
 namespace formatters {
 
 /// Summary provider for `std::coroutine_handle` from  libc++, libstdc++ and
@@ -50,7 +48,6 @@
   lldb::ValueObjectSP m_resume_ptr_sp;
   lldb::ValueObjectSP m_destroy_ptr_sp;
   lldb::ValueObjectSP m_promise_ptr_sp;
-  std::unique_ptr m_ast_importer;
 };
 
 SyntheticChildrenFrontEnd *
Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -8,7 +8,6 @@
 
 #include "Coroutines.h"
 
-#include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/VariableList.h"
@@ -97,8 +96,7 @@
 
 lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
 StdlibCoroutineHandleSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp),
-  m_ast_importer(std::make_unique()) {
+: SyntheticChildrenFrontEnd(*valobj_sp) {
   if (valobj_sp)
 Update();
 }
@@ -174,8 +172,7 @@
 if (Function *destroy_func =
 ExtractDestroyFunction(target_sp, frame_ptr_addr)) {
   if (CompilerType inferred_type = InferPromiseType(*destroy_func)) {
-// Copy the type over to the correct `TypeSystemClang` instance
-promise_type = m_ast_importer->CopyType(*ast_ctx, inferred_type);
+promise_type = inferred_type;
   }
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 54d4a25 - [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

[Adrian Vogelsgesang via lldb-commits]

Author: Adrian Vogelsgesang
Date: 2023-02-08T10:22:50-08:00
New Revision: 54d4a2550d3167d51a9d386d9823a06aca459531

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

LOG: [LLDB] Fix assertion failure by removing `CopyType` in 
`std::coroutine_handle` pretty printer

The pretty printer for `std::coroutine_handle` was running into
> Assertion failed: (target_ctx != source_ctx && "Can't import into itself")
from ClangASTImporter.h, line 270.

This commit fixes the issue by removing the `CopyType` call from the
pretty printer. While this call was necessary in the past, it seems to
be no longer required, at least all test cases are still passing. Maybe
something changed in the meantime around the handling of `TypesystemClang`
instances. I don't quite understand why `CopyType` was necessary earlier.

I am not sure how to add a regression test for this, though. It seems
the issue is already triggered by the exising `TestCoroutineHandle.py`,
but API tests seem to ignore all violations of `lldbassert` and still
report the test as "passed", even if assertions were triggered

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

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.h

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
index bd428a812531a..c5170757496f1 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -8,7 +8,6 @@
 
 #include "Coroutines.h"
 
-#include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/VariableList.h"
@@ -97,8 +96,7 @@ bool 
lldb_private::formatters::StdlibCoroutineHandleSummaryProvider(
 
 lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
 StdlibCoroutineHandleSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp),
-  m_ast_importer(std::make_unique()) {
+: SyntheticChildrenFrontEnd(*valobj_sp) {
   if (valobj_sp)
 Update();
 }
@@ -174,8 +172,7 @@ bool 
lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
 if (Function *destroy_func =
 ExtractDestroyFunction(target_sp, frame_ptr_addr)) {
   if (CompilerType inferred_type = InferPromiseType(*destroy_func)) {
-// Copy the type over to the correct `TypeSystemClang` instance
-promise_type = m_ast_importer->CopyType(*ast_ctx, inferred_type);
+promise_type = inferred_type;
   }
 }
   }

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h 
b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
index e2e640ab48430..b26cc9ed6132d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
@@ -16,8 +16,6 @@
 
 namespace lldb_private {
 
-class ClangASTImporter;
-
 namespace formatters {
 
 /// Summary provider for `std::coroutine_handle` from  libc++, libstdc++ and
@@ -50,7 +48,6 @@ class StdlibCoroutineHandleSyntheticFrontEnd
   lldb::ValueObjectSP m_resume_ptr_sp;
   lldb::ValueObjectSP m_destroy_ptr_sp;
   lldb::ValueObjectSP m_promise_ptr_sp;
-  std::unique_ptr m_ast_importer;
 };
 
 SyntheticChildrenFrontEnd *



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

[Lldb-commits] [PATCH] D143501: [WIP][clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Nice! Does `expr -- std::basic_string s` still work after this change? 
Not that anyone would want to type this over `std::string` ...




Comment at: clang/test/CodeGen/debug-info-preferred-names.cpp:1
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - 
-debugger-tuning=lldb | FileCheck --check-prefixes=COMMON,LLDB %s
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - 
-debugger-tuning=gdb | FileCheck --check-prefixes=COMMON,GDB %s

Is `-debug-info-kind=limited` needed here? Using it here is odd since LLDB 
doesn't really support it and favors `=standalone` instead.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py:61
 
 if self.expectedCompilerVersion(['>', '16.0']):
+string_type = "std::string"

Out of curiosity: How does this function work when building the testsuite with 
a compiler that isn't Clang?
I vaguely recall there was someone in the community running a bot that built 
the LLDB testsuite against GCC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143501

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

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

akyrtzi added a comment.

In D141910#4112144 , @domada wrote:

> @akyrtzi Thank you for your feedback. Can I land the patch?

Fine be me.


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

https://reviews.llvm.org/D141910

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

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

domada added a comment.

In D141910#4112164 , @tschuett wrote:

> For AArch64 the default alignment is 0? I would have expected 128.

The refactored function `TargetInfo::getSimdDefaultAlign` is used only for 
calculation of default alignment for `#pragma omp simd aligned(A)`. If user 
does not specify any alignment in OpenMP simd pragma, then we will assume that 
the alignment of `A` is equal to 128 for PPC, WebAssembly or for some X86 
targets and we insert proper assumptions into LLVM IR (please refer to: clang 
test for more details 
.
 For other targets like AArch64 we don't insert any assumptions into LLVM IR 
code.

I just refactored the code and I return the same values as previous function.


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

https://reviews.llvm.org/D141910

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

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[Emmmer S via Phabricator via lldb-commits]

Emmmer added inline comments.



Comment at: lldb/source/Target/Process.cpp:2373
+reported_after = false;
+  return reported_after;
+}

DavidSpickett wrote:
> DavidSpickett wrote:
> > Would this be any clearer with multiple returns? Or one giant return, but 
> > the logic is a bit cryptic then.
> > 
> > ```
> >   const ArchSpec  = GetTarget().GetArchitecture();
> >   if (!arch.IsValid())
> > return true;
> > 
> >   llvm::Triple triple = arch.GetTriple();
> >   return !(triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || 
> > triple.isArmMClass() || triple.isARM());
> > ```
> > 
> > Better as:
> > ```
> >   const ArchSpec  = GetTarget().GetArchitecture();
> >   if (!arch.IsValid())
> > return false;
> > 
> >   llvm::Triple triple = arch.GetTriple();
> >   if (triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || 
> > triple.isArmMClass() || triple.isARM())
> > return false;
> > 
> >   return true;
> > ```
> > 
> > Also, do we know what RISC-V does?
> @Emmmer any idea?
It seems standard RISC-V uses the `before` mode, that is 
> The action for this trigger will be taken just before the instruction that 
> triggered it is committed, but after all preceding instructions are committed.

If I understand this code correctly (correct me if I was wrong), we should 
return `false` for RISC-V. 

But in the debug-spec [1], RISC-V seems to allow both `before` and `after` 
modes, which can be detected through CSRs. When debug-spec lands, we may change 
the code accordingly.

[1]: 
https://github.com/riscv/riscv-debug-spec/blob/6309972901417a0fa72d812d2ffe89e432f00dff/xml/hwbp_registers.xml#L365


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

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

[Lldb-commits] [PATCH] D143564: [LLDB] Add missing newline to "image lookup" output

[David Spickett via Phabricator via lldb-commits]

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

When using --name, due to a missing newline, multiple symbol results
were not correctly printed:

  (lldb) image lookup -r -n "As<.*"
  2 matches found in <...>/tbi_lisp:
  Address: tbi_lisp<...>
  Summary: tbi_lisp<...> at Symbol.cpp:75Address: tbi_lisp<...>
  Summary: tbi_lisp<...> at Symbol.cpp:82

It should be:

  (lldb) image lookup -r -n "As<.*"
  2 matches found in /home/david.spickett/tbi_lisp/tbi_lisp:
  Address: tbi_lisp<...>
  Summary: tbi_lisp<...> at Symbol.cpp:75
  Address: tbi_lisp<...>
  Summary: tbi_lisp<...> at Symbol.cpp:82

With Address/Summary on separate lines.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143564

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/test/Shell/Commands/command-target-modules-lookup.test


Index: lldb/test/Shell/Commands/command-target-modules-lookup.test
===
--- lldb/test/Shell/Commands/command-target-modules-lookup.test
+++ lldb/test/Shell/Commands/command-target-modules-lookup.test
@@ -10,3 +10,10 @@
 # CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc()
 # CHECK-NEXT: Address: [[MODULE]][0x0038] ([[MODULE]]..text + 56)
 # CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc(double)
+
+# RUN: %lldb %t -b -o "target modules lookup -r -n \".*someFunc\"" | FileCheck 
%s -DMODULE=%basename_t.tmp --check-prefix CHECKFN
+# CHECKFN: 2 matches found in {{.*}}[[MODULE]]:
+# CHECKFN-NEXT: Address: [[MODULE]][0x] ([[MODULE]]..text + 0)
+# CHECKFN-NEXT: Summary: [[MODULE]]`someFunc(int, int, int)
+# CHECKFN-NEXT: Address: [[MODULE]][0x001c] ([[MODULE]]..text + 28)
+# CHECKFN-NEXT: Summary: [[MODULE]]`someFunc(char, int)
\ No newline at end of file
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1582,6 +1582,8 @@
   sc.GetAddressRange(eSymbolContextEverything, 0, true, range);
 
   DumpAddress(exe_scope, range.GetBaseAddress(), verbose, all_ranges, 
strm);
+  if (i != (num_matches - 1))
+strm.EOL();
 }
   }
   strm.IndentLess();


Index: lldb/test/Shell/Commands/command-target-modules-lookup.test
===
--- lldb/test/Shell/Commands/command-target-modules-lookup.test
+++ lldb/test/Shell/Commands/command-target-modules-lookup.test
@@ -10,3 +10,10 @@
 # CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc()
 # CHECK-NEXT: Address: [[MODULE]][0x0038] ([[MODULE]]..text + 56)
 # CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc(double)
+
+# RUN: %lldb %t -b -o "target modules lookup -r -n \".*someFunc\"" | FileCheck %s -DMODULE=%basename_t.tmp --check-prefix CHECKFN
+# CHECKFN: 2 matches found in {{.*}}[[MODULE]]:
+# CHECKFN-NEXT: Address: [[MODULE]][0x] ([[MODULE]]..text + 0)
+# CHECKFN-NEXT: Summary: [[MODULE]]`someFunc(int, int, int)
+# CHECKFN-NEXT: Address: [[MODULE]][0x001c] ([[MODULE]]..text + 28)
+# CHECKFN-NEXT: Summary: [[MODULE]]`someFunc(char, int)
\ No newline at end of file
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1582,6 +1582,8 @@
   sc.GetAddressRange(eSymbolContextEverything, 0, true, range);
 
   DumpAddress(exe_scope, range.GetBaseAddress(), verbose, all_ranges, strm);
+  if (i != (num_matches - 1))
+strm.EOL();
 }
   }
   strm.IndentLess();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a subscriber: Emmmer.
DavidSpickett added inline comments.



Comment at: lldb/source/Target/Process.cpp:2373
+reported_after = false;
+  return reported_after;
+}

DavidSpickett wrote:
> Would this be any clearer with multiple returns? Or one giant return, but the 
> logic is a bit cryptic then.
> 
> ```
>   const ArchSpec  = GetTarget().GetArchitecture();
>   if (!arch.IsValid())
> return true;
> 
>   llvm::Triple triple = arch.GetTriple();
>   return !(triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || 
> triple.isArmMClass() || triple.isARM());
> ```
> 
> Better as:
> ```
>   const ArchSpec  = GetTarget().GetArchitecture();
>   if (!arch.IsValid())
> return false;
> 
>   llvm::Triple triple = arch.GetTriple();
>   if (triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || 
> triple.isArmMClass() || triple.isARM())
> return false;
> 
>   return true;
> ```
> 
> Also, do we know what RISC-V does?
@Emmmer any idea?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

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

[Lldb-commits] [PATCH] D142715: [LLDB] Apply FixCodeAddress to all forms of address arguments

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142715

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

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Change lldb from depending on the remote gdb stub to tell it whether 
> watchpoints are received before or after the instruction, to knowing the 
> architectures where watchpoint exceptions are received before the instruction 
> executes, and allow the remote gdb stub to override this behavior in its 
> qHostInfo response.

Sounds good to me. Can you simplify the commit message some now that you've got 
the changes you want?

Seems like the main things are:

- Split up querying watchpoint reporting style, and number of watchpoint slots.
- Have process determine the reporting style by target, with the option for the 
remote to override. Meaning this query never fails.
- Only conclude that watchpoints are unsupported if the remote tells us that it 
has 0 slots.

(the last one I think we always did but good for context)




Comment at: lldb/source/Target/Process.cpp:2360
+  std::optional subclass_override = DoGetWatchpointReportedAfter();
+  if (subclass_override) {
+return *subclass_override;

Can do `if (std::optional subclass) {...` here.

Also no `{}` for single line if.



Comment at: lldb/source/Target/Process.cpp:2367
+return reported_after;
+  }
+  llvm::Triple triple = arch.GetTriple();

No `{}` needed here too.



Comment at: lldb/source/Target/Process.cpp:2373
+reported_after = false;
+  return reported_after;
+}

Would this be any clearer with multiple returns? Or one giant return, but the 
logic is a bit cryptic then.

```
  const ArchSpec  = GetTarget().GetArchitecture();
  if (!arch.IsValid())
return true;

  llvm::Triple triple = arch.GetTriple();
  return !(triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || 
triple.isArmMClass() || triple.isARM());
```

Better as:
```
  const ArchSpec  = GetTarget().GetArchitecture();
  if (!arch.IsValid())
return false;

  llvm::Triple triple = arch.GetTriple();
  if (triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || 
triple.isArmMClass() || triple.isARM())
return false;

  return true;
```

Also, do we know what RISC-V does?



Comment at: lldb/source/Target/StopInfo.cpp:833
-  m_should_stop = true;
-  return m_should_stop;
-}

This is no longer needed because `GetWatchpointReportedAfter` will fall back to 
the process' defaults if the GDB layer does not/can not provide a value, 
correct?



Comment at: lldb/source/Target/Target.cpp:804
 "Target supports (%u) hardware watchpoint slots.\n",
-num_supported_hardware_watchpoints);
+*num_supported_hardware_watchpoints);
 return false;

This should just append "Target supports 0 hardware...".

In theory a compiler could figure that out but also it's a bit confusing 
because it makes the reader wonder what value other than 0 is supposed to end 
up here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

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

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

For AArch64 the default alignment is 0? I would have expected 128.


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

https://reviews.llvm.org/D141910

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

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

domada added a comment.

@akyrtzi Thank you for your feedback. Can I land the patch?


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

https://reviews.llvm.org/D141910

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