Skip to content

Commit

Permalink
Remove Platform references from the Host module
Browse files Browse the repository at this point in the history
Summary:
These were used by Host::LaunchProcess to "resolve" the executable it
was about to launch. The only parts of Platform::ResolveExecutable, which
seem to be relevant here are the FileSpec::ResolvePath and
ResolveExecutableLocation calls.

The rest (most) of that function deals with selecting an architecture
out of a fat binary and making sure we are able to create a Module with that
slice. These are reasonable actions when selecting a binary to debug,
but not for a generic process launching framework (it's technically even
wrong because we should be able to launch a binary with execute
permissions only, but trying to parse such file will obviously fail).

I remove the platform call by inlining the relevant FileSpec calls and
ignoring the rest of the Platform::ResolveExecutable code.  The
architecture found by the slice-searching code is being ignored already
anyway, as we use the one specified in the LaunchInfo, so the only
effect of this should be a different error message in case the
executable does not contain the requested architecture -- before we
would get an error message from the Platform class, but now we will get
an error from the actual posix_spawn syscall (this is only relevant on
mac, as it's the only target supporting fat binaries).

Launching targets for debugging should not be affected as here the
executable is pre-resolved at the point when the Target is created.

Reviewers: jingham, clayborg

Subscribers: lldb-commits, emaste

Differential Revision: https://reviews.llvm.org/D41902

llvm-svn: 322935
  • Loading branch information
labath committed Jan 19, 2018
1 parent d24ddcd commit 91f14e6
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 57 deletions.
36 changes: 10 additions & 26 deletions lldb/source/Host/common/MonitoringProcessLauncher.cpp
Expand Up @@ -8,10 +8,7 @@
//===----------------------------------------------------------------------===//

#include "lldb/Host/MonitoringProcessLauncher.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Host/HostProcess.h"
#include "lldb/Target/Platform.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/ProcessLaunchInfo.h"
#include "lldb/Utility/Log.h"
Expand All @@ -32,36 +29,23 @@ MonitoringProcessLauncher::LaunchProcess(const ProcessLaunchInfo &launch_info,
ProcessLaunchInfo resolved_info(launch_info);

error.Clear();
char exe_path[PATH_MAX];

PlatformSP host_platform_sp(Platform::GetHostPlatform());

const ArchSpec &arch_spec = resolved_info.GetArchitecture();

FileSpec exe_spec(resolved_info.GetExecutableFile());

llvm::sys::fs::file_status stats;
status(exe_spec.GetPath(), stats);
if (!is_regular_file(stats)) {
ModuleSpec module_spec(exe_spec, arch_spec);
lldb::ModuleSP exe_module_sp;
error =
host_platform_sp->ResolveExecutable(module_spec, exe_module_sp, NULL);

if (error.Fail())
return HostProcess();

if (exe_module_sp) {
exe_spec = exe_module_sp->GetFileSpec();
status(exe_spec.GetPath(), stats);
}
if (!exists(stats)) {
exe_spec.ResolvePath();
status(exe_spec.GetPath(), stats);
}
if (!exists(stats)) {
exe_spec.ResolveExecutableLocation();
status(exe_spec.GetPath(), stats);
}

if (exists(stats)) {
exe_spec.GetPath(exe_path, sizeof(exe_path));
} else {
resolved_info.GetExecutableFile().GetPath(exe_path, sizeof(exe_path));
error.SetErrorStringWithFormat("executable doesn't exist: '%s'", exe_path);
if (!exists(stats)) {
error.SetErrorStringWithFormatv("executable doesn't exist: '{0}'",
exe_spec);
return HostProcess();
}

Expand Down
1 change: 0 additions & 1 deletion lldb/source/Host/freebsd/Host.cpp
Expand Up @@ -29,7 +29,6 @@
#include "lldb/Core/Module.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Target/Platform.h"
#include "lldb/Target/Process.h"
#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataBufferHeap.h"
Expand Down
46 changes: 18 additions & 28 deletions lldb/source/Host/macosx/Host.mm
Expand Up @@ -55,12 +55,9 @@
#include <unistd.h>

#include "lldb/Core/Communication.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Host/ThreadLauncher.h"
#include "lldb/Target/Platform.h"
#include "lldb/Target/Process.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/CleanUp.h"
Expand Down Expand Up @@ -1463,36 +1460,28 @@ static bool ShouldLaunchUsingXPC(ProcessLaunchInfo &launch_info) {

Status Host::LaunchProcess(ProcessLaunchInfo &launch_info) {
Status error;
char exe_path[PATH_MAX];
PlatformSP host_platform_sp(Platform::GetHostPlatform());
FileSpec exe_spec(launch_info.GetExecutableFile());

ModuleSpec exe_module_spec(launch_info.GetExecutableFile(),
launch_info.GetArchitecture());

if (!llvm::sys::fs::is_regular_file(
exe_module_spec.GetFileSpec().GetPath())) {
lldb::ModuleSP exe_module_sp;
error = host_platform_sp->ResolveExecutable(exe_module_spec, exe_module_sp,
NULL);

if (error.Fail())
return error;

if (exe_module_sp)
exe_module_spec.GetFileSpec() = exe_module_sp->GetFileSpec();
llvm::sys::fs::file_status stats;
status(exe_spec.GetPath(), stats);
if (!exists(stats)) {
exe_spec.ResolvePath();
status(exe_spec.GetPath(), stats);
}

if (exe_module_spec.GetFileSpec().Exists()) {
exe_module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
} else {
launch_info.GetExecutableFile().GetPath(exe_path, sizeof(exe_path));
error.SetErrorStringWithFormat("executable doesn't exist: '%s'", exe_path);
if (!exists(stats)) {
exe_spec.ResolveExecutableLocation();
status(exe_spec.GetPath(), stats);
}
if (!exists(stats)) {
error.SetErrorStringWithFormatv("executable doesn't exist: '{0}'",
launch_info.GetExecutableFile());
return error;
}

if (launch_info.GetFlags().Test(eLaunchFlagLaunchInTTY)) {
#if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
return LaunchInNewTerminalWithAppleScript(exe_path, launch_info);
return LaunchInNewTerminalWithAppleScript(exe_spec.GetPath().c_str(),
launch_info);
#else
error.SetErrorString("launching a process in a new terminal is not "
"supported on iOS devices");
Expand All @@ -1503,9 +1492,10 @@ ModuleSpec exe_module_spec(launch_info.GetExecutableFile(),
lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;

if (ShouldLaunchUsingXPC(launch_info)) {
error = LaunchProcessXPC(exe_path, launch_info, pid);
error = LaunchProcessXPC(exe_spec.GetPath().c_str(), launch_info, pid);
} else {
error = LaunchProcessPosixSpawn(exe_path, launch_info, pid);
error =
LaunchProcessPosixSpawn(exe_spec.GetPath().c_str(), launch_info, pid);
}

if (pid != LLDB_INVALID_PROCESS_ID) {
Expand Down
1 change: 0 additions & 1 deletion lldb/source/Host/netbsd/Host.cpp
Expand Up @@ -28,7 +28,6 @@
#include "lldb/Core/Module.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Target/Platform.h"
#include "lldb/Target/Process.h"
#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataBufferHeap.h"
Expand Down
1 change: 0 additions & 1 deletion lldb/source/Host/openbsd/Host.cpp
Expand Up @@ -25,7 +25,6 @@
#include "lldb/Core/Module.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Target/Platform.h"
#include "lldb/Target/Process.h"
#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataBufferHeap.h"
Expand Down

0 comments on commit 91f14e6

Please sign in to comment.