wrapper: resolve /proc/self/cwd only when seen.

This wrapper may run on Mac, which doesn't have /proc/self/cwd
available. This also adds a test to be sure this selective Readlink()
behavior continues to work.

BUG=None
TEST=SDK Tryjob; tests pass

Change-Id: I6f6aaeb7ea45ab8b3d68299422f0594a689ecfcd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2095763
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Tiancong Wang <tcwang@google.com>
Tested-by: George Burgess <gbiv@chromium.org>

2 files changed, 90 insertions, 10 deletions

diff --git a/compiler_wrapper/env.go b/compiler_wrapper/env.go
index 56b3a913..2c48ad30 100644
--- a/compiler_wrapper/env.go
+++ b/compiler_wrapper/env.go
@@ -28,19 +28,25 @@ type processEnv struct {
 }
 
 func newProcessEnv() (env, error) {
-	// Note: We are not using os.getwd() as this sometimes uses the value of the PWD
-	// env variable. This has the following problems:
-	// - if PWD=/proc/self/cwd, os.getwd() will return "/proc/self/cwd",
-	//   and we need to read the link to get the actual wd. However, we can't always
-	//   do this as we are calculating
-	//   the path to clang, and following a symlinked cwd first would make
-	//   this calculation invalid.
-	// - the old python wrapper doesn't respect the PWD env variable either, so if we
-	//   did we would fail the comparison to the old wrapper.
-	wd, err := os.Readlink("/proc/self/cwd")
+	wd, err := os.Getwd()
 	if err != nil {
 		return nil, wrapErrorwithSourceLocf(err, "failed to read working directory")
 	}
+
+	// Note: On Linux, Getwd may resolve to /proc/self/cwd, since it checks the PWD environment
+	// variable. We need to read the link to get the actual working directory. We can't always
+	// do this as we are calculating the path to clang, since following a symlinked cwd first
+	// would make this calculation invalid.
+	//
+	// FIXME(gbiv): It's not clear why always Readlink()ing here an issue. crrev.com/c/1764624
+	// might provide helpful context?
+	if wd == "/proc/self/cwd" {
+		wd, err = os.Readlink(wd)
+		if err != nil {
+			return nil, wrapErrorwithSourceLocf(err, "resolving /proc/self/cwd")
+		}
+	}
+
 	return &processEnv{wd: wd}, nil
 }
 
diff --git a/compiler_wrapper/env_test.go b/compiler_wrapper/env_test.go
index 8580d4aa..e03d60a8 100644
--- a/compiler_wrapper/env_test.go
+++ b/compiler_wrapper/env_test.go
@@ -7,6 +7,7 @@ package main
 import (
 	"bytes"
 	"flag"
+	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -163,6 +164,79 @@ func TestProcessEnvRunCmdDeleteEnv(t *testing.T) {
 	})
 }
 
+func TestNewProcessEnvResolvesPwdAwayProperly(t *testing.T) {
+	// This test cannot be t.Parallel(), since it modifies our environment.
+	const envPwd = "PWD"
+
+	oldEnvPwd := os.Getenv(envPwd)
+	defer func() {
+		if oldEnvPwd == "" {
+			os.Unsetenv(envPwd)
+		} else {
+			os.Setenv(envPwd, oldEnvPwd)
+		}
+	}()
+
+	os.Unsetenv(envPwd)
+
+	initialWd, err := os.Getwd()
+	if initialWd == "/proc/self/cwd" {
+		t.Fatalf("Working directory should never be %q when env is unset", initialWd)
+	}
+
+	defer func() {
+		if err := os.Chdir(initialWd); err != nil {
+			t.Errorf("Changing back to %q failed: %v", initialWd, err)
+		}
+	}()
+
+	tempDir, err := ioutil.TempDir("", "wrapper_env_test")
+	if err != nil {
+		t.Fatalf("Failed making temp dir: %v", err)
+	}
+
+	// Nothing we can do if this breaks, unfortunately.
+	defer os.RemoveAll(tempDir)
+
+	tempDirLink := tempDir + ".symlink"
+	if err := os.Symlink(tempDir, tempDirLink); err != nil {
+		t.Fatalf("Failed creating symlink %q => %q: %v", tempDirLink, tempDir, err)
+	}
+
+	if err := os.Chdir(tempDir); err != nil {
+		t.Fatalf("Failed chdir'ing to tempdir at %q: %v", tempDirLink, err)
+	}
+
+	if err := os.Setenv(envPwd, tempDirLink); err != nil {
+		t.Fatalf("Failed setting pwd to tempdir at %q: %v", tempDirLink, err)
+	}
+
+	// Ensure that we don't resolve symlinks if they're present in our CWD somehow, except for
+	// /proc/self/cwd, which tells us nothing about where we are.
+	env, err := newProcessEnv()
+	if err != nil {
+		t.Fatalf("Failed making a new env: %v", err)
+	}
+
+	if wd := env.getwd(); wd != tempDirLink {
+		t.Errorf("Environment setup had a wd of %q; wanted %q", wd, tempDirLink)
+	}
+
+	const cwdLink = "/proc/self/cwd"
+	if err := os.Setenv(envPwd, cwdLink); err != nil {
+		t.Fatalf("Failed setting pwd to /proc/self/cwd: %v", err)
+	}
+
+	env, err = newProcessEnv()
+	if err != nil {
+		t.Fatalf("Failed making a new env: %v", err)
+	}
+
+	if wd := env.getwd(); wd != tempDir {
+		t.Errorf("Environment setup had a wd of %q; wanted %q", cwdLink, tempDir)
+	}
+}
+
 func execEcho(ctx *testContext, cmd *command) {
 	env := &processEnv{}
 	err := env.exec(createEcho(ctx, cmd))