diff options
author | George Burgess IV <gbiv@google.com> | 2020-03-04 17:32:44 -0800 |
---|---|---|
committer | George Burgess <gbiv@chromium.org> | 2020-03-05 02:03:19 +0000 |
commit | c94f243ccfe39115e7a648ffbb6f7594b0d2e9ea (patch) | |
tree | e8a736271471839c217e18a2c770500ee005796d /compiler_wrapper | |
parent | 30171e1539d5f63491365d6363a74791d7d0432e (diff) | |
download | toolchain-utils-c94f243ccfe39115e7a648ffbb6f7594b0d2e9ea.tar.gz |
wrapper: correctly Join() paths for Android
filepath.Join(".", "foo") folds to "foo", which is fine as long as we're
not Exec()ing the result of that Join(). If we do, we might execute a
binary we weren't expecting to (e.g., one looked up from ${PATH}, rather
than the one at ${CWD}).
BUG=None
TEST=`go test`
Change-Id: Ifbd247c2e489c786d0a0d3342b5d1b61a7320796
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2088432
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Diffstat (limited to 'compiler_wrapper')
-rw-r--r-- | compiler_wrapper/compile_with_fallback_test.go | 2 | ||||
-rw-r--r-- | compiler_wrapper/compiler_wrapper.go | 30 | ||||
-rw-r--r-- | compiler_wrapper/compiler_wrapper_test.go | 37 |
3 files changed, 62 insertions, 7 deletions
diff --git a/compiler_wrapper/compile_with_fallback_test.go b/compiler_wrapper/compile_with_fallback_test.go index 4ea847f6..b36c3e18 100644 --- a/compiler_wrapper/compile_with_fallback_test.go +++ b/compiler_wrapper/compile_with_fallback_test.go @@ -229,7 +229,7 @@ func TestCompileWithFallbackLogCommandAndErrors(t *testing.T) { log := readCompileWithFallbackErrorLog(ctx) if log != `==================COMMAND:==================== -clang.real main.cc -fno-color-diagnostics -a -b +./clang.real main.cc -fno-color-diagnostics -a -b someerror ============================================== diff --git a/compiler_wrapper/compiler_wrapper.go b/compiler_wrapper/compiler_wrapper.go index cecde1c6..c8c3b6f9 100644 --- a/compiler_wrapper/compiler_wrapper.go +++ b/compiler_wrapper/compiler_wrapper.go @@ -38,6 +38,29 @@ func callCompiler(env env, cfg *config, inputCmd *command) int { return exitCode } +// Given the main builder path and the absolute path to our wrapper, returns the path to the +// 'real' compiler we should invoke. +func calculateAndroidWrapperPath(mainBuilderPath string, absWrapperPath string) string { + // FIXME: This combination of using the directory of the symlink but the basename of the + // link target is strange but is the logic that old android wrapper uses. Change this to use + // directory and basename either from the absWrapperPath or from the builder.path, but don't + // mix anymore. + + // We need to be careful here: path.Join Clean()s its result, so `./foo` will get + // transformed to `foo`, which isn't good since we're passing this path to exec. + basePart := filepath.Base(absWrapperPath) + ".real" + if !strings.ContainsRune(mainBuilderPath, filepath.Separator) { + return basePart + } + + dirPart := filepath.Dir(mainBuilderPath) + if cleanResult := filepath.Join(dirPart, basePart); strings.ContainsRune(cleanResult, filepath.Separator) { + return cleanResult + } + + return "." + string(filepath.Separator) + basePart +} + func callCompilerInternal(env env, cfg *config, inputCmd *command) (exitCode int, err error) { if err := checkUnsupportedFlags(inputCmd); err != nil { return 0, err @@ -52,12 +75,7 @@ func callCompilerInternal(env env, cfg *config, inputCmd *command) (exitCode int var compilerCmd *command clangSyntax := processClangSyntaxFlag(mainBuilder) if cfg.isAndroidWrapper { - // FIXME: This combination of using the directory of the symlink but the - // basename of the link target is strange but is the logic that old android - // wrapper uses. Change this to use directory and basename either from the - // absWrapperPath or from the builder.path, but don't mix anymore. - mainBuilder.path = filepath.Join(filepath.Dir(mainBuilder.path), filepath.Base(mainBuilder.absWrapperPath)+".real") - + mainBuilder.path = calculateAndroidWrapperPath(mainBuilder.path, mainBuilder.absWrapperPath) switch mainBuilder.target.compilerType { case clangType: mainBuilder.addPreUserArgs(mainBuilder.cfg.clangFlags...) diff --git a/compiler_wrapper/compiler_wrapper_test.go b/compiler_wrapper/compiler_wrapper_test.go index 67cbda92..a132ec5c 100644 --- a/compiler_wrapper/compiler_wrapper_test.go +++ b/compiler_wrapper/compiler_wrapper_test.go @@ -148,3 +148,40 @@ func TestPrintOtherCompilerError(t *testing.T) { t.Errorf("Unexpected string. Got: %s", buffer.String()) } } + +func TestCalculateAndroidWrapperPath(t *testing.T) { + t.Parallel() + + testCases := []struct { + mainBuilderPath string + absWrapperPath string + want string + }{ + { + mainBuilderPath: "/foo/bar", + absWrapperPath: "/bar/baz", + want: "/foo/baz.real", + }, + { + mainBuilderPath: "/my_wrapper", + absWrapperPath: "/bar/baz", + want: "/baz.real", + }, + { + mainBuilderPath: "no_seps", + absWrapperPath: "/bar/baz", + want: "baz.real", + }, + { + mainBuilderPath: "./a_sep", + absWrapperPath: "/bar/baz", + want: "./baz.real", + }, + } + + for _, tc := range testCases { + if result := calculateAndroidWrapperPath(tc.mainBuilderPath, tc.absWrapperPath); result != tc.want { + t.Errorf("Failed calculating the wrapper path with (%q, %q); got %q, want %q", tc.mainBuilderPath, tc.absWrapperPath, result, tc.want) + } + } +} |