Skip to content

Commit

Permalink
Add a Symbolizer::GetEnvP() method that allows symbolizer implement…
Browse files Browse the repository at this point in the history
…ations to customise the environment of the symbolizer binary.

Summary:
This change introduces the `Symbolizer::GetEnvP()` method that returns a
pointer to environment array used for spawning the symbolizer process.
The motivation is to allow implementations to customise the environment
if required.  The default implementation just returns
`__sanitizer::GetEnviron()` which (provided it's implemented) should
preserve the existing behaviours of the various implementations.

This change has been plumbed through the `internal_spawn(...)` and
`StartSubprocess(...)` process spawning implementations.

For the `StartSubprocess()` implementation we need to call `execve()`
rather than `execv()` to pass the environment. However, it appears that
`internal_execve(...)` exists in sanitizer_common so this patch use that
which seems like a nice clean up.

Support in the Windows implementation of
`SymbolizerProcess:StartSymbolizerSubprocess()` has not been added
because the Windows sanitizer runtime doesn't implement `GetEnviron()`.

rdar://problem/58789439

Reviewers: kubamracek, yln, dvyukov, vitalybuka, eugenis, phosek, aizatsky, rnk

Subscribers: #sanitizers, llvm-commits

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D76666
  • Loading branch information
delcypher committed Mar 24, 2020
1 parent ec184dd commit b684c1a
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 14 deletions.
4 changes: 2 additions & 2 deletions compiler-rt/lib/sanitizer_common/sanitizer_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ bool IsAbsolutePath(const char *path);
// The child process will close all fds after STDERR_FILENO
// before passing control to a program.
pid_t StartSubprocess(const char *filename, const char *const argv[],
fd_t stdin_fd = kInvalidFd, fd_t stdout_fd = kInvalidFd,
fd_t stderr_fd = kInvalidFd);
const char *const envp[], fd_t stdin_fd = kInvalidFd,
fd_t stdout_fd = kInvalidFd, fd_t stderr_fd = kInvalidFd);
// Checks if specified process is still running
bool IsProcessRunning(pid_t pid);
// Waits for the process to finish and returns its exit code.
Expand Down
11 changes: 6 additions & 5 deletions compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,8 @@ int internal_sysctlbyname(const char *sname, void *oldp, uptr *oldlenp,
(size_t)newlen);
}

static fd_t internal_spawn_impl(const char *argv[], pid_t *pid) {
static fd_t internal_spawn_impl(const char *argv[], const char *envp[],
pid_t *pid) {
fd_t master_fd = kInvalidFd;
fd_t slave_fd = kInvalidFd;

Expand Down Expand Up @@ -302,8 +303,8 @@ static fd_t internal_spawn_impl(const char *argv[], pid_t *pid) {

// posix_spawn
char **argv_casted = const_cast<char **>(argv);
char **env = GetEnviron();
res = posix_spawn(pid, argv[0], &acts, &attrs, argv_casted, env);
char **envp_casted = const_cast<char **>(envp);
res = posix_spawn(pid, argv[0], &acts, &attrs, argv_casted, envp_casted);
if (res != 0) return kInvalidFd;

// Disable echo in the new terminal, disable CR.
Expand All @@ -320,7 +321,7 @@ static fd_t internal_spawn_impl(const char *argv[], pid_t *pid) {
return fd;
}

fd_t internal_spawn(const char *argv[], pid_t *pid) {
fd_t internal_spawn(const char *argv[], const char *envp[], pid_t *pid) {
// The client program may close its stdin and/or stdout and/or stderr thus
// allowing open/posix_openpt to reuse file descriptors 0, 1 or 2. In this
// case the communication is broken if either the parent or the child tries to
Expand All @@ -335,7 +336,7 @@ fd_t internal_spawn(const char *argv[], pid_t *pid) {
break;
}

fd_t fd = internal_spawn_impl(argv, pid);
fd_t fd = internal_spawn_impl(argv, envp, pid);

for (; count > 0; count--) {
internal_close(low_fds[count]);
Expand Down
2 changes: 1 addition & 1 deletion compiler-rt/lib/sanitizer_common/sanitizer_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ uptr internal_ptrace(int request, int pid, void *addr, void *data);
uptr internal_waitpid(int pid, int *status, int options);

int internal_fork();
fd_t internal_spawn(const char *argv[], pid_t *pid);
fd_t internal_spawn(const char *argv[], const char *envp[], pid_t *pid);

int internal_sysctl(const int *name, unsigned int namelen, void *oldp,
uptr *oldlenp, const void *newp, uptr newlen);
Expand Down
6 changes: 4 additions & 2 deletions compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ void AdjustStackSize(void *attr_) {
#endif // !SANITIZER_GO

pid_t StartSubprocess(const char *program, const char *const argv[],
fd_t stdin_fd, fd_t stdout_fd, fd_t stderr_fd) {
const char *const envp[], fd_t stdin_fd, fd_t stdout_fd,
fd_t stderr_fd) {
auto file_closer = at_scope_exit([&] {
if (stdin_fd != kInvalidFd) {
internal_close(stdin_fd);
Expand Down Expand Up @@ -469,7 +470,8 @@ pid_t StartSubprocess(const char *program, const char *const argv[],

for (int fd = sysconf(_SC_OPEN_MAX); fd > 2; fd--) internal_close(fd);

execv(program, const_cast<char **>(&argv[0]));
internal_execve(program, const_cast<char **>(&argv[0]),
const_cast<char *const *>(envp));
internal__exit(1);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ class SymbolizerProcess {
// Customizable by subclasses.
virtual bool StartSymbolizerSubprocess();
virtual bool ReadFromSymbolizer(char *buffer, uptr max_length);
// Return the environment to run the symbolizer in.
virtual char **GetEnvP() { return GetEnviron(); }

private:
virtual bool ReachedEndOfOutput(const char *buffer, uptr length) const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ bool SymbolizerProcess::StartSymbolizerSubprocess() {

if (use_posix_spawn_) {
#if SANITIZER_MAC
fd_t fd = internal_spawn(argv, &pid);
fd_t fd = internal_spawn(argv, const_cast<const char **>(GetEnvP()), &pid);
if (fd == kInvalidFd) {
Report("WARNING: failed to spawn external symbolizer (errno: %d)\n",
errno);
Expand All @@ -173,7 +173,7 @@ bool SymbolizerProcess::StartSymbolizerSubprocess() {
return false;
}

pid = StartSubprocess(path_, argv, /* stdin */ outfd[0],
pid = StartSubprocess(path_, argv, GetEnvP(), /* stdin */ outfd[0],
/* stdout */ infd[1]);
if (pid < 0) {
internal_close(infd[0]);
Expand Down
3 changes: 2 additions & 1 deletion compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,8 @@ char **GetEnviron() {
}

pid_t StartSubprocess(const char *program, const char *const argv[],
fd_t stdin_fd, fd_t stdout_fd, fd_t stderr_fd) {
const char *const envp[], fd_t stdin_fd, fd_t stdout_fd,
fd_t stderr_fd) {
// FIXME: implement on this platform
// Should be implemented based on
// SymbolizerProcess::StarAtSymbolizerSubprocess
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ TEST(SanitizerCommon, StartSubprocessTest) {
const char *shell = "/bin/sh";
#endif
const char *argv[] = {shell, "-c", "echo -n 'hello'", (char *)NULL};
int pid = StartSubprocess(shell, argv,
int pid = StartSubprocess(shell, argv, GetEnviron(),
/* stdin */ kInvalidFd, /* stdout */ pipe_fds[1]);
ASSERT_GT(pid, 0);

Expand Down

0 comments on commit b684c1a

Please sign in to comment.