tatyana-krasnukha updated this revision to Diff 311128.
tatyana-krasnukha added a comment.
Removed `do_select`'s default value.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93052/new/
https://reviews.llvm.org/D93052
Files:
lldb/include/lldb/Target/TargetList.h
lldb/source/API/SBDebugger.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/TargetList.cpp
lldb/source/Target/TraceSessionFileParser.cpp
lldb/unittests/Process/ProcessEventDataTest.cpp
lldb/unittests/Thread/ThreadTest.cpp
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;
}
Index: lldb/unittests/Process/ProcessEventDataTest.cpp
===================================================================
--- lldb/unittests/Process/ProcessEventDataTest.cpp
+++ lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -111,16 +111,11 @@
typedef std::shared_ptr<Event> 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/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/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<std::recursive_mutex> 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<std::recursive_mutex> 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<std::recursive_mutex> 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/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/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/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/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/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/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/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/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;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
