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>
diff --git a/compiler_wrapper/env.go b/compiler_wrapper/env.go
index 56b3a91..2c48ad3 100644
--- a/compiler_wrapper/env.go
+++ b/compiler_wrapper/env.go
@@ -28,19 +28,25 @@
}
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 8580d4a..e03d60a 100644
--- a/compiler_wrapper/env_test.go
+++ b/compiler_wrapper/env_test.go
@@ -7,6 +7,7 @@
import (
"bytes"
"flag"
+ "io/ioutil"
"os"
"os/exec"
"path/filepath"
@@ -163,6 +164,79 @@
})
}
+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))
