[Lldb-commits] [lldb] [lldb] Set return status to Failed when Python command raises uncaught exception (PR #113996)

Dave Lee via lldb-commits lldb-commits at lists.llvm.org
Sun Nov 24 17:21:20 PST 2024


https://github.com/kastiglione updated https://github.com/llvm/llvm-project/pull/113996

>From 75f96b84c9bd2b3211edb5fa8806092711133251 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Mon, 28 Oct 2024 15:49:50 -0700
Subject: [PATCH 1/6] [lldb] Set return status to Failed when Python command
 raises uncaught exception

---
 lldb/bindings/python/python-wrapper.swig      | 13 +++++++--
 .../script/exception/TestCommandException.py  | 27 +++++++++++++++++++
 .../command/script/exception/throw_command.py |  6 +++++
 3 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 lldb/test/API/commands/command/script/exception/TestCommandException.py
 create mode 100644 lldb/test/API/commands/command/script/exception/throw_command.py

diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index b72a462d04643b..dfe762e026788a 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -592,6 +592,8 @@ void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBExecutionContext(PyOb
   return sb_ptr;
 }
 
+#include "lldb/Interpreter/CommandReturnObject.h"
+
 bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommand(
     const char *python_function_name, const char *session_dictionary_name,
     lldb::DebuggerSP debugger, const char *args,
@@ -621,6 +623,9 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommand(
     pfunc(debugger_arg, PythonString(args),
           SWIGBridge::ToSWIGWrapper(std::move(exe_ctx_ref_sp)), cmd_retobj_arg.obj(), dict);
 
+  if (PyErr_Occurred())
+    cmd_retobj.SetStatus(eReturnStatusFailed);
+
   return true;
 }
 
@@ -642,6 +647,9 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommandObject(
   pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger)), PythonString(args),
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), cmd_retobj_arg.obj());
 
+  if (PyErr_Occurred())
+    cmd_retobj.SetStatus(eReturnStatusFailed);
+
   return true;
 }
 
@@ -740,8 +748,6 @@ lldb_private::python::SWIGBridge::LLDBSwigPythonHandleOptionArgumentCompletionFo
   return dict_sp;
 }
 
-#include "lldb/Interpreter/CommandReturnObject.h"
-
 bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallParsedCommandObject(
     PyObject *implementor, lldb::DebuggerSP debugger, lldb_private::StructuredDataImpl &args_impl,
     lldb_private::CommandReturnObject &cmd_retobj,
@@ -760,6 +766,9 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallParsedCommandObject(
   pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger)), SWIGBridge::ToSWIGWrapper(args_impl),
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), SWIGBridge::ToSWIGWrapper(cmd_retobj).obj());
 
+  if (PyErr_Occurred())
+    cmd_retobj.SetStatus(eReturnStatusFailed);
+
   return true;
 }
 
diff --git a/lldb/test/API/commands/command/script/exception/TestCommandException.py b/lldb/test/API/commands/command/script/exception/TestCommandException.py
new file mode 100644
index 00000000000000..73484137d82b3e
--- /dev/null
+++ b/lldb/test/API/commands/command/script/exception/TestCommandException.py
@@ -0,0 +1,27 @@
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestCase(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test(self):
+        """
+        Check that a Python command, which raises an unhandled exception, has
+        its status set to failed.
+        """
+        command_path = os.path.join(self.getSourceDir(), "throw_command.py")
+        self.runCmd(f"command script import {command_path}")
+
+        with open(os.devnull, "w") as devnull:
+            self.dbg.SetErrorFileHandle(devnull, False)
+            result = lldb.SBCommandReturnObject()
+            self.ci.HandleCommand("throw", result)
+
+        self.assertEqual(
+            result.GetStatus(),
+            lldb.eReturnStatusFailed,
+            "command unexpectedly succeeded",
+        )
diff --git a/lldb/test/API/commands/command/script/exception/throw_command.py b/lldb/test/API/commands/command/script/exception/throw_command.py
new file mode 100644
index 00000000000000..7c4989850cb19a
--- /dev/null
+++ b/lldb/test/API/commands/command/script/exception/throw_command.py
@@ -0,0 +1,6 @@
+import lldb
+
+
+ at lldb.command()
+def throw(debugger, cmd, ctx, result, _):
+    raise Exception("command failed")

>From b64f5ac38b31a1098ca31183f2bfdae4a6069306 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Tue, 29 Oct 2024 09:37:32 -0700
Subject: [PATCH 2/6] Reset error file handle

---
 .../commands/command/script/exception/TestCommandException.py    | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lldb/test/API/commands/command/script/exception/TestCommandException.py b/lldb/test/API/commands/command/script/exception/TestCommandException.py
index 73484137d82b3e..6a673b7589d87e 100644
--- a/lldb/test/API/commands/command/script/exception/TestCommandException.py
+++ b/lldb/test/API/commands/command/script/exception/TestCommandException.py
@@ -19,6 +19,7 @@ def test(self):
             self.dbg.SetErrorFileHandle(devnull, False)
             result = lldb.SBCommandReturnObject()
             self.ci.HandleCommand("throw", result)
+            self.dbg.SetErrorFileHandle(None, False)
 
         self.assertEqual(
             result.GetStatus(),

>From af28f7711654b985a75636d69550bb7897a9459a Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Tue, 29 Oct 2024 10:48:11 -0700
Subject: [PATCH 3/6] Use AppendError to mark status and supply reason

---
 lldb/bindings/python/python-wrapper.swig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index dfe762e026788a..0888a18a125c67 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -624,7 +624,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommand(
           SWIGBridge::ToSWIGWrapper(std::move(exe_ctx_ref_sp)), cmd_retobj_arg.obj(), dict);
 
   if (PyErr_Occurred())
-    cmd_retobj.SetStatus(eReturnStatusFailed);
+    cmd_retobj.AppendError("Scripted command threw an uncaught exception");
 
   return true;
 }
@@ -648,7 +648,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommandObject(
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), cmd_retobj_arg.obj());
 
   if (PyErr_Occurred())
-    cmd_retobj.SetStatus(eReturnStatusFailed);
+    cmd_retobj.AppendError("Scripted command threw an uncaught exception");
 
   return true;
 }
@@ -767,7 +767,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallParsedCommandObject(
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), SWIGBridge::ToSWIGWrapper(cmd_retobj).obj());
 
   if (PyErr_Occurred())
-    cmd_retobj.SetStatus(eReturnStatusFailed);
+    cmd_retobj.AppendError("Scripted command threw an uncaught exception");
 
   return true;
 }

>From 15a8fbff7e0ab4b33ec69210abdefdb374ce9615 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Tue, 29 Oct 2024 10:57:25 -0700
Subject: [PATCH 4/6] Comment on the intent of AppendError

---
 lldb/bindings/python/python-wrapper.swig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index 0888a18a125c67..252376e9ca32e8 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -624,6 +624,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommand(
           SWIGBridge::ToSWIGWrapper(std::move(exe_ctx_ref_sp)), cmd_retobj_arg.obj(), dict);
 
   if (PyErr_Occurred())
+    // Set status to Failed.
     cmd_retobj.AppendError("Scripted command threw an uncaught exception");
 
   return true;
@@ -648,6 +649,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommandObject(
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), cmd_retobj_arg.obj());
 
   if (PyErr_Occurred())
+    // Set status to Failed.
     cmd_retobj.AppendError("Scripted command threw an uncaught exception");
 
   return true;
@@ -767,6 +769,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallParsedCommandObject(
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), SWIGBridge::ToSWIGWrapper(cmd_retobj).obj());
 
   if (PyErr_Occurred())
+    // Set status to Failed.
     cmd_retobj.AppendError("Scripted command threw an uncaught exception");
 
   return true;

>From 6f923e0421c5f6ffc7ce7d8bf360f0e8518b1dbc Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Wed, 30 Oct 2024 14:32:31 -0700
Subject: [PATCH 5/6] Include Python function/class name in exception message

---
 lldb/bindings/python/python-wrapper.swig | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index 252376e9ca32e8..de7904afac2091 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -625,7 +625,8 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommand(
 
   if (PyErr_Occurred())
     // Set status to Failed.
-    cmd_retobj.AppendError("Scripted command threw an uncaught exception");
+    cmd_retobj.AppendErrorWithFormatv("Scripted command ({0}) threw an uncaught exception",
+                                      python_function_name);
 
   return true;
 }
@@ -648,9 +649,15 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommandObject(
   pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger)), PythonString(args),
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), cmd_retobj_arg.obj());
 
-  if (PyErr_Occurred())
+  if (PyErr_Occurred()) {
+    py_err_cleaner.~PyErr_Cleaner();
+    auto command_class = self.ResolveName("__class__");
+    auto module_name = command_class.ResolveName<PythonString>("__module__");
+    auto class_name = command_class.ResolveName<PythonString>("__name__");
     // Set status to Failed.
-    cmd_retobj.AppendError("Scripted command threw an uncaught exception");
+    cmd_retobj.AppendErrorWithFormatv("Scripted command ({0}.{1}) threw an uncaught exception",
+                                      module_name.GetString(), class_name.GetString());
+  }
 
   return true;
 }
@@ -768,9 +775,15 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallParsedCommandObject(
   pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger)), SWIGBridge::ToSWIGWrapper(args_impl),
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), SWIGBridge::ToSWIGWrapper(cmd_retobj).obj());
 
-  if (PyErr_Occurred())
+  if (PyErr_Occurred()) {
+    py_err_cleaner.~PyErr_Cleaner();
+    auto command_class = self.ResolveName("__class__");
+    auto module_name = command_class.ResolveName<PythonString>("__module__");
+    auto class_name = command_class.ResolveName<PythonString>("__name__");
     // Set status to Failed.
-    cmd_retobj.AppendError("Scripted command threw an uncaught exception");
+    cmd_retobj.AppendErrorWithFormatv("Scripted command ({0}.{1}) threw an uncaught exception",
+                                      module_name.GetString(), class_name.GetString());
+  }
 
   return true;
 }

>From e75a64dd4185dcd824540cd9af0770656ee2a739 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Sun, 24 Nov 2024 17:15:16 -0800
Subject: [PATCH 6/6] Minor hygenic improvements

---
 lldb/bindings/python/python-wrapper.swig | 10 ++++++----
 lldb/bindings/python/python.swig         |  1 +
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index de7904afac2091..92fd30161e5b46 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -5,6 +5,10 @@ public:
   PyErr_Cleaner(bool print = false) : m_print(print) {}
 
   ~PyErr_Cleaner() {
+    Clean();
+  }
+
+  void Clean() {
     if (PyErr_Occurred()) {
       if (m_print && !PyErr_ExceptionMatches(PyExc_SystemExit))
         PyErr_Print();
@@ -592,8 +596,6 @@ void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBExecutionContext(PyOb
   return sb_ptr;
 }
 
-#include "lldb/Interpreter/CommandReturnObject.h"
-
 bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommand(
     const char *python_function_name, const char *session_dictionary_name,
     lldb::DebuggerSP debugger, const char *args,
@@ -650,7 +652,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommandObject(
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), cmd_retobj_arg.obj());
 
   if (PyErr_Occurred()) {
-    py_err_cleaner.~PyErr_Cleaner();
+    py_err_cleaner.Clean();
     auto command_class = self.ResolveName("__class__");
     auto module_name = command_class.ResolveName<PythonString>("__module__");
     auto class_name = command_class.ResolveName<PythonString>("__name__");
@@ -776,7 +778,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallParsedCommandObject(
         SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), SWIGBridge::ToSWIGWrapper(cmd_retobj).obj());
 
   if (PyErr_Occurred()) {
-    py_err_cleaner.~PyErr_Cleaner();
+    py_err_cleaner.Clean();
     auto command_class = self.ResolveName("__class__");
     auto module_name = command_class.ResolveName<PythonString>("__module__");
     auto class_name = command_class.ResolveName<PythonString>("__name__");
diff --git a/lldb/bindings/python/python.swig b/lldb/bindings/python/python.swig
index 278c0eed2bab27..b0c1af7bf24b07 100644
--- a/lldb/bindings/python/python.swig
+++ b/lldb/bindings/python/python.swig
@@ -117,6 +117,7 @@ def lldb_iter(obj, getsize, getelem):
 #include "../source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h"
 #include "../source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h"
 #include "../bindings/python/python-swigsafecast.swig"
+#include "lldb/Interpreter/CommandReturnObject.h"
 using namespace lldb_private;
 using namespace lldb_private::python;
 using namespace lldb;



More information about the lldb-commits mailing list