Index: lldb/include/lldb/Target/TargetList.h =================================================================== --- lldb/include/lldb/Target/TargetList.h +++ lldb/include/lldb/Target/TargetList.h @@ -173,7 +173,9 @@ uint32_t SignalIfRunning(lldb::pid_t pid, int signo); - uint32_t SetSelectedTarget(Target *target); + void SetSelectedTarget(uint32_t index); + + void SetSelectedTarget(const lldb::TargetSP &target); lldb::TargetSP GetSelectedTarget(); @@ -185,17 +187,21 @@ uint32_t m_selected_target_idx; private: - Status CreateTargetInternal(Debugger &debugger, llvm::StringRef user_exe_path, - llvm::StringRef triple_str, - LoadDependentFiles load_dependent_files, - const OptionGroupPlatform *platform_options, - lldb::TargetSP &target_sp); - - Status CreateTargetInternal(Debugger &debugger, llvm::StringRef user_exe_path, - const ArchSpec &arch, - LoadDependentFiles get_dependent_modules, - lldb::PlatformSP &platform_sp, - lldb::TargetSP &target_sp); + static Status CreateTargetInternal( + Debugger &debugger, llvm::StringRef user_exe_path, + llvm::StringRef triple_str, LoadDependentFiles load_dependent_files, + const OptionGroupPlatform *platform_options, lldb::TargetSP &target_sp); + + static Status CreateTargetInternal(Debugger &debugger, + llvm::StringRef user_exe_path, + const ArchSpec &arch, + LoadDependentFiles get_dependent_modules, + lldb::PlatformSP &platform_sp, + lldb::TargetSP &target_sp); + + void AddTargetInternal(lldb::TargetSP target_sp, bool do_select); + + void SetSelectedTargetInternal(uint32_t index); TargetList(const TargetList &) = delete; const TargetList &operator=(const TargetList &) = delete; Index: lldb/source/API/SBDebugger.cpp =================================================================== --- lldb/source/API/SBDebugger.cpp +++ lldb/source/API/SBDebugger.cpp @@ -811,10 +811,8 @@ add_dependent_modules ? eLoadDependentsYes : eLoadDependentsNo, nullptr, target_sp); - if (error.Success()) { - m_opaque_sp->GetTargetList().SetSelectedTarget(target_sp.get()); + if (error.Success()) sb_target.SetSP(target_sp); - } } LLDB_LOGF(log, @@ -840,10 +838,8 @@ add_dependent_modules ? eLoadDependentsYes : eLoadDependentsNo, nullptr, target_sp); - if (error.Success()) { - m_opaque_sp->GetTargetList().SetSelectedTarget(target_sp.get()); + if (error.Success()) sb_target.SetSP(target_sp); - } } Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API)); LLDB_LOGF(log, @@ -998,7 +994,7 @@ TargetSP target_sp(sb_target.GetSP()); if (m_opaque_sp) { - m_opaque_sp->GetTargetList().SetSelectedTarget(target_sp.get()); + m_opaque_sp->GetTargetList().SetSelectedTarget(target_sp); } if (log) { SBStream sstr; Index: lldb/source/Commands/CommandObjectProcess.cpp =================================================================== --- lldb/source/Commands/CommandObjectProcess.cpp +++ lldb/source/Commands/CommandObjectProcess.cpp @@ -364,7 +364,6 @@ result.AppendError(error.AsCString("Error creating target")); return false; } - GetDebugger().GetTargetList().SetSelectedTarget(target); } // Record the old executable module, we want to issue a warning if the Index: lldb/source/Commands/CommandObjectTarget.cpp =================================================================== --- lldb/source/Commands/CommandObjectTarget.cpp +++ lldb/source/Commands/CommandObjectTarget.cpp @@ -50,6 +50,7 @@ #include "lldb/Utility/State.h" #include "lldb/Utility/Timer.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/FormatAdapters.h" @@ -318,123 +319,124 @@ m_add_dependents.m_load_dependent_files, &m_platform_options, target_sp)); - if (target_sp) { - // Only get the platform after we create the target because we might - // have switched platforms depending on what the arguments were to - // CreateTarget() we can't rely on the selected platform. - - PlatformSP platform_sp = target_sp->GetPlatform(); - - if (remote_file) { - if (platform_sp) { - // I have a remote file.. two possible cases - if (file_spec && FileSystem::Instance().Exists(file_spec)) { - // if the remote file does not exist, push it there - if (!platform_sp->GetFileExists(remote_file)) { - Status err = platform_sp->PutFile(file_spec, remote_file); - if (err.Fail()) { - result.AppendError(err.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - } - } else { - // there is no local file and we need one - // in order to make the remote ---> local transfer we need a - // platform - // TODO: if the user has passed in a --platform argument, use it - // to fetch the right platform - if (!platform_sp) { - result.AppendError( - "unable to perform remote debugging without a platform"); + if (!target_sp) { + result.AppendError(error.AsCString()); + result.SetStatus(eReturnStatusFailed); + return false; + } + + auto on_error = llvm::make_scope_exit( + [&target_list = debugger.GetTargetList(), &target_sp]() { + target_list.DeleteTarget(target_sp); + }); + + // Only get the platform after we create the target because we might + // have switched platforms depending on what the arguments were to + // CreateTarget() we can't rely on the selected platform. + + PlatformSP platform_sp = target_sp->GetPlatform(); + + if (remote_file) { + if (platform_sp) { + // I have a remote file.. two possible cases + if (file_spec && FileSystem::Instance().Exists(file_spec)) { + // if the remote file does not exist, push it there + if (!platform_sp->GetFileExists(remote_file)) { + Status err = platform_sp->PutFile(file_spec, remote_file); + if (err.Fail()) { + result.AppendError(err.AsCString()); result.SetStatus(eReturnStatusFailed); return false; } - if (file_path) { - // copy the remote file to the local file - Status err = platform_sp->GetFile(remote_file, file_spec); - if (err.Fail()) { - result.AppendError(err.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - } else { - // make up a local file - result.AppendError("remote --> local transfer without local " - "path is not implemented yet"); + } + } else { + // there is no local file and we need one + // in order to make the remote ---> local transfer we need a + // platform + // TODO: if the user has passed in a --platform argument, use it + // to fetch the right platform + if (file_path) { + // copy the remote file to the local file + Status err = platform_sp->GetFile(remote_file, file_spec); + if (err.Fail()) { + result.AppendError(err.AsCString()); result.SetStatus(eReturnStatusFailed); return false; } + } else { + // make up a local file + result.AppendError("remote --> local transfer without local " + "path is not implemented yet"); + result.SetStatus(eReturnStatusFailed); + return false; } - } else { - result.AppendError("no platform found for target"); - result.SetStatus(eReturnStatusFailed); - return false; } + } else { + result.AppendError("no platform found for target"); + result.SetStatus(eReturnStatusFailed); + return false; } + } - if (symfile || remote_file) { - ModuleSP module_sp(target_sp->GetExecutableModule()); - if (module_sp) { - if (symfile) - module_sp->SetSymbolFileFileSpec(symfile); - if (remote_file) { - std::string remote_path = remote_file.GetPath(); - target_sp->SetArg0(remote_path.c_str()); - module_sp->SetPlatformFileSpec(remote_file); - } + if (symfile || remote_file) { + ModuleSP module_sp(target_sp->GetExecutableModule()); + if (module_sp) { + if (symfile) + module_sp->SetSymbolFileFileSpec(symfile); + if (remote_file) { + std::string remote_path = remote_file.GetPath(); + target_sp->SetArg0(remote_path.c_str()); + module_sp->SetPlatformFileSpec(remote_file); } } + } - debugger.GetTargetList().SetSelectedTarget(target_sp.get()); - if (must_set_platform_path) { - ModuleSpec main_module_spec(file_spec); - ModuleSP module_sp = - target_sp->GetOrCreateModule(main_module_spec, true /* notify */); - if (module_sp) - module_sp->SetPlatformFileSpec(remote_file); - } + if (must_set_platform_path) { + ModuleSpec main_module_spec(file_spec); + ModuleSP module_sp = + target_sp->GetOrCreateModule(main_module_spec, true /* notify */); + if (module_sp) + module_sp->SetPlatformFileSpec(remote_file); + } - if (core_file) { - FileSpec core_file_dir; - core_file_dir.GetDirectory() = core_file.GetDirectory(); - target_sp->AppendExecutableSearchPaths(core_file_dir); + if (core_file) { + FileSpec core_file_dir; + core_file_dir.GetDirectory() = core_file.GetDirectory(); + target_sp->AppendExecutableSearchPaths(core_file_dir); - ProcessSP process_sp(target_sp->CreateProcess( - GetDebugger().GetListener(), llvm::StringRef(), &core_file, - false)); + ProcessSP process_sp(target_sp->CreateProcess( + GetDebugger().GetListener(), llvm::StringRef(), &core_file, false)); - if (process_sp) { - // Seems weird that we Launch a core file, but that is what we - // do! - error = process_sp->LoadCore(); + if (process_sp) { + // Seems weird that we Launch a core file, but that is what we + // do! + error = process_sp->LoadCore(); - if (error.Fail()) { - result.AppendError( - error.AsCString("can't find plug-in for core file")); - result.SetStatus(eReturnStatusFailed); - return false; - } else { - result.AppendMessageWithFormatv("Core file '{0}' ({1}) was loaded.\n", core_file.GetPath(), - target_sp->GetArchitecture().GetArchitectureName()); - result.SetStatus(eReturnStatusSuccessFinishNoResult); - } - } else { - result.AppendErrorWithFormatv( - "Unable to find process plug-in for core file '{0}'\n", - core_file.GetPath()); + if (error.Fail()) { + result.AppendError( + error.AsCString("can't find plug-in for core file")); result.SetStatus(eReturnStatusFailed); + return false; + } else { + result.AppendMessageWithFormatv( + "Core file '{0}' ({1}) was loaded.\n", core_file.GetPath(), + target_sp->GetArchitecture().GetArchitectureName()); + result.SetStatus(eReturnStatusSuccessFinishNoResult); + on_error.release(); } } else { - result.AppendMessageWithFormat( - "Current executable set to '%s' (%s).\n", - file_spec.GetPath().c_str(), - target_sp->GetArchitecture().GetArchitectureName()); - result.SetStatus(eReturnStatusSuccessFinishNoResult); + result.AppendErrorWithFormatv( + "Unable to find process plug-in for core file '{0}'\n", + core_file.GetPath()); + result.SetStatus(eReturnStatusFailed); } } else { - result.AppendError(error.AsCString()); - result.SetStatus(eReturnStatusFailed); + result.AppendMessageWithFormat( + "Current executable set to '%s' (%s).\n", + file_spec.GetPath().c_str(), + target_sp->GetArchitecture().GetArchitectureName()); + result.SetStatus(eReturnStatusSuccessFinishNoResult); + on_error.release(); } } else { result.AppendErrorWithFormat("'%s' takes exactly one executable path " @@ -442,6 +444,7 @@ m_cmd_name.c_str()); result.SetStatus(eReturnStatusFailed); } + return result.Succeeded(); } @@ -507,18 +510,11 @@ TargetList &target_list = GetDebugger().GetTargetList(); const uint32_t num_targets = target_list.GetNumTargets(); if (target_idx < num_targets) { - TargetSP target_sp(target_list.GetTargetAtIndex(target_idx)); - if (target_sp) { - Stream &strm = result.GetOutputStream(); - target_list.SetSelectedTarget(target_sp.get()); - bool show_stopped_process_status = false; - DumpTargetList(target_list, show_stopped_process_status, strm); - result.SetStatus(eReturnStatusSuccessFinishResult); - } else { - result.AppendErrorWithFormat("target #%u is NULL in target list\n", - target_idx); - result.SetStatus(eReturnStatusFailed); - } + target_list.SetSelectedTarget(target_idx); + Stream &strm = result.GetOutputStream(); + bool show_stopped_process_status = false; + DumpTargetList(target_list, show_stopped_process_status, strm); + result.SetStatus(eReturnStatusSuccessFinishResult); } else { if (num_targets > 0) { result.AppendErrorWithFormat( Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp =================================================================== --- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -377,7 +377,6 @@ } if (target && error.Success()) { - debugger.GetTargetList().SetSelectedTarget(target); if (log) { ModuleSP exe_module_sp = target->GetExecutableModule(); LLDB_LOGF(log, "PlatformPOSIX::%s set selected target to %p %s", @@ -463,9 +462,6 @@ } } - // Mark target as currently selected target. - debugger.GetTargetList().SetSelectedTarget(target); - // Now create the gdb-remote process. LLDB_LOG(log, "having target create process with gdb-remote plugin"); process_sp = Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp =================================================================== --- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp +++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp @@ -272,8 +272,6 @@ if (!target || error.Fail()) return process_sp; - debugger.GetTargetList().SetSelectedTarget(target); - const char *plugin_name = attach_info.GetProcessPluginName(); process_sp = target->CreateProcess( attach_info.GetListenerForProcess(debugger), plugin_name, nullptr, false); Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp =================================================================== --- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -495,8 +495,6 @@ error.Clear(); if (target && error.Success()) { - debugger.GetTargetList().SetSelectedTarget(target); - // 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(), @@ -581,8 +579,6 @@ error.Clear(); if (target && error.Success()) { - debugger.GetTargetList().SetSelectedTarget(target); - // The darwin always currently uses the GDB remote debugger plug-in // so even when debugging locally we are debugging remotely! process_sp = Index: lldb/source/Target/Platform.cpp =================================================================== --- lldb/source/Target/Platform.cpp +++ lldb/source/Target/Platform.cpp @@ -1831,8 +1831,6 @@ if (!target || error.Fail()) return nullptr; - debugger.GetTargetList().SetSelectedTarget(target); - lldb::ProcessSP process_sp = target->CreateProcess(debugger.GetListener(), plugin_name, nullptr, true); Index: lldb/source/Target/TargetList.cpp =================================================================== --- lldb/source/Target/TargetList.cpp +++ lldb/source/Target/TargetList.cpp @@ -48,9 +48,13 @@ LoadDependentFiles load_dependent_files, const OptionGroupPlatform *platform_options, TargetSP &target_sp) { - return CreateTargetInternal(debugger, user_exe_path, triple_str, - load_dependent_files, platform_options, - target_sp); + auto result = TargetList::CreateTargetInternal( + debugger, user_exe_path, triple_str, load_dependent_files, + platform_options, target_sp); + + if (target_sp && result.Success()) + AddTargetInternal(target_sp, /*do_select*/ true); + return result; } Status TargetList::CreateTarget(Debugger &debugger, @@ -58,8 +62,13 @@ const ArchSpec &specified_arch, LoadDependentFiles load_dependent_files, PlatformSP &platform_sp, TargetSP &target_sp) { - return CreateTargetInternal(debugger, user_exe_path, specified_arch, - load_dependent_files, platform_sp, target_sp); + auto result = TargetList::CreateTargetInternal( + debugger, user_exe_path, specified_arch, load_dependent_files, + platform_sp, target_sp); + + if (target_sp && result.Success()) + AddTargetInternal(target_sp, /*do_select*/ true); + return result; } Status TargetList::CreateTargetInternal( @@ -388,9 +397,6 @@ target_sp->AppendExecutableSearchPaths(file_dir); } - std::lock_guard guard(m_target_list_mutex); - m_selected_target_idx = m_target_list.size(); - m_target_list.push_back(target_sp); // Now prime this from the dummy target: target_sp->PrimeFromDummyTarget(debugger.GetDummyTarget()); @@ -552,18 +558,29 @@ return UINT32_MAX; } -uint32_t TargetList::SetSelectedTarget(Target *target) { +void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) { + lldbassert(std::find(m_target_list.begin(), m_target_list.end(), target_sp) == + m_target_list.end() && + "target already exists it the list"); + m_target_list.push_back(std::move(target_sp)); + if (do_select) + SetSelectedTargetInternal(m_target_list.size() - 1); +} + +void TargetList::SetSelectedTargetInternal(uint32_t index) { + lldbassert(!m_target_list.empty()); + m_selected_target_idx = index < m_target_list.size() ? index : 0; +} + +void TargetList::SetSelectedTarget(uint32_t index) { std::lock_guard guard(m_target_list_mutex); - collection::const_iterator pos, begin = m_target_list.begin(), - end = m_target_list.end(); - for (pos = begin; pos != end; ++pos) { - if (pos->get() == target) { - m_selected_target_idx = std::distance(begin, pos); - return m_selected_target_idx; - } - } - m_selected_target_idx = 0; - return m_selected_target_idx; + SetSelectedTargetInternal(index); +} + +void TargetList::SetSelectedTarget(const TargetSP &target_sp) { + std::lock_guard guard(m_target_list_mutex); + auto it = std::find(m_target_list.begin(), m_target_list.end(), target_sp); + SetSelectedTargetInternal(std::distance(m_target_list.begin(), it)); } lldb::TargetSP TargetList::GetSelectedTarget() { Index: lldb/source/Target/TraceSessionFileParser.cpp =================================================================== --- lldb/source/Target/TraceSessionFileParser.cpp +++ lldb/source/Target/TraceSessionFileParser.cpp @@ -123,8 +123,6 @@ ParsedProcess parsed_process; parsed_process.target_sp = target_sp; - m_debugger.GetTargetList().SetSelectedTarget(target_sp.get()); - ProcessSP process_sp = target_sp->CreateProcess( /*listener*/ nullptr, "trace", /*crash_file*/ nullptr, Index: lldb/unittests/Process/ProcessEventDataTest.cpp =================================================================== --- lldb/unittests/Process/ProcessEventDataTest.cpp +++ lldb/unittests/Process/ProcessEventDataTest.cpp @@ -111,16 +111,11 @@ typedef std::shared_ptr EventSP; TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) { - Status error; PlatformSP platform_sp; TargetSP target_sp; - error = debugger_sp->GetTargetList().CreateTarget( + debugger_sp->GetTargetList().CreateTarget( *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp); - if (target_sp) { - debugger_sp->GetTargetList().SetSelectedTarget(target_sp.get()); - } - return target_sp; } Index: lldb/unittests/Thread/ThreadTest.cpp =================================================================== --- lldb/unittests/Thread/ThreadTest.cpp +++ lldb/unittests/Thread/ThreadTest.cpp @@ -83,16 +83,11 @@ } // namespace TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) { - Status error; PlatformSP platform_sp; TargetSP target_sp; - error = debugger_sp->GetTargetList().CreateTarget( + debugger_sp->GetTargetList().CreateTarget( *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp); - if (target_sp) { - debugger_sp->GetTargetList().SetSelectedTarget(target_sp.get()); - } - return target_sp; }