[Lldb-commits] [PATCH] D144688: [lldb] Fix watchpoint command function stopping behavior

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This patch fixes an issue where we would discard the return value of the
user-provided function for watchpoint commands.

That causes lldb to not to take into account the function expected
stopping behavior. To prevent that, this patch prepends  a return
statement to the function call, to ensure we behave as the user wanted.

By doing so, watchpoint commands now behave the same way was breakpoint
commands.

rdar://105461140

Signed-off-by: Med Ismail Bennani <medismail.bennani at gmail.com>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144688

Files:
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  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,8 @@
         print ("I stopped the first time")
         frame.EvaluateExpression("cookie = 888")
         num_hits += 1
-        frame.thread.process.Continue()
+        return True
     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,20 @@
         self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT,
                     substrs=['stop reason = watchpoint'])
 
+        # 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",
+                    substrs=['(int32_t)', 'cookie = 888'])
+
+        # 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/Commands/CommandObjectWatchpointCommand.cpp
===================================================================
--- lldb/source/Commands/CommandObjectWatchpointCommand.cpp
+++ lldb/source/Commands/CommandObjectWatchpointCommand.cpp
@@ -422,7 +422,8 @@
           // automatize what the user would do manually: make their watchpoint
           // command be a function call
           else if (!m_options.m_function_name.empty()) {
-            std::string oneliner(m_options.m_function_name);
+            std::string oneliner("return ");
+            oneliner += m_options.m_function_name;
             oneliner += "(frame, wp, internal_dict)";
             script_interp->SetWatchpointCommandCallback(
                 wp_options, oneliner.c_str());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144688.500016.patch
Type: text/x-patch
Size: 3820 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230224/c0210084/attachment-0001.bin>