Skip to content

Commit c2ad9f8

Browse files
medismailbenmedismailben
committed
Revert "[lldb] Check for abstract methods implementation in Scripted Plugin Objects (#71260)"
This reverts commit cc9ad72 since it breaks some tests upstream: https://lab.llvm.org/buildbot/#/builders/68/builds/63112 ******************** Failed Tests (4): lldb-api :: functionalities/gdb_remote_client/TestThreadSelectionBug.py lldb-api :: functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py lldb-api :: functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py lldb-api :: functionalities/postmortem/mach-core/TestMachCore.py
1 parent c43e627 commit c2ad9f8

14 files changed

+29
-251
lines changed

‎lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h

-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ class ScriptedInterface {
3030
return m_object_instance_sp;
3131
}
3232

33-
virtual llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const = 0;
34-
3533
template <typename Ret>
3634
static Ret ErrorWithMessage(llvm::StringRef caller_name,
3735
llvm::StringRef error_msg, Status &error,

‎lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h

-4
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ class ScriptedPlatformInterface : virtual public ScriptedInterface {
2424
StructuredData::DictionarySP args_sp,
2525
StructuredData::Generic *script_obj = nullptr) = 0;
2626

27-
llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
28-
return {};
29-
}
30-
3127
virtual StructuredData::DictionarySP ListProcesses() { return {}; }
3228

3329
virtual StructuredData::DictionarySP GetProcessInfo(lldb::pid_t) {

‎lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h

-4
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@ class ScriptedProcessInterface : virtual public ScriptedInterface {
2626
StructuredData::DictionarySP args_sp,
2727
StructuredData::Generic *script_obj = nullptr) = 0;
2828

29-
llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
30-
return {};
31-
}
32-
3329
virtual StructuredData::DictionarySP GetCapabilities() { return {}; }
3430

3531
virtual Status Attach(const ProcessAttachInfo &attach_info) {

‎lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h

-4
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@ class ScriptedThreadInterface : virtual public ScriptedInterface {
2525
StructuredData::DictionarySP args_sp,
2626
StructuredData::Generic *script_obj = nullptr) = 0;
2727

28-
llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
29-
return {};
30-
}
31-
3228
virtual lldb::tid_t GetThreadID() { return LLDB_INVALID_THREAD_ID; }
3329

3430
virtual std::optional<std::string> GetName() { return std::nullopt; }

‎lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h

-7
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,6 @@ class OperatingSystemPythonInterface
2929
StructuredData::DictionarySP args_sp,
3030
StructuredData::Generic *script_obj = nullptr) override;
3131

32-
llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
33-
return llvm::SmallVector<llvm::StringLiteral>({"get_thread_info"
34-
"get_register_data",
35-
"get_stop_reason",
36-
"get_register_context"});
37-
}
38-
3932
StructuredData::DictionarySP CreateThread(lldb::tid_t tid,
4033
lldb::addr_t context) override;
4134

‎lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h

-6
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,6 @@ class ScriptedPlatformPythonInterface : public ScriptedPlatformInterface,
2828
StructuredData::DictionarySP args_sp,
2929
StructuredData::Generic *script_obj = nullptr) override;
3030

31-
llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
32-
return llvm::SmallVector<llvm::StringLiteral>(
33-
{"list_processes", "attach_to_process", "launch_process",
34-
"kill_process"});
35-
}
36-
3731
StructuredData::DictionarySP ListProcesses() override;
3832

3933
StructuredData::DictionarySP GetProcessInfo(lldb::pid_t) override;

‎lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h

-5
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,6 @@ class ScriptedProcessPythonInterface : public ScriptedProcessInterface,
2929
StructuredData::DictionarySP args_sp,
3030
StructuredData::Generic *script_obj = nullptr) override;
3131

32-
llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
33-
return llvm::SmallVector<llvm::StringLiteral>(
34-
{"read_memory_at_address", "is_alive", "get_scripted_thread_plugin"});
35-
}
36-
3732
StructuredData::DictionarySP GetCapabilities() override;
3833

3934
Status Attach(const ProcessAttachInfo &attach_info) override;

‎lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h

+29-127
Original file line numberDiff line numberDiff line change
@@ -32,66 +32,27 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
3232
ScriptedPythonInterface(ScriptInterpreterPythonImpl &interpreter);
3333
~ScriptedPythonInterface() override = default;
3434

35-
enum class AbstractMethodCheckerCases {
36-
eNotImplemented,
37-
eNotAllocated,
38-
eNotCallable,
39-
eValid
40-
};
41-
42-
llvm::Expected<std::map<llvm::StringLiteral, AbstractMethodCheckerCases>>
43-
CheckAbstractMethodImplementation(
44-
const python::PythonDictionary &class_dict) const {
45-
46-
using namespace python;
47-
48-
std::map<llvm::StringLiteral, AbstractMethodCheckerCases> checker;
49-
#define SET_ERROR_AND_CONTINUE(method_name, error) \
50-
{ \
51-
checker[method_name] = error; \
52-
continue; \
53-
}
54-
55-
for (const llvm::StringLiteral &method_name : GetAbstractMethods()) {
56-
if (!class_dict.HasKey(method_name))
57-
SET_ERROR_AND_CONTINUE(method_name,
58-
AbstractMethodCheckerCases::eNotImplemented)
59-
auto callable_or_err = class_dict.GetItem(method_name);
60-
if (!callable_or_err)
61-
SET_ERROR_AND_CONTINUE(method_name,
62-
AbstractMethodCheckerCases::eNotAllocated)
63-
if (!PythonCallable::Check(callable_or_err.get().get()))
64-
SET_ERROR_AND_CONTINUE(method_name,
65-
AbstractMethodCheckerCases::eNotCallable)
66-
checker[method_name] = AbstractMethodCheckerCases::eValid;
67-
}
68-
69-
#undef HANDLE_ERROR
70-
71-
return checker;
72-
}
73-
7435
template <typename... Args>
7536
llvm::Expected<StructuredData::GenericSP>
7637
CreatePluginObject(llvm::StringRef class_name,
7738
StructuredData::Generic *script_obj, Args... args) {
7839
using namespace python;
7940
using Locker = ScriptInterpreterPythonImpl::Locker;
8041

81-
auto create_error = [](std::string message) {
82-
return llvm::createStringError(llvm::inconvertibleErrorCode(), message);
83-
};
84-
8542
bool has_class_name = !class_name.empty();
8643
bool has_interpreter_dict =
8744
!(llvm::StringRef(m_interpreter.GetDictionaryName()).empty());
8845
if (!has_class_name && !has_interpreter_dict && !script_obj) {
8946
if (!has_class_name)
90-
return create_error("Missing script class name.");
47+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
48+
"Missing script class name.");
9149
else if (!has_interpreter_dict)
92-
return create_error("Invalid script interpreter dictionary.");
50+
return llvm::createStringError(
51+
llvm::inconvertibleErrorCode(),
52+
"Invalid script interpreter dictionary.");
9353
else
94-
return create_error("Missing scripting object.");
54+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
55+
"Missing scripting object.");
9556
}
9657

9758
Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
@@ -106,23 +67,26 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
10667
auto dict =
10768
PythonModule::MainModule().ResolveName<python::PythonDictionary>(
10869
m_interpreter.GetDictionaryName());
109-
if (!dict.IsAllocated())
110-
return create_error(
111-
llvm::formatv("Could not find interpreter dictionary: %s",
112-
m_interpreter.GetDictionaryName()));
70+
if (!dict.IsAllocated()) {
71+
return llvm::createStringError(
72+
llvm::inconvertibleErrorCode(),
73+
"Could not find interpreter dictionary: %s",
74+
m_interpreter.GetDictionaryName());
75+
}
11376

114-
auto init =
77+
auto method =
11578
PythonObject::ResolveNameWithDictionary<python::PythonCallable>(
11679
class_name, dict);
117-
if (!init.IsAllocated())
118-
return create_error(llvm::formatv("Could not find script class: %s",
119-
class_name.data()));
80+
if (!method.IsAllocated())
81+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
82+
"Could not find script class: %s",
83+
class_name.data());
12084

12185
std::tuple<Args...> original_args = std::forward_as_tuple(args...);
12286
auto transformed_args = TransformArgs(original_args);
12387

12488
std::string error_string;
125-
llvm::Expected<PythonCallable::ArgInfo> arg_info = init.GetArgInfo();
89+
llvm::Expected<PythonCallable::ArgInfo> arg_info = method.GetArgInfo();
12690
if (!arg_info) {
12791
llvm::handleAllErrors(
12892
arg_info.takeError(),
@@ -135,87 +99,25 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
13599
}
136100

137101
llvm::Expected<PythonObject> expected_return_object =
138-
create_error("Resulting object is not initialized.");
102+
llvm::createStringError(llvm::inconvertibleErrorCode(),
103+
"Resulting object is not initialized.");
139104

140105
std::apply(
141-
[&init, &expected_return_object](auto &&...args) {
106+
[&method, &expected_return_object](auto &&...args) {
142107
llvm::consumeError(expected_return_object.takeError());
143-
expected_return_object = init(args...);
108+
expected_return_object = method(args...);
144109
},
145110
transformed_args);
146111

147-
if (!expected_return_object)
148-
return expected_return_object.takeError();
149-
result = expected_return_object.get();
112+
if (llvm::Error e = expected_return_object.takeError())
113+
return std::move(e);
114+
result = std::move(expected_return_object.get());
150115
}
151116

152117
if (!result.IsValid())
153-
return create_error("Resulting object is not a valid Python Object.");
154-
if (!result.HasAttribute("__class__"))
155-
return create_error("Resulting object doesn't have '__class__' member.");
156-
157-
PythonObject obj_class = result.GetAttributeValue("__class__");
158-
if (!obj_class.IsValid())
159-
return create_error("Resulting class object is not a valid.");
160-
if (!obj_class.HasAttribute("__name__"))
161-
return create_error(
162-
"Resulting object class doesn't have '__name__' member.");
163-
PythonString obj_class_name =
164-
obj_class.GetAttributeValue("__name__").AsType<PythonString>();
165-
166-
PythonObject object_class_mapping_proxy =
167-
obj_class.GetAttributeValue("__dict__");
168-
if (!obj_class.HasAttribute("__dict__"))
169-
return create_error(
170-
"Resulting object class doesn't have '__dict__' member.");
171-
172-
PythonCallable dict_converter = PythonModule::BuiltinsModule()
173-
.ResolveName("dict")
174-
.AsType<PythonCallable>();
175-
if (!dict_converter.IsAllocated())
176-
return create_error(
177-
"Python 'builtins' module doesn't have 'dict' class.");
178-
179-
PythonDictionary object_class_dict =
180-
dict_converter(object_class_mapping_proxy).AsType<PythonDictionary>();
181-
if (!object_class_dict.IsAllocated())
182-
return create_error("Coudn't create dictionary from resulting object "
183-
"class mapping proxy object.");
184-
185-
auto checker_or_err = CheckAbstractMethodImplementation(object_class_dict);
186-
if (!checker_or_err)
187-
return checker_or_err.takeError();
188-
189-
for (const auto &method_checker : *checker_or_err)
190-
switch (method_checker.second) {
191-
case AbstractMethodCheckerCases::eNotImplemented:
192-
LLDB_LOG(GetLog(LLDBLog::Script),
193-
"Abstract method {0}.{1} not implemented.",
194-
obj_class_name.GetString(), method_checker.first);
195-
break;
196-
case AbstractMethodCheckerCases::eNotAllocated:
197-
LLDB_LOG(GetLog(LLDBLog::Script),
198-
"Abstract method {0}.{1} not allocated.",
199-
obj_class_name.GetString(), method_checker.first);
200-
break;
201-
case AbstractMethodCheckerCases::eNotCallable:
202-
LLDB_LOG(GetLog(LLDBLog::Script),
203-
"Abstract method {0}.{1} not callable.",
204-
obj_class_name.GetString(), method_checker.first);
205-
break;
206-
case AbstractMethodCheckerCases::eValid:
207-
LLDB_LOG(GetLog(LLDBLog::Script),
208-
"Abstract method {0}.{1} implemented & valid.",
209-
obj_class_name.GetString(), method_checker.first);
210-
break;
211-
}
212-
213-
for (const auto &method_checker : *checker_or_err)
214-
if (method_checker.second != AbstractMethodCheckerCases::eValid)
215-
return create_error(
216-
llvm::formatv("Abstract method {0}.{1} missing. Enable lldb "
217-
"script log for more details.",
218-
obj_class_name.GetString(), method_checker.first));
118+
return llvm::createStringError(
119+
llvm::inconvertibleErrorCode(),
120+
"Resulting object is not a valid Python Object.");
219121

220122
m_object_instance_sp = StructuredData::GenericSP(
221123
new StructuredPythonObject(std::move(result)));

‎lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h

-5
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,6 @@ class ScriptedThreadPythonInterface : public ScriptedThreadInterface,
2828
StructuredData::DictionarySP args_sp,
2929
StructuredData::Generic *script_obj = nullptr) override;
3030

31-
llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
32-
return llvm::SmallVector<llvm::StringLiteral>(
33-
{"get_stop_reason", "get_register_context"});
34-
}
35-
3631
lldb::tid_t GetThreadID() override;
3732

3833
std::optional<std::string> GetName() override;

‎lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

-14
Original file line numberDiff line numberDiff line change
@@ -663,20 +663,6 @@ bool PythonDictionary::Check(PyObject *py_obj) {
663663
return PyDict_Check(py_obj);
664664
}
665665

666-
bool PythonDictionary::HasKey(const llvm::Twine &key) const {
667-
if (!IsValid())
668-
return false;
669-
670-
PythonString key_object(key.isSingleStringRef() ? key.getSingleStringRef()
671-
: key.str());
672-
673-
if (int res = PyDict_Contains(m_py_obj, key_object.get()) > 0)
674-
return res;
675-
676-
PyErr_Print();
677-
return false;
678-
}
679-
680666
uint32_t PythonDictionary::GetSize() const {
681667
if (IsValid())
682668
return PyDict_Size(m_py_obj);

‎lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

-2
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,6 @@ class PythonDictionary : public TypedPythonObject<PythonDictionary> {
562562

563563
static bool Check(PyObject *py_obj);
564564

565-
bool HasKey(const llvm::Twine &key) const;
566-
567565
uint32_t GetSize() const;
568566

569567
PythonList GetKeys() const;

‎lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

-49
Original file line numberDiff line numberDiff line change
@@ -60,55 +60,6 @@ def move_blueprint_to_dsym(self, blueprint_name):
6060
)
6161
shutil.copy(blueprint_origin_path, blueprint_destination_path)
6262

63-
def test_missing_methods_scripted_register_context(self):
64-
"""Test that we only instanciate scripted processes if they implement
65-
all the required abstract methods."""
66-
self.build()
67-
68-
os.environ["SKIP_SCRIPTED_PROCESS_LAUNCH"] = "1"
69-
70-
def cleanup():
71-
del os.environ["SKIP_SCRIPTED_PROCESS_LAUNCH"]
72-
73-
self.addTearDownHook(cleanup)
74-
75-
target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
76-
self.assertTrue(target, VALID_TARGET)
77-
log_file = self.getBuildArtifact("script.log")
78-
self.runCmd("log enable lldb script -f " + log_file)
79-
self.assertTrue(os.path.isfile(log_file))
80-
script_path = os.path.join(
81-
self.getSourceDir(), "missing_methods_scripted_process.py"
82-
)
83-
self.runCmd("command script import " + script_path)
84-
85-
launch_info = lldb.SBLaunchInfo(None)
86-
launch_info.SetProcessPluginName("ScriptedProcess")
87-
launch_info.SetScriptedProcessClassName(
88-
"missing_methods_scripted_process.MissingMethodsScriptedProcess"
89-
)
90-
error = lldb.SBError()
91-
92-
process = target.Launch(launch_info, error)
93-
94-
self.assertFailure(error)
95-
96-
with open(log_file, "r") as f:
97-
log = f.read()
98-
99-
self.assertIn(
100-
"Abstract method MissingMethodsScriptedProcess.read_memory_at_address not implemented",
101-
log,
102-
)
103-
self.assertIn(
104-
"Abstract method MissingMethodsScriptedProcess.is_alive not implemented",
105-
log,
106-
)
107-
self.assertIn(
108-
"Abstract method MissingMethodsScriptedProcess.get_scripted_thread_plugin not implemented",
109-
log,
110-
)
111-
11263
@skipUnlessDarwin
11364
def test_invalid_scripted_register_context(self):
11465
"""Test that we can launch an lldb scripted process with an invalid

0 commit comments

Comments
 (0)