diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h --- a/lldb/include/lldb/Utility/Log.h +++ b/lldb/include/lldb/Utility/Log.h @@ -211,8 +211,6 @@ static uint32_t GetFlags(llvm::raw_ostream &stream, const ChannelMap::value_type &entry, llvm::ArrayRef categories); - static void DisableLoggingChild(); - Log(const Log &) = delete; void operator=(const Log &) = delete; }; diff --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp --- a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp +++ b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp @@ -38,43 +38,40 @@ using namespace lldb; using namespace lldb_private; -static void FixupEnvironment(Environment &env) { -#ifdef __ANDROID__ - // If there is no PATH variable specified inside the environment then set the - // path to /system/bin. It is required because the default path used by - // execve() is wrong on android. - env.try_emplace("PATH", "/system/bin"); -#endif +// Begin code running in the child process +// NB: This code needs to be async-signal safe, since we're invoking fork from +// multithreaded contexts. + +static void write_string(int error_fd, const char *str) { + int r = write(error_fd, str, strlen(str)); + (void)r; } [[noreturn]] static void ExitWithError(int error_fd, const char *operation) { int err = errno; - llvm::raw_fd_ostream os(error_fd, true); - os << operation << " failed: " << llvm::sys::StrError(err); - os.flush(); + write_string(error_fd, operation); + write_string(error_fd, " failed: "); + // strerror is not guaranteed to be async-signal safe, but it usually is. + write_string(error_fd, strerror(err)); _exit(1); } -static void DisableASLRIfRequested(int error_fd, const ProcessLaunchInfo &info) { +static void DisableASLR(int error_fd) { #if defined(__linux__) - if (info.GetFlags().Test(lldb::eLaunchFlagDisableASLR)) { - const unsigned long personality_get_current = 0xffffffff; - int value = personality(personality_get_current); - if (value == -1) - ExitWithError(error_fd, "personality get"); - - value = personality(ADDR_NO_RANDOMIZE | value); - if (value == -1) - ExitWithError(error_fd, "personality set"); - } + const unsigned long personality_get_current = 0xffffffff; + int value = personality(personality_get_current); + if (value == -1) + ExitWithError(error_fd, "personality get"); + + value = personality(ADDR_NO_RANDOMIZE | value); + if (value == -1) + ExitWithError(error_fd, "personality set"); #endif } -static void DupDescriptor(int error_fd, const FileSpec &file_spec, int fd, - int flags) { - int target_fd = llvm::sys::RetryAfterSignal(-1, ::open, - file_spec.GetCString(), flags, 0666); +static void DupDescriptor(int error_fd, const char *file, int fd, int flags) { + int target_fd = llvm::sys::RetryAfterSignal(-1, ::open, file, flags, 0666); if (target_fd == -1) ExitWithError(error_fd, "DupDescriptor-open"); @@ -89,44 +86,67 @@ return; } -[[noreturn]] static void ChildFunc(int error_fd, - const ProcessLaunchInfo &info) { - if (info.GetFlags().Test(eLaunchFlagLaunchInSeparateProcessGroup)) { +namespace { +struct ForkFileAction { + ForkFileAction(const FileAction &act); + + FileAction::Action action; + int fd; + std::string path; + int arg; +}; + +struct ForkLaunchInfo { + ForkLaunchInfo(const ProcessLaunchInfo &info); + + bool separate_process_group; + bool debug; + bool disable_aslr; + std::string wd; + const char **argv; + Environment::Envp envp; + std::vector actions; + + bool has_action(int fd) const { + for (const ForkFileAction &action : actions) { + if (action.fd == fd) + return true; + } + return false; + } +}; +} // namespace + +[[noreturn]] static void ChildFunc(int error_fd, const ForkLaunchInfo &info) { + if (info.separate_process_group) { if (setpgid(0, 0) != 0) ExitWithError(error_fd, "setpgid"); } - for (size_t i = 0; i < info.GetNumFileActions(); ++i) { - const FileAction &action = *info.GetFileActionAtIndex(i); - switch (action.GetAction()) { + for (const ForkFileAction &action : info.actions) { + switch (action.action) { case FileAction::eFileActionClose: - if (close(action.GetFD()) != 0) + if (close(action.fd) != 0) ExitWithError(error_fd, "close"); break; case FileAction::eFileActionDuplicate: - if (dup2(action.GetFD(), action.GetActionArgument()) == -1) + if (dup2(action.fd, action.arg) == -1) ExitWithError(error_fd, "dup2"); break; case FileAction::eFileActionOpen: - DupDescriptor(error_fd, action.GetFileSpec(), action.GetFD(), - action.GetActionArgument()); + DupDescriptor(error_fd, action.path.c_str(), action.fd, action.arg); break; case FileAction::eFileActionNone: break; } } - const char **argv = info.GetArguments().GetConstArgumentVector(); - // Change working directory - if (info.GetWorkingDirectory() && - 0 != ::chdir(info.GetWorkingDirectory().GetCString())) + if (!info.wd.empty() && 0 != ::chdir(info.wd.c_str())) ExitWithError(error_fd, "chdir"); - DisableASLRIfRequested(error_fd, info); - Environment env = info.GetEnvironment(); - FixupEnvironment(env); - Environment::Envp envp = env.getEnvp(); + if (info.disable_aslr) + DisableASLR(error_fd); // Clear the signal mask to prevent the child from being affected by any // masking done by the parent. @@ -135,7 +155,7 @@ pthread_sigmask(SIG_SETMASK, &set, nullptr) != 0) ExitWithError(error_fd, "pthread_sigmask"); - if (info.GetFlags().Test(eLaunchFlagDebug)) { + if (info.debug) { // Do not inherit setgid powers. if (setgid(getgid()) != 0) ExitWithError(error_fd, "setgid"); @@ -144,6 +164,8 @@ // Close everything besides stdin, stdout, and stderr that has no file // action to avoid leaking. Only do this when debugging, as elsewhere we // actually rely on passing open descriptors to child processes. + // NB: This code is not async-signal safe, but we currently do not launch + // processes for debugging from within multithreaded contexts. const llvm::StringRef proc_fd_path = "/proc/self/fd"; std::error_code ec; @@ -158,7 +180,7 @@ // Don't close first three entries since they are stdin, stdout and // stderr. - if (fd > 2 && !info.GetFileActionForFD(fd) && fd != error_fd) + if (fd > 2 && !info.has_action(fd) && fd != error_fd) files_to_close.push_back(fd); } for (int file_to_close : files_to_close) @@ -167,7 +189,7 @@ // Since /proc/self/fd didn't work, trying the slow way instead. int max_fd = sysconf(_SC_OPEN_MAX); for (int fd = 3; fd < max_fd; ++fd) - if (!info.GetFileActionForFD(fd) && fd != error_fd) + if (!info.has_action(fd) && fd != error_fd) close(fd); } @@ -177,7 +199,7 @@ } // Execute. We should never return... - execve(argv[0], const_cast(argv), envp); + execve(info.argv[0], const_cast(info.argv), info.envp); #if defined(__linux__) if (errno == ETXTBSY) { @@ -190,7 +212,7 @@ // Since this state should clear up quickly, wait a while and then give it // one more go. usleep(50000); - execve(argv[0], const_cast(argv), envp); + execve(info.argv[0], const_cast(info.argv), info.envp); } #endif @@ -199,12 +221,43 @@ ExitWithError(error_fd, "execve"); } +// End of code running in the child process. + +ForkFileAction::ForkFileAction(const FileAction &act) + : action(act.GetAction()), fd(act.GetFD()), path(act.GetPath().str()), + arg(act.GetActionArgument()) {} + +static std::vector +MakeForkActions(const ProcessLaunchInfo &info) { + std::vector result; + for (size_t i = 0; i < info.GetNumFileActions(); ++i) + result.emplace_back(*info.GetFileActionAtIndex(i)); + return result; +} + +static Environment::Envp FixupEnvironment(Environment env) { +#ifdef __ANDROID__ + // If there is no PATH variable specified inside the environment then set the + // path to /system/bin. It is required because the default path used by + // execve() is wrong on android. + env.try_emplace("PATH", "/system/bin"); +#endif + return env.getEnvp(); +} + +ForkLaunchInfo::ForkLaunchInfo(const ProcessLaunchInfo &info) + : separate_process_group( + info.GetFlags().Test(eLaunchFlagLaunchInSeparateProcessGroup)), + debug(info.GetFlags().Test(eLaunchFlagDebug)), + disable_aslr(info.GetFlags().Test(eLaunchFlagDisableASLR)), + wd(info.GetWorkingDirectory().GetPath()), + argv(info.GetArguments().GetConstArgumentVector()), + envp(FixupEnvironment(info.GetEnvironment())), + actions(MakeForkActions(info)) {} + HostProcess ProcessLauncherPosixFork::LaunchProcess(const ProcessLaunchInfo &launch_info, Status &error) { - char exe_path[PATH_MAX]; - launch_info.GetExecutableFile().GetPath(exe_path, sizeof(exe_path)); - // A pipe used by the child process to report errors. PipePosix pipe; const bool child_processes_inherit = false; @@ -212,6 +265,8 @@ if (error.Fail()) return HostProcess(); + const ForkLaunchInfo fork_launch_info(launch_info); + ::pid_t pid = ::fork(); if (pid == -1) { // Fork failed @@ -222,7 +277,7 @@ if (pid == 0) { // child process pipe.CloseReadFileDescriptor(); - ChildFunc(pipe.ReleaseWriteFileDescriptor(), launch_info); + ChildFunc(pipe.ReleaseWriteFileDescriptor(), fork_launch_info); } // parent process diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp --- a/lldb/source/Utility/Log.cpp +++ b/lldb/source/Utility/Log.cpp @@ -30,7 +30,6 @@ #include #else #include -#include #endif using namespace lldb_private; @@ -180,9 +179,6 @@ } void Log::Initialize() { -#ifdef LLVM_ON_UNIX - pthread_atfork(nullptr, nullptr, &Log::DisableLoggingChild); -#endif InitializeLldbChannel(); } @@ -346,11 +342,3 @@ message << payload << "\n"; WriteMessage(message.str()); } - -void Log::DisableLoggingChild() { - // Disable logging by clearing out the atomic variable after forking -- if we - // forked while another thread held the channel mutex, we would deadlock when - // trying to write to the log. - for (auto &c: *g_channel_map) - c.second.m_channel.log_ptr.store(nullptr, std::memory_order_relaxed); -}