Use execve with properly merged env variables.

This reverts 478cfee03066d58ca3899753de95a58f35a96835
(Modify hostenv and use execv instead of execve).

The original problem was actually that execve
doesn't merge updates into the environment variables,
in contrast to exec.Command.

Now that we have our own merging logic (to e.g. support
removal of variables), we can just use that.

BUG=chromium:773875
TEST=emerge-kevin sci-libs/tensorflow

Change-Id: I4a8412a73f4e79ad7b6d502602f442b2524efb06
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1804354
Reviewed-by: George Burgess <gbiv@chromium.org>
Tested-by: Tobias Bosch <tbosch@google.com>

2 files changed, 6 insertions, 30 deletions

diff --git a/compiler_wrapper/env.go b/compiler_wrapper/env.go
index c8f31357..4d83b17a 100644
--- a/compiler_wrapper/env.go
+++ b/compiler_wrapper/env.go
@@ -71,7 +71,7 @@ func (env *processEnv) stderr() io.Writer {
 }
 
 func (env *processEnv) exec(cmd *command) error {
-	return libcExec(cmd)
+	return libcExec(env, cmd)
 }
 
 func (env *processEnv) run(cmd *command, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
diff --git a/compiler_wrapper/libc_exec.go b/compiler_wrapper/libc_exec.go
index 2ee9b7f5..268221dc 100644
--- a/compiler_wrapper/libc_exec.go
+++ b/compiler_wrapper/libc_exec.go
@@ -9,33 +9,8 @@ package main
 // #include <string.h>
 // #include <unistd.h>
 //
-// int libc_exec(const char *pathname, char *const argv[], char *const env_updates[]) {
-//	// Note: We are not using execve and pass the new environment here
-//	// as that sometimes doesn't work well with the gentoo sandbox to
-//	// pick up changes to SANDBOX_WRITE env variable (needed for ccache).
-//	// Instead, we are updating our own environment and call execv.
-//	// This update of global state is ok as we won't execute anything else
-//	// after the exec.
-//	// Note: We don't update the environment already in go as these somehow
-//	// don't seem to update the real environment...
-//	int i;
-//	for (i = 0; env_updates[i] != NULL; ++i) {
-//		const char* update = env_updates[i];
-//		const char* pos = strchr(update, '=');
-//		if (pos == NULL) {
-//			continue;
-//		}
-//		char key[pos - update + 1];
-//		key[pos - update] = 0;
-//		strncpy(key, update, pos - update);
-//		if (pos[1] == 0) {
-//			// update has no value
-//			unsetenv(key);
-//		} else {
-//			setenv(key, &pos[1], /*overwrite=*/1);
-//		}
-//	}
-//	if (execv(pathname, argv) != 0) {
+// int libc_exec(const char *pathname, char *const argv[], char *const envp[]) {
+//	if (execve(pathname, argv, envp) != 0) {
 //		return errno;
 //	}
 //	return 0;
@@ -51,7 +26,7 @@ import (
 // LD_PRELOAD to work properly (e.g. gentoo sandbox).
 // Note that this changes the go binary to be a dynamically linked one.
 // See crbug.com/1000863 for details.
-func libcExec(cmd *command) error {
+func libcExec(env env, cmd *command) error {
 	freeList := []unsafe.Pointer{}
 	defer func() {
 		for _, ptr := range freeList {
@@ -83,7 +58,8 @@ func libcExec(cmd *command) error {
 	}
 
 	execCmd := exec.Command(cmd.Path, cmd.Args...)
-	if errno := C.libc_exec(goStrToC(execCmd.Path), goSliceToC(execCmd.Args), goSliceToC(cmd.EnvUpdates)); errno != 0 {
+	mergedEnv := mergeEnvValues(env.environ(), cmd.EnvUpdates)
+	if errno := C.libc_exec(goStrToC(execCmd.Path), goSliceToC(execCmd.Args), goSliceToC(mergedEnv)); errno != 0 {
 		return newErrorwithSourceLocf("exec error: %d", errno)
 	}