intel/llvm
1
0
Fork 0
mirror of https://github.com/intel/llvm.git synced 2026-01-21 12:19:23 +08:00
Code Issues Packages Projects Releases Wiki Activity

[lldb] "target create" shouldn't save target if the command failed

Browse Source 
TargetList::CreateTarget automatically adds created target to the list, however,
CommandObjectTargetCreate does some additional preparation after creating a target
and which can fail. The command should remove created target if it failed. Since
the function has many ways to return, scope guard does this work safely.

Changes to the TargetList make target adding and selection more transparent.

Other changes remove unnecessary SetSelectedTarget after CreateTarget.

Differential Revision: https://reviews.llvm.org/D93052
This commit is contained in:
Tatyana Krasnukha
2020-12-11 11:09:39 +03:00
committed by Tatyana Krasnukha
parent e52bc1d2bb
commit 2634ec6ce9
12 changed files with 181 additions and 191 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
lldb/include/lldb/Target/TargetList.h
View File

@@ -173,7 +173,9 @@ public:
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 @@ protected:
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);
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);
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,
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;

10
lldb/source/API/SBDebugger.cpp
View File

@@ -811,10 +811,8 @@ SBTarget SBDebugger::CreateTargetWithFileAndArch(const char *filename,
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 @@ SBTarget SBDebugger::CreateTarget(const char *filename) {
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 @@ void SBDebugger::SetSelectedTarget(SBTarget &sb_target) {
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;

1
lldb/source/Commands/CommandObjectProcess.cpp
View File

@@ -364,7 +364,6 @@ protected:
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

250
lldb/source/Commands/CommandObjectTarget.cpp
View File

@@ -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 @@ protected:
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");
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");
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);
}
}
}
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 (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));
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());
result.SetStatus(eReturnStatusFailed);
}
} else {
result.AppendMessageWithFormat(
"Current executable set to '%s' (%s).\n",
file_spec.GetPath().c_str(),
target_sp->GetArchitecture().GetArchitectureName());
result.SetStatus(eReturnStatusSuccessFinishNoResult);
}
} else {
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;
}
}
} 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;
}
}
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 (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);
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 (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.AppendErrorWithFormatv(
"Unable to find process plug-in for core file '{0}'\n",
core_file.GetPath());
result.SetStatus(eReturnStatusFailed);
}
} else {
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 @@ protected:
m_cmd_name.c_str());
result.SetStatus(eReturnStatusFailed);
}
return result.Succeeded();
}
@@ -507,18 +510,11 @@ protected:
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(

4
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
View File

@@ -377,7 +377,6 @@ lldb::ProcessSP PlatformPOSIX::Attach(ProcessAttachInfo &attach_info,
}
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",
@@ -462,9 +461,6 @@ PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger,
}
}
// 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 =

2
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
View File

@@ -272,8 +272,6 @@ lldb::ProcessSP PlatformWindows::Attach(ProcessAttachInfo &attach_info,
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);

4
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
View File

@@ -495,8 +495,6 @@ lldb::ProcessSP PlatformRemoteGDBServer::DebugProcess(
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 @@ lldb::ProcessSP PlatformRemoteGDBServer::Attach(
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 =

2
lldb/source/Target/Platform.cpp
View File

@@ -1831,8 +1831,6 @@ lldb::ProcessSP Platform::DoConnectProcess(llvm::StringRef connect_url,
if (!target || error.Fail())
return nullptr;
debugger.GetTargetList().SetSelectedTarget(target);
lldb::ProcessSP process_sp =
target->CreateProcess(debugger.GetListener(), plugin_name, nullptr, true);

55
lldb/source/Target/TargetList.cpp
View File

@@ -48,9 +48,13 @@ Status TargetList::CreateTarget(Debugger &debugger,
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 @@ Status TargetList::CreateTarget(Debugger &debugger,
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 @@ Status TargetList::CreateTargetInternal(Debugger &debugger,
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 @@ uint32_t TargetList::GetIndexOfTarget(lldb::TargetSP target_sp) const {
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() {

2
lldb/source/Target/TraceSessionFileParser.cpp
View File

@@ -123,8 +123,6 @@ TraceSessionFileParser::ParseProcess(const JSONProcess &process) {
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,

7
lldb/unittests/Process/ProcessEventDataTest.cpp
View File

@@ -112,16 +112,11 @@ typedef std::shared_ptr<Process::ProcessEventData> ProcessEventDataSP;
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;
}

7
lldb/unittests/Thread/ThreadTest.cpp
View File

@@ -83,16 +83,11 @@ public:
} // 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;
}