This revision was automatically updated to reflect the committed changes.
mib marked an inline comment as done.
Closed by commit rG7e01924e4e56: [lldb/Plugins] Improve error reporting with
reading memory in Scripted Process (authored by mib).
Changed prior to commit:
https://reviews.llvm.org/D134033?vs=476299&id=476598#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134033/new/
https://reviews.llvm.org/D134033
Files:
lldb/bindings/python/python-swigsafecast.swig
lldb/examples/python/scripted_process/crashlog_scripted_process.py
lldb/examples/python/scripted_process/scripted_process.py
lldb/include/lldb/API/SBError.h
lldb/source/API/SBError.cpp
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===================================================================
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -270,3 +270,7 @@
lldb::StreamSP stream) {
return false;
}
+
+python::PythonObject lldb_private::python::ToSWIGWrapper(const Status &status) {
+ return python::PythonObject();
+}
Index: lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
===================================================================
--- lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
@@ -65,9 +65,8 @@
def get_registers_for_thread(self, tid: int):
return {}
- def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+ def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
data = lldb.SBData()
- error = lldb.SBError()
bytes_read = self.corefile_process.ReadMemory(addr, size, error)
if error.Fail():
Index: lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
===================================================================
--- lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
@@ -20,8 +20,9 @@
def get_registers_for_thread(self, tid: int):
return {}
- def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
- return None
+ def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+ error.SetErrorString("This is an invalid scripted process!")
+ return lldb.SBData()
def get_loaded_images(self):
return self.loaded_images
Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===================================================================
--- lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -20,11 +20,12 @@
def get_registers_for_thread(self, tid: int):
return {}
- def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+ def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
data = lldb.SBData().CreateDataFromCString(
self.target.GetByteOrder(),
self.target.GetCodeByteSize(),
"Hello, world!")
+
return data
def get_loaded_images(self):
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===================================================================
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -77,6 +77,12 @@
self.assertEqual(process.GetProcessID(), 666)
self.assertEqual(process.GetNumThreads(), 0)
+ addr = 0x500000000
+ buff = process.ReadMemory(addr, 4, error)
+ self.assertEqual(buff, None)
+ self.assertTrue(error.Fail())
+ self.assertEqual(error.GetCString(), "This is an invalid scripted process!")
+
with open(log_file, 'r') as f:
log = f.read()
@@ -109,9 +115,14 @@
process = target.Launch(launch_info, error)
self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
self.assertEqual(process.GetProcessID(), 42)
-
self.assertEqual(process.GetNumThreads(), 1)
+ addr = 0x500000000
+ message = "Hello, world!"
+ buff = process.ReadCStringFromMemory(addr, len(message) + 1, error)
+ self.assertSuccess(error)
+ self.assertEqual(buff, message)
+
thread = process.GetSelectedThread()
self.assertTrue(thread, "Invalid thread.")
self.assertEqual(thread.GetThreadID(), 0x19)
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
@@ -9,10 +9,14 @@
#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPYTHONINTERFACE_H
#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPYTHONINTERFACE_H
-#include "lldb/Host/Config.h"
-
#if LLDB_ENABLE_PYTHON
+#include <sstream>
+#include <tuple>
+#include <type_traits>
+#include <utility>
+
+#include "lldb/Host/Config.h"
#include "lldb/Interpreter/ScriptedInterface.h"
#include "lldb/Utility/DataBufferHeap.h"
@@ -34,7 +38,7 @@
}
template <typename T = StructuredData::ObjectSP, typename... Args>
- T Dispatch(llvm::StringRef method_name, Status &error, Args... args) {
+ T Dispatch(llvm::StringRef method_name, Status &error, Args &&...args) {
using namespace python;
using Locker = ScriptInterpreterPythonImpl::Locker;
@@ -56,59 +60,116 @@
return ErrorWithMessage<T>(caller_signature,
"Python implementor not allocated.", error);
- PythonObject pmeth(
- PyRefType::Owned,
- PyObject_GetAttrString(implementor.get(), method_name.str().c_str()));
+ std::tuple<Args...> original_args = std::forward_as_tuple(args...);
+ auto transformed_args = TransformArgs(original_args);
+
+ llvm::Expected<PythonObject> expected_return_object =
+ llvm::make_error<llvm::StringError>("Not initialized.",
+ llvm::inconvertibleErrorCode());
+ std::apply(
+ [&implementor, &method_name, &expected_return_object](auto &&...args) {
+ llvm::consumeError(expected_return_object.takeError());
+ expected_return_object =
+ implementor.CallMethod(method_name.data(), args...);
+ },
+ transformed_args);
+
+ if (llvm::Error e = expected_return_object.takeError()) {
+ error.SetErrorString(llvm::toString(std::move(e)).c_str());
+ return ErrorWithMessage<T>(caller_signature,
+ "Python method could not be called.", error);
+ }
- if (PyErr_Occurred())
- PyErr_Clear();
+ PythonObject py_return = std::move(expected_return_object.get());
- if (!pmeth.IsAllocated())
- return ErrorWithMessage<T>(caller_signature,
- "Python method not allocated.", error);
+ if (!py_return.IsAllocated())
+ return ErrorWithMessage<T>(caller_signature, "Returned object is null.",
+ error);
- if (PyCallable_Check(pmeth.get()) == 0) {
- if (PyErr_Occurred())
- PyErr_Clear();
- return ErrorWithMessage<T>(caller_signature,
- "Python method not callable.", error);
- }
+ // Now that we called the python method with the transformed arguments,
+ // we need to interate again over both the original and transformed
+ // parameter pack, and transform back the parameter that were passed in
+ // the original parameter pack as references or pointers.
+ if (sizeof...(Args) > 0)
+ if (!ReassignPtrsOrRefsArgs(original_args, transformed_args))
+ return ErrorWithMessage<T>(
+ caller_signature,
+ "Couldn't re-assign reference and pointer arguments.", error);
- if (PyErr_Occurred())
- PyErr_Clear();
+ return ExtractValueFromPythonObject<T>(py_return, error);
+ }
- // TODO: make `const char *` when removing support for Python 2.
- char *format = nullptr;
- std::string format_buffer;
+ Status GetStatusFromMethod(llvm::StringRef method_name);
- if (sizeof...(Args) > 0) {
- FormatArgs(format_buffer, args...);
- // TODO: make `const char *` when removing support for Python 2.
- format = const_cast<char *>(format_buffer.c_str());
+ template <typename T> struct transformation { using type = T; };
+ template <typename T, typename U> struct reverse_transformation {
+ static void Apply(ScriptedPythonInterface &obj, T &original_arg,
+ U transformed_arg, Status &error) {
+ // If U is not a PythonObject, don't touch it!
+ return;
+ }
+ };
+
+ template <> struct transformation<Status> {
+ using type = python::PythonObject;
+ };
+ template <typename T> struct reverse_transformation<T, python::PythonObject> {
+ static void Apply(ScriptedPythonInterface &obj, T &original_arg,
+ python::PythonObject transformed_arg, Status &error) {
+ original_arg =
+ obj.ExtractValueFromPythonObject<T>(transformed_arg, error);
}
+ };
- // TODO: make `const char *` when removing support for Python 2.
- PythonObject py_return(
- PyRefType::Owned,
- PyObject_CallMethod(implementor.get(),
- const_cast<char *>(method_name.data()), format,
- args...));
+ template <typename T> typename transformation<T>::type Transform(T object) {
+ // No Transformation for generic usage
+ return {object};
+ }
- if (PyErr_Occurred()) {
- PyErr_Print();
- PyErr_Clear();
- return ErrorWithMessage<T>(caller_signature,
- "Python method could not be called.", error);
- }
+ template <> typename transformation<Status>::type Transform(Status arg) {
+ // Call SWIG Wrapper function
+ return python::ToSWIGWrapper(arg);
+ }
- if (!py_return.IsAllocated())
- return ErrorWithMessage<T>(caller_signature, "Returned object is null.",
- error);
+ template <std::size_t... I, typename... Args>
+ auto TransformTuple(const std::tuple<Args...> &args,
+ std::index_sequence<I...>) {
+ return std::make_tuple(Transform(std::get<I>(args))...);
+ }
- return ExtractValueFromPythonObject<T>(py_return, error);
+ // This will iterate over the Dispatch parameter pack and replace in-place
+ // every `lldb_private` argument that has a SB counterpart.
+ template <typename... Args>
+ auto TransformArgs(const std::tuple<Args...> &args) {
+ return TransformTuple(args, std::make_index_sequence<sizeof...(Args)>());
}
- Status GetStatusFromMethod(llvm::StringRef method_name);
+ template <typename T, typename U>
+ void TransformBack(T &original_arg, U transformed_arg, Status &error) {
+ reverse_transformation<T, U>::Apply(*this, original_arg, transformed_arg,
+ error);
+ }
+
+ template <std::size_t... I, typename... Ts, typename... Us>
+ bool ReassignPtrsOrRefsArgs(std::tuple<Ts...> &original_args,
+ std::tuple<Us...> &transformed_args,
+ std::index_sequence<I...>) {
+ Status error;
+ (TransformBack(std::get<I>(original_args), std::get<I>(transformed_args),
+ error),
+ ...);
+ return error.Success();
+ }
+
+ template <typename... Ts, typename... Us>
+ bool ReassignPtrsOrRefsArgs(std::tuple<Ts...> &original_args,
+ std::tuple<Us...> &transformed_args) {
+ if (sizeof...(Ts) != sizeof...(Us))
+ return false;
+
+ return ReassignPtrsOrRefsArgs(original_args, transformed_args,
+ std::make_index_sequence<sizeof...(Ts)>());
+ }
template <typename T, typename... Args>
void FormatArgs(std::string &fmt, T arg, Args... args) const {
@@ -117,7 +178,7 @@
}
template <typename T> void FormatArgs(std::string &fmt, T arg) const {
- fmt += GetPythonValueFormatString(arg);
+ fmt += python::PythonFormat<T>::format;
}
void FormatArgs(std::string &fmt) const {}
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
@@ -55,11 +55,11 @@
python::PythonObject &p, Status &error) {
if (lldb::SBError *sb_error = reinterpret_cast<lldb::SBError *>(
LLDBSWIGPython_CastPyObjectToSBError(p.get())))
- error = m_interpreter.GetStatusFromSBError(*sb_error);
+ return m_interpreter.GetStatusFromSBError(*sb_error);
else
error.SetErrorString("Couldn't cast lldb::SBError to lldb::Status.");
- return error;
+ return {};
}
template <>
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
@@ -8,6 +8,7 @@
#include "lldb/Host/Config.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Status.h"
#include "lldb/lldb-enumerations.h"
#if LLDB_ENABLE_PYTHON
@@ -120,8 +121,15 @@
lldb::DataExtractorSP ScriptedProcessPythonInterface::ReadMemoryAtAddress(
lldb::addr_t address, size_t size, Status &error) {
- return Dispatch<lldb::DataExtractorSP>("read_memory_at_address", error,
- address, size);
+ Status py_error;
+ lldb::DataExtractorSP data_sp = Dispatch<lldb::DataExtractorSP>(
+ "read_memory_at_address", py_error, address, size, error);
+
+ // If there was an error on the python call, surface it to the user.
+ if (py_error.Fail())
+ error = py_error;
+
+ return data_sp;
}
StructuredData::ArraySP ScriptedProcessPythonInterface::GetLoadedImages() {
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -23,7 +23,68 @@
#include "lldb/lldb-types.h"
#include "llvm/Support/Error.h"
+namespace lldb {
+class SBEvent;
+class SBCommandReturnObject;
+class SBValue;
+class SBStream;
+class SBStructuredData;
+} // namespace lldb
+
namespace lldb_private {
+namespace python {
+
+typedef struct swig_type_info swig_type_info;
+
+python::PythonObject ToSWIGHelper(void *obj, swig_type_info *info);
+
+/// A class that automatically clears an SB object when it goes out of scope.
+/// Use for cases where the SB object points to a temporary/unowned entity.
+template <typename T> class ScopedPythonObject : PythonObject {
+public:
+ ScopedPythonObject(T *sb, swig_type_info *info)
+ : PythonObject(ToSWIGHelper(sb, info)), m_sb(sb) {}
+ ~ScopedPythonObject() {
+ if (m_sb)
+ *m_sb = T();
+ }
+ ScopedPythonObject(ScopedPythonObject &&rhs)
+ : PythonObject(std::move(rhs)), m_sb(std::exchange(rhs.m_sb, nullptr)) {}
+ ScopedPythonObject(const ScopedPythonObject &) = delete;
+ ScopedPythonObject &operator=(const ScopedPythonObject &) = delete;
+ ScopedPythonObject &operator=(ScopedPythonObject &&) = delete;
+
+ const PythonObject &obj() const { return *this; }
+
+private:
+ T *m_sb;
+};
+
+PythonObject ToSWIGWrapper(lldb::ValueObjectSP value_sp);
+PythonObject ToSWIGWrapper(lldb::TargetSP target_sp);
+PythonObject ToSWIGWrapper(lldb::ProcessSP process_sp);
+PythonObject ToSWIGWrapper(lldb::ThreadPlanSP thread_plan_sp);
+PythonObject ToSWIGWrapper(lldb::BreakpointSP breakpoint_sp);
+PythonObject ToSWIGWrapper(const Status &status);
+PythonObject ToSWIGWrapper(const StructuredDataImpl &data_impl);
+PythonObject ToSWIGWrapper(lldb::ThreadSP thread_sp);
+PythonObject ToSWIGWrapper(lldb::StackFrameSP frame_sp);
+PythonObject ToSWIGWrapper(lldb::DebuggerSP debugger_sp);
+PythonObject ToSWIGWrapper(lldb::WatchpointSP watchpoint_sp);
+PythonObject ToSWIGWrapper(lldb::BreakpointLocationSP bp_loc_sp);
+PythonObject ToSWIGWrapper(lldb::ExecutionContextRefSP ctx_sp);
+PythonObject ToSWIGWrapper(const TypeSummaryOptions &summary_options);
+PythonObject ToSWIGWrapper(const SymbolContext &sym_ctx);
+
+PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBValue> value_sb);
+PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBStream> stream_sb);
+PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBStructuredData> data_sb);
+
+python::ScopedPythonObject<lldb::SBCommandReturnObject>
+ToSWIGWrapper(CommandReturnObject &cmd_retobj);
+python::ScopedPythonObject<lldb::SBEvent> ToSWIGWrapper(Event *event);
+
+} // namespace python
// GetPythonValueFormatString provides a system independent type safe way to
// convert a variable's type into a python value format. Python value formats
Index: lldb/source/API/SBError.cpp
===================================================================
--- lldb/source/API/SBError.cpp
+++ lldb/source/API/SBError.cpp
@@ -25,6 +25,11 @@
m_opaque_up = clone(rhs.m_opaque_up);
}
+SBError::SBError(const lldb_private::Status &status)
+ : m_opaque_up(new Status(status)) {
+ LLDB_INSTRUMENT_VA(this, status);
+}
+
SBError::~SBError() = default;
const SBError &SBError::operator=(const SBError &rhs) {
Index: lldb/include/lldb/API/SBError.h
===================================================================
--- lldb/include/lldb/API/SBError.h
+++ lldb/include/lldb/API/SBError.h
@@ -23,6 +23,8 @@
SBError(const lldb::SBError &rhs);
+ SBError(const lldb_private::Status &error);
+
~SBError();
const SBError &operator=(const lldb::SBError &rhs);
Index: lldb/examples/python/scripted_process/scripted_process.py
===================================================================
--- lldb/examples/python/scripted_process/scripted_process.py
+++ lldb/examples/python/scripted_process/scripted_process.py
@@ -98,13 +98,14 @@
pass
@abstractmethod
- def read_memory_at_address(self, addr, size):
+ def read_memory_at_address(self, addr, size, error):
""" Get a memory buffer from the scripted process at a certain address,
of a certain size.
Args:
addr (int): Address from which we should start reading.
size (int): Size of the memory to read.
+ error (lldb.SBError): Error object.
Returns:
lldb.SBData: An `lldb.SBData` buffer with the target byte size and
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===================================================================
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -103,7 +103,7 @@
def get_registers_for_thread(self, tid: int):
return {}
- def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+ def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
# NOTE: CrashLogs don't contain any memory.
return lldb.SBData()
Index: lldb/bindings/python/python-swigsafecast.swig
===================================================================
--- lldb/bindings/python/python-swigsafecast.swig
+++ lldb/bindings/python/python-swigsafecast.swig
@@ -5,28 +5,6 @@
return {PyRefType::Owned, SWIG_NewPointerObj(obj, info, SWIG_POINTER_OWN)};
}
-/// A class that automatically clears an SB object when it goes out of scope.
-/// Use for cases where the SB object points to a temporary/unowned entity.
-template <typename T> class ScopedPythonObject : PythonObject {
-public:
- ScopedPythonObject(T *sb, swig_type_info *info)
- : PythonObject(ToSWIGHelper(sb, info)), m_sb(sb) {}
- ~ScopedPythonObject() {
- if (m_sb)
- *m_sb = T();
- }
- ScopedPythonObject(ScopedPythonObject &&rhs)
- : PythonObject(std::move(rhs)), m_sb(std::exchange(rhs.m_sb, nullptr)) {}
- ScopedPythonObject(const ScopedPythonObject &) = delete;
- ScopedPythonObject &operator=(const ScopedPythonObject &) = delete;
- ScopedPythonObject &operator=(ScopedPythonObject &&) = delete;
-
- const PythonObject &obj() const { return *this; }
-
-private:
- T *m_sb;
-};
-
PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBValue> value_sb) {
return ToSWIGHelper(value_sb.release(), SWIGTYPE_p_lldb__SBValue);
}
@@ -55,6 +33,10 @@
SWIGTYPE_p_lldb__SBBreakpoint);
}
+PythonObject ToSWIGWrapper(const Status& status) {
+ return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
+}
+
PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBStream> stream_sb) {
return ToSWIGHelper(stream_sb.release(), SWIGTYPE_p_lldb__SBStream);
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
