From bd590a5f895f50ceb7669de1b4a69bbcc8adb38b Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 16 Sep 2021 11:14:16 +0200 Subject: [PATCH] [lldb] Make Platform::DebugProcess take a Target reference instead of a pointer. There are just two callers of this function, and both of them have a valid target pointer, so there's no need for all implementations to concern themselves with whether the pointer is null. --- lldb/include/lldb/Target/Platform.h | 8 ++-- .../source/Commands/CommandObjectPlatform.cpp | 2 +- .../MacOSX/PlatformAppleSimulator.cpp | 11 ++--- .../Platform/MacOSX/PlatformAppleSimulator.h | 2 +- .../Platform/MacOSX/PlatformDarwin.cpp | 9 ++-- .../Plugins/Platform/MacOSX/PlatformDarwin.h | 2 +- .../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 35 +++----------- .../Plugins/Platform/POSIX/PlatformPOSIX.h | 6 +-- .../Platform/Windows/PlatformWindows.cpp | 6 +-- .../Platform/Windows/PlatformWindows.h | 2 +- .../gdb-server/PlatformRemoteGDBServer.cpp | 46 +++++++------------ .../gdb-server/PlatformRemoteGDBServer.h | 5 +- lldb/source/Target/Platform.cpp | 15 +++--- lldb/source/Target/Target.cpp | 2 +- 14 files changed, 51 insertions(+), 100 deletions(-) diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h index df46466655c3..878739c46fa4 100644 --- a/lldb/include/lldb/Target/Platform.h +++ b/lldb/include/lldb/Target/Platform.h @@ -363,11 +363,9 @@ public: /// platforms will want to subclass this function in order to be able to /// intercept STDIO and possibly launch a separate process that will debug /// the debuggee. - virtual lldb::ProcessSP - DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, - Target *target, // Can be nullptr, if nullptr create a new - // target, else use existing one - Status &error); + virtual lldb::ProcessSP DebugProcess(ProcessLaunchInfo &launch_info, + Debugger &debugger, Target &target, + Status &error); virtual lldb::ProcessSP ConnectProcess(llvm::StringRef connect_url, llvm::StringRef plugin_name, diff --git a/lldb/source/Commands/CommandObjectPlatform.cpp b/lldb/source/Commands/CommandObjectPlatform.cpp index 7760eba290d4..c4444d4d880e 100644 --- a/lldb/source/Commands/CommandObjectPlatform.cpp +++ b/lldb/source/Commands/CommandObjectPlatform.cpp @@ -1171,7 +1171,7 @@ protected: target->GetRunArguments(m_options.launch_info.GetArguments()); ProcessSP process_sp(platform_sp->DebugProcess( - m_options.launch_info, debugger, target, error)); + m_options.launch_info, debugger, *target, error)); if (process_sp && process_sp->IsAlive()) { result.SetStatus(eReturnStatusSuccessFinishNoResult); return true; diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp index 925a3d110b1d..0919ba23e371 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp @@ -177,11 +177,10 @@ Status PlatformAppleSimulator::DisconnectRemote() { #endif } -lldb::ProcessSP PlatformAppleSimulator::DebugProcess( - ProcessLaunchInfo &launch_info, Debugger &debugger, - Target *target, // Can be NULL, if NULL create a new target, else use - // existing one - Status &error) { +lldb::ProcessSP +PlatformAppleSimulator::DebugProcess(ProcessLaunchInfo &launch_info, + Debugger &debugger, Target &target, + Status &error) { #if defined(__APPLE__) ProcessSP process_sp; // Make sure we stop at the entry point @@ -195,7 +194,7 @@ lldb::ProcessSP PlatformAppleSimulator::DebugProcess( if (error.Success()) { if (launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID) { ProcessAttachInfo attach_info(launch_info); - process_sp = Attach(attach_info, debugger, target, error); + process_sp = Attach(attach_info, debugger, &target, error); if (process_sp) { launch_info.SetHijackListener(attach_info.GetHijackListener()); diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h index 2e6ce02635c5..fc428ad2be46 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h @@ -60,7 +60,7 @@ public: lldb::ProcessSP DebugProcess(lldb_private::ProcessLaunchInfo &launch_info, lldb_private::Debugger &debugger, - lldb_private::Target *target, + lldb_private::Target &target, lldb_private::Status &error) override; bool GetSupportedArchitectureAtIndex(uint32_t idx, diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 62da06095139..943b157c6963 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1226,12 +1226,9 @@ PlatformDarwin::GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) { return 1; } -lldb::ProcessSP -PlatformDarwin::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, - Target *target, // Can be NULL, if NULL create - // a new target, else use existing - // one - Status &error) { +lldb::ProcessSP PlatformDarwin::DebugProcess(ProcessLaunchInfo &launch_info, + Debugger &debugger, Target &target, + Status &error) { ProcessSP process_sp; if (IsHost()) { diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h index 22f7ad4ff823..c3862f14a040 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h @@ -71,7 +71,7 @@ public: lldb::ProcessSP DebugProcess(lldb_private::ProcessLaunchInfo &launch_info, lldb_private::Debugger &debugger, - lldb_private::Target *target, + lldb_private::Target &target, lldb_private::Status &error) override; void CalculateTrapHandlerSymbolNames() override; diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp index 71917af43378..4a93e58598ef 100644 --- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -410,13 +410,11 @@ lldb::ProcessSP PlatformPOSIX::Attach(ProcessAttachInfo &attach_info, return process_sp; } -lldb::ProcessSP -PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, - Target *target, // Can be NULL, if NULL create a new - // target, else use existing one - Status &error) { +lldb::ProcessSP PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info, + Debugger &debugger, Target &target, + Status &error) { Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM)); - LLDB_LOG(log, "target {0}", target); + LLDB_LOG(log, "target {0}", &target); ProcessSP process_sp; @@ -442,29 +440,10 @@ PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, // worry about the target getting them as well. launch_info.SetLaunchInSeparateProcessGroup(true); - // Ensure we have a target. - if (target == nullptr) { - LLDB_LOG(log, "creating new target"); - TargetSP new_target_sp; - error = debugger.GetTargetList().CreateTarget( - debugger, "", "", eLoadDependentsNo, nullptr, new_target_sp); - if (error.Fail()) { - LLDB_LOG(log, "failed to create new target: {0}", error); - return process_sp; - } - - target = new_target_sp.get(); - if (!target) { - error.SetErrorString("CreateTarget() returned nullptr"); - LLDB_LOG(log, "error: {0}", error); - return process_sp; - } - } - // Now create the gdb-remote process. LLDB_LOG(log, "having target create process with gdb-remote plugin"); process_sp = - target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr, + target.CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr, true); if (!process_sp) { @@ -518,8 +497,8 @@ PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, LLDB_LOG(log, "not using process STDIO pty"); } else { LLDB_LOG(log, "{0}", error); - // FIXME figure out appropriate cleanup here. Do we delete the target? Do - // we delete the process? Does our caller do that? + // FIXME figure out appropriate cleanup here. Do we delete the process? + // Does our caller do that? } return process_sp; diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h index 1cba4c5eb2e9..511797ce6bb7 100644 --- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h +++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h @@ -47,11 +47,7 @@ public: lldb::ProcessSP DebugProcess(lldb_private::ProcessLaunchInfo &launch_info, lldb_private::Debugger &debugger, - lldb_private::Target *target, // Can be nullptr, - // if nullptr - // create a new - // target, else use - // existing one + lldb_private::Target &target, lldb_private::Status &error) override; std::string GetPlatformSpecificConnectionInformation() override; diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp index fd28eec63808..fdb6758c8b7a 100644 --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp +++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp @@ -198,7 +198,7 @@ Status PlatformWindows::DisconnectRemote() { } ProcessSP PlatformWindows::DebugProcess(ProcessLaunchInfo &launch_info, - Debugger &debugger, Target *target, + Debugger &debugger, Target &target, Status &error) { // Windows has special considerations that must be followed when launching or // attaching to a process. The key requirement is that when launching or @@ -230,9 +230,9 @@ ProcessSP PlatformWindows::DebugProcess(ProcessLaunchInfo &launch_info, if (launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID) { // This is a process attach. Don't need to launch anything. ProcessAttachInfo attach_info(launch_info); - return Attach(attach_info, debugger, target, error); + return Attach(attach_info, debugger, &target, error); } else { - ProcessSP process_sp = target->CreateProcess( + ProcessSP process_sp = target.CreateProcess( launch_info.GetListener(), launch_info.GetProcessPluginName(), nullptr, false); diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.h b/lldb/source/Plugins/Platform/Windows/PlatformWindows.h index 8c02d8e0c988..aff6924e448d 100644 --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.h +++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.h @@ -42,7 +42,7 @@ public: lldb::ProcessSP DebugProcess(lldb_private::ProcessLaunchInfo &launch_info, lldb_private::Debugger &debugger, - lldb_private::Target *target, + lldb_private::Target &target, lldb_private::Status &error) override; lldb::ProcessSP Attach(lldb_private::ProcessAttachInfo &attach_info, diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp index 528208665a4e..01a0ec5ccaa8 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -475,11 +475,10 @@ Status PlatformRemoteGDBServer::KillProcess(const lldb::pid_t pid) { return Status(); } -lldb::ProcessSP PlatformRemoteGDBServer::DebugProcess( - ProcessLaunchInfo &launch_info, Debugger &debugger, - Target *target, // Can be NULL, if NULL create a new target, else use - // existing one - Status &error) { +lldb::ProcessSP +PlatformRemoteGDBServer::DebugProcess(ProcessLaunchInfo &launch_info, + Debugger &debugger, Target &target, + Status &error) { lldb::ProcessSP process_sp; if (IsRemote()) { if (IsConnected()) { @@ -489,32 +488,21 @@ lldb::ProcessSP PlatformRemoteGDBServer::DebugProcess( error.SetErrorStringWithFormat("unable to launch a GDB server on '%s'", GetHostname()); } else { - if (target == nullptr) { - TargetSP new_target_sp; + // The darwin always currently uses the GDB remote debugger plug-in + // so even when debugging locally we are debugging remotely! + process_sp = target.CreateProcess(launch_info.GetListener(), + "gdb-remote", nullptr, true); - error = debugger.GetTargetList().CreateTarget( - debugger, "", "", eLoadDependentsNo, nullptr, new_target_sp); - target = new_target_sp.get(); - } else - error.Clear(); - - if (target && error.Success()) { - // The darwin always currently uses the GDB remote debugger plug-in - // so even when debugging locally we are debugging remotely! - process_sp = target->CreateProcess(launch_info.GetListener(), - "gdb-remote", nullptr, true); - - if (process_sp) { + if (process_sp) { + error = process_sp->ConnectRemote(connect_url.c_str()); + // Retry the connect remote one time... + if (error.Fail()) error = process_sp->ConnectRemote(connect_url.c_str()); - // Retry the connect remote one time... - if (error.Fail()) - error = process_sp->ConnectRemote(connect_url.c_str()); - if (error.Success()) - error = process_sp->Launch(launch_info); - else if (debugserver_pid != LLDB_INVALID_PROCESS_ID) { - printf("error: connect remote failed (%s)\n", error.AsCString()); - KillSpawnedProcess(debugserver_pid); - } + if (error.Success()) + error = process_sp->Launch(launch_info); + else if (debugserver_pid != LLDB_INVALID_PROCESS_ID) { + printf("error: connect remote failed (%s)\n", error.AsCString()); + KillSpawnedProcess(debugserver_pid); } } } diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h index 834140f8a22a..4f0e3c2100d2 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h @@ -62,10 +62,7 @@ public: Status KillProcess(const lldb::pid_t pid) override; lldb::ProcessSP DebugProcess(ProcessLaunchInfo &launch_info, - Debugger &debugger, - Target *target, // Can be NULL, if NULL create a - // new target, else use existing - // one + Debugger &debugger, Target &target, Status &error) override; lldb::ProcessSP Attach(ProcessAttachInfo &attach_info, Debugger &debugger, diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp index 23141c2842f2..a9b97f795795 100644 --- a/lldb/source/Target/Platform.cpp +++ b/lldb/source/Target/Platform.cpp @@ -1088,14 +1088,11 @@ Status Platform::KillProcess(const lldb::pid_t pid) { return Status(); } -lldb::ProcessSP -Platform::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, - Target *target, // Can be nullptr, if nullptr create a - // new target, else use existing one - Status &error) { +lldb::ProcessSP Platform::DebugProcess(ProcessLaunchInfo &launch_info, + Debugger &debugger, Target &target, + Status &error) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM)); - LLDB_LOGF(log, "Platform::%s entered (target %p)", __FUNCTION__, - static_cast(target)); + LLDB_LOG(log, "target = {0})", &target); ProcessSP process_sp; // Make sure we stop at the entry point @@ -1117,7 +1114,7 @@ Platform::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, filter_callback = get_filter_func(++i, iteration_complete)) { if (filter_callback) { // Give this ProcessLaunchInfo filter a chance to adjust the launch info. - error = (*filter_callback)(launch_info, target); + error = (*filter_callback)(launch_info, &target); if (!error.Success()) { LLDB_LOGF(log, "Platform::%s() StructuredDataPlugin launch " @@ -1135,7 +1132,7 @@ Platform::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, __FUNCTION__, launch_info.GetProcessID()); if (launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID) { ProcessAttachInfo attach_info(launch_info); - process_sp = Attach(attach_info, debugger, target, error); + process_sp = Attach(attach_info, debugger, &target, error); if (process_sp) { LLDB_LOGF(log, "Platform::%s Attach() succeeded, Process plugin: %s", __FUNCTION__, process_sp->GetPluginName().AsCString()); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 159928981c92..e24aa58fb940 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -2993,7 +2993,7 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) { DeleteCurrentProcess(); m_process_sp = - GetPlatform()->DebugProcess(launch_info, debugger, this, error); + GetPlatform()->DebugProcess(launch_info, debugger, *this, error); } else { LLDB_LOGF(log,