[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-02-27 Thread Erich Keane via Phabricator via lldb-commits 
erichkeane added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:3036
 if (!std::is_trivially_destructible::value) {
-  auto DestroyPtr = [](void *V) { static_cast(V)->~T(); };
-  AddDeallocation(DestroyPtr, Ptr);
+  auto DestroyPtr = [](void *V) { ((T *)V)->~T(); };
+  AddDeallocation(DestroyPtr, (void *)Ptr);

bolshakov-a wrote:
> erichkeane wrote:
> > This change is weird... what is going on here?
> Here is not very beautiful attempt to workaround const-ness of 
> `TemplateArgument::V::Value` pointer passed here from the added 
> `TemplateArgument` constructor. The change in this line isn't acually needed 
> and made only for consistence with the next line, I think. Alternatively, I 
> can
> 1) refactor `addDestruction` and `AddDeallocation` to work with pointers to 
> constants, or
> 2) add `const_cast` to `AddDeallocation` call in the next line, or
> 3) make `TemplateArgument::V::Value` pointer non-const.
> 
> I'm biased to the first variant.
I'd lean towards #3, it ends up being consistent with the rest of the things 
here.  #1 is interesting, but that results in these functions violating 
const-correctness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140996

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

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-27 Thread Dan Liew via Phabricator via lldb-commits 
delcypher added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1316
+auto_generated_function.AppendString("def __user_code():");
+for (int i = 0; i < num_lines; ++i) {
+  sstr.Clear();

Why do we need to loop over multiple lines in this is a "oneliner"?

Is `num_lines == 1` equivalent to `is_oneliner`?


If yes, then the `is_oneliner` parameter is not needed.
If no, then `is_oneliner` should probably be renamed to make sure this 
confusion cannot be made.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1330
+} else {
+  return 
Status("ScriptInterpreterPythonImpl::GenerateFunction(is_oneliner="
+"false) = ERROR: python function is multiline.");

Why are we erroring here? It looks like this is something that the old code 
supported. I would is expect `is_oneliner` to imply there are multiple lines in 
which case shouldn't we support that here?


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

https://reviews.llvm.org/D144688

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

[Lldb-commits] [PATCH] D144904: [Linux] Add kernel.yama.ptrace_scope note when ptrace fails to attach.

2023-02-27 Thread Jordan Rupprecht via Phabricator via lldb-commits 
rupprecht created this revision.
rupprecht added reviewers: jingham, DavidSpickett, labath.
Herald added a project: All.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

A common reason for LLDB failing to attach to an already-running process on 
Linux is the Yama security module: 
https://www.kernel.org/doc/Documentation/security/Yama.txt. This patch adds an 
explaination and suggested fix when it detects that case happening.

This was previously proposed in D106226 , but 
hasn't been updated in a while. The last request was to put the check in a 
target-specific location, which is the approach this patch takes. I believe 
Yama only exists on Linux, so it's put in that package.

This has no end-to-end test because I'm not sure how to set `ptrace_scope` in a 
test environment -- if there are suggestions on how to do that, I'd be happy to 
add it. (Also, setting it to `3` is comically irreversible). I tested this 
locally.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144904

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/Procfs.cpp
  lldb/source/Plugins/Process/Linux/Procfs.h
  lldb/unittests/Process/Linux/ProcfsTests.cpp

Index: lldb/unittests/Process/Linux/ProcfsTests.cpp
===
--- lldb/unittests/Process/Linux/ProcfsTests.cpp
+++ lldb/unittests/Process/Linux/ProcfsTests.cpp
@@ -102,3 +102,19 @@
   ASSERT_TRUE((bool)cpu_ids);
   ASSERT_GT((int)cpu_ids->size(), 0) << "We must see at least one core";
 }
+
+TEST(Perf, RealPtraceScope) {
+  // We first check we can read /proc/sys/kernel/yama/ptrace_scope
+  auto buffer_or_error =
+  errorOrToExpected(getProcFile("sys/kernel/yama/ptrace_scope"));
+  if (!buffer_or_error)
+GTEST_SKIP() << toString(buffer_or_error.takeError());
+
+  // At this point we shouldn't fail parsing the core ids
+  Expected ptrace_scope = GetPtraceScope();
+  ASSERT_TRUE((bool)ptrace_scope) << ptrace_scope.takeError();
+  ASSERT_GE(*ptrace_scope, 0)
+  << "Sensible values of ptrace_scope are between 0 and 3";
+  ASSERT_LE(*ptrace_scope, 3)
+  << "Sensible values of ptrace_scope are between 0 and 3";
+}
Index: lldb/source/Plugins/Process/Linux/Procfs.h
===
--- lldb/source/Plugins/Process/Linux/Procfs.h
+++ lldb/source/Plugins/Process/Linux/Procfs.h
@@ -28,5 +28,11 @@
 /// if errors didn't happen.
 llvm::Expected> GetAvailableLogicalCoreIDs();
 
+/// \return
+/// The current value of /proc/sys/kernel/yama/ptrace_scope, parsed as an
+/// integer, or an error if the proc file cannot be read or has non-integer
+/// contents.
+llvm::Expected GetPtraceScope();
+
 } // namespace process_linux
 } // namespace lldb_private
Index: lldb/source/Plugins/Process/Linux/Procfs.cpp
===
--- lldb/source/Plugins/Process/Linux/Procfs.cpp
+++ lldb/source/Plugins/Process/Linux/Procfs.cpp
@@ -8,6 +8,7 @@
 
 #include "Procfs.h"
 #include "lldb/Host/linux/Support.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Threading.h"
 #include 
@@ -68,3 +69,18 @@
   }
   return *logical_cores_ids;
 }
+
+llvm::Expected lldb_private::process_linux::GetPtraceScope() {
+  ErrorOr> ptrace_scope_file =
+  getProcFile("sys/kernel/yama/ptrace_scope");
+  if (!*ptrace_scope_file)
+return errorCodeToError(ptrace_scope_file.getError());
+  // The contents should be something like "1\n". Trim it so we get "1".
+  StringRef buffer = (*ptrace_scope_file)->getBuffer().trim();
+  int ptrace_scope_value;
+  if (buffer.getAsInteger(10, ptrace_scope_value)) {
+return createStringError(inconvertibleErrorCode(),
+ "Invalid ptrace_scope value: '%s'", buffer.data());
+  }
+  return ptrace_scope_value;
+}
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Utility/StringExtractor.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 
@@ -214,6 +215,43 @@
   return error;
 }
 
+static llvm::Error AddPtraceScopeNote(llvm::Error original_error) {
+  Expected ptrace_scope = GetPtraceScope();
+  if (auto E = ptrace_scope.takeError()) {
+Log *log = GetLog(POSIXLog::Process);
+LLDB_LOG(log, "error reading value of ptrace_scope: {0}", E);
+
+// The original error is probably more interesting than not being able to
+// read or interpret ptrace_scope.
+return original_error;
+  }
+
+  // We only have s

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-27 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1316
+auto_generated_function.AppendString("def __user_code():");
+for (int i = 0; i < num_lines; ++i) {
+  sstr.Clear();

delcypher wrote:
> Why do we need to loop over multiple lines in this is a "oneliner"?
> 
> Is `num_lines == 1` equivalent to `is_oneliner`?
> 
> 
> If yes, then the `is_oneliner` parameter is not needed.
> If no, then `is_oneliner` should probably be renamed to make sure this 
> confusion cannot be made.
+1



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1330
+} else {
+  return 
Status("ScriptInterpreterPythonImpl::GenerateFunction(is_oneliner="
+"false) = ERROR: python function is multiline.");

delcypher wrote:
> Why are we erroring here? It looks like this is something that the old code 
> supported. I would is expect `is_oneliner` to imply there are multiple lines 
> in which case shouldn't we support that here?
+1


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

https://reviews.llvm.org/D144688

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

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-27 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1316
+auto_generated_function.AppendString("def __user_code():");
+for (int i = 0; i < num_lines; ++i) {
+  sstr.Clear();

bulbazord wrote:
> delcypher wrote:
> > Why do we need to loop over multiple lines in this is a "oneliner"?
> > 
> > Is `num_lines == 1` equivalent to `is_oneliner`?
> > 
> > 
> > If yes, then the `is_oneliner` parameter is not needed.
> > If no, then `is_oneliner` should probably be renamed to make sure this 
> > confusion cannot be made.
> +1
You can do:

(lldb) breakpoint command add -s python -o "first_python_line" -o 
"second_python_line"

which give you a "one-line" command with two lines.


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

https://reviews.llvm.org/D144688

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

[Lldb-commits] [PATCH] D144929: Add SBCommandInterpreter::UserCommandExists

2023-02-27 Thread Jim Ingham via Phabricator via lldb-commits 
jingham created this revision.
jingham added reviewers: JDevlieghere, mib.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Turns out there's no API to find out if a user command exists.  There's 
SBCommandInterpreter::CommandExists but that only checks for built-in commands. 
 I needed this to write a test for an upcoming patch, and it's a useful thing 
to know.  I added docs so it's clear CommandExists is for builtin commands and 
this one is for `command script add` commands.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144929

Files:
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/test/API/commands/command/script/TestCommandScript.py


Index: lldb/test/API/commands/command/script/TestCommandScript.py
===
--- lldb/test/API/commands/command/script/TestCommandScript.py
+++ lldb/test/API/commands/command/script/TestCommandScript.py
@@ -19,6 +19,11 @@
 def pycmd_tests(self):
 self.runCmd("command source py_import")
 
+# Test that we did indeed add these commands as user commands:
+interp = self.dbg.GetCommandInterpreter()
+self.expectTrue(interp.UserCommandExists("foobar"), "foobar exists")
+self.expectFalse(interp.CommandExists("foobar"), "It is not a 
builtin.")
+
 # Test a bunch of different kinds of python callables with
 # both 4 and 5 positional arguments.
 self.expect("foobar", substrs=["All good"])
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -118,6 +118,13 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_INSTRUMENT_VA(this, cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? 
m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_INSTRUMENT_VA(this, cmd);
 
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -45,8 +45,34 @@
 
   bool IsValid() const;
 
+  /// Return whether a built-in command with the passed in
+  /// name or command path exists.
+  ///
+  /// \param[in] cmd
+  ///   The command or command path to search for.
+  ///
+  /// \return
+  ///   \b true if the command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Return whether a user defined command with the passed in
+  /// name or command path exists.
+  ///
+  /// \param[in] cmd
+  ///   The command or command path to search for.
+  ///
+  /// \return
+  ///   \b true if the command exists, \b false otherwise.
+  bool UserCommandExists(const char *cmd);
+
+  /// Return whether the passed in name or command path
+  /// exists and is an alias to some other command.
+  ///
+  /// \param[in] cmd
+  ///   The command or command path to search for.
+  ///
+  /// \return
+  ///   \b true if the command exists, \b false otherwise.
   bool AliasExists(const char *cmd);
 
   lldb::SBBroadcaster GetBroadcaster();


Index: lldb/test/API/commands/command/script/TestCommandScript.py
===
--- lldb/test/API/commands/command/script/TestCommandScript.py
+++ lldb/test/API/commands/command/script/TestCommandScript.py
@@ -19,6 +19,11 @@
 def pycmd_tests(self):
 self.runCmd("command source py_import")
 
+# Test that we did indeed add these commands as user commands:
+interp = self.dbg.GetCommandInterpreter()
+self.expectTrue(interp.UserCommandExists("foobar"), "foobar exists")
+self.expectFalse(interp.CommandExists("foobar"), "It is not a builtin.")
+
 # Test a bunch of different kinds of python callables with
 # both 4 and 5 positional arguments.
 self.expect("foobar", substrs=["All good"])
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -118,6 +118,13 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_INSTRUMENT_VA(this, cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_INSTRUMENT_VA(this, cmd);
 
Index: lldb/include/lldb/API/SBCommandInterpreter.h
=

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-27 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib updated this revision to Diff 500975.
mib added a comment.

Rename `is_oneliner` flag to `is_callback` and swap the logic.


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

https://reviews.llvm.org/D144688

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBBreakpoint.cpp
  lldb/source/API/SBBreakpointLocation.cpp
  lldb/source/API/SBBreakpointName.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  
lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
  lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
  
lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py

Index: lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
===
--- lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
+++ lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
@@ -9,7 +9,12 @@
 print ("I stopped the first time")
 frame.EvaluateExpression("cookie = 888")
 num_hits += 1
-frame.thread.process.Continue()
+return True
+if num_hits == 1:
+print ("I stopped the second time, but with no return")
+frame.EvaluateExpression("cookie = 666")
+num_hits += 1
 else:
 print ("I stopped the %d time" % (num_hits))
 frame.EvaluateExpression("cookie = 999")
+return False # This cause the process to continue.
Index: lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
===
--- lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
+++ lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
@@ -16,5 +16,5 @@
 modify(global);
 
 printf("global=%d\n", global);
-printf("cookie=%d\n", cookie);
+printf("cookie=%d\n", cookie); // Set another breakpoint here.
 }
Index: lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
===
--- lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
+++ lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
@@ -1,4 +1,4 @@
-"""
+"""
 Test 'watchpoint command'.
 """
 
@@ -22,6 +22,8 @@
 # Find the line number to break inside main().
 self.line = line_number(
 self.source, '// Set break point at this line.')
+self.second_line = line_number(
+self.source, '// Set another breakpoint here.')
 # And the watchpoint variable declaration line number.
 self.decl = line_number(self.source,
 '// Watchpoint variable declaration.')
@@ -143,6 +145,32 @@
 self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT,
 substrs=['stop reason = watchpoint'])
 
+# We should have hit the watchpoint once, set cookie to 888, since the
+# user callback returned True.
+self.expect("frame variable --show-globals cookie",
+substrs=['(int32_t)', 'cookie = 888'])
+
+self.runCmd("process continue")
+
+# We should be stopped again due to the watchpoint (write type).
+# The stop reason of the thread should be watchpoint.
+self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT,
+substrs=['stop reason = watchpoint'])
+
+# We should have hit the watchpoint a second time, set cookie to 666,
+# even if the user callback didn't return anything and then continue.
+self.expect("frame variable --show-globals cookie",
+substrs=['(int32_t)', 'cookie = 666'])
+
+# Add a breakpoint to set a watchpoint when stopped on the breakpoint.
+lldbutil.run_break_set_by_file_and_line(
+self, None, self.second_line, num_expected_locations=1)
+
+self.runCmd("process continue")
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stop reason = breakpoint'])
+
 # We should have hit the watchpoint once, set cookie to 888, then continued to the
 # second hit and set it to 999
 self.expect("frame variable --show-globals cookie",
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPyt

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-27 Thread Dan Liew via Phabricator via lldb-commits 
delcypher created this revision.
Herald added a project: All.
delcypher requested review of this revision.
Herald added a project: LLDB.

This patch adds the following methods:

- `GetType()`
- `IsWatchVariable()`
- `GetWatchSpec()`

These effectively expose methods that `lldb_private::Watchpoint` already
had. Tests are included that exercise these new methods.

The motivation for exposing these are as follows:

- `GetType()` - With this information and the address from a watchpoint it is 
now possible to construct an SBValue from an SBWatchpoint. Previously this 
wasn't possible. The included test case illustrates doing this.
- `IsWatchVariable()` - This allows the caller to determine whether the 
watchpoint is a variable watchpoint or an expression watchpoint.
- `GetWatchSpec()` - This allows (at least for variable watchpoints) to use a 
sensible name for SBValues created from an SBWatchpoint.

The implementation of `GetWatchSpec()` isn't ideal. It currently caches
the string because `lldb_private::Watchpoint` does not publicly expose
the underlying memory its using to store the spec string. This is
probably fine though because the cost for this is only really paid if
`GetWatchSpec()` is called (unlikely given that its a new API).

rdar://105606978

Reviewersa jingham, mib, bulbazord, jasonmolenda, JDevlieghere


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144937

Files:
  lldb/bindings/interface/SBWatchpointDocstrings.i
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/API/SBWatchpoint.h
  lldb/source/API/SBWatchpoint.cpp
  lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
  lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py

Index: lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
===
--- lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
+++ lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
@@ -64,6 +64,11 @@
 self.DebugSBValue(value)
 self.DebugSBValue(pointee)
 
+# Check some API calls for expression watchpoint
+self.assertFalse(watchpoint.IsWatchVariable())
+self.assertEqual(watchpoint.GetWatchSpec(), '')
+self.assertEqual(watchpoint.GetType().GetDisplayTypeName(), 'char')
+
 # Hide stdout if not running with '-t' option.
 if not self.TraceOn():
 self.HideStdout()
Index: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
===
--- lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
+++ lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
@@ -19,12 +19,24 @@
 # Find the line number to break inside main().
 self.line = line_number(
 self.source, '// Set break point at this line.')
+self.build()
 
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
 def test_watch_val(self):
 """Exercise SBValue.Watch() API to set a watchpoint."""
-self.build()
+self._test_watch_val(variable_watchpoint=False)
+pass
+
+@expectedFailureAll(archs=['s390x'])
+def test_watch_variable(self):
+"""
+   Exercise some watchpoint APIs when the watchpoint
+   is created as a variable watchpoint.
+"""
+self._test_watch_val(variable_watchpoint=True)
+
+def _test_watch_val(self, variable_watchpoint):
 exe = self.getBuildArtifact("a.out")
 
 # Create a target by the debugger.
@@ -50,12 +62,27 @@
 frame0 = thread.GetFrameAtIndex(0)
 
 # Watch 'global' for read and write.
-value = frame0.FindValue('global', lldb.eValueTypeVariableGlobal)
-error = lldb.SBError()
-watchpoint = value.Watch(True, True, True, error)
-self.assertTrue(value and watchpoint,
-"Successfully found the variable and set a watchpoint")
-self.DebugSBValue(value)
+if variable_watchpoint:
+# FIXME: There should probably be an API to create a variable watchpoint
+self.runCmd('watchpoint set variable -w read_write -- global')
+watchpoint = target.GetWatchpointAtIndex(0)
+self.assertTrue(watchpoint.IsWatchVariable())
+self.assertEqual(watchpoint.GetWatchSpec(), 'global')
+# Synthesize an SBValue from the watchpoint
+watchpoint_addr = lldb.SBAddress(watchpoint.GetWatchAddress(), target)
+value = target.CreateValueFromAddress(
+watchpoint.GetWatchSpec(), watchpoint_addr, watchpoint.GetType())
+else:
+value = frame0.FindValue('global', lldb.eValueTypeVariableGlobal)
+error = lldb.SBError()
+watchpoint = value.Watch(True, True, True, error)
+self.assertTrue(value and watchpoint,
+

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-27 Thread Dan Liew via Phabricator via lldb-commits 
delcypher updated this revision to Diff 500993.
delcypher added a comment.
Herald added a subscriber: JDevlieghere.

Fix reviewers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144937

Files:
  lldb/bindings/interface/SBWatchpointDocstrings.i
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/API/SBWatchpoint.h
  lldb/source/API/SBWatchpoint.cpp
  lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
  lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py

Index: lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
===
--- lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
+++ lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
@@ -64,6 +64,11 @@
 self.DebugSBValue(value)
 self.DebugSBValue(pointee)
 
+# Check some API calls for expression watchpoint
+self.assertFalse(watchpoint.IsWatchVariable())
+self.assertEqual(watchpoint.GetWatchSpec(), '')
+self.assertEqual(watchpoint.GetType().GetDisplayTypeName(), 'char')
+
 # Hide stdout if not running with '-t' option.
 if not self.TraceOn():
 self.HideStdout()
Index: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
===
--- lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
+++ lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
@@ -19,12 +19,24 @@
 # Find the line number to break inside main().
 self.line = line_number(
 self.source, '// Set break point at this line.')
+self.build()
 
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
 def test_watch_val(self):
 """Exercise SBValue.Watch() API to set a watchpoint."""
-self.build()
+self._test_watch_val(variable_watchpoint=False)
+pass
+
+@expectedFailureAll(archs=['s390x'])
+def test_watch_variable(self):
+"""
+   Exercise some watchpoint APIs when the watchpoint
+   is created as a variable watchpoint.
+"""
+self._test_watch_val(variable_watchpoint=True)
+
+def _test_watch_val(self, variable_watchpoint):
 exe = self.getBuildArtifact("a.out")
 
 # Create a target by the debugger.
@@ -50,12 +62,27 @@
 frame0 = thread.GetFrameAtIndex(0)
 
 # Watch 'global' for read and write.
-value = frame0.FindValue('global', lldb.eValueTypeVariableGlobal)
-error = lldb.SBError()
-watchpoint = value.Watch(True, True, True, error)
-self.assertTrue(value and watchpoint,
-"Successfully found the variable and set a watchpoint")
-self.DebugSBValue(value)
+if variable_watchpoint:
+# FIXME: There should probably be an API to create a variable watchpoint
+self.runCmd('watchpoint set variable -w read_write -- global')
+watchpoint = target.GetWatchpointAtIndex(0)
+self.assertTrue(watchpoint.IsWatchVariable())
+self.assertEqual(watchpoint.GetWatchSpec(), 'global')
+# Synthesize an SBValue from the watchpoint
+watchpoint_addr = lldb.SBAddress(watchpoint.GetWatchAddress(), target)
+value = target.CreateValueFromAddress(
+watchpoint.GetWatchSpec(), watchpoint_addr, watchpoint.GetType())
+else:
+value = frame0.FindValue('global', lldb.eValueTypeVariableGlobal)
+error = lldb.SBError()
+watchpoint = value.Watch(True, True, True, error)
+self.assertTrue(value and watchpoint,
+"Successfully found the variable and set a watchpoint")
+self.DebugSBValue(value)
+self.assertFalse(watchpoint.IsWatchVariable())
+self.assertEqual(watchpoint.GetWatchSpec(), '')
+
+self.assertEqual(watchpoint.GetType().GetDisplayTypeName(), 'int32_t')
 
 # Hide stdout if not running with '-t' option.
 if not self.TraceOn():
Index: lldb/source/API/SBWatchpoint.cpp
===
--- lldb/source/API/SBWatchpoint.cpp
+++ lldb/source/API/SBWatchpoint.cpp
@@ -17,6 +17,7 @@
 #include "lldb/Breakpoint/Watchpoint.h"
 #include "lldb/Breakpoint/WatchpointList.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Symbol/CompilerType.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Stream.h"
@@ -29,12 +30,13 @@
 SBWatchpoint::SBWatchpoint() { LLDB_INSTRUMENT_VA(this); }
 
 SBWatchpoint::SBWatchpoint(const lldb::WatchpointSP &wp_sp)
-: m_opaque_wp(wp_sp) {
+: m_opaque_wp(wp_sp), m_cached_watch_spec() {
   LLDB_INSTRUMENT_VA(this, wp_sp);
 }
 
 SBWatchpoint::SBWatchpoin

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-27 Thread Dan Liew via Phabricator via lldb-commits 
delcypher updated this revision to Diff 500994.
delcypher added a comment.

Fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144937

Files:
  lldb/bindings/interface/SBWatchpointDocstrings.i
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/API/SBWatchpoint.h
  lldb/source/API/SBWatchpoint.cpp
  lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
  lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py

Index: lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
===
--- lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
+++ lldb/test/API/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
@@ -64,6 +64,11 @@
 self.DebugSBValue(value)
 self.DebugSBValue(pointee)
 
+# Check some API calls for expression watchpoint
+self.assertFalse(watchpoint.IsWatchVariable())
+self.assertEqual(watchpoint.GetWatchSpec(), '')
+self.assertEqual(watchpoint.GetType().GetDisplayTypeName(), 'char')
+
 # Hide stdout if not running with '-t' option.
 if not self.TraceOn():
 self.HideStdout()
Index: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
===
--- lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
+++ lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py
@@ -19,12 +19,24 @@
 # Find the line number to break inside main().
 self.line = line_number(
 self.source, '// Set break point at this line.')
+self.build()
 
 # Read-write watchpoints not supported on SystemZ
 @expectedFailureAll(archs=['s390x'])
 def test_watch_val(self):
 """Exercise SBValue.Watch() API to set a watchpoint."""
-self.build()
+self._test_watch_val(variable_watchpoint=False)
+pass
+
+@expectedFailureAll(archs=['s390x'])
+def test_watch_variable(self):
+"""
+   Exercise some watchpoint APIs when the watchpoint
+   is created as a variable watchpoint.
+"""
+self._test_watch_val(variable_watchpoint=True)
+
+def _test_watch_val(self, variable_watchpoint):
 exe = self.getBuildArtifact("a.out")
 
 # Create a target by the debugger.
@@ -50,12 +62,27 @@
 frame0 = thread.GetFrameAtIndex(0)
 
 # Watch 'global' for read and write.
-value = frame0.FindValue('global', lldb.eValueTypeVariableGlobal)
-error = lldb.SBError()
-watchpoint = value.Watch(True, True, True, error)
-self.assertTrue(value and watchpoint,
-"Successfully found the variable and set a watchpoint")
-self.DebugSBValue(value)
+if variable_watchpoint:
+# FIXME: There should probably be an API to create a variable watchpoint
+self.runCmd('watchpoint set variable -w read_write -- global')
+watchpoint = target.GetWatchpointAtIndex(0)
+self.assertTrue(watchpoint.IsWatchVariable())
+self.assertEqual(watchpoint.GetWatchSpec(), 'global')
+# Synthesize an SBValue from the watchpoint
+watchpoint_addr = lldb.SBAddress(watchpoint.GetWatchAddress(), target)
+value = target.CreateValueFromAddress(
+watchpoint.GetWatchSpec(), watchpoint_addr, watchpoint.GetType())
+else:
+value = frame0.FindValue('global', lldb.eValueTypeVariableGlobal)
+error = lldb.SBError()
+watchpoint = value.Watch(True, True, True, error)
+self.assertTrue(value and watchpoint,
+"Successfully found the variable and set a watchpoint")
+self.DebugSBValue(value)
+self.assertFalse(watchpoint.IsWatchVariable())
+self.assertEqual(watchpoint.GetWatchSpec(), '')
+
+self.assertEqual(watchpoint.GetType().GetDisplayTypeName(), 'int32_t')
 
 # Hide stdout if not running with '-t' option.
 if not self.TraceOn():
Index: lldb/source/API/SBWatchpoint.cpp
===
--- lldb/source/API/SBWatchpoint.cpp
+++ lldb/source/API/SBWatchpoint.cpp
@@ -17,6 +17,7 @@
 #include "lldb/Breakpoint/Watchpoint.h"
 #include "lldb/Breakpoint/WatchpointList.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Symbol/CompilerType.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Stream.h"
@@ -29,12 +30,13 @@
 SBWatchpoint::SBWatchpoint() { LLDB_INSTRUMENT_VA(this); }
 
 SBWatchpoint::SBWatchpoint(const lldb::WatchpointSP &wp_sp)
-: m_opaque_wp(wp_sp) {
+: m_opaque_wp(wp_sp), m_cached_watch_spec() {
   LLDB_INSTRUMENT_VA(this, wp_sp);
 }
 
 SBWatchpoint::SBWatchpoint(const SBWatchpoint &rhs)
-: m_opaque_wp(

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-27 Thread Dan Liew via Phabricator via lldb-commits 
delcypher accepted this revision.
delcypher added a comment.

@mib Thanks for working on this. LGTM (with very minor nits), but you should 
probably wait for someone who works on LLDB more frequently than me to give you 
the ok.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1310
   auto_generated_function.AppendString(
-  " global_dict.update (internal_dict)"); // Add the session dictionary
-  // to the
-  // global dictionary.
-
-  // Wrap everything up inside the function, increasing the indentation.
-
-  auto_generated_function.AppendString(" if True:");
-  for (int i = 0; i < num_lines; ++i) {
-sstr.Clear();
-sstr.Printf("   %s", input.GetStringAtIndex(i));
-auto_generated_function.AppendString(sstr.GetData());
+  "global_dict.update (internal_dict)"); // Add the session dictionary
+ // to the global dictionary.

Nit: the space between `.update` and `(internal_dict)` is probably an old 
mistake. We can probably tidy that up seeing as we're already touching the line.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1328
+"__return_val = None"); // Initialize user callback return value.
+auto_generated_function.AppendString("def __user_code():");
+for (int i = 0; i < num_lines; ++i) {

Nit: Might be worth leaving a comment here on why we're wrapping the code in a 
nested function.


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

https://reviews.llvm.org/D144688

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