[Lldb-commits] [lldb] Add a breakpoint override resolver feature to lldb (PR #195392)

[Jonas Devlieghere via lldb-commits]

================
@@ -3584,6 +3584,221 @@ class CommandObjectBreakpointWrite : public CommandObjectParsed {
   CommandOptions m_options;
 };
 
+#pragma mark override add
+#define LLDB_OPTIONS_breakpoint_override_add
+#include "CommandOptions.inc"
+
+class CommandObjectBreakpointOverrideAdd : public CommandObjectParsed {
+public:
+  CommandObjectBreakpointOverrideAdd(CommandInterpreter &interpreter)
+      : CommandObjectParsed(interpreter, "breakpoint override add",
+                            "Add a scripted breakpoint override resolver.",
+                            nullptr),
+        m_python_class_options("breakpoint override resolver", true, 'P') {
+    // We're picking up all the normal options, commands and disable.
+    m_all_options.Append(&m_python_class_options,
+                         LLDB_OPT_SET_1 | LLDB_OPT_SET_2, LLDB_OPT_SET_1);
+    m_all_options.Append(&m_dummy_options, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
+    m_all_options.Append(&m_options, llvm::ArrayRef<llvm::StringRef>());
+    m_all_options.Finalize();
+  }
+
+  ~CommandObjectBreakpointOverrideAdd() override = default;
+
+  class CommandOptions : public OptionGroup {
+  public:
+    CommandOptions() = default;
+
+    ~CommandOptions() override = default;
+
+    Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+                          ExecutionContext *execution_context) override {
+      Status error;
+      const int short_option = GetDefinitions()[option_idx].short_option;
+
+      switch (short_option) {
+      case 'd':
+        m_description.assign(std::string(option_arg));
+        break;
+      default:
+        llvm_unreachable("Unimplemented option");
+      }
+
+      return error;
+    }
+
+    void OptionParsingStarting(ExecutionContext *execution_context) override {
+      m_description.clear();
+    }
+
+    llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+      return llvm::ArrayRef(g_breakpoint_override_add_options);
+    }
+
+    // Instance variables to hold the values for command options.
+
+    std::string m_description;
+  };
+  Options *GetOptions() override { return &m_all_options; }
+
+protected:
+  void DoExecute(Args &command, CommandReturnObject &result) override {
+    Target &target =
+        m_dummy_options.m_use_dummy ? GetDummyTarget() : GetTarget();
+    uint64_t id = target.AddBreakpointResolverOverride(
+        m_python_class_options.GetName(),
+        m_python_class_options.GetStructuredData(), m_options.m_description);
+    result.AppendMessageWithFormatv("{0}", id);
+    result.SetStatus(eReturnStatusSuccessFinishResult);
+  }
+
+private:
+  BreakpointDummyOptionGroup m_dummy_options;
+  OptionGroupPythonClassWithDict m_python_class_options;
+  CommandOptions m_options;
+  OptionGroupOptions m_all_options;
+};
+
+class CommandObjectBreakpointOverrideDelete : public CommandObjectParsed {
+public:
+  CommandObjectBreakpointOverrideDelete(CommandInterpreter &interpreter)
+      : CommandObjectParsed(interpreter, "breakpoint override delete",
+                            "Add a scripted breakpoint override resolver.",
+                            nullptr) {
+    AddSimpleArgumentList(eArgTypeIndex, eArgRepeatOptional);
+    m_all_options.Append(&m_dummy_options, LLDB_OPT_SET_1, LLDB_OPT_SET_1);
+    m_all_options.Finalize();
+  }
+
+  ~CommandObjectBreakpointOverrideDelete() override = default;
+
+protected:
+  void DoExecute(Args &command, CommandReturnObject &result) override {
+    Target &target =
+        m_dummy_options.m_use_dummy ? GetDummyTarget() : GetTarget();
+
+    const size_t argc = command.GetArgumentCount();
+    if (argc == 0) {
+      if (m_interpreter.Confirm("Delete all breakpoint overrides?", false)) {
+        target.ClearBreakpointResolverOverrides();
+      }
+      result.SetStatus(eReturnStatusSuccessFinishNoResult);
+      return;
+    }
+
+    for (auto &entry : command.entries()) {
+      uint64_t id;
+      bool success;
+      if (!entry.ref().getAsInteger(0, id))
+        success = target.RemoveBreakpointResolverOverride(id);
+      else {
+        result.AppendErrorWithFormatv("Index not an integer: {0}", entry.ref());
+        result.SetStatus(eReturnStatusFailed);
+        return;
+      }
+      if (!success) {
+        result.AppendErrorWithFormatv("Cannot delete override: {0}", id);
+        result.SetStatus(eReturnStatusFailed);
+        return;
+      }
+    }
+    result.SetStatus(eReturnStatusSuccessFinishNoResult);
+  }
+
+private:
+  BreakpointDummyOptionGroup m_dummy_options;
+  OptionGroupOptions m_all_options;
+};
+
+class CommandObjectBreakpointOverrideList : public CommandObjectParsed {
+public:
+  CommandObjectBreakpointOverrideList(CommandInterpreter &interpreter)
+      : CommandObjectParsed(
+            interpreter, "breakpoint override list",
+            "List the current scripted breakpoint override resolvers.",
+            nullptr) {
+    AddSimpleArgumentList(eArgTypeIndex, eArgRepeatOptional);
+    m_all_options.Append(&m_dummy_options, LLDB_OPT_SET_1, LLDB_OPT_SET_1);
+    m_all_options.Finalize();
+  }
+
+  ~CommandObjectBreakpointOverrideList() override = default;
+
+protected:
+  void DoExecute(Args &command, CommandReturnObject &result) override {
+    Target &target =
+        m_dummy_options.m_use_dummy ? GetDummyTarget() : GetTarget();
+
+    const size_t argc = command.GetArgumentCount();
+    std::vector<uint64_t> idxs;
+    if (argc != 0) {
+      for (auto &entry : command.entries()) {
+        uint64_t id;
+        if (entry.ref().getAsInteger(0, id))
+          idxs.push_back(id);
+        else {
+          result.AppendErrorWithFormatv("Index not an integer: {0}",
+                                        entry.ref());
+          result.SetStatus(eReturnStatusFailed);
+          return;
+        }
----------------
JDevlieghere wrote:

This is inverted. Confusingly, `getAsInteger` returns true on error:

> If the string is invalid or if only a subset of the string is valid, this returns true to signify the error. The 
string is considered erroneous if empty or if it overflows T.

Also, if part of the if-else needs braces, the other one does too even if it's a single line.

```suggestion
        if (!entry.ref().getAsInteger(0, id)) {
          idxs.push_back(id);
        } else {
          result.AppendErrorWithFormatv("Index not an integer: {0}",
                                        entry.ref());
          result.SetStatus(eReturnStatusFailed);
          return;
        }
```

https://github.com/llvm/llvm-project/pull/195392