RFC https://discourse.llvm.org/t/rfc-python-callback-for-target-get-module/71580
Use SWIG for the locate module callback the same as other Python callbacks.
TestLocateModuleCallback.py verifies the functionalities.
Differential D153735
[lldb][LocateModuleCallback] Implement API, Python interface splhack on Jun 25 2023, 6:18 PM. Authored by
Details RFC https://discourse.llvm.org/t/rfc-python-callback-for-target-get-module/71580 Use SWIG for the locate module callback the same as other Python callbacks.
Diff Detail
Event TimelineComment Actions Move SetTargetGetModuleCallback from SBDebugger to SBPlatform.
Comment Actions
Comment Actions It would be nice to pass the baton down into the platform layer if possible so that internal users could use the callback + baton. See comments in the other diff and see if you agree Comment Actions @clayborg yes, it'd be possible to pass the baton down to Platform.h since it is just void *, so no dependencies required. But, first, let me explain the callback and baton flow. For Python, user will set a callable Python object as the callback. Both Python callback and C++ API callback will go into SBPlatform SetLocateModuleCallback. | | | | | | | user callback |<-SBModuleSpec->| SBPlatform lambda |<-ModuleSpec->| Platform | | baton | SBFileSpec | captures callback| FileSpec | | | | | baton | | | This summary is what things are retained by which layer.
There are three things.
And back to 'passing the baton down to Platform.h'. m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec, FileSpec &module_file_spec, FileSpec &symbol_file_spec) { CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec); symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file)); return Status(); });
Comment Actions Thanks for the detailed explanation. We don't want internal users to add a callback with a baton to Platform.h as this would interfere with the APIs, so it is fine to not have a baton in Platform.h. Comment Actions @splhack the shell tests are all asserting on Darwin systems (on my desktop and on the bots https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/ ): [ RUN ] LocateModuleCallbackTest.GetOrCreateModuleCallbackSuccessWithModuleAndBreakpadSymbol Assertion failed: (default_platform_sp), function Debugger, file Debugger.cpp, line 837. Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 TargetTests 0x000000010048a5b0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 80 1 TargetTests 0x000000010048ab4c PrintStackTraceSignalHandler(void*) + 28 2 TargetTests 0x000000010048887c llvm::sys::RunSignalHandlers() + 152 3 TargetTests 0x000000010048bd08 SignalHandler(int) + 276 4 libsystem_platform.dylib 0x0000000183a65a24 _sigtramp + 56 5 libsystem_pthread.dylib 0x0000000183a36cc0 pthread_kill + 288 6 libsystem_c.dylib 0x0000000183946a80 abort + 180 7 libsystem_c.dylib 0x0000000183945d9c err + 0 8 TargetTests 0x000000010050afa0 lldb_private::Debugger::Debugger(void (*)(char const*, void*), void*) + 1700 9 TargetTests 0x0000000100509be8 lldb_private::Debugger::Debugger(void (*)(char const*, void*), void*) + 44 10 TargetTests 0x0000000100509b10 lldb_private::Debugger::CreateInstance(void (*)(char const*, void*), void*) + 68 11 TargetTests 0x00000001001e9434 (anonymous namespace)::LocateModuleCallbackTest::SetUp() + 124 In these tests, 835 // Always add our default platform to the platform list -> 836 PlatformSP default_platform_sp(Platform::GetHostPlatform()); 837 assert(default_platform_sp); 838 m_platform_list.Append(default_platform_sp, true); Platform::GetHostPlatform() is returning no platform. I don't know the contracts for this call offhand and if there might be an issue with how the debugger was created that is causing this. Comment Actions @jasonmolenda thanks for the report. let me check. At least ninja check-lldb-unit check-lldb-api-python_api-sbplatform is ok on macOS arm64. Comment Actions I'm checking other Debugger::CreateInstance calls in unittests. Comment Actions @jasonmolenda D155135 should fix the Debugger::CreateInstance assert. |
If we are putting this into SBPlatform, this should probably be named "SBPlatformGetModuleCallback".
The name might be a bit more clear if it was "SBPlatformLocateModuleCallback"? Open to suggestions or fine to lave this as "SBPlatformGetModuleCallback" if everyone likes that.