[Lldb-commits] [PATCH] D96548: [lldb] [Process/FreeBSDRemote] Introduce aarch64 hw break/watchpoint support

2021-03-05 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 328436.
mgorny added a comment.

Updated to enable on FreeBSD 13+ unconditionally. I've left the `#define` to 
avoid repeating the version in a few places.


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

https://reviews.llvm.org/D96548

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h

Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
@@ -0,0 +1,79 @@
+//===-- NativeRegisterContextDBReg_arm64.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef lldb_NativeRegisterContextDBReg_arm64_h
+#define lldb_NativeRegisterContextDBReg_arm64_h
+
+#include "Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h"
+
+#include 
+
+namespace lldb_private {
+
+class NativeRegisterContextDBReg_arm64
+: public virtual NativeRegisterContextRegisterInfo {
+public:
+  uint32_t NumSupportedHardwareBreakpoints() override;
+
+  uint32_t SetHardwareBreakpoint(lldb::addr_t addr, size_t size) override;
+
+  bool ClearHardwareBreakpoint(uint32_t hw_idx) override;
+
+  Status ClearAllHardwareBreakpoints() override;
+
+  Status GetHardwareBreakHitIndex(uint32_t &bp_index,
+  lldb::addr_t trap_addr) override;
+
+  bool BreakpointIsEnabled(uint32_t bp_index);
+
+  uint32_t NumSupportedHardwareWatchpoints() override;
+
+  uint32_t SetHardwareWatchpoint(lldb::addr_t addr, size_t size,
+ uint32_t watch_flags) override;
+
+  bool ClearHardwareWatchpoint(uint32_t hw_index) override;
+
+  Status ClearAllHardwareWatchpoints() override;
+
+  Status GetWatchpointHitIndex(uint32_t &wp_index,
+   lldb::addr_t trap_addr) override;
+
+  lldb::addr_t GetWatchpointHitAddress(uint32_t wp_index) override;
+
+  lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
+
+  uint32_t GetWatchpointSize(uint32_t wp_index);
+
+  bool WatchpointIsEnabled(uint32_t wp_index);
+
+  // Debug register type select
+  enum DREGType { eDREGTypeWATCH = 0, eDREGTypeBREAK };
+
+protected:
+  // Debug register info for hardware breakpoints and watchpoints management.
+  struct DREG {
+lldb::addr_t address;  // Breakpoint/watchpoint address value.
+lldb::addr_t hit_addr; // Address at which last watchpoint trigger exception
+   // occurred.
+lldb::addr_t real_addr; // Address value that should cause target to stop.
+uint32_t control;   // Breakpoint/watchpoint control value.
+  };
+
+  std::array m_hbp_regs; // hardware breakpoints
+  std::array m_hwp_regs; // hardware watchpoints
+
+  uint32_t m_max_hbp_supported;
+  uint32_t m_max_hwp_supported;
+
+  virtual llvm::Error ReadHardwareDebugInfo() = 0;
+  virtual llvm::Error WriteHardwareDebugRegs(DREGType hwbType) = 0;
+};
+
+} // namespace lldb_private
+
+#endif // #ifndef lldb_NativeRegisterContextDBReg_arm64_h
Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
@@ -0,0 +1,466 @@
+//===-- NativeRegisterContextDBReg_arm64.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NativeRegisterContextDBReg_arm64.h"
+
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/RegisterValue.h"
+
+using namespace lldb_private;
+
+// E (bit 0), used to enable breakpoint/watchpoint
+constexpr uint32_t g_enable_bit = 1;
+// PAC (bits 2:1): 0b10
+constexpr uint32_t g_pac_bits = (2 << 1);
+
+// Returns appropriate control register bits for the specified size
+static constexpr inline uint64_t GetSizeBits(int size) {
+  // BAS (bits 12:5) hold a bi

[Lldb-commits] [PATCH] D96458: [LLDB] Add support for Arm64/Linux dynamic register sets

2021-03-05 Thread Diana Picus via Phabricator via lldb-commits
rovka added a comment.

Hi Omair,

I had a look and I think the commit message is slightly misleading because it 
says "Arm64 has dynamic features like SVE, Pointer Authentication and MTE", but 
then you don't set m_reg_infos_is_dynamic to true for SVE. This got me a bit 
confused at first. Maybe it would help to elaborate a bit on this point in the 
commit message (and maybe also in the comments if you think it makes sense).

I also have a few nits inline.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:81
+  Status error = ReadSVEHeader();
+  uint32_t opt_regset = RegisterInfoPOSIX_arm64::eRegsetEnableDefault;
+  if (error.Success()) {

I was about to suggest using lldb_private::Flags for this, but after looking a 
bit more through the code it's not clear how this is intended to be used. The 
definition of eRegsetEnableSVE doesn't look flag-like at all (I would expect to 
see 1 << 1 rather than just plain 1, to make it clear what future values should 
look like), and also in ConfigureRegisterContext you check if (opt_regsets > 
eRegsetEnableSVE), which to me doesn't seem very idiomatic for working with 
flags. 

What do you think about increasing the level of abstraction a bit and using a 
small wrapper class for the regset options, that can answer directly questions 
like IsRegsetDynamic()?



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:88
+opt_regset |= RegisterInfoPOSIX_arm64::eRegsetEnableSVE;
+GetRegisterInfo().ConfigureRegisterInfos(opt_regset);
+  } else {

Nitpick: You should hoist the call out of the if, so it's easier to add other 
regsets in the future.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h:132
+  // Contains pair of (start, end) register numbers of a register set with 
start
+  // and end included.
+  std::map> m_per_regset_regnum_range;

Nitpick: Please use square brackets instead of parantheses, otherwise it's 
confusing (parentheses suggest an open interval).


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

https://reviews.llvm.org/D96458

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


[Lldb-commits] [PATCH] D98001: [lldb/API] Add CommandInterpreter::{Get, Set}PrintErrors to SBAPI (NFC)

2021-03-05 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 328569.
mib added a comment.

Add test.


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

https://reviews.llvm.org/D98001

Files:
  lldb/bindings/interface/SBCommandInterpreterRunOptions.i
  lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
  lldb/source/API/SBCommandInterpreterRunOptions.cpp
  lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py

Index: lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
===
--- lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
+++ lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
@@ -73,3 +73,40 @@
 self.assertGreater(n_errors, 0)
 self.assertTrue(quit_requested)
 self.assertFalse(has_crashed)
+
+class SBCommandInterpreterRunOptionsCase(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+mydir = TestBase.compute_mydir(__file__)
+
+def test_command_interpreter_run_options(self):
+"""Test SBCommandInterpreterRunOptions default values, getters & setters """
+
+opts = lldb.SBCommandInterpreterRunOptions()
+
+# Check getters with default values
+self.assertEqual(opts.GetStopOnContinue(), False)
+self.assertEqual(opts.GetStopOnError(), False)
+self.assertEqual(opts.GetStopOnCrash(), False)
+self.assertEqual(opts.GetEchoCommands(), True)
+self.assertEqual(opts.GetPrintResults(), True)
+self.assertEqual(opts.GetPrintErrors(), True)
+self.assertEqual(opts.GetAddToHistory(), True)
+
+# Invert values
+opts.SetStopOnContinue(not opts.GetStopOnContinue())
+opts.SetStopOnError(not opts.GetStopOnError())
+opts.SetStopOnCrash(not opts.GetStopOnCrash())
+opts.SetEchoCommands(not opts.GetEchoCommands())
+opts.SetPrintResults(not opts.GetPrintResults())
+opts.SetPrintErrors(not opts.GetPrintErrors())
+opts.SetAddToHistory(not opts.GetAddToHistory())
+
+# Check the value changed
+self.assertEqual(opts.GetStopOnContinue(), True)
+self.assertEqual(opts.GetStopOnError(), True)
+self.assertEqual(opts.GetStopOnCrash(), True)
+self.assertEqual(opts.GetEchoCommands(), False)
+self.assertEqual(opts.GetPrintResults(), False)
+self.assertEqual(opts.GetPrintErrors(), False)
+self.assertEqual(opts.GetAddToHistory(), False)
\ No newline at end of file
Index: lldb/source/API/SBCommandInterpreterRunOptions.cpp
===
--- lldb/source/API/SBCommandInterpreterRunOptions.cpp
+++ lldb/source/API/SBCommandInterpreterRunOptions.cpp
@@ -131,6 +131,20 @@
   m_opaque_up->SetPrintResults(print_results);
 }
 
+bool SBCommandInterpreterRunOptions::GetPrintErrors() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBCommandInterpreterRunOptions,
+   GetPrintErrors);
+
+  return m_opaque_up->GetPrintErrors();
+}
+
+void SBCommandInterpreterRunOptions::SetPrintErrors(bool print_errors) {
+  LLDB_RECORD_METHOD(void, SBCommandInterpreterRunOptions, SetPrintErrors,
+ (bool), print_errors);
+
+  m_opaque_up->SetPrintErrors(print_errors);
+}
+
 bool SBCommandInterpreterRunOptions::GetAddToHistory() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBCommandInterpreterRunOptions,
GetAddToHistory);
@@ -269,6 +283,10 @@
  GetPrintResults, ());
   LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetPrintResults,
(bool));
+  LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreterRunOptions,
+ GetPrintErrors, ());
+  LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetPrintErrors,
+   (bool));
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreterRunOptions,
  GetAddToHistory, ());
   LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetAddToHistory,
Index: lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
===
--- lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
+++ lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
@@ -56,6 +56,10 @@
 
   void SetPrintResults(bool);
 
+  bool GetPrintErrors() const;
+
+  void SetPrintErrors(bool);
+
   bool GetAddToHistory() const;
 
   void SetAddToHistory(bool);
Index: lldb/bindings/interface/SBCommandInterpreterRunOptions.i
===
--- lldb/bindings/interface/SBCommandInterpreterRunOptions.i
+++ lldb/bindings/interface/SBCommandInterpreterRunOptions.i
@@ -18,6 +18,7 @@
 * StopOnCrash:false
 * EchoCommands:   true
 * PrintResults:   true
+* PrintErrors:true
 * AddToHistory:   true
 
 ") SBCommandInterpreterRunOptions;
@@ -58,6 +59

[Lldb-commits] [PATCH] D98001: [lldb/API] Add CommandInterpreter::{Get, Set}PrintErrors to SBAPI (NFC)

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

LGTM with the `\n`.




Comment at: 
lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py:113
+self.assertEqual(opts.GetAddToHistory(), False)
\ No newline at end of file


No newline at end of file


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

https://reviews.llvm.org/D98001

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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-03-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D97786#2603656 , @cmtice wrote:

> I'm not sure about using target.debug-file-search-paths, and what the changes 
> Pavel is suggesting would entail.

I imagine it would involve calling Symbols::LocateExecutableSymbolFile to 
locate the dwo file, similar to how we do that for dwp files (see 
`SymbolFileDWARF::GetDwpSymbolFile`). (Disclaimer: I have not tried doing this, 
so I don't know if that function would work out-of-the-box.)
Note that I myself am not convinced that this is the right solution (that 
function is rather heavy), but it does solve the problem of "how do we let the 
user specify/choose the location of the dwo files" (answer: put the path in the 
target.debug-file-search-paths setting), and it would search the cwd and the 
module directory automatically. And the "heavy-ness" of the function is 
moderated by the fact that it is a no-op for absolute paths to existing files.

> But the real question I am trying to resolve here is not "what are all the 
> places we should be searching for the .dwo files?" but "When we're given a 
> *relative path* in the DWARF for finding the files, what should it be 
> relative TO?".

I'm sorry, but what's the difference between those two questions? Is it about 
the fact that the second question sort of implies that there should only be one 
correct location where we should be searching?

> I still think the most correct answer to that question is "relative to the 
> location of the binary" (I know, this is assuming that the binary has not 
> been moved since it was built, or all bets are off).  If you like, I can 
> update this patch so that it continues to search relative to the cwd (where 
> the debugger was launched), and IN ADDITION, will search relative to where 
> the binary is.

I don't really have a strong opinion either way -- the thing I'm mostly 
interested in is some sort of consistency between lldb & gdb. Have you already 
discussed this with the gdb folks? If they want to move to the binary-relative 
mechanism, then I don't see a problem with us doing the same. If they want to 
keep the existing behavior, then I think it would be good to preserve that in 
lldb as well (but I don't have a problem with searching in other places too; 
and `LocateExecutableSymbolFile` is one way to achieve that).


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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [lldb] 36eab46 - [lldb/Interpreter] Add `interpreter.repeat-previous-command` setting

2021-03-05 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2021-03-05T19:33:32+01:00
New Revision: 36eab4634f4cd4594e6d1409a66bc8f2d8fda04f

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

LOG: [lldb/Interpreter] Add `interpreter.repeat-previous-command` setting

This patch introduces a new interpreter setting to prevent LLDB from
re-executing the previous command when passing an empty command.

This can be very useful when performing actions that requires a long
time to complete.

To preserve the original behaviour, the setting defaults to `true`.

rdar://74983516

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/InterpreterProperties.td
lldb/test/API/commands/settings/TestSettings.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 7f897bf20185..5b5f145b7e25 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -504,6 +504,8 @@ class CommandInterpreter : public Broadcaster,
   bool GetEchoCommentCommands() const;
   void SetEchoCommentCommands(bool enable);
 
+  bool GetRepeatPreviousCommand() const;
+
   const CommandObject::CommandMap &GetUserCommands() const {
 return m_user_dict;
   }

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index b0ff634f0365..eeef14bf36c2 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -223,6 +223,12 @@ bool CommandInterpreter::GetSpaceReplPrompts() const {
   nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
 }
 
+bool CommandInterpreter::GetRepeatPreviousCommand() const {
+  const uint32_t idx = ePropertyRepeatPreviousCommand;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
+}
+
 void CommandInterpreter::Initialize() {
   LLDB_SCOPED_TIMER();
 
@@ -1695,6 +1701,11 @@ bool CommandInterpreter::HandleCommand(const char 
*command_line,
   }
 
   if (empty_command) {
+if (!GetRepeatPreviousCommand()) {
+  result.SetStatus(eReturnStatusSuccessFinishNoResult);
+  return true;
+}
+
 if (m_command_history.IsEmpty()) {
   result.AppendError("empty command");
   result.SetStatus(eReturnStatusFailed);

diff  --git a/lldb/source/Interpreter/InterpreterProperties.td 
b/lldb/source/Interpreter/InterpreterProperties.td
index e5346d14a6e1..1148c1b01def 100644
--- a/lldb/source/Interpreter/InterpreterProperties.td
+++ b/lldb/source/Interpreter/InterpreterProperties.td
@@ -29,4 +29,8 @@ let Definition = "interpreter" in {
 Global,
 DefaultTrue,
 Desc<"If true, commands will be echoed even if they are pure comment 
lines.">;
+  def RepeatPreviousCommand: Property<"repeat-previous-command", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"If true, LLDB will repeat the previous command if no command was 
passed to the interpreter. If false, LLDB won't repeat the previous command but 
only return a new prompt.">;
 }

diff  --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index c3cfdabe75de..7eab23a195bb 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -25,6 +25,42 @@ def 
test_apropos_should_also_search_settings_description(self):
  "environment variables",
  "executable's environment"])
 
+def test_set_interpreter_repeat_prev_command(self):
+"""Test the `interpreter.repeat-previous-command` setting."""
+self.build()
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+setting = "interpreter.repeat-previous-command"
+
+def cleanup(setting):
+self.runCmd(
+"settings clear %s" %
+setting, check=False)
+
+# Execute the cleanup function during test case tear down.
+self.addTearDownHook(cleanup(setting))
+
+# First, check for the setting default value.
+self.expect("setting show %s" % setting,
+substrs=["interpreter.repeat-previous-command (boolean) = 
true"])
+
+# Then, invert the setting, and check that was set correctly
+self.runCmd("setting set %s false" % setting)
+self.expect("setting show %s" % setting,
+substrs=["interpreter.repeat-previous-comm

[Lldb-commits] [lldb] c964741 - [lldb/API] Add CommandInterpreter::{Get, Set}PrintErrors to SBAPI (NFC)

2021-03-05 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2021-03-05T19:33:33+01:00
New Revision: c964741996bcc3550c3598bb7237bd4551b03016

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

LOG: [lldb/API] Add CommandInterpreter::{Get,Set}PrintErrors to SBAPI (NFC)

This patch exposes the getter and setter methods for the command
interpreter `print_errors` run option.

rdar://74816984

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/bindings/interface/SBCommandInterpreterRunOptions.i
lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
lldb/source/API/SBCommandInterpreterRunOptions.cpp
lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py

Removed: 




diff  --git a/lldb/bindings/interface/SBCommandInterpreterRunOptions.i 
b/lldb/bindings/interface/SBCommandInterpreterRunOptions.i
index 1a618a228bbe..28437abb60c6 100644
--- a/lldb/bindings/interface/SBCommandInterpreterRunOptions.i
+++ b/lldb/bindings/interface/SBCommandInterpreterRunOptions.i
@@ -18,6 +18,7 @@ A default SBCommandInterpreterRunOptions object has:
 * StopOnCrash:false
 * EchoCommands:   true
 * PrintResults:   true
+* PrintErrors:true
 * AddToHistory:   true
 
 ") SBCommandInterpreterRunOptions;
@@ -58,6 +59,12 @@ public:
 void
 SetPrintResults (bool);
 
+bool
+GetPrintErrors () const;
+
+void
+SetPrintErrors (bool);
+
 bool
 GetAddToHistory () const;
 

diff  --git a/lldb/include/lldb/API/SBCommandInterpreterRunOptions.h 
b/lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
index 3c00513faaa7..efaacd39eb0f 100644
--- a/lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
+++ b/lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
@@ -56,6 +56,10 @@ class LLDB_API SBCommandInterpreterRunOptions {
 
   void SetPrintResults(bool);
 
+  bool GetPrintErrors() const;
+
+  void SetPrintErrors(bool);
+
   bool GetAddToHistory() const;
 
   void SetAddToHistory(bool);

diff  --git a/lldb/source/API/SBCommandInterpreterRunOptions.cpp 
b/lldb/source/API/SBCommandInterpreterRunOptions.cpp
index da800e8b7804..317ec6d37127 100644
--- a/lldb/source/API/SBCommandInterpreterRunOptions.cpp
+++ b/lldb/source/API/SBCommandInterpreterRunOptions.cpp
@@ -131,6 +131,20 @@ void SBCommandInterpreterRunOptions::SetPrintResults(bool 
print_results) {
   m_opaque_up->SetPrintResults(print_results);
 }
 
+bool SBCommandInterpreterRunOptions::GetPrintErrors() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBCommandInterpreterRunOptions,
+   GetPrintErrors);
+
+  return m_opaque_up->GetPrintErrors();
+}
+
+void SBCommandInterpreterRunOptions::SetPrintErrors(bool print_errors) {
+  LLDB_RECORD_METHOD(void, SBCommandInterpreterRunOptions, SetPrintErrors,
+ (bool), print_errors);
+
+  m_opaque_up->SetPrintErrors(print_errors);
+}
+
 bool SBCommandInterpreterRunOptions::GetAddToHistory() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBCommandInterpreterRunOptions,
GetAddToHistory);
@@ -269,6 +283,10 @@ template <> void 
RegisterMethods(Registry &R) {
  GetPrintResults, ());
   LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetPrintResults,
(bool));
+  LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreterRunOptions,
+ GetPrintErrors, ());
+  LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetPrintErrors,
+   (bool));
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreterRunOptions,
  GetAddToHistory, ());
   LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetAddToHistory,

diff  --git 
a/lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py 
b/lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
index 86d7f7822772..2db7fdd66133 100644
--- a/lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
@@ -73,3 +73,40 @@ def test_run_session_with_error_and_quit(self):
 self.assertGreater(n_errors, 0)
 self.assertTrue(quit_requested)
 self.assertFalse(has_crashed)
+
+class SBCommandInterpreterRunOptionsCase(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+mydir = TestBase.compute_mydir(__file__)
+
+def test_command_interpreter_run_options(self):
+"""Test SBCommandInterpreterRunOptions default values, getters & 
setters """
+
+opts = lldb.SBCommandInterpreterRunOptions()
+
+# Check getters with default values
+self.assertEqual(opts.GetStopOnContinue(), False)
+self.assertEqual(opts.GetStopOnError

[Lldb-commits] [PATCH] D97999: [lldb/Interpreter] Add `interpreter.repeat-previous-command` setting

2021-03-05 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG36eab4634f4c: [lldb/Interpreter] Add 
`interpreter.repeat-previous-command` setting (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97999

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/InterpreterProperties.td
  lldb/test/API/commands/settings/TestSettings.py

Index: lldb/test/API/commands/settings/TestSettings.py
===
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -25,6 +25,42 @@
  "environment variables",
  "executable's environment"])
 
+def test_set_interpreter_repeat_prev_command(self):
+"""Test the `interpreter.repeat-previous-command` setting."""
+self.build()
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+setting = "interpreter.repeat-previous-command"
+
+def cleanup(setting):
+self.runCmd(
+"settings clear %s" %
+setting, check=False)
+
+# Execute the cleanup function during test case tear down.
+self.addTearDownHook(cleanup(setting))
+
+# First, check for the setting default value.
+self.expect("setting show %s" % setting,
+substrs=["interpreter.repeat-previous-command (boolean) = true"])
+
+# Then, invert the setting, and check that was set correctly
+self.runCmd("setting set %s false" % setting)
+self.expect("setting show %s" % setting,
+substrs=["interpreter.repeat-previous-command (boolean) = false"])
+
+
+ci  = self.dbg.GetCommandInterpreter()
+self.assertTrue(ci.IsValid(), "Invalid command interpreter.")
+# Now, test the functionnality
+res = lldb.SBCommandReturnObject()
+ci.HandleCommand('breakpoint set -n main', res)
+self.assertTrue(res.Succeeded(), "Command failed.")
+ci.HandleCommand('', res)
+self.assertTrue(res.Succeeded(), "Empty command failed.")
+self.assertEqual(self.dbg.GetSelectedTarget().GetNumBreakpoints(), 1)
+
 def test_append_target_env_vars(self):
 """Test that 'append target.run-args' works."""
 # Append the env-vars.
Index: lldb/source/Interpreter/InterpreterProperties.td
===
--- lldb/source/Interpreter/InterpreterProperties.td
+++ lldb/source/Interpreter/InterpreterProperties.td
@@ -29,4 +29,8 @@
 Global,
 DefaultTrue,
 Desc<"If true, commands will be echoed even if they are pure comment lines.">;
+  def RepeatPreviousCommand: Property<"repeat-previous-command", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"If true, LLDB will repeat the previous command if no command was passed to the interpreter. If false, LLDB won't repeat the previous command but only return a new prompt.">;
 }
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -223,6 +223,12 @@
   nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
 }
 
+bool CommandInterpreter::GetRepeatPreviousCommand() const {
+  const uint32_t idx = ePropertyRepeatPreviousCommand;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
+}
+
 void CommandInterpreter::Initialize() {
   LLDB_SCOPED_TIMER();
 
@@ -1695,6 +1701,11 @@
   }
 
   if (empty_command) {
+if (!GetRepeatPreviousCommand()) {
+  result.SetStatus(eReturnStatusSuccessFinishNoResult);
+  return true;
+}
+
 if (m_command_history.IsEmpty()) {
   result.AppendError("empty command");
   result.SetStatus(eReturnStatusFailed);
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -504,6 +504,8 @@
   bool GetEchoCommentCommands() const;
   void SetEchoCommentCommands(bool enable);
 
+  bool GetRepeatPreviousCommand() const;
+
   const CommandObject::CommandMap &GetUserCommands() const {
 return m_user_dict;
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D98001: [lldb/API] Add CommandInterpreter::{Get, Set}PrintErrors to SBAPI (NFC)

2021-03-05 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc964741996bc: [lldb/API] Add 
CommandInterpreter::{Get,Set}PrintErrors to SBAPI (NFC) (authored by mib).

Changed prior to commit:
  https://reviews.llvm.org/D98001?vs=328569&id=328582#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98001

Files:
  lldb/bindings/interface/SBCommandInterpreterRunOptions.i
  lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
  lldb/source/API/SBCommandInterpreterRunOptions.cpp
  lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py

Index: lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
===
--- lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
+++ lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
@@ -73,3 +73,40 @@
 self.assertGreater(n_errors, 0)
 self.assertTrue(quit_requested)
 self.assertFalse(has_crashed)
+
+class SBCommandInterpreterRunOptionsCase(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+mydir = TestBase.compute_mydir(__file__)
+
+def test_command_interpreter_run_options(self):
+"""Test SBCommandInterpreterRunOptions default values, getters & setters """
+
+opts = lldb.SBCommandInterpreterRunOptions()
+
+# Check getters with default values
+self.assertEqual(opts.GetStopOnContinue(), False)
+self.assertEqual(opts.GetStopOnError(), False)
+self.assertEqual(opts.GetStopOnCrash(), False)
+self.assertEqual(opts.GetEchoCommands(), True)
+self.assertEqual(opts.GetPrintResults(), True)
+self.assertEqual(opts.GetPrintErrors(), True)
+self.assertEqual(opts.GetAddToHistory(), True)
+
+# Invert values
+opts.SetStopOnContinue(not opts.GetStopOnContinue())
+opts.SetStopOnError(not opts.GetStopOnError())
+opts.SetStopOnCrash(not opts.GetStopOnCrash())
+opts.SetEchoCommands(not opts.GetEchoCommands())
+opts.SetPrintResults(not opts.GetPrintResults())
+opts.SetPrintErrors(not opts.GetPrintErrors())
+opts.SetAddToHistory(not opts.GetAddToHistory())
+
+# Check the value changed
+self.assertEqual(opts.GetStopOnContinue(), True)
+self.assertEqual(opts.GetStopOnError(), True)
+self.assertEqual(opts.GetStopOnCrash(), True)
+self.assertEqual(opts.GetEchoCommands(), False)
+self.assertEqual(opts.GetPrintResults(), False)
+self.assertEqual(opts.GetPrintErrors(), False)
+self.assertEqual(opts.GetAddToHistory(), False)
Index: lldb/source/API/SBCommandInterpreterRunOptions.cpp
===
--- lldb/source/API/SBCommandInterpreterRunOptions.cpp
+++ lldb/source/API/SBCommandInterpreterRunOptions.cpp
@@ -131,6 +131,20 @@
   m_opaque_up->SetPrintResults(print_results);
 }
 
+bool SBCommandInterpreterRunOptions::GetPrintErrors() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBCommandInterpreterRunOptions,
+   GetPrintErrors);
+
+  return m_opaque_up->GetPrintErrors();
+}
+
+void SBCommandInterpreterRunOptions::SetPrintErrors(bool print_errors) {
+  LLDB_RECORD_METHOD(void, SBCommandInterpreterRunOptions, SetPrintErrors,
+ (bool), print_errors);
+
+  m_opaque_up->SetPrintErrors(print_errors);
+}
+
 bool SBCommandInterpreterRunOptions::GetAddToHistory() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBCommandInterpreterRunOptions,
GetAddToHistory);
@@ -269,6 +283,10 @@
  GetPrintResults, ());
   LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetPrintResults,
(bool));
+  LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreterRunOptions,
+ GetPrintErrors, ());
+  LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetPrintErrors,
+   (bool));
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreterRunOptions,
  GetAddToHistory, ());
   LLDB_REGISTER_METHOD(void, SBCommandInterpreterRunOptions, SetAddToHistory,
Index: lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
===
--- lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
+++ lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
@@ -56,6 +56,10 @@
 
   void SetPrintResults(bool);
 
+  bool GetPrintErrors() const;
+
+  void SetPrintErrors(bool);
+
   bool GetAddToHistory() const;
 
   void SetAddToHistory(bool);
Index: lldb/bindings/interface/SBCommandInterpreterRunOptions.i
===
--- lldb/bindings/interface/SBCommand

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

So we have had a vote for allowing multiple callbacks. Any other main issues 
with the approach in this patch? If everyone is mostly happy, please chime in 
and I will make unit tests and vscode tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

I'm quite happy with it besides the multiple callback thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] IMPORTANT NOTICE - Subscription to Mailman lists disabled immediately

2021-03-05 Thread Tanya Lattner via lldb-commits
All,

We need to immediately disable subscription capabilities to all LLVM Mailman 
lists.

The current Mailman server is being abused by subscribing valid email addresses 
to our lists and because the list requires confirmation, the email address gets 
“spam”. An email address is subscribed upwards of 100 times in a short period 
of time in many cases. AWS has threatened to turn off our instance unless we 
take immediate action. Given the time frame of the situation (24 hours to 
resolve), we have no choice but to disable all new subscription capabilities as 
we can not distinguish between a real subscription attempt versus the abuse. 

Those currently subscribed should see no changes or impact to their workflow. 

I am sure this raises a lot of questions for the LLVM community and we are 
working hard and as quickly as possible on a permanent solution to this 
situation.

Thanks,
Tanya Lattner
LLVM Foundation

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


[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

2021-03-05 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2162
+  llvm::StringRef parent_type_name = GetDWARFDeclContext(parent)
+ .GetQualifiedNameAsConstString()
+ .GetStringRef();

ConstString here is needlessly expensive to construct and it is then used only 
once. Use plain `const char *` or `std::string` is also much cheaper.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2162
 
 void SymbolFileDWARF::FindGlobalVariables(const RegularExpression ®ex,
   uint32_t max_matches,

jankratochvil wrote:
> ConstString here is needlessly expensive to construct and it is then used 
> only once. Use plain `const char *` or `std::string` is also much cheaper.
This function also needs to be patched (with a testcase) as this command works:
```
(lldb) target variable Vars::inline_static
(double) Vars::inline_static = 1.5
```
But this one does not (and it should work):
```
(lldb) target variable -r Vars::inline_static
error: can't find global variable 'Vars::inline_static'
```



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2166
+  // This type is from another scope, skip it.
+  if (!parent_type_name.endswith(context))
+return true;

Could this case have a testcase so that it cannot find such false positives?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3129
 
+VariableSP SymbolFileDWARF::ParseStaticConstMemberDIE(
+const lldb_private::SymbolContext &sc, const DWARFDIE &die) {

Could you try to merge this functionality into 
`SymbolFileDWARF::ParseVariableDIE`? There is now some code duplication. I hope 
the merge will not become too ugly. (@labath was also suggesting this.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

This way of doing progress is going to look odd in anything that uses multiple 
debuggers.  If I'm reading the code aright, if I have two debuggers, and a 
target in Debugger A starts doing something that would cause progress traffic, 
both debuggers will show activity.  Given that the candidate for progress that 
is most common is down in SymbolFile, it's going to be hard to fix that.  You'd 
have to have some way of marking the API boundary from "where you have a 
Target" to "where you don't have a Target" and record (maybe with TSD) that 
this target is using this API on this thread...  Then you could use that to 
send the progress to the right actor.

Note, this is the way Xcode runs lldb, so this isn't a trivial concern.  OTOH, 
it might be more trouble than it's worth to fix.

However, if this is explicitly a "All Debuggers" feature, it might be better to 
have callbacks held outside of any debugger, a static method on SBDebugger, 
perhaps?

I also wonder a little bit why you went with a callback rather than sending 
progress events?  You wouldn't have had to invent a callback mechanism, then.  
Whoever cared about progress events would sign up for the "ProgressEvent" bit 
on their Debugger, and could do what they like.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D97739#2607869 , @jingham wrote:

> This way of doing progress is going to look odd in anything that uses 
> multiple debuggers.  If I'm reading the code aright, if I have two debuggers, 
> and a target in Debugger A starts doing something that would cause progress 
> traffic, both debuggers will show activity.

that is true, but it is a global module repository that benefits both 
debuggers. And I very rarely debug two things at the same time, so most of the 
time for most people this will be beneficial and shouldn't cause too much 
confusion.

We can also add debugger specific progress in the future when we do have a long 
running task that can be associated with a specific debugger.

> Given that the candidate for progress that is most common is down in 
> SymbolFile, it's going to be hard to fix that.  You'd have to have some way 
> of marking the API boundary from "where you have a Target" to "where you 
> don't have a Target" and record (maybe with TSD) that this target is using 
> this API on this thread...  Then you could use that to send the progress to 
> the right actor.

Yeah, we would need to try and catch all of the areas that end up making 
queries just to avoid broadcasting extra progress to multiple debuggers. But 
again, 99.9% of the time people are debugging one thing.

> Note, this is the way Xcode runs lldb, so this isn't a trivial concern.  
> OTOH, it might be more trouble than it's worth to fix.  But we should make 
> clear that each callback will receive events caused by any of the extant 
> debuggers, regardless of which debugger it was registered for.  That very 
> much changes how you would present the results.

It isn't too often that people debug multiple things at the same time. It does 
happen. But it would be ok for the activity in each window to show what is 
going on for things that can't be tied back to a specific debugger since it 
will benefit them all. Kind of like the old Apple mail activity view if anyone 
remembers that, it would be find for each window to show the same thing.

This at least allows us to supply feedback for long running operations where 
clients right now have no way of knowing why the delays are happening and often 
just kill the debug session when it is taking too long. So some feedback is 
much better than none IMHO.

> I also wonder a little bit why you went with a callback rather than sending 
> progress events?  You wouldn't have had to invent a callback mechanism, then. 
>  Whoever cared about progress events would sign up for the "ProgressEvent" 
> bit on their Debugger, and could do what they like.

We can go with SBBroadcaster and SBListener events if that is what everyone 
thinks is the best way forward. SBDebugger currently doesn't vend any events so 
that would need to be added. SBDebugger but does use a logging callback which 
probably could be also converted to SBEvents.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

> SBDebugger currently doesn't vend any events so that would need to be added.

I should clarify this: SBDebugger doesn't have any SBDebugger specific SBEvents 
that it vends or that people can sign up for. It does vend events for the 
targets / processes that it owns, just no SBDebugger specific events.

Anyone else have any opinion on SBEvent versus a callback? I like the SBEvent 
idea because this allows API access to this which means this can be tested 
easily in the python API tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

One serious vote against the SBEvent way of doing things is that it might stop 
progress notification from appearing in the UI immediately if someone is 
currently calling something that causes a long running operation from the main 
thread that is running the SBEvent loop...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

I think that the mechanism used to show progress should be lock-free and mostly 
unblocked, as it's merely an observer without side effects within LLDB. So from 
the last thing you said it seems that SBEvent doesn't like a good solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

I agree that we should avoid SBEvent after thinking about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

In D97739#2607993 , @clayborg wrote:

> I agree that we should avoid SBEvent after thinking about it.

First off, doing long running operations on a thread that is the one handling 
the major lldb events is obviously a bad idea.  Secondly, there's no reason 
that the listener for progress events has to call "WaitForEvents" along with 
all the process events it's waiting for on the debugger.  You could just set up 
a listener thread for all these events (they would have their own event bit) if 
you wanted somebody just monitoring progress events.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D97739#2608066 , @jingham wrote:

> In D97739#2607993 , @clayborg wrote:
>
>> I agree that we should avoid SBEvent after thinking about it.
>
> First off, doing long running operations on a thread that is the one handling 
> the major lldb events is obviously a bad idea.  The driver doesn't do it this 
> way.  Commands get run on the main thread, and events get processed on the 
> event-handler thread.

True, but the diver isn't a great example of using the API like an IDE would. 
And anything can kick off expensive events in the debugger. Xcode, as you know, 
as a thread that listens for events and does process control. This process 
control thread is responsible for many things including fetching the frame 
variables when needed. So just the act of having a dynamic type that needs to 
lookup a class by name could cause this expensive event to trigger. And with 
many other APIs, you never know what might trigger some expensive lookup to 
happen. Just the act of consuming a process stopped event on the process 
control thread is enough to trigger a python script or other LLDB commands in 
the event handler itself that could cause the expensive event to trigger and 
cause long delays. So there are plenty of reasons that this could cause delays.

> Secondly, there's no reason that the listener for progress events has to call 
> "WaitForEvents" along with all the process events it's waiting for on the 
> debugger.  You could just set up a listener thread for all these events (they 
> would have their own event bit) if you wanted somebody just monitoring 
> progress events.

That is true. But spinning up a thread just to listen for progress events seems 
like a bit of overkill, but it can easily be done.

> It also has the advantage that callbacks can't stall the progress of lldb, 
> since the event just gets fired off, and then you go on your way...

Using events does also has the advantage being able to receive all progress 
events on the same thread. Though since most GUI programs can only draw to the 
window server on the main thread, they will still need to setup things to 
forward these events to the main thread. Same goes for the callback methods 
though.

I am happy to try the SBEvent approach if anyone else chimes in


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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


[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

2021-03-05 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment.

In D97739#2607961 , @clayborg wrote:

> In D97739#2607869 , @jingham wrote:
>
>> This way of doing progress is going to look odd in anything that uses 
>> multiple debuggers.  If I'm reading the code aright, if I have two 
>> debuggers, and a target in Debugger A starts doing something that would 
>> cause progress traffic, both debuggers will show activity.
>
> that is true, but it is a global module repository that benefits both 
> debuggers. And I very rarely debug two things at the same time, so most of 
> the time for most people this will be beneficial and shouldn't cause too much 
> confusion.

Just one tidbit here. Most users are actually routinely running tens of 
debuggers at the same time, because tests run in parallel and they have a 
debugger attached by default. Now if you have a long running operation kick in 
in your unit tests, you might already have a different kind of issue, but I’d 
like to avoid a world where the IDE displays spurious and wrong information 
because of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

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