[lldb/Command] Add --force option for `watchpoint delete` command

[mib]

Summary

[lldb/Command] Add --force option for watchpoint delete command

Currently, there is no option to delete all the watchpoint without LLDB
asking for a confirmation. Besides making the watchpoint delete command
homogeneous with the breakpoint delete command, this option could also
become handy to trigger automated watchpoint deletion i.e. using
breakpoint actions.

rdar://42560586

Signed-off-by: Med Ismail Bennani <[e-mail]>

[JDevlieghere]

LGTM with the inline comment(s) addressed.

lldb/source/Commands/CommandObjectWatchpoint.cpp

463

Given that error is never actually populated, it might be nice to use return {} to make it clear that we're returning a default constructed instance.

529

I know it's not code you touched but moving this into the first if-clause would allow for an early return.

[friss]

lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py

167–184

If you cannot reuse another test (see bellow), all the setup can be replaced by lldbutil.run_to_line_breakpoint or lldbutil.run_to_source_breakpoint

199–201

Can't we just add this to the end of another test without spinning up a new process?

Did you verify that the test failed before your patch? I'm asking because I don't know what m_interpreter.Confirm() does when there's no PTY connected. Does it default to no or yes?

204

You don't test the result of this command, so you don't actually test that deleting the breakpoints really happened. Is there an SB API to list watchpoints? If there is, it would be a better fit for this test instead of matching the output of a command.

208–211

I see, this is the actual test that no watchpoints are present. I'm fine with adding this new test, I think testing it this way makes sense, but please also find a way to make sure the list of watchpoints is empty.

[mib]

Merged --force flag and auto-confirm setting test.
Refactored implementation.

[mib]

lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py

199–201

I merge both tests (auto-confirm enabled & --force flag) into one.

FWIU, m_interpreter.Confirm(message, default_answer) checks first the auto-confirm setting, When enabled, it returns the default answer (true in this case) otherwise, it triggers the IOHandler for the user input.

lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py

def test_rw_watchpoint(self):
         # The hit count should now be 2.
         self.expect("watchpoint list -v",
                     substrs=['hit_count = 2'])
 
     # Read-write watchpoints not supported on SystemZ
     @expectedFailureAll(archs=['s390x'])
     def test_rw_watchpoint_delete(self):
         """Test delete watchpoint and expect not to stop for watchpoint."""
-        self.build(dictionary=self.d)
-        self.setTearDownCleanup(dictionary=self.d)
-
-        exe = self.getBuildArtifact(self.exe_name)
-        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
-
-        # Add a breakpoint to set a watchpoint when stopped on the breakpoint.
-        lldbutil.run_break_set_by_file_and_line(
-            self, None, self.line, num_expected_locations=1)
-
-        # Run the program.
-        self.runCmd("run", RUN_SUCCEEDED)
-
-        # We should be stopped again due to the breakpoint.
-        # The stop reason of the thread should be breakpoint.
-        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-                    substrs=['stopped',
-                             'stop reason = breakpoint'])
+        self.build()
+        lldbutil.run_to_line_breakpoint(self, lldb.SBFileSpec(self.source),
+                                        self.line)
 
         # Now let's set a read_write-type watchpoint for 'global'.
         # There should be two watchpoint hits (see main.c).
         self.expect(
             "watchpoint set variable -w read_write global",
             WATCHPOINT_CREATED,
             substrs=[
                 'Watchpoint created',
                 'size = 4',
                 'type = rw',
                 '%s:%d' %
                 (self.source,
                  self.decl)])
 
         # Delete the watchpoint immediately, but set auto-confirm to true
         # first.
         self.runCmd("settings set auto-confirm true")
         self.expect("watchpoint delete",
                     substrs=['All watchpoints removed.'])
         # Restore the original setting of auto-confirm.
         self.runCmd("settings clear auto-confirm")
 
-        # Use the '-v' option to do verbose listing of the watchpoint.
-        self.runCmd("watchpoint list -v")
+        target = self.dbg.GetSelectedTarget()
+        self.assertTrue(target and not target.GetNumWatchpoints())
+
+        # Now let's set a read_write-type watchpoint for 'global'.
+        # There should be two watchpoint hits (see main.c).
+        self.expect(
+            "watchpoint set variable -w read_write global",
+            WATCHPOINT_CREATED,
+            substrs=[
+                'Watchpoint created',
+                'size = 4',
+                'type = rw',
+                '%s:%d' %
+                (self.source,
+                 self.decl)])
+
+
+        # Delete the watchpoint immediately using the force option.
+        self.expect("watchpoint delete --force",
+                    substrs=['All watchpoints removed.'])
+
+        self.assertTrue(target and not target.GetNumWatchpoints())
 
         self.runCmd("process continue")
 
         # There should be no more watchpoint hit and the process status should
         # be 'exited'.
         self.expect("process status",
                     substrs=['exited'])
 
     # Read-write watchpoints not supported on SystemZ
     @expectedFailureAll(archs=['s390x'])
     def test_rw_watchpoint_set_ignore_count(self):
         """Test watchpoint ignore count and expect to not to stop at all."""
         self.build(dictionary=self.d)
         self.setTearDownCleanup(dictionary=self.d)
 
         exe = self.getBuildArtifact(self.exe_name)
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         # Add a breakpoint to set a watchpoint when stopped on the breakpoint.
         lldbutil.run_break_set_by_file_and_line(
             self, None, self.line, num_expected_locations=1)
 
         # Run the program.
         self.runCmd("run", RUN_SUCCEEDED)
 
         # We should be stopped again due to the breakpoint.
         # The stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped',
                              'stop reason = breakpoint'])
 
         # Now let's set a read_write-type watchpoint for 'global'.
         # There should be two watchpoint hits (see main.c).
         self.expect(
             "watchpoint set variable -w read_write global",
             WATCHPOINT_CREATED,
             substrs=[
                 'Watchpoint created',
                 'size = 4',
                 'type = rw',
                 '%s:%d' %
                 (self.source,
                  self.decl)])
 
         # Set the ignore count of the watchpoint immediately.
         self.expect("watchpoint ignore -i 2",
                     substrs=['All watchpoints ignored.'])
 
         # Use the '-v' option to do verbose listing of the watchpoint.
         # Expect to find an ignore_count of 2.
         self.expect("watchpoint list -v",
                     substrs=['hit_count = 0', 'ignore_count = 2'])
 
         self.runCmd("process continue")
 
         # There should be no more watchpoint hit and the process status should
         # be 'exited'.
         self.expect("process status",
                     substrs=['exited'])
 
         # Use the '-v' option to do verbose listing of the watchpoint.
         # Expect to find a hit_count of 2 as well.
         self.expect("watchpoint list -v",
                     substrs=['hit_count = 2', 'ignore_count = 2'])
 

lldb/source/Commands/CommandObjectWatchpoint.cpp

if (command.GetArgumentCount() == 0) {
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
     }
 
     return result.Succeeded();
   }
 };
 
 // CommandObjectWatchpointDelete
+#define LLDB_OPTIONS_watchpoint_delete
+#include "CommandOptions.inc"
+
+// CommandObjectWatchpointDelete
 #pragma mark Delete
 
 class CommandObjectWatchpointDelete : public CommandObjectParsed {
 public:
   CommandObjectWatchpointDelete(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "watchpoint delete",
                             "Delete the specified watchpoint(s).  If no "
                             "watchpoints are specified, delete them all.",
-                            nullptr, eCommandRequiresTarget) {
+                            nullptr, eCommandRequiresTarget),
+        m_options() {
     CommandArgumentEntry arg;
     CommandObject::AddIDsArgumentData(arg, eArgTypeWatchpointID,
                                       eArgTypeWatchpointIDRange);
     // Add the entry for the first argument for this command to the object's
     // arguments vector.
     m_arguments.push_back(arg);
   }
 
   ~CommandObjectWatchpointDelete() override = default;
 
+  Options *GetOptions() override { return &m_options; }
+
+  class CommandOptions : public Options {
+  public:
+    CommandOptions() : Options(), m_force(false) {}
+
+    ~CommandOptions() override = default;
+
+    Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+                          ExecutionContext *execution_context) override {
+      const int short_option = m_getopt_table[option_idx].val;
+
+      switch (short_option) {
+      case 'f':
+        m_force = true;
+        break;
+      default:
+        llvm_unreachable("Unimplemented option");
+      }
+
+      return {};
+    }
+
+    void OptionParsingStarting(ExecutionContext *execution_context) override {
+      m_force = false;
+    }
+
+    llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+      return llvm::makeArrayRef(g_watchpoint_delete_options);
+    }
+
+    // Instance variables to hold the values for command options.
+    bool m_force;
+  };
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target *target = &GetSelectedTarget();
     if (!CheckTargetForWatchpointOperations(target, result))
       return false;
 
     std::unique_lock<std::recursive_mutex> lock;
     target->GetWatchpointList().GetListMutex(lock);
 
     const WatchpointList &watchpoints = target->GetWatchpointList();
 
     size_t num_watchpoints = watchpoints.GetSize();
 
     if (num_watchpoints == 0) {
       result.AppendError("No watchpoints exist to be deleted.");
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
 
-    if (command.GetArgumentCount() == 0) {
-      if (!m_interpreter.Confirm(
+    if (command.empty()) {
+      if (!m_options.m_force &&
+          !m_interpreter.Confirm(
               "About to delete all watchpoints, do you want to do that?",
               true)) {
         result.AppendMessage("Operation cancelled...");
       } else {
         target->RemoveAllWatchpoints();
         result.AppendMessageWithFormat("All watchpoints removed. (%" PRIu64
                                        " watchpoints)\n",
                                        (uint64_t)num_watchpoints);
       }
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    } else {
-      // Particular watchpoints selected; delete them.
-      std::vector<uint32_t> wp_ids;
-      if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(
-              target, command, wp_ids)) {
-        result.AppendError("Invalid watchpoints specification.");
-        result.SetStatus(eReturnStatusFailed);
-        return false;
-      }
+      return result.Succeeded();
+    }
+
+    if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(target, command,
+                                                               wp_ids)) {
 
-      int count = 0;
-      const size_t size = wp_ids.size();
-      for (size_t i = 0; i < size; ++i)
-        if (target->RemoveWatchpointByID(wp_ids[i]))
-          ++count;
-      result.AppendMessageWithFormat("%d watchpoints deleted.\n", count);
-      result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    }
 
     return result.Succeeded();
   }
+
+private:
+  CommandOptions m_options;
 };
 
 // CommandObjectWatchpointIgnore
 
 #pragma mark Ignore::CommandOptions
 #define LLDB_OPTIONS_watchpoint_ignore
 #include "CommandOptions.inc"
 

lldb/source/Commands/Options.td

def watchpoint_command_add_script_type : Option<"script-type", "s">,
     EnumArg<"None", "ScriptOptionEnum()">, Desc<"Specify the language for the"
     " commands - if none is specified, the lldb command interpreter will be "
     "used.">;
   def watchpoint_command_add_python_function : Option<"python-function", "F">,
     Group<2>, Arg<"PythonFunction">, Desc<"Give the name of a Python function "
     "to run as command for this watchpoint. Be sure to give a module name if "
     "appropriate.">;
 }
+
+let Command = "watchpoint delete" in {
+  def watchpoint_delete_force : Option<"force", "f">, Group<1>,
+    Desc<"Delete all watchpoints without querying for confirmation.">;
+}