mib updated this revision to Diff 476019.
mib marked 2 inline comments as done.
mib retitled this revision from "[lldb/Plugins] Improve error reporting with
reading/writing memory in a Scripted Process (WIP)" to "[lldb/Plugins] Improve
error reporting with reading memory in Scripted Process".
mib edited the summary of this revision.
mib added a reviewer: kastiglione.
mib added a comment.
Redone the whole patch (taking into account @labath's previous comments)
It makes use of:
- Generic Variadic Lambdas
- Fold Expressions
- Index Sequences
- Template Partial Specializations
- Variadic template parameters and tuples
...
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.cpp
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.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
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,7 +20,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:
+ error.SetErrorString("This is an invalid scripted process!")
return None
def get_loaded_images(self):
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,13 +9,17 @@
#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"
+#if LLDB_ENABLE_PYTHON
+
#include "PythonDataObjects.h"
#include "SWIGPythonBridge.h"
#include "ScriptInterpreterPythonImpl.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;
@@ -81,18 +85,31 @@
char *format = nullptr;
std::string format_buffer;
+ std::tuple<Args...> original_args = std::forward_as_tuple(args...);
+ auto transformed_args = TransformArgs(original_args);
+
if (sizeof...(Args) > 0) {
- FormatArgs(format_buffer, args...);
+ std::apply(
+ [&format_buffer, this](auto... args) {
+ this->FormatArgs(format_buffer, args...);
+ },
+ transformed_args);
// TODO: make `const char *` when removing support for Python 2.
format = const_cast<char *>(format_buffer.c_str());
}
- // 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...));
+ PyObject *py_obj_ptr = nullptr;
+
+ std::apply(
+ [&py_obj_ptr, &implementor, &method_name, &format](auto... args) {
+ // TODO: make `const char *` when removing support for Python 2.
+ py_obj_ptr = PyObject_CallMethod(
+ implementor.get(), const_cast<char *>(method_name.data()), format,
+ args...);
+ },
+ transformed_args);
+
+ PythonObject py_return = PythonObject(PyRefType::Owned, py_obj_ptr);
if (PyErr_Occurred()) {
PyErr_Print();
@@ -101,6 +118,16 @@
"Python method could not be called.", 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 (!py_return.IsAllocated())
return ErrorWithMessage<T>(caller_signature, "Returned object is null.",
error);
@@ -110,6 +137,76 @@
Status GetStatusFromMethod(llvm::StringRef method_name);
+ template <typename T> struct transformation { using type = T; };
+ template <> struct transformation<Status> { using type = PyObject *; };
+
+ template <typename T> typename transformation<T>::type Transform(T object) {
+ // No Transformation for generic usage
+ return {object};
+ }
+
+ template <> typename transformation<Status>::type Transform(Status arg) {
+ // Call SWIG Wrapper function
+ python::PythonObject py_obj = python::ToSWIGWrapper(arg);
+ return py_obj.release();
+ }
+
+ 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))...);
+ }
+
+ // 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)>());
+ }
+
+ 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 PyObject*, don't touch it!
+ return;
+ }
+ };
+
+ template <typename T> struct reverse_transformation<T, PyObject *> {
+ static void Apply(ScriptedPythonInterface &obj, T &original_arg,
+ PyObject *transformed_arg, Status &error) {
+ python::PythonObject p(python::PyRefType::Borrowed, transformed_arg);
+ original_arg = obj.ExtractValueFromPythonObject<T>(p, error);
+ }
+ };
+
+ 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 {
FormatArgs(fmt, arg);
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
@@ -120,8 +120,10 @@
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;
+ // TODO: Log py_error after call if failed
+ return Dispatch<lldb::DataExtractorSP>("read_memory_at_address", py_error,
+ address, size, error);
}
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(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
@@ -32,6 +93,7 @@
// change.
template <typename T> const char *GetPythonValueFormatString(T t);
+template <> const char *GetPythonValueFormatString(PyObject *);
template <> const char *GetPythonValueFormatString(char *);
template <> const char *GetPythonValueFormatString(char);
template <> const char *GetPythonValueFormatString(unsigned char);
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp
@@ -16,11 +16,14 @@
#include "SWIGPythonBridge.h"
+// DOC: https://docs.python.org/3/c-api/arg.html#building-values
+
using namespace lldb;
namespace lldb_private {
template <typename T> const char *GetPythonValueFormatString(T t);
+template <> const char *GetPythonValueFormatString(PyObject *) { return "O"; }
template <> const char *GetPythonValueFormatString(char *) { return "s"; }
template <> const char *GetPythonValueFormatString(char) { return "b"; }
template <> const char *GetPythonValueFormatString(unsigned char) {
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.ToError())) {
+ 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(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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
