[Lldb-commits] [lldb] [lldb] Do not accept invalid `process save-core` plugins (PR #142684)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)

<details>
<summary>Changes</summary>


Fixes #<!-- -->142581

---
Full diff: https://github.com/llvm/llvm-project/pull/142684.diff


6 Files Affected:

- (modified) lldb/source/Commands/CommandObjectProcess.cpp (+1-1) 
- (modified) lldb/source/Symbol/SaveCoreOptions.cpp (-1) 
- (added) lldb/test/Shell/Commands/command-process-save-core-not-a-plugin.test (+19) 
- (modified) lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp (+9-3) 
- (modified) lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp (+10-4) 
- (modified) lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp (+8-2) 


``````````diff
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index d0f5eaf2dfd9a..b1f243c9e2777 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -1303,7 +1303,7 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
         llvm_unreachable("Unimplemented option");
       }
 
-      return {};
+      return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index e51ae27954934..d884b00a47b00 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -24,7 +24,6 @@ Status SaveCoreOptions::SetPluginName(const char *name) {
   if (!PluginManager::IsRegisteredObjectFilePluginName(name)) {
     return Status::FromErrorStringWithFormat(
         "plugin name '%s' is not a valid ObjectFile plugin name", name);
-    return error;
   }
 
   m_plugin_name = name;
diff --git a/lldb/test/Shell/Commands/command-process-save-core-not-a-plugin.test b/lldb/test/Shell/Commands/command-process-save-core-not-a-plugin.test
new file mode 100644
index 0000000000000..c034c8ebbf87d
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-process-save-core-not-a-plugin.test
@@ -0,0 +1,19 @@
+# This checks that lldb returns an error if process save-core is called
+# with a plugin that does not exist.
+
+# RUN: %clang_host -g %S/Inputs/main.c -o %t
+# RUN: %lldb %t -s %s -o exit 2>&1 | FileCheck %s
+
+b main
+# CHECK-LABEL: b main
+# CHECK: Breakpoint 1: where = {{.*}}`main
+
+run
+# CHECK-LABEL: run
+# CHECK: Process {{.*}} stopped
+# CHECK: stop reason = breakpoint 1
+# CHECK:   frame #0: {{.*}}`main at main.c
+
+process save-core --plugin-name=notaplugin dump
+# CHECK-LABEL: process save-core --plugin-name=notaplugin dump
+# CHECK: error: plugin name 'notaplugin' is not a valid ObjectFile plugin name
diff --git a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
index 3fa167686d2f9..2b48350dfba1b 100644
--- a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
@@ -8,6 +8,7 @@
 
 #include "DAP.h"
 #include "EventHelper.h"
+#include "LLDBUtils.h"
 #include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
 #include "llvm/Support/Error.h"
@@ -30,16 +31,21 @@ Error NextRequestHandler::Run(const NextArguments &args) const {
   if (!thread.IsValid())
     return make_error<DAPError>("invalid thread");
 
+  if (!SBDebugger::StateIsStoppedState(dap.target.GetProcess().GetState()))
+    return make_error<NotStoppedError>();
+
   // Remember the thread ID that caused the resume so we can set the
   // "threadCausedFocus" boolean value in the "stopped" events.
   dap.focus_tid = thread.GetThreadID();
+  lldb::SBError error;
   if (args.granularity == eSteppingGranularityInstruction) {
-    thread.StepInstruction(/*step_over=*/true);
+    thread.StepInstruction(/*step_over=*/true, error);
   } else {
-    thread.StepOver(args.singleThread ? eOnlyThisThread : eOnlyDuringStepping);
+    thread.StepOver(args.singleThread ? eOnlyThisThread : eOnlyDuringStepping,
+                    error);
   }
 
-  return Error::success();
+  return ToError(error);
 }
 
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
index 15f242a9e18ff..6742c791a5486 100644
--- a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
@@ -8,6 +8,7 @@
 
 #include "DAP.h"
 #include "EventHelper.h"
+#include "LLDBUtils.h"
 #include "Protocol/ProtocolRequests.h"
 #include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
@@ -39,9 +40,13 @@ Error StepInRequestHandler::Run(const StepInArguments &args) const {
   // "threadCausedFocus" boolean value in the "stopped" events.
   dap.focus_tid = thread.GetThreadID();
 
+  if (!SBDebugger::StateIsStoppedState(dap.target.GetProcess().GetState()))
+    return make_error<NotStoppedError>();
+
+  lldb::SBError error;
   if (args.granularity == eSteppingGranularityInstruction) {
-    thread.StepInstruction(/*step_over=*/false);
-    return Error::success();
+    thread.StepInstruction(/*step_over=*/false, error);
+    return ToError(error);
   }
 
   std::string step_in_target;
@@ -50,8 +55,9 @@ Error StepInRequestHandler::Run(const StepInArguments &args) const {
     step_in_target = it->second;
 
   RunMode run_mode = args.singleThread ? eOnlyThisThread : eOnlyDuringStepping;
-  thread.StepInto(step_in_target.c_str(), run_mode);
-  return Error::success();
+  thread.StepInto(step_in_target.c_str(), LLDB_INVALID_LINE_NUMBER, error,
+                  run_mode);
+  return ToError(error);
 }
 
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
index 6b98582262a68..e896e03720b6b 100644
--- a/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
@@ -8,6 +8,7 @@
 
 #include "DAP.h"
 #include "EventHelper.h"
+#include "LLDBUtils.h"
 #include "Protocol/ProtocolRequests.h"
 #include "RequestHandler.h"
 #include "llvm/Support/Error.h"
@@ -32,12 +33,17 @@ Error StepOutRequestHandler::Run(const StepOutArguments &arguments) const {
   if (!thread.IsValid())
     return make_error<DAPError>("invalid thread");
 
+  if (!lldb::SBDebugger::StateIsStoppedState(
+          dap.target.GetProcess().GetState()))
+    return make_error<NotStoppedError>();
+
   // Remember the thread ID that caused the resume so we can set the
   // "threadCausedFocus" boolean value in the "stopped" events.
   dap.focus_tid = thread.GetThreadID();
-  thread.StepOut();
+  lldb::SBError error;
+  thread.StepOut(error);
 
-  return Error::success();
+  return ToError(error);
 }
 
 } // namespace lldb_dap

``````````

</details>


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