diff options
author | Tobias Bosch <tbosch@google.com> | 2019-06-24 09:31:39 -0700 |
---|---|---|
committer | Tobias Bosch <tbosch@google.com> | 2019-06-28 19:00:23 +0000 |
commit | 900dbc92800d8fc927905db29cb302461054cf97 (patch) | |
tree | c74ecbfb01ee4ee679090139596aef3546f19d9b /compiler_wrapper | |
parent | 739e6abb2cd03b60e579df31ad55870a4a00260a (diff) | |
download | toolchain-utils-900dbc92800d8fc927905db29cb302461054cf97.tar.gz |
Introduce infrastructure for calling and testing nested
commands, error messages and exit codes.
Also:
- implements the -Xclang-path= flag as use case of calling
a nested command.
- adds tests for forwarding errors, comparing against the
old wrapper, and exit codes.
- captures the source locations of errors in error messages.
- compares exit codes of new wrapper and old wrapper.
BUG=chromium:773875
TEST=unit test
Change-Id: I919e58091d093d68939809f676f799a68ec7a34e
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1676833
Reviewed-by: George Burgess <gbiv@chromium.org>
Tested-by: Tobias Bosch <tbosch@google.com>
Diffstat (limited to 'compiler_wrapper')
32 files changed, 1299 insertions, 389 deletions
diff --git a/compiler_wrapper/ccache_flag.go b/compiler_wrapper/ccache_flag.go index 95dd296a..3d3e77ad 100644 --- a/compiler_wrapper/ccache_flag.go +++ b/compiler_wrapper/ccache_flag.go @@ -8,11 +8,11 @@ func processCCacheFlag(sysroot string, builder *commandBuilder) { useCCache := true builder.transformArgs(func(arg builderArg) string { - if arg.Value == "-noccache" { + if arg.value == "-noccache" { useCCache = false return "" } - return arg.Value + return arg.value }) if builder.cfg.useCCache && useCCache { diff --git a/compiler_wrapper/ccache_flag_test.go b/compiler_wrapper/ccache_flag_test.go index b46ab388..c547b9fe 100644 --- a/compiler_wrapper/ccache_flag_test.go +++ b/compiler_wrapper/ccache_flag_test.go @@ -6,7 +6,7 @@ import ( func TestCallCCacheGivenConfig(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyPath(cmd, "/usr/bin/ccache"); err != nil { t.Error(err) @@ -19,7 +19,7 @@ func TestCallCCacheGivenConfig(t *testing.T) { func TestNotCallCCacheGivenConfig(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyPath(cmd, gccX86_64+".real"); err != nil { t.Error(err) @@ -29,7 +29,7 @@ func TestNotCallCCacheGivenConfig(t *testing.T) { func TestNotCallCCacheGivenConfigAndNoCCacheArg(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-noccache", mainCc))) if err := verifyPath(cmd, gccX86_64+".real"); err != nil { t.Error(err) @@ -42,7 +42,7 @@ func TestNotCallCCacheGivenConfigAndNoCCacheArg(t *testing.T) { func TestSetCacheDir(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyEnvUpdate(cmd, "CCACHE_DIR=/var/cache/distfiles/ccache"); err != nil { t.Error(err) @@ -52,7 +52,7 @@ func TestSetCacheDir(t *testing.T) { func TestSetCacheBaseDirToSysroot(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyEnvUpdate(cmd, "CCACHE_BASEDIR="+ctx.tempDir+"/usr/x86_64-cros-linux-gnu"); err != nil { @@ -63,7 +63,7 @@ func TestSetCacheBaseDirToSysroot(t *testing.T) { func TestSetCacheUmask(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyEnvUpdate(cmd, "CCACHE_UMASK=002"); err != nil { t.Error(err) @@ -73,14 +73,14 @@ func TestSetCacheUmask(t *testing.T) { func TestUpdateSandboxRewrite(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyNoEnvUpdate(cmd, "SANDBOX_WRITE"); err != nil { t.Error(err) } ctx.env = []string{"SANDBOX_WRITE=xyz"} - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyEnvUpdate(cmd, "SANDBOX_WRITE=xyz:/var/cache/distfiles/ccache"); err != nil { @@ -91,14 +91,14 @@ func TestUpdateSandboxRewrite(t *testing.T) { func TestClearCacheDisable(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyNoEnvUpdate(cmd, "SANDBOX_WRITE"); err != nil { t.Error(err) } ctx.env = []string{"CCACHE_DISABLE=true"} - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyEnvUpdate(cmd, "CCACHE_DISABLE="); err != nil { t.Error(err) @@ -108,7 +108,7 @@ func TestClearCacheDisable(t *testing.T) { func TestAddCCacheCpp2FlagForClang(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc))) if err := verifyEnvUpdate(cmd, "CCACHE_CPP2=yes"); err != nil { t.Error(err) @@ -118,7 +118,7 @@ func TestAddCCacheCpp2FlagForClang(t *testing.T) { func TestOmitCCacheCpp2FlagForGcc(t *testing.T) { withCCacheEnabledTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyNoEnvUpdate(cmd, "CCACHE_CPP2"); err != nil { t.Error(err) diff --git a/compiler_wrapper/clang_flags.go b/compiler_wrapper/clang_flags.go index 9679ace9..40826f99 100644 --- a/compiler_wrapper/clang_flags.go +++ b/compiler_wrapper/clang_flags.go @@ -1,12 +1,14 @@ package main import ( + "bytes" "path/filepath" "strings" ) func processClangFlags(rootPath string, builder *commandBuilder) error { - clangDir := builder.env.getenv("CLANG") + env := builder.env + clangDir := env.getenv("CLANG") if clangDir == "" { clangDir = filepath.Join(rootPath, "usr/bin/") @@ -15,15 +17,14 @@ func processClangFlags(rootPath string, builder *commandBuilder) error { // relative form. This is neccesary to remove absolute path from compile // outputs. var err error - clangDir, err = filepath.Rel(builder.env.getwd(), clangDir) + clangDir, err = filepath.Rel(env.getwd(), clangDir) if err != nil { - return err + return wrapErrorwithSourceLocf(err, "failed to make clangDir %s relative to %s.", clangDir, env.getwd()) } } } else { clangDir = filepath.Dir(clangDir) } - builder.path = filepath.Join(clangDir, builder.target.compiler) // GCC flags to remove from the clang command line. // TODO: Once clang supports GCC compatibility mode, remove @@ -61,42 +62,76 @@ func processClangFlags(rootPath string, builder *commandBuilder) error { "-Wunused-but-set-variable": "-Wunused-variable", } - builder.transformArgs(func(arg builderArg) string { - if mapped, ok := gccToClang[arg.Value]; ok { - return mapped + // Note: not using builder.transformArgs as we need to add multiple arguments + // based on a single input argument, and also be able to return errors. + newArgs := []builderArg{} + + for _, arg := range builder.args { + // Adds an argument with the given value, preserving the + // fromUser value of the original argument. + addNewArg := func(value string) { + newArgs = append(newArgs, builderArg{ + fromUser: arg.fromUser, + value: value, + }) } - if unsupported[arg.Value] { - return "" + if mapped, ok := gccToClang[arg.value]; ok { + addNewArg(mapped) + continue } - for _, prefix := range unsupportedPrefixes { - if strings.HasPrefix(arg.Value, prefix) { - return "" - } + if unsupported[arg.value] { + continue + } + + if hasAtLeastOnePrefix(arg.value, unsupportedPrefixes) { + continue } if builder.target.arch == "armv7a" && builder.target.sys == "linux" { - if armUnsupported[arg.Value] { - return "" + if armUnsupported[arg.value] { + continue } } - if clangOnly := "-Xclang-only="; strings.HasPrefix(arg.Value, clangOnly) { - return arg.Value[len(clangOnly):] + if clangOnly := "-Xclang-only="; strings.HasPrefix(arg.value, clangOnly) { + addNewArg(arg.value[len(clangOnly):]) + continue } - return arg.Value - }) + if clangPath := "-Xclang-path="; strings.HasPrefix(arg.value, clangPath) { + readResourceCmd := &command{ + path: filepath.Join(clangDir, builder.target.compiler), + args: []string{"--print-resource-dir"}, + } + stdoutBuffer := bytes.Buffer{} + if err := env.run(readResourceCmd, &stdoutBuffer, env.stderr()); err != nil { + return wrapErrorwithSourceLocf(err, + "failed to call clang to read the resouce-dir: %s %s", + readResourceCmd.path, readResourceCmd.args) + } + + clangPathValue := arg.value[len(clangPath):] + clangDir = clangPathValue - // Specify the target for clang. - linkerPath, err := getLinkerPath(builder.env, builder.target.target+"-ld", rootPath) - if err != nil { - return err + addNewArg("-resource-dir=" + stdoutBuffer.String()) + addNewArg("--gcc-toolchain=/usr") + continue + } + + addNewArg(arg.value) } - relLinkerPath, err := filepath.Rel(builder.env.getwd(), linkerPath) + builder.args = newArgs + + builder.path = filepath.Join(clangDir, builder.target.compiler) + + // Specify the target for clang. + linkerPath := getLinkerPath(env, builder.target.target+"-ld", rootPath) + relLinkerPath, err := filepath.Rel(env.getwd(), linkerPath) if err != nil { - return err + return wrapErrorwithSourceLocf(err, "failed to make linker path %s relative to %s", + linkerPath, env.getwd()) } builder.addPostUserArgs("-B" + relLinkerPath) if startswithI86(builder.target.arch) { @@ -115,14 +150,14 @@ func processClangFlags(rootPath string, builder *commandBuilder) error { } // Return the a directory which contains an 'ld' that gcc is using. -func getLinkerPath(env env, linkerCmd string, rootPath string) (string, error) { +func getLinkerPath(env env, linkerCmd string, rootPath string) string { // We did not pass the tuple i686-pc-linux-gnu to x86-32 clang. Instead, // we passed '-m32' to clang. As a result, clang does not want to use the // i686-pc-linux-gnu-ld, so we need to add this to help clang find the right // linker. for _, path := range strings.Split(env.getenv("PATH"), ":") { if linkerPath, err := filepath.EvalSymlinks(filepath.Join(path, linkerCmd)); err == nil { - return filepath.Dir(linkerPath), nil + return filepath.Dir(linkerPath) } } @@ -133,5 +168,14 @@ func getLinkerPath(env env, linkerCmd string, rootPath string) (string, error) { // ${SDK_LOCATION}/bin is not necessarily in the ${PATH}. To fix this, we // provide the directory that contains the cross linker wrapper to clang. // Outside chroot, it is the top bin directory form the sdk tarball. - return filepath.Join(rootPath, "bin"), nil + return filepath.Join(rootPath, "bin") +} + +func hasAtLeastOnePrefix(s string, prefixes []string) bool { + for _, prefix := range prefixes { + if strings.HasPrefix(s, prefix) { + return true + } + } + return false } diff --git a/compiler_wrapper/clang_flags_test.go b/compiler_wrapper/clang_flags_test.go index f50757fa..07efb89e 100644 --- a/compiler_wrapper/clang_flags_test.go +++ b/compiler_wrapper/clang_flags_test.go @@ -1,7 +1,11 @@ package main import ( + "errors" + "fmt" + "io" "path/filepath" + "strings" "testing" ) @@ -16,8 +20,8 @@ func TestClangBasename(t *testing.T) { } for _, tt := range tests { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, - ctx.newCommand(tt.in, "-noccache", mainCc))) + cmd := ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand(tt.in, mainCc))) if err := verifyPath(cmd, tt.out); err != nil { t.Error(err) } @@ -28,8 +32,8 @@ func TestClangBasename(t *testing.T) { func TestClangPathGivenClangEnv(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.env = []string{"CLANG=/a/b/clang"} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, - ctx.newCommand(clangX86_64, "-noccache", mainCc))) + cmd := ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand(clangX86_64, mainCc))) if err := verifyPath(cmd, "/a/b/clang"); err != nil { t.Error(err) } @@ -39,8 +43,8 @@ func TestClangPathGivenClangEnv(t *testing.T) { func TestAbsoluteClangPathBasedOnRootPath(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.rootRelPath = "somepath" - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, - ctx.newCommand(filepath.Join(ctx.tempDir, clangX86_64), "-noccache", mainCc))) + cmd := ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand(filepath.Join(ctx.tempDir, clangX86_64), mainCc))) if err := verifyPath(cmd, filepath.Join(ctx.tempDir, "somepath/usr/bin/clang")); err != nil { t.Error(err) } @@ -50,14 +54,65 @@ func TestAbsoluteClangPathBasedOnRootPath(t *testing.T) { func TestRelativeClangPathBasedOnRootPath(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.rootRelPath = "somepath" - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, - ctx.newCommand(clangX86_64, "-noccache", mainCc))) + cmd := ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand(clangX86_64, mainCc))) if err := verifyPath(cmd, "somepath/usr/bin/clang"); err != nil { t.Error(err) } }) } +func TestUseXclangPathAndCalcResourceDirByNestedClangCall(t *testing.T) { + withTestContext(t, func(ctx *testContext) { + ctx.cfg.rootRelPath = "somepath" + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + if ctx.cmdCount > 1 { + return nil + } + if err := verifyPath(cmd, "somepath/usr/bin/clang"); err != nil { + t.Error(err) + } + if err := verifyArgOrder(cmd, "--print-resource-dir"); err != nil { + t.Error(err) + } + fmt.Fprint(stdout, "someResourcePath") + return nil + } + cmd := ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand(clangX86_64, "-Xclang-path=somedir", mainCc))) + if err := verifyPath(cmd, "somedir/clang"); err != nil { + t.Error(err) + } + if err := verifyArgOrder(cmd, "-resource-dir=someResourcePath", + "--gcc-toolchain=/usr", mainCc); err != nil { + t.Error(err) + } + }) +} + +func TestXclangPathFailIfNestedClangCallFails(t *testing.T) { + withTestContext(t, func(ctx *testContext) { + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + fmt.Fprint(stderr, "someclangerror") + return errors.New("someerror") + } + stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg, + ctx.newCommand(clangX86_64, "-Xclang-path=somedir", mainCc))) + if err := verifyInternalError(stderr); err != nil { + t.Fatal(err) + } + if !strings.Contains(stderr, "clang") { + t.Errorf("could not find compiler path on stderr. Got: %s", stderr) + } + if !strings.Contains(stderr, "someerror") { + t.Errorf("could not find original error on stderr. Got: %s", stderr) + } + if !strings.Contains(stderr, "someclangerror") { + t.Errorf("stderr was not forwarded. Got: %s", stderr) + } + }) +} + func TestConvertGccToClangFlags(t *testing.T) { withTestContext(t, func(ctx *testContext) { var tests = []struct { @@ -73,7 +128,7 @@ func TestConvertGccToClangFlags(t *testing.T) { } for _, tt := range tests { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, tt.in, mainCc))) if err := verifyArgCount(cmd, 0, tt.in); err != nil { t.Error(err) @@ -110,7 +165,7 @@ func TestFilterUnsupportedClangFlags(t *testing.T) { } for _, tt := range tests { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(tt.compiler, tt.flag, mainCc))) if err := verifyArgCount(cmd, tt.expectedCount, tt.flag); err != nil { t.Error(err) @@ -129,7 +184,7 @@ func TestClangArchFlags(t *testing.T) { {"./x86_64-cros-linux-gnu-clang", []string{mainCc, "-target", "x86_64-cros-linux-gnu"}}, } for _, tt := range tests { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(tt.compiler, mainCc))) if err := verifyArgOrder(cmd, tt.flags...); err != nil { t.Error(err) @@ -143,7 +198,7 @@ func TestClangLinkerPathProbesBinariesOnPath(t *testing.T) { linkerPath := filepath.Join(ctx.tempDir, "a/b/c") ctx.writeFile(filepath.Join(linkerPath, "x86_64-cros-linux-gnu-ld"), "") ctx.env = []string{"PATH=nonExistantPath:" + linkerPath} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand("./x86_64-cros-linux-gnu-clang", mainCc))) if err := verifyArgOrder(cmd, "-Ba/b/c"); err != nil { t.Error(err) @@ -159,7 +214,7 @@ func TestClangLinkerPathEvaluatesSymlinksForBinariesOnPath(t *testing.T) { ctx.symlink(realLinkerPath, linkedLinkerPath) ctx.env = []string{"PATH=nonExistantPath:" + filepath.Dir(linkedLinkerPath)} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand("./x86_64-cros-linux-gnu-clang", mainCc))) if err := verifyArgOrder(cmd, "-Ba/original/path"); err != nil { t.Error(err) @@ -169,7 +224,7 @@ func TestClangLinkerPathEvaluatesSymlinksForBinariesOnPath(t *testing.T) { func TestClangFallbackLinkerPathRelativeToRootDir(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc))) if err := verifyArgOrder(cmd, "-Bbin"); err != nil { t.Error(err) @@ -180,7 +235,7 @@ func TestClangFallbackLinkerPathRelativeToRootDir(t *testing.T) { func TestClangLinkerPathRelativeToRootDir(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.rootRelPath = "somepath" - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc))) if err := verifyArgOrder(cmd, "-Bsomepath/bin"); err != nil { t.Error(err) diff --git a/compiler_wrapper/command.go b/compiler_wrapper/command.go index 618a1323..63301229 100644 --- a/compiler_wrapper/command.go +++ b/compiler_wrapper/command.go @@ -1,13 +1,9 @@ package main import ( - "errors" - "fmt" "os" "os/exec" "path/filepath" - "reflect" - "sort" "strings" ) @@ -26,45 +22,26 @@ func newProcessCommand() *command { func newExecCmd(env env, cmd *command) *exec.Cmd { execCmd := exec.Command(cmd.path, cmd.args...) - execCmd.Env = env.environ() + execCmd.Env = append(env.environ(), cmd.envUpdates...) + ensurePathEnv(execCmd) execCmd.Dir = env.getwd() return execCmd } -// TODO: Move to test once we no longer compare the calculated command against -// the command produced by the old wrapper in the released binary. -func (actual *command) verifySimilarTo(expected *command) error { - var differences []string - if actual.path != expected.path { - differences = append(differences, fmt.Sprintf("Paths are different. Expected: %q. Actual: %q", expected.path, actual.path)) - } - - if !reflect.DeepEqual(actual.args, expected.args) { - differences = append(differences, fmt.Sprintf("Args are different. Expected: %q. Actual: %q", expected.args, actual.args)) - } - - // Sort the environment as we don't care in which order - // it was modified. - actualEnvUpdates := actual.envUpdates - sort.Strings(actualEnvUpdates) - expectedEnvUpdates := expected.envUpdates - sort.Strings(expectedEnvUpdates) - - if !reflect.DeepEqual(actualEnvUpdates, expectedEnvUpdates) { - differences = append(differences, fmt.Sprintf("Env updates are different. Expected: %q. Actual: %q", expectedEnvUpdates, actualEnvUpdates)) - } - - if len(differences) > 0 { - return errors.New("commands differ:\n" + strings.Join(differences, "\n")) +func ensurePathEnv(cmd *exec.Cmd) { + for _, env := range cmd.Env { + if strings.HasPrefix(env, "PATH=") { + return + } } - return nil + cmd.Env = append(cmd.Env, "PATH=") } func newCommandBuilder(env env, cfg *config, cmd *command) (*commandBuilder, error) { basename := filepath.Base(cmd.path) nameParts := strings.Split(basename, "-") if len(nameParts) != 5 { - return nil, fmt.Errorf("expected 5 parts in the compiler name. Actual: %s", basename) + return nil, newErrorwithSourceLocf("expected 5 parts in the compiler name. Actual: %s", basename) } compiler := nameParts[4] @@ -72,12 +49,8 @@ func newCommandBuilder(env env, cfg *config, cmd *command) (*commandBuilder, err switch { case strings.HasPrefix(compiler, "clang"): compilerType = clangType - case strings.HasPrefix(compiler, "gcc"): - compilerType = gccType - case strings.HasPrefix(compiler, "g++"): - compilerType = gccType default: - return nil, fmt.Errorf("expected clang or gcc. Actual: %s", basename) + compilerType = gccType } return &commandBuilder{ path: cmd.path, @@ -106,8 +79,8 @@ type commandBuilder struct { } type builderArg struct { - Value string - FromUser bool + value string + fromUser bool } type compilerType int32 @@ -130,20 +103,20 @@ type builderTarget struct { func createBuilderArgs(fromUser bool, args []string) []builderArg { builderArgs := make([]builderArg, len(args)) for i, arg := range args { - builderArgs[i] = builderArg{Value: arg, FromUser: fromUser} + builderArgs[i] = builderArg{value: arg, fromUser: fromUser} } return builderArgs } func (builder *commandBuilder) wrapPath(path string) { - builder.args = append([]builderArg{{Value: builder.path, FromUser: false}}, builder.args...) + builder.args = append([]builderArg{{value: builder.path, fromUser: false}}, builder.args...) builder.path = path } func (builder *commandBuilder) addPreUserArgs(args ...string) { index := 0 for _, arg := range builder.args { - if arg.FromUser { + if arg.fromUser { break } index++ @@ -163,8 +136,8 @@ func (builder *commandBuilder) transformArgs(transform func(arg builderArg) stri newArg := transform(arg) if newArg != "" { newArgs = append(newArgs, builderArg{ - Value: newArg, - FromUser: arg.FromUser, + value: newArg, + fromUser: arg.fromUser, }) } } @@ -178,7 +151,7 @@ func (builder *commandBuilder) updateEnv(updates ...string) { func (builder *commandBuilder) build() *command { cmdArgs := make([]string, len(builder.args)) for i, builderArg := range builder.args { - cmdArgs[i] = builderArg.Value + cmdArgs[i] = builderArg.value } return &command{ path: builder.path, diff --git a/compiler_wrapper/compiler_wrapper.go b/compiler_wrapper/compiler_wrapper.go index 6034fb0f..4ef30391 100644 --- a/compiler_wrapper/compiler_wrapper.go +++ b/compiler_wrapper/compiler_wrapper.go @@ -1,21 +1,84 @@ package main import ( - "log" + "fmt" + "io" "path/filepath" "strings" + "syscall" ) -func calcCompilerCommand(env env, cfg *config, wrapperCmd *command) (*command, error) { - absWrapperDir, err := getAbsWrapperDir(env, wrapperCmd.path) +func callCompiler(env env, cfg *config, inputCmd *command) int { + exitCode := 0 + var compilerErr error + if shouldForwardToOldWrapper(env, inputCmd) { + // TODO: Once this is only checking for bisect, create a command + // that directly calls the bisect driver in calcCompilerCommand. + exitCode, compilerErr = forwardToOldWrapper(env, cfg, inputCmd) + } else if cfg.oldWrapperPath != "" { + exitCode, compilerErr = callCompilerWithRunAndCompareToOldWrapper(env, cfg, inputCmd) + } else { + compilerErr = callCompilerWithExec(env, cfg, inputCmd) + } + if compilerErr != nil { + printCompilerError(env.stderr(), compilerErr) + exitCode = 1 + } + return exitCode +} + +func callCompilerWithRunAndCompareToOldWrapper(env env, cfg *config, inputCmd *command) (exitCode int, err error) { + recordingEnv := &commandRecordingEnv{ + env: env, + } + compilerCmd, err := calcCompilerCommand(recordingEnv, cfg, inputCmd) if err != nil { + return exitCode, err + } + exitCode = 0 + // Note: we are not using env.exec here so that we can compare the exit code + // against the old wrapper too. + if err := recordingEnv.run(compilerCmd, env.stdout(), env.stderr()); err != nil { + if userErr, ok := getCCacheError(compilerCmd, err); ok { + return exitCode, userErr + } + var ok bool + if exitCode, ok = getExitCode(err); !ok { + return exitCode, wrapErrorwithSourceLocf(err, "failed to execute %s %s", compilerCmd.path, compilerCmd.args) + } + } + if err := compareToOldWrapper(env, cfg, inputCmd, recordingEnv.cmdResults); err != nil { + return exitCode, err + } + return exitCode, nil +} + +func callCompilerWithExec(env env, cfg *config, inputCmd *command) error { + compilerCmd, err := calcCompilerCommand(env, cfg, inputCmd) + if err != nil { + return err + } + if err := env.exec(compilerCmd); err != nil { + // Note: No need to check for exit code error as exec will + // stop this control flow once the command started executing. + if userErr, ok := getCCacheError(compilerCmd, err); ok { + return userErr + } + return wrapErrorwithSourceLocf(err, "failed to execute %s %s", compilerCmd.path, compilerCmd.args) + } + return nil +} + +func calcCompilerCommand(env env, cfg *config, inputCmd *command) (*command, error) { + if err := checkUnsupportedFlags(inputCmd); err != nil { return nil, err } - rootPath := filepath.Join(absWrapperDir, cfg.rootRelPath) - if err := checkUnsupportedFlags(wrapperCmd); err != nil { + absWrapperDir, err := getAbsWrapperDir(env, inputCmd.path) + if err != nil { return nil, err } - builder, err := newCommandBuilder(env, cfg, wrapperCmd) + rootPath := filepath.Join(absWrapperDir, cfg.rootRelPath) + builder, err := newCommandBuilder(env, cfg, inputCmd) if err != nil { return nil, err } @@ -47,56 +110,34 @@ func calcCompilerCommand(env env, cfg *config, wrapperCmd *command) (*command, e return builder.build(), nil } -func calcCompilerCommandAndCompareToOld(env env, cfg *config, wrapperCmd *command) (*command, error) { - compilerCmd, err := calcCompilerCommand(env, cfg, wrapperCmd) - if err != nil { - return nil, err - } - if cfg.oldWrapperPath == "" { - return compilerCmd, nil - } - oldCmds, err := calcOldCompilerCommands(env, cfg, wrapperCmd) - if err != nil { - return nil, err - } - if err := compilerCmd.verifySimilarTo(oldCmds[0]); err != nil { - return nil, err - } - return compilerCmd, nil -} - func getAbsWrapperDir(env env, wrapperPath string) (string, error) { if !filepath.IsAbs(wrapperPath) { wrapperPath = filepath.Join(env.getwd(), wrapperPath) } evaledCmdPath, err := filepath.EvalSymlinks(wrapperPath) if err != nil { - log.Printf("Unable to EvalSymlinks for %s. Error: %s", evaledCmdPath, err) - return "", err + return "", wrapErrorwithSourceLocf(err, "failed to evaluate symlinks for %s", wrapperPath) } return filepath.Dir(evaledCmdPath), nil } -// Whether the command should be executed by the old wrapper as we don't -// support it yet. -func shouldForwardToOldWrapper(env env, wrapperCmd *command) bool { - for _, arg := range wrapperCmd.args { - switch { - case strings.HasPrefix(arg, "-Xclang-path="): - fallthrough - case arg == "-clang-syntax": - return true - } +func getCCacheError(compilerCmd *command, compilerCmdErr error) (ccacheErr userError, ok bool) { + if en, ok := compilerCmdErr.(syscall.Errno); ok && en == syscall.ENOENT && + strings.Contains(compilerCmd.path, "ccache") { + ccacheErr = + newUserErrorf("ccache not found under %s. Please install it", + compilerCmd.path) + return ccacheErr, ok + } + return ccacheErr, false +} + +func printCompilerError(writer io.Writer, compilerErr error) { + if _, ok := compilerErr.(userError); ok { + fmt.Fprintf(writer, "%s\n", compilerErr) + } else { + fmt.Fprintf(writer, + "Internal error. Please report to chromeos-toolchain@google.com.\n%s\n", + compilerErr) } - switch { - case env.getenv("WITH_TIDY") != "": - fallthrough - case env.getenv("FORCE_DISABLE_WERROR") != "": - fallthrough - case env.getenv("GETRUSAGE") != "": - fallthrough - case env.getenv("BISECT_STAGE") != "": - return true - } - return false } diff --git a/compiler_wrapper/compiler_wrapper_test.go b/compiler_wrapper/compiler_wrapper_test.go index 6ab25e4e..5abf7ce9 100644 --- a/compiler_wrapper/compiler_wrapper_test.go +++ b/compiler_wrapper/compiler_wrapper_test.go @@ -1,13 +1,20 @@ package main import ( + "bytes" + "errors" + "fmt" + "io" + "path/filepath" + "strings" + "syscall" "testing" ) func TestAddCommonFlags(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.commonFlags = []string{"-someflag"} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgOrder(cmd, "-someflag", mainCc); err != nil { t.Error(err) @@ -18,7 +25,7 @@ func TestAddCommonFlags(t *testing.T) { func TestAddGccConfigFlags(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.gccFlags = []string{"-someflag"} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgOrder(cmd, "-someflag", mainCc); err != nil { t.Error(err) @@ -29,7 +36,7 @@ func TestAddGccConfigFlags(t *testing.T) { func TestAddClangConfigFlags(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.clangFlags = []string{"-someflag"} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc))) if err := verifyArgOrder(cmd, "-someflag", mainCc); err != nil { t.Error(err) @@ -37,42 +44,98 @@ func TestAddClangConfigFlags(t *testing.T) { }) } -func TestShouldForwardToOldWrapperBecauseOfArgs(t *testing.T) { +func TestLogGeneralExecError(t *testing.T) { withTestContext(t, func(ctx *testContext) { - testdata := []struct { - arg string - shouldForward bool - }{ - {"abc", false}, - {"-Xclang-path=abc", true}, - {"-clang-syntax", true}, - {"-clang-syntaxabc", false}, + testOldWrapperPaths := []string{ + "", + filepath.Join(ctx.tempDir, "fakewrapper"), } - for _, tt := range testdata { - if actual := shouldForwardToOldWrapper(ctx, ctx.newCommand(clangX86_64, tt.arg)); actual != tt.shouldForward { - t.Fatalf("Forward to old wrapper incorrect for arg %s. Expected %t but was %t.", tt.arg, tt.shouldForward, actual) + for _, testOldWrapperPath := range testOldWrapperPaths { + ctx.cfg.oldWrapperPath = testOldWrapperPath + // Note: No need to write the old wrapper as we don't execute + // it due to the general error from the new error. + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + return errors.New("someerror") + } + stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) + if err := verifyInternalError(stderr); err != nil { + t.Fatal(err) + } + if !strings.Contains(stderr, gccX86_64) { + t.Errorf("could not find compiler path on stderr. Got: %s", stderr) + } + if !strings.Contains(stderr, "someerror") { + t.Errorf("could not find original error on stderr. Got: %s", stderr) } } }) } -func TestShouldForwardToOldWrapperBecauseOfEnv(t *testing.T) { +func TestLogMissingCCacheExecError(t *testing.T) { withTestContext(t, func(ctx *testContext) { - testdata := []struct { - env string - shouldForward bool - }{ - {"PATH=abc", false}, - {"WITH_TIDY=abc", true}, - {"FORCE_DISABLE_WERROR=abc", true}, - {"GETRUSAGE=abc", true}, - {"BISECT_STAGE=abc", true}, + ctx.cfg.useCCache = true + + testOldWrapperPaths := []string{ + "", + filepath.Join(ctx.tempDir, "fakewrapper"), } - for _, tt := range testdata { - ctx.env = []string{tt.env} - if actual := shouldForwardToOldWrapper(ctx, ctx.newCommand(clangX86_64)); actual != tt.shouldForward { - t.Fatalf("Forward to old wrapper incorrect for env %s. Expected %t but was %t.", tt.env, tt.shouldForward, actual) + for _, testOldWrapperPath := range testOldWrapperPaths { + ctx.cfg.oldWrapperPath = testOldWrapperPath + // Note: No need to write the old wrapper as we don't execute + // it due to the general error from the new error. + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + return syscall.ENOENT + } + ctx.stderrBuffer.Reset() + stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) + if err := verifyNonInternalError(stderr, "ccache not found under .*. Please install it"); err != nil { + t.Fatal(err) } } }) } + +func TestLogExitCodeErrorWhenComparingToOldWrapper(t *testing.T) { + withTestContext(t, func(ctx *testContext) { + ctx.cfg.mockOldWrapperCmds = false + ctx.cfg.oldWrapperPath = filepath.Join(ctx.tempDir, "fakewrapper") + + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + writeMockWrapper(ctx, &mockWrapperConfig{ + Cmds: []*mockWrapperCmd{ + { + Path: cmd.path, + Args: cmd.args, + ExitCode: 2, + }, + }, + }) + fmt.Fprint(stderr, "someerror") + return newExitCodeError(2) + } + + exitCode := callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc)) + if exitCode != 2 { + t.Fatalf("Expected exit code 2. Got: %d", exitCode) + } + if err := verifyNonInternalError(ctx.stderrString(), "someerror"); err != nil { + t.Fatal(err) + } + }) +} + +func TestPrintUserCompilerError(t *testing.T) { + buffer := bytes.Buffer{} + printCompilerError(&buffer, newUserErrorf("abcd")) + if buffer.String() != "abcd\n" { + t.Errorf("Unexpected string. Got: %s", buffer.String()) + } +} + +func TestPrintOtherCompilerError(t *testing.T) { + buffer := bytes.Buffer{} + printCompilerError(&buffer, errors.New("abcd")) + if buffer.String() != "Internal error. Please report to chromeos-toolchain@google.com.\nabcd\n" { + t.Errorf("Unexpected string. Got: %s", buffer.String()) + } +} diff --git a/compiler_wrapper/config.go b/compiler_wrapper/config.go index 1e78fbef..34bfc122 100644 --- a/compiler_wrapper/config.go +++ b/compiler_wrapper/config.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "strconv" ) @@ -17,8 +16,11 @@ type config struct { // Toolchain root path relative to the wrapper binary. rootRelPath string // Path of the old wrapper using the toolchain root. - oldWrapperPath string - overrideOldWrapperConfig bool + oldWrapperPath string + // Whether to mock out the calls that the old wrapper does. + mockOldWrapperCmds bool + // Whether to overwrite the config in the old wrapper. + overwriteOldWrapperCfg bool } // UseCCache can be set via a linker flag. @@ -36,7 +38,7 @@ var ConfigName = "unknown" func getRealConfig() (*config, error) { useCCache, err := strconv.ParseBool(UseCCache) if err != nil { - return nil, fmt.Errorf("Parse error for UseCCache: %s", err) + return nil, wrapErrorwithSourceLocf(err, "invalid format for UseCCache") } config, err := getConfig(useCCache, ConfigName) if err != nil { @@ -52,7 +54,7 @@ func getConfig(useCCache bool, configName string) (*config, error) { case "cros.nonhardened": return getCrosNonHardenedConfig(useCCache), nil default: - return nil, fmt.Errorf("Unknown config name: %s", configName) + return nil, newErrorwithSourceLocf("unknown config name: %s", configName) } } diff --git a/compiler_wrapper/config_test.go b/compiler_wrapper/config_test.go index 0dcd58dc..bd7aec03 100644 --- a/compiler_wrapper/config_test.go +++ b/compiler_wrapper/config_test.go @@ -8,7 +8,7 @@ import ( func TestFullHardeningConfigAndGcc(t *testing.T) { withTestContext(t, func(ctx *testContext) { initFullHardeningConfig(ctx) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyPath(cmd, "/usr/bin/ccache"); err != nil { t.Error(err) @@ -24,7 +24,7 @@ func TestFullHardeningConfigAndGcc(t *testing.T) { func TestFullHardeningConfigAndClang(t *testing.T) { withTestContext(t, func(ctx *testContext) { initFullHardeningConfig(ctx) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc))) if err := verifyPath(cmd, "/usr/bin/ccache"); err != nil { t.Error(err) @@ -46,7 +46,7 @@ func TestFullHardeningConfigAndClang(t *testing.T) { func TestNonHardeningConfigAndGcc(t *testing.T) { withTestContext(t, func(ctx *testContext) { initNonHardeningConfig(ctx) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyPath(cmd, "/usr/bin/ccache"); err != nil { t.Error(err) @@ -62,7 +62,7 @@ func TestNonHardeningConfigAndGcc(t *testing.T) { func TestNonHardeningConfigAndClang(t *testing.T) { withTestContext(t, func(ctx *testContext) { initNonHardeningConfig(ctx) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc))) if err := verifyPath(cmd, "/usr/bin/ccache"); err != nil { t.Error(err) @@ -151,14 +151,14 @@ func isHardened(cfg *config) bool { func initFullHardeningConfig(ctx *testContext) { useCCache := true - *ctx.cfg = *getCrosHardenedConfig(useCCache) - ctx.setOldWrapperPath(oldHardenedWrapperPathForTest) + ctx.updateConfig(oldHardenedWrapperPathForTest, getCrosHardenedConfig(useCCache)) + ctx.cfg.overwriteOldWrapperCfg = false } func initNonHardeningConfig(ctx *testContext) { useCCache := true - *ctx.cfg = *getCrosNonHardenedConfig(useCCache) - ctx.setOldWrapperPath(oldNonHardenedWrapperPathForTest) + ctx.updateConfig(oldNonHardenedWrapperPathForTest, getCrosNonHardenedConfig(useCCache)) + ctx.cfg.overwriteOldWrapperCfg = false } func resetGlobals() { diff --git a/compiler_wrapper/env.go b/compiler_wrapper/env.go index 3c106d4a..6cf1a1b9 100644 --- a/compiler_wrapper/env.go +++ b/compiler_wrapper/env.go @@ -1,13 +1,20 @@ package main import ( + "bytes" + "io" "os" + "syscall" ) type env interface { getenv(key string) string environ() []string getwd() string + stdout() io.Writer + stderr() io.Writer + run(cmd *command, stdout io.Writer, stderr io.Writer) error + exec(cmd *command) error } type processEnv struct { @@ -17,7 +24,7 @@ type processEnv struct { func newProcessEnv() (env, error) { wd, err := os.Getwd() if err != nil { - return nil, err + return nil, wrapErrorwithSourceLocf(err, "failed to read working directory") } return &processEnv{wd: wd}, nil } @@ -35,3 +42,58 @@ func (env *processEnv) environ() []string { func (env *processEnv) getwd() string { return env.wd } + +func (env *processEnv) stdout() io.Writer { + return os.Stdout +} + +func (env *processEnv) stderr() io.Writer { + return os.Stderr +} + +func (env *processEnv) exec(cmd *command) error { + execCmd := newExecCmd(env, cmd) + return syscall.Exec(execCmd.Path, execCmd.Args, execCmd.Env) +} + +func (env *processEnv) run(cmd *command, stdout io.Writer, stderr io.Writer) error { + execCmd := newExecCmd(env, cmd) + execCmd.Stdout = stdout + execCmd.Stderr = stderr + return execCmd.Run() +} + +type commandRecordingEnv struct { + env + cmdResults []*commandResult +} +type commandResult struct { + cmd *command + stdout string + stderr string + exitCode int +} + +var _ env = (*commandRecordingEnv)(nil) + +func (env *commandRecordingEnv) exec(cmd *command) error { + // Note: We will only get here if the exec failed, + // e.g. when the underlying command could not be called. + // In this case, we don't compare commands, so nothing to record. + return env.exec(cmd) +} + +func (env *commandRecordingEnv) run(cmd *command, stdout io.Writer, stderr io.Writer) error { + stdoutBuffer := &bytes.Buffer{} + stderrBuffer := &bytes.Buffer{} + err := env.env.run(cmd, io.MultiWriter(stdout, stdoutBuffer), io.MultiWriter(stderr, stderrBuffer)) + if exitCode, ok := getExitCode(err); ok { + env.cmdResults = append(env.cmdResults, &commandResult{ + cmd: cmd, + stdout: stdoutBuffer.String(), + stderr: stderrBuffer.String(), + exitCode: exitCode, + }) + } + return err +} diff --git a/compiler_wrapper/errors.go b/compiler_wrapper/errors.go new file mode 100644 index 00000000..525b986e --- /dev/null +++ b/compiler_wrapper/errors.go @@ -0,0 +1,57 @@ +package main + +import ( + "fmt" + "os/exec" + "runtime" + "strings" + "syscall" +) + +type userError struct { + err string +} + +var _ error = userError{} + +func (err userError) Error() string { + return err.err +} + +func newUserErrorf(format string, v ...interface{}) userError { + return userError{err: fmt.Sprintf(format, v...)} +} + +func newErrorwithSourceLocf(format string, v ...interface{}) error { + return newErrorwithSourceLocfInternal(2, format, v...) +} + +func wrapErrorwithSourceLocf(err error, format string, v ...interface{}) error { + return newErrorwithSourceLocfInternal(2, "%s: %s", fmt.Sprintf(format, v...), err.Error()) +} + +// Based on the implementation of log.Output +func newErrorwithSourceLocfInternal(skip int, format string, v ...interface{}) error { + _, file, line, ok := runtime.Caller(skip) + if !ok { + file = "???" + line = 0 + } + if lastSlash := strings.LastIndex(file, "/"); lastSlash >= 0 { + file = file[lastSlash+1:] + } + + return fmt.Errorf("%s:%d: %s", file, line, fmt.Sprintf(format, v...)) +} + +func getExitCode(err error) (exitCode int, ok bool) { + if err == nil { + return 0, true + } + if exiterr, ok := err.(*exec.ExitError); ok { + if status, ok := exiterr.Sys().(syscall.WaitStatus); ok { + return status.ExitStatus(), true + } + } + return 0, false +} diff --git a/compiler_wrapper/errors_test.go b/compiler_wrapper/errors_test.go new file mode 100644 index 00000000..626d3148 --- /dev/null +++ b/compiler_wrapper/errors_test.go @@ -0,0 +1,28 @@ +package main + +import ( + "errors" + "testing" +) + +func TestNewErrorwithSourceLocfMessage(t *testing.T) { + err := newErrorwithSourceLocf("a%sc", "b") + if err.Error() != "errors_test.go:9: abc" { + t.Errorf("Error message incorrect. Got: %s", err.Error()) + } +} + +func TestWrapErrorwithSourceLocfMessage(t *testing.T) { + cause := errors.New("someCause") + err := wrapErrorwithSourceLocf(cause, "a%sc", "b") + if err.Error() != "errors_test.go:17: abc: someCause" { + t.Errorf("Error message incorrect. Got: %s", err.Error()) + } +} + +func TestNewUserErrorf(t *testing.T) { + err := newUserErrorf("a%sc", "b") + if err.Error() != "abc" { + t.Errorf("Error message incorrect. Got: %s", err.Error()) + } +} diff --git a/compiler_wrapper/gcc_flags.go b/compiler_wrapper/gcc_flags.go index 352b38e4..ffbde316 100644 --- a/compiler_wrapper/gcc_flags.go +++ b/compiler_wrapper/gcc_flags.go @@ -12,13 +12,13 @@ func processGccFlags(builder *commandBuilder) { } builder.transformArgs(func(arg builderArg) string { - if unsupported[arg.Value] { + if unsupported[arg.value] { return "" } - if mapped, ok := clangToGcc[arg.Value]; ok { + if mapped, ok := clangToGcc[arg.value]; ok { return mapped } - return arg.Value + return arg.value }) builder.path += ".real" diff --git a/compiler_wrapper/gcc_flags_test.go b/compiler_wrapper/gcc_flags_test.go index 9e34216d..87c4153e 100644 --- a/compiler_wrapper/gcc_flags_test.go +++ b/compiler_wrapper/gcc_flags_test.go @@ -6,7 +6,7 @@ import ( func TestCallRealGcc(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyPath(cmd, gccX86_64+".real"); err != nil { t.Error(err) @@ -14,11 +14,11 @@ func TestCallRealGcc(t *testing.T) { }) } -func TestCallRealGPlusPlus(t *testing.T) { +func TestCallRealGccForOtherNames(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, - ctx.newCommand("./x86_64-cros-linux-gnu-g++", mainCc))) - if err := verifyPath(cmd, "\\./x86_64-cros-linux-gnu-g\\+\\+\\.real"); err != nil { + cmd := ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand("./x86_64-cros-linux-gnu-somename", mainCc))) + if err := verifyPath(cmd, "\\./x86_64-cros-linux-gnu-somename.real"); err != nil { t.Error(err) } }) @@ -36,7 +36,7 @@ func TestConvertClangToGccFlags(t *testing.T) { } for _, tt := range tests { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, tt.in, mainCc))) if err := verifyArgCount(cmd, 0, tt.in); err != nil { t.Error(err) @@ -50,7 +50,7 @@ func TestConvertClangToGccFlags(t *testing.T) { func TestFilterUnsupportedGccFlags(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-Xcompiler", mainCc))) if err := verifyArgCount(cmd, 0, "-Xcompiler"); err != nil { t.Error(err) diff --git a/compiler_wrapper/gomacc_flag_test.go b/compiler_wrapper/gomacc_flag_test.go index 5e8d66c9..9173aad5 100644 --- a/compiler_wrapper/gomacc_flag_test.go +++ b/compiler_wrapper/gomacc_flag_test.go @@ -11,7 +11,7 @@ func TestCallGomaccIfEnvIsGivenAndValid(t *testing.T) { // Create a file so the gomacc path is valid. ctx.writeFile(gomaPath, "") ctx.env = []string{"GOMACC_PATH=" + gomaPath} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyPath(cmd, gomaPath); err != nil { t.Error(err) @@ -27,7 +27,7 @@ func TestOmitGomaccIfEnvIsGivenButInvalid(t *testing.T) { // Note: This path does not point to a valid file. gomaPath := path.Join(ctx.tempDir, "gomacc") ctx.env = []string{"GOMACC_PATH=" + gomaPath} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyPath(cmd, gccX86_64+".real"); err != nil { t.Error(err) @@ -37,7 +37,7 @@ func TestOmitGomaccIfEnvIsGivenButInvalid(t *testing.T) { func TestOmitGomaccByDefault(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyPath(cmd, gccX86_64+".real"); err != nil { t.Error(err) diff --git a/compiler_wrapper/main.go b/compiler_wrapper/main.go index 9e131329..10132543 100644 --- a/compiler_wrapper/main.go +++ b/compiler_wrapper/main.go @@ -21,12 +21,10 @@ package main import ( "log" - "os/exec" - "syscall" + "os" ) func main() { - wrapperCmd := newProcessCommand() env, err := newProcessEnv() if err != nil { log.Fatal(err) @@ -35,23 +33,8 @@ func main() { if err != nil { log.Fatal(err) } - if shouldForwardToOldWrapper(env, wrapperCmd) { - err := forwardToOldWrapper(env, cfg, wrapperCmd) - if err != nil { - log.Fatal(err) - } - return - } - - cmd, err := calcCompilerCommandAndCompareToOld(env, cfg, wrapperCmd) - if err != nil { - log.Fatal(err) - } - if err := execCmd(newExecCmd(env, cmd)); err != nil { - log.Fatal(err) - } -} - -func execCmd(cmd *exec.Cmd) error { - return syscall.Exec(cmd.Path, cmd.Args, cmd.Env) + // Note: callCompiler will exec the command. Only in case of + // an error or when we are comparing against the old wrapper + // will this os.Exit be called. + os.Exit(callCompiler(env, cfg, newProcessCommand())) } diff --git a/compiler_wrapper/oldwrapper.go b/compiler_wrapper/oldwrapper.go index 46077731..c208c8f0 100644 --- a/compiler_wrapper/oldwrapper.go +++ b/compiler_wrapper/oldwrapper.go @@ -6,34 +6,81 @@ import ( "io" "io/ioutil" "os" - "os/exec" "path/filepath" + "reflect" "regexp" + "sort" "strings" "text/template" ) -func calcOldCompilerCommands(env env, cfg *config, wrapperCmd *command) ([]*command, error) { +const forwardToOldWrapperFilePattern = "old_wrapper_forward" +const compareToOldWrapperFilePattern = "old_wrapper_compare" + +// Whether the command should be executed by the old wrapper as we don't +// support it yet. +func shouldForwardToOldWrapper(env env, inputCmd *command) bool { + for _, arg := range inputCmd.args { + if arg == "-clang-syntax" { + return true + } + } + switch { + case env.getenv("WITH_TIDY") != "": + fallthrough + case env.getenv("FORCE_DISABLE_WERROR") != "": + fallthrough + case env.getenv("GETRUSAGE") != "": + fallthrough + case env.getenv("BISECT_STAGE") != "": + return true + } + return false +} + +func forwardToOldWrapper(env env, cfg *config, inputCmd *command) (exitCode int, err error) { + oldWrapperCfg, err := newOldWrapperConfig(env, cfg, inputCmd) + if err != nil { + return 0, err + } + return callOldWrapper(env, oldWrapperCfg, inputCmd, forwardToOldWrapperFilePattern, env.stdout(), env.stderr()) +} + +func compareToOldWrapper(env env, cfg *config, inputCmd *command, newCmdResults []*commandResult) error { + oldWrapperCfg, err := newOldWrapperConfig(env, cfg, inputCmd) + if err != nil { + return err + } + oldWrapperCfg.LogCmds = true + oldWrapperCfg.MockCmds = cfg.mockOldWrapperCmds + for _, cmdResult := range newCmdResults { + oldWrapperCfg.CmdResults = append(oldWrapperCfg.CmdResults, oldWrapperCmdResult{ + Stdout: cmdResult.stdout, + Stderr: cmdResult.stderr, + Exitcode: cmdResult.exitCode, + }) + } + oldWrapperCfg.OverwriteConfig = cfg.overwriteOldWrapperCfg + stdoutBuffer := bytes.Buffer{} stderrBuffer := bytes.Buffer{} - pipes := exec.Cmd{ - Stdin: strings.NewReader(""), - Stdout: &stdoutBuffer, - Stderr: &stderrBuffer, - } - mockForks := true - if err := callOldWrapper(env, cfg, wrapperCmd, &pipes, mockForks); err != nil { - return nil, fmt.Errorf("error: %s. %s", err, stderrBuffer.String()) + exitCode, err := callOldWrapper(env, oldWrapperCfg, inputCmd, compareToOldWrapperFilePattern, &stdoutBuffer, &stderrBuffer) + if err != nil { + return err } + oldCmdResults := parseOldWrapperCommands(stdoutBuffer.String(), stderrBuffer.String(), exitCode) + return diffCommandResults(oldCmdResults, newCmdResults) +} - // Parse the nested commands. - allStderrLines := strings.Split(stderrBuffer.String(), "\n") - var commands []*command +func parseOldWrapperCommands(stdout string, stderr string, exitCode int) []*commandResult { + allStderrLines := strings.Split(stderr, "\n") + remainingStderrLines := []string{} + cmdResults := []*commandResult{} for _, line := range allStderrLines { const commandPrefix = "command:" const envupdatePrefix = ".EnvUpdate:" - envUpdateIdx := strings.Index(line, ".EnvUpdate:") - if strings.Index(line, commandPrefix) >= 0 { + envUpdateIdx := strings.Index(line, envupdatePrefix) + if strings.Index(line, commandPrefix) == 0 { if envUpdateIdx == -1 { envUpdateIdx = len(line) - 1 } @@ -50,61 +97,123 @@ func calcOldCompilerCommands(env env, cfg *config, wrapperCmd *command) ([]*comm args: args[1:], envUpdates: envUpdate, } - commands = append(commands, command) + cmdResults = append(cmdResults, &commandResult{cmd: command}) + } else { + remainingStderrLines = append(remainingStderrLines, line) } } - return commands, nil -} - -func forwardToOldWrapper(env env, cfg *config, wrapperCmd *command) error { - pipes := exec.Cmd{ - Stdin: os.Stdin, - Stdout: os.Stdout, - Stderr: os.Stderr, + remainingStderr := strings.TrimSpace(strings.Join(remainingStderrLines, "\n")) + if len(cmdResults) > 0 { + lastCmdResult := cmdResults[len(cmdResults)-1] + lastCmdResult.exitCode = exitCode + lastCmdResult.stderr = remainingStderr + lastCmdResult.stdout = strings.TrimSpace(stdout) } - mockForks := false - return callOldWrapper(env, cfg, wrapperCmd, &pipes, mockForks) + + return cmdResults } -func callOldWrapper(env env, cfg *config, wrapperCmd *command, pipes *exec.Cmd, mockForks bool) error { - mockFile, err := ioutil.TempFile("", "compiler_wrapper_mock") - if err != nil { - return err +func diffCommandResults(oldCmdResults []*commandResult, newCmdResults []*commandResult) error { + maxLen := len(newCmdResults) + if maxLen < len(oldCmdResults) { + maxLen = len(oldCmdResults) } - defer os.Remove(mockFile.Name()) + hasDifferences := false + var cmdDifferences []string + for i := 0; i < maxLen; i++ { + var differences []string + if i >= len(newCmdResults) { + differences = append(differences, "missing command") + } else if i >= len(oldCmdResults) { + differences = append(differences, "extra command") + } else { + newCmdResult := newCmdResults[i] + oldCmdResult := oldCmdResults[i] - if err := writeOldWrapperMock(mockFile, env, cfg, wrapperCmd, mockForks); err != nil { - return err + if i == maxLen-1 && newCmdResult.exitCode != oldCmdResult.exitCode { + // We do not capture errors in nested commands from the old wrapper, + // so only compare the exit codes of the final compiler command. + differences = append(differences, "exit code") + } + + if newCmdResult.cmd.path != oldCmdResult.cmd.path { + differences = append(differences, "path") + } + + if !reflect.DeepEqual(newCmdResult.cmd.args, oldCmdResult.cmd.args) { + differences = append(differences, "args") + } + + // Sort the environment as we don't care in which order + // it was modified. + newEnvUpdates := newCmdResult.cmd.envUpdates + sort.Strings(newEnvUpdates) + oldEnvUpdates := oldCmdResult.cmd.envUpdates + sort.Strings(oldEnvUpdates) + + if !reflect.DeepEqual(newEnvUpdates, oldEnvUpdates) { + differences = append(differences, "env updates") + } + } + if len(differences) > 0 { + hasDifferences = true + } else { + differences = []string{"none"} + } + cmdDifferences = append(cmdDifferences, + fmt.Sprintf("Index %d: %s", i, strings.Join(differences, ","))) } - if err := mockFile.Close(); err != nil { - return err + if hasDifferences { + return newErrorwithSourceLocf("commands differ:\n%s\nOld commands:\n%s\nNew commands:\n%s", + strings.Join(cmdDifferences, "\n"), + dumpCommandResults(oldCmdResults), + dumpCommandResults(newCmdResults), + ) } + return nil +} - // Call the wrapper. - cmd := newExecCmd(env, wrapperCmd) - ensurePathEnv(cmd) - // Note: Using a self executable wrapper does not work due to a race condition - // on unix systems. See https://github.com/golang/go/issues/22315 - cmd.Args = append([]string{"/usr/bin/python2", "-S", mockFile.Name()}, cmd.Args[1:]...) - cmd.Path = cmd.Args[0] - cmd.Stdin = pipes.Stdin - cmd.Stdout = pipes.Stdout - cmd.Stderr = pipes.Stderr - return cmd.Run() +func dumpCommandResults(results []*commandResult) string { + lines := []string{} + for _, result := range results { + lines = append(lines, fmt.Sprintf("cmd: %#v; result: %#v", result.cmd, result)) + } + return strings.Join(lines, "\n") +} + +// Note: field names are upper case so they can be used in +// a template via reflection. +type oldWrapperConfig struct { + CmdPath string + OldWrapperContent string + RootRelPath string + LogCmds bool + MockCmds bool + CmdResults []oldWrapperCmdResult + OverwriteConfig bool + CommonFlags []string + GccFlags []string + ClangFlags []string +} + +type oldWrapperCmdResult struct { + Stdout string + Stderr string + Exitcode int } -func writeOldWrapperMock(writer io.Writer, env env, cfg *config, wrapperCmd *command, mockForks bool) error { +func newOldWrapperConfig(env env, cfg *config, inputCmd *command) (*oldWrapperConfig, error) { absOldWrapperPath := cfg.oldWrapperPath if !filepath.IsAbs(absOldWrapperPath) { - absWrapperDir, err := getAbsWrapperDir(env, wrapperCmd.path) + absWrapperDir, err := getAbsWrapperDir(env, inputCmd.path) if err != nil { - return err + return nil, err } absOldWrapperPath = filepath.Join(absWrapperDir, cfg.oldWrapperPath) } oldWrapperContentBytes, err := ioutil.ReadFile(absOldWrapperPath) if err != nil { - return err + return nil, wrapErrorwithSourceLocf(err, "failed to read old wrapper") } oldWrapperContent := string(oldWrapperContentBytes) // Disable the original call to main() @@ -114,42 +223,69 @@ func writeOldWrapperMock(writer io.Writer, env env, cfg *config, wrapperCmd *com oldWrapperContent = regexp.MustCompile(`True\s+#\s+@CCACHE_DEFAULT@`).ReplaceAllString(oldWrapperContent, "False #") } - // Note: Fieldnames need to be upper case so that they can be read via reflection. - mockData := struct { - CmdPath string - OldWrapperContent string - RootRelPath string - MockForks bool - OverwriteConfig bool - CommonFlags []string - GccFlags []string - ClangFlags []string - }{ - wrapperCmd.path, - oldWrapperContent, - cfg.rootRelPath, - mockForks, - cfg.overrideOldWrapperConfig, - cfg.commonFlags, - cfg.gccFlags, - cfg.clangFlags, + return &oldWrapperConfig{ + CmdPath: inputCmd.path, + OldWrapperContent: oldWrapperContent, + RootRelPath: cfg.rootRelPath, + CommonFlags: cfg.commonFlags, + GccFlags: cfg.gccFlags, + ClangFlags: cfg.clangFlags, + }, nil +} + +func callOldWrapper(env env, cfg *oldWrapperConfig, inputCmd *command, filepattern string, stdout io.Writer, stderr io.Writer) (exitCode int, err error) { + mockFile, err := ioutil.TempFile("", filepattern) + if err != nil { + return 0, wrapErrorwithSourceLocf(err, "failed to create tempfile") } + defer os.Remove(mockFile.Name()) const mockTemplate = `{{.OldWrapperContent}} -{{if .MockForks}} +import subprocess + init_env = os.environ.copy() +mockResults = [{{range .CmdResults}} { + 'stdout': '{{.Stdout}}', + 'stderr': '{{.Stderr}}', + 'exitcode': {{.Exitcode}}, +},{{end}}] + def serialize_cmd(args): + {{if .LogCmds}} current_env = os.environ envupdate = [k + "=" + current_env.get(k, '') for k in set(list(current_env.keys()) + list(init_env.keys())) if current_env.get(k, '') != init_env.get(k, '')] - print('command:%s.EnvUpdate:%s\n' % (' '.join(args), ' '.join(envupdate)), file=sys.stderr) + print('command:%s.EnvUpdate:%s' % (' '.join(args), ' '.join(envupdate)), file=sys.stderr) + {{end}} + +def check_output_mock(args): + serialize_cmd(args) + {{if .MockCmds}} + result = mockResults.pop(0) + print(result['stderr'], file=sys.stderr) + if result['exitcode']: + raise subprocess.CalledProcessError(result['exitcode']) + return result['stdout'] + {{else}} + return old_check_output(args) + {{end}} + +old_check_output = subprocess.check_output +subprocess.check_output = check_output_mock def execv_mock(binary, args): serialize_cmd([binary] + args[1:]) - sys.exit(0) + {{if .MockCmds}} + result = mockResults.pop(0) + print(result['stdout'], file=sys.stdout) + print(result['stderr'], file=sys.stderr) + sys.exit(result['exitcode']) + {{else}} + old_execv(binary, args) + {{end}} +old_execv = os.execv os.execv = execv_mock -{{end}} sys.argv[0] = '{{.CmdPath}}' @@ -163,20 +299,31 @@ CLANG_FLAGS_TO_ADD=set([{{range .ClangFlags}}'{{.}}',{{end}}]) sys.exit(main()) ` - tmpl, err := template.New("mock").Parse(mockTemplate) if err != nil { - return err + return 0, wrapErrorwithSourceLocf(err, "failed to parse old wrapper template") } + if err := tmpl.Execute(mockFile, cfg); err != nil { + return 0, wrapErrorwithSourceLocf(err, "failed execute old wrapper template") + } + if err := mockFile.Close(); err != nil { + return 0, wrapErrorwithSourceLocf(err, "failed to close temp file") + } + buf := bytes.Buffer{} + tmpl.Execute(&buf, cfg) - return tmpl.Execute(writer, mockData) -} - -func ensurePathEnv(cmd *exec.Cmd) { - for _, env := range cmd.Env { - if strings.HasPrefix(env, "PATH=") { - return + // Note: Using a self executable wrapper does not work due to a race condition + // on unix systems. See https://github.com/golang/go/issues/22315 + if err := env.run(&command{ + path: "/usr/bin/python2", + args: append([]string{"-S", mockFile.Name()}, inputCmd.args...), + envUpdates: inputCmd.envUpdates, + }, stdout, stderr); err != nil { + if exitCode, ok := getExitCode(err); ok { + return exitCode, nil } + return 0, wrapErrorwithSourceLocf(err, "failed to call old wrapper. Command: %s %s", + inputCmd.path, inputCmd.args) } - cmd.Env = append(cmd.Env, "PATH=") + return 0, nil } diff --git a/compiler_wrapper/oldwrapper_test.go b/compiler_wrapper/oldwrapper_test.go new file mode 100644 index 00000000..2bb90d08 --- /dev/null +++ b/compiler_wrapper/oldwrapper_test.go @@ -0,0 +1,352 @@ +package main + +import ( + "bytes" + "errors" + "fmt" + "io" + "path/filepath" + "strings" + "testing" + "text/template" +) + +func TestNoForwardToOldWrapperBecauseOfArgs(t *testing.T) { + withTestContext(t, func(ctx *testContext) { + testArgs := []string{ + "abc", + "-clang-syntaxabc", + } + for _, testArg := range testArgs { + forwarded := false + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + if isForwardToOldWrapperCmd(cmd) { + forwarded = true + } + return nil + } + ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand(clangX86_64, testArg))) + if forwarded { + t.Errorf("forwarded to old wrapper for arg %s", testArg) + } + } + }) +} + +func TestForwardToOldWrapperBecauseOfArgs(t *testing.T) { + withForwardToOldWrapperTestContext(t, func(ctx *testContext) { + testArgs := []string{"-clang-syntax"} + for _, testArg := range testArgs { + forwarded := false + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + forwarded = true + if !isForwardToOldWrapperCmd(cmd) { + return newErrorwithSourceLocf("expected call to old wrapper. Got: %s", cmd.path) + } + if err := verifyArgCount(cmd, 1, testArg); err != nil { + return err + } + return nil + } + ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, testArg))) + if !forwarded { + t.Errorf("not forwarded to old wrapper for arg %s", testArg) + } + } + }) +} + +func TestNoForwardToOldWrapperBecauseOfEnv(t *testing.T) { + withTestContext(t, func(ctx *testContext) { + testEnvs := []string{"PATH=abc"} + for _, testEnv := range testEnvs { + ctx.env = []string{testEnv} + forwarded := false + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + if isForwardToOldWrapperCmd(cmd) { + forwarded = true + } + return nil + } + ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64))) + if forwarded { + t.Errorf("forwarded to old wrapper for env %s", testEnv) + } + } + }) +} + +func TestForwardToOldWrapperBecauseOfEnv(t *testing.T) { + withForwardToOldWrapperTestContext(t, func(ctx *testContext) { + testEnvs := []string{ + "WITH_TIDY=abc", + "FORCE_DISABLE_WERROR=abc", + "GETRUSAGE=abc", + "BISECT_STAGE=abc", + } + for _, testEnv := range testEnvs { + ctx.env = []string{testEnv} + forwarded := false + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + forwarded = true + if !isForwardToOldWrapperCmd(cmd) { + return newErrorwithSourceLocf("expected call to old wrapper. Got: %s", cmd.path) + } + return nil + } + ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64))) + if !forwarded { + t.Errorf("not forwarded to old wrapper for env %s", testEnv) + } + } + }) +} + +func TestForwardStdOutAndStderrFromOldWrapperOnSuccess(t *testing.T) { + withForwardToOldWrapperTestContext(t, func(ctx *testContext) { + ctx.env = []string{"BISECT_STAGE=abc"} + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + fmt.Fprint(stdout, "somemessage") + fmt.Fprint(stderr, "someerror") + return nil + } + + ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64))) + if ctx.stdoutString() != "somemessage" { + t.Errorf("stdout was not exactly forwarded. Got: %s", ctx.stdoutString()) + } + if ctx.stderrString() != "someerror" { + t.Errorf("stderr was not exactly forwarded. Got: %s", ctx.stderrString()) + } + }) +} + +func TestReportExitCodeErrorsWhenForwardingToOldWrapper(t *testing.T) { + withForwardToOldWrapperTestContext(t, func(ctx *testContext) { + ctx.env = []string{"BISECT_STAGE=abc"} + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + fmt.Fprint(stderr, "someerror") + return newExitCodeError(2) + } + + exitCode := callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64)) + if exitCode != 2 { + t.Fatalf("Expected exit code 2. Got %d", exitCode) + } + if err := verifyNonInternalError(ctx.stderrString(), "someerror"); err != nil { + t.Fatal(err) + } + }) +} + +func TestReportGeneralErrorsWhenForwardingToOldWrapper(t *testing.T) { + withForwardToOldWrapperTestContext(t, func(ctx *testContext) { + ctx.env = []string{"BISECT_STAGE=abc"} + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + fmt.Fprint(stderr, "someoldwrappererror") + return errors.New("someerror") + } + + stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64))) + if err := verifyInternalError(stderr); err != nil { + t.Fatal(err) + } + if !strings.Contains(stderr, "someerror") { + t.Errorf("error message was not forwarded. Got: %s", stderr) + } + if !strings.Contains(stderr, "someoldwrappererror") { + t.Errorf("stderr was not forwarded. Got: %s", stderr) + } + }) +} + +func TestCompareToOldWrapperCompilerCommand(t *testing.T) { + withTestContext(t, func(ctx *testContext) { + ctx.cfg.mockOldWrapperCmds = false + ctx.cfg.oldWrapperPath = filepath.Join(ctx.tempDir, "fakewrapper") + + pathSuffix := "" + extraArgs := []string{} + exitCode := 0 + + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + writeMockWrapper(ctx, &mockWrapperConfig{ + Cmds: []*mockWrapperCmd{ + { + Path: cmd.path + pathSuffix, + Args: append(cmd.args, extraArgs...), + ExitCode: exitCode, + }, + }, + }) + return nil + } + + // Note: This will cause only the compiler command. + inputCmd := ctx.newCommand(gccX86_64) + + ctx.stderrBuffer.Reset() + pathSuffix = "xyz" + extraArgs = nil + exitCode = 0 + stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg, inputCmd)) + if !strings.Contains(stderr, "Index 0: path") { + t.Errorf("expected path difference error. Got: %s", stderr) + } + + ctx.stderrBuffer.Reset() + pathSuffix = "" + extraArgs = []string{"xyz"} + exitCode = 0 + stderr = ctx.mustFail(callCompiler(ctx, ctx.cfg, inputCmd)) + if !strings.Contains(stderr, "Index 0: args") { + t.Errorf("expected args difference error. Got: %s", stderr) + } + + ctx.stderrBuffer.Reset() + pathSuffix = "" + extraArgs = nil + exitCode = 1 + stderr = ctx.mustFail(callCompiler(ctx, ctx.cfg, inputCmd)) + if !strings.Contains(stderr, "Index 0: exit code") { + t.Errorf("expected exit code difference error. Got: %s", stderr) + } + + pathSuffix = "" + extraArgs = nil + exitCode = 0 + ctx.must(callCompiler(ctx, ctx.cfg, inputCmd)) + }) +} + +func TestCompareToOldWrapperNestedCommand(t *testing.T) { + withTestContext(t, func(ctx *testContext) { + ctx.cfg.mockOldWrapperCmds = false + ctx.cfg.oldWrapperPath = filepath.Join(ctx.tempDir, "fakewrapper") + + pathSuffix := "" + extraArgs := []string{} + wrapperCfg := &mockWrapperConfig{} + + ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error { + isNestedCmd := len(wrapperCfg.Cmds) == 0 + var wrapperCmd *mockWrapperCmd + if isNestedCmd { + wrapperCmd = &mockWrapperCmd{ + Path: cmd.path + pathSuffix, + Args: append(cmd.args, extraArgs...), + } + } else { + wrapperCmd = &mockWrapperCmd{ + Path: cmd.path, + Args: cmd.args, + } + } + wrapperCfg.Cmds = append(wrapperCfg.Cmds, wrapperCmd) + if !isNestedCmd { + writeMockWrapper(ctx, wrapperCfg) + } + return nil + } + + // Note: This will cause a nested command call. + inputCmd := ctx.newCommand(clangX86_64, "-Xclang-path=somedir", mainCc) + + ctx.stderrBuffer.Reset() + wrapperCfg = &mockWrapperConfig{} + pathSuffix = "xyz" + extraArgs = nil + stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg, inputCmd)) + if !strings.Contains(stderr, "Index 0: path") { + t.Errorf("expected path difference error. Got: %s", stderr) + } + if !strings.Contains(stderr, "Index 1: none") { + t.Errorf("expected no difference for cmd index 1. Got: %s", stderr) + } + + ctx.stderrBuffer.Reset() + wrapperCfg = &mockWrapperConfig{} + pathSuffix = "" + extraArgs = []string{"xyz"} + stderr = ctx.mustFail(callCompiler(ctx, ctx.cfg, inputCmd)) + if !strings.Contains(stderr, "Index 0: args") { + t.Errorf("expected args difference error. Got: %s", stderr) + } + if !strings.Contains(stderr, "Index 1: none") { + t.Errorf("expected no difference for cmd index 1. Got: %s", stderr) + } + + wrapperCfg = &mockWrapperConfig{} + pathSuffix = "" + extraArgs = nil + ctx.must(callCompiler(ctx, ctx.cfg, inputCmd)) + }) +} + +func withForwardToOldWrapperTestContext(t *testing.T, work func(ctx *testContext)) { + withTestContext(t, func(ctx *testContext) { + // Need to make sure the old wrapper file exists as oldwrapper.go + // tries to read it. + ctx.cfg.oldWrapperPath = filepath.Join(ctx.tempDir, "somewrapper") + ctx.writeFile(ctx.cfg.oldWrapperPath, "") + work(ctx) + }) +} + +func writeMockWrapper(ctx *testContext, cfg *mockWrapperConfig) { + const mockTemplate = ` +from __future__ import print_function +import os +import sys +import subprocess + +mockCmds = [{{range .Cmds}} { + 'path': '{{.Path}}', + 'args': [{{range .Args}}'{{.}}',{{end}}], + 'exitcode': {{.ExitCode}}, +},{{end}}] + +def execv_impl(binary, args): + cmd = mockCmds.pop(0) + sys.exit(cmd['exitcode']) +os.execv = execv_impl + +def check_output_impl(args): + cmd = mockCmds.pop(0) + if cmd['exitcode']: + raise subprocess.CalledProcessError(cmd['exitcode']) + return "" +subprocess.check_output = check_output_impl + +def main(): + while len(mockCmds) > 1: + subprocess.check_output([mockCmds[0]['path']] + mockCmds[0]['args']) + + os.execv(mockCmds[0]['path'], [mockCmds[0]['path']] + mockCmds[0]['args']) + +if __name__ == '__main__': + sys.exit(main()) +` + tmpl, err := template.New("mock").Parse(mockTemplate) + if err != nil { + ctx.t.Fatalf("failed to parse old wrapper template. Error: %s", err) + } + buf := bytes.Buffer{} + if err := tmpl.Execute(&buf, cfg); err != nil { + ctx.t.Fatalf("failed to execute the template. Error: %s", err) + } + ctx.writeFile(ctx.cfg.oldWrapperPath, buf.String()) +} + +// Note: Fields have to be uppercase so that they can be used with template. +type mockWrapperConfig struct { + Cmds []*mockWrapperCmd +} + +// Note: Fields have to be uppercase so that they can be used with template. +type mockWrapperCmd struct { + Path string + Args []string + ExitCode int +} diff --git a/compiler_wrapper/pie_flags.go b/compiler_wrapper/pie_flags.go index 73b3a224..167f1193 100644 --- a/compiler_wrapper/pie_flags.go +++ b/compiler_wrapper/pie_flags.go @@ -13,11 +13,11 @@ func processPieFlags(builder *commandBuilder) { fpie := false if builder.target.abi != "eabi" { for _, arg := range builder.args { - if arg.FromUser { - if fpieMap[arg.Value] { + if arg.fromUser { + if fpieMap[arg.value] { fpie = true } - if pieMap[arg.Value] { + if pieMap[arg.value] { pie = true } } @@ -25,15 +25,15 @@ func processPieFlags(builder *commandBuilder) { } builder.transformArgs(func(arg builderArg) string { // Remove -nopie as it is a non-standard flag. - if arg.Value == "-nopie" { + if arg.value == "-nopie" { return "" } - if fpie && !arg.FromUser && arg.Value == "-fPIE" { + if fpie && !arg.fromUser && arg.value == "-fPIE" { return "" } - if pie && !arg.FromUser && arg.Value == "-pie" { + if pie && !arg.fromUser && arg.value == "-pie" { return "" } - return arg.Value + return arg.value }) } diff --git a/compiler_wrapper/pie_flags_test.go b/compiler_wrapper/pie_flags_test.go index 56250fcd..58ed54cf 100644 --- a/compiler_wrapper/pie_flags_test.go +++ b/compiler_wrapper/pie_flags_test.go @@ -7,8 +7,8 @@ import ( func TestAddPieFlags(t *testing.T) { withTestContext(t, func(ctx *testContext) { initPieConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) - + cmd := ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgOrder(cmd, "-pie", mainCc); err != nil { t.Error(err) } @@ -21,7 +21,7 @@ func TestAddPieFlags(t *testing.T) { func TestOmitPieFlagsWhenNoPieArgGiven(t *testing.T) { withTestContext(t, func(ctx *testContext) { initPieConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-nopie", mainCc))) if err := verifyArgCount(cmd, 0, "-nopie"); err != nil { t.Error(err) @@ -33,7 +33,7 @@ func TestOmitPieFlagsWhenNoPieArgGiven(t *testing.T) { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-fno-pie", mainCc))) if err := verifyArgCount(cmd, 0, "-pie"); err != nil { t.Error(err) @@ -47,7 +47,7 @@ func TestOmitPieFlagsWhenNoPieArgGiven(t *testing.T) { func TestOmitPieFlagsWhenKernelDefined(t *testing.T) { withTestContext(t, func(ctx *testContext) { initPieConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-D__KERNEL__", mainCc))) if err := verifyArgCount(cmd, 0, "-pie"); err != nil { t.Error(err) @@ -61,7 +61,7 @@ func TestOmitPieFlagsWhenKernelDefined(t *testing.T) { func TestAddPieFlagsForEabiEvenIfNoPieGiven(t *testing.T) { withTestContext(t, func(ctx *testContext) { initPieConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64Eabi, "-nopie", mainCc))) if err := verifyArgCount(cmd, 0, "-nopie"); err != nil { t.Error(err) diff --git a/compiler_wrapper/sanitizer_flags.go b/compiler_wrapper/sanitizer_flags.go index de4f76c4..1823dcb1 100644 --- a/compiler_wrapper/sanitizer_flags.go +++ b/compiler_wrapper/sanitizer_flags.go @@ -9,7 +9,7 @@ func processSanitizerFlags(builder *commandBuilder) { for _, arg := range builder.args { // TODO: This should probably be -fsanitize= to not match on // e.g. -fsanitize-blacklist - if arg.FromUser && strings.HasPrefix(arg.Value, "-fsanitize") { + if arg.fromUser && strings.HasPrefix(arg.value, "-fsanitize") { filterSanitizerFlags = true break } @@ -24,10 +24,10 @@ func processSanitizerFlags(builder *commandBuilder) { } builder.transformArgs(func(arg builderArg) string { - if unsupportedSanitizerFlags[arg.Value] { + if unsupportedSanitizerFlags[arg.value] { return "" } - return arg.Value + return arg.value }) } } diff --git a/compiler_wrapper/sanitizer_flags_test.go b/compiler_wrapper/sanitizer_flags_test.go index b54398a6..2b348779 100644 --- a/compiler_wrapper/sanitizer_flags_test.go +++ b/compiler_wrapper/sanitizer_flags_test.go @@ -6,26 +6,25 @@ import ( func TestFilterUnsupportedSanitizerFlagsIfSanitizeGiven(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-fsanitize=kernel-address", "-Wl,--no-undefined", mainCc))) if err := verifyArgCount(cmd, 0, "-Wl,--no-undefined"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, - ctx.newCommand(gccX86_64, "-fsanitize=kernel-address", - "-Wl,-z,defs", mainCc))) + cmd = ctx.must(callCompiler(ctx, ctx.cfg, + ctx.newCommand(gccX86_64, "-fsanitize=kernel-address", "-Wl,-z,defs", mainCc))) if err := verifyArgCount(cmd, 0, "-Wl,-z,defs"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-fsanitize=kernel-address", "-D_FORTIFY_SOURCE=1", mainCc))) if err := verifyArgCount(cmd, 0, "-D_FORTIFY_SOURCE=1"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-fsanitize=kernel-address", "-D_FORTIFY_SOURCE=2", mainCc))) if err := verifyArgCount(cmd, 0, "-D_FORTIFY_SOURCE=2"); err != nil { t.Error(err) @@ -35,25 +34,25 @@ func TestFilterUnsupportedSanitizerFlagsIfSanitizeGiven(t *testing.T) { func TestKeepSanitizerFlagsIfNoSanitizeGiven(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-Wl,--no-undefined", mainCc))) if err := verifyArgCount(cmd, 1, "-Wl,--no-undefined"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-Wl,-z,defs", mainCc))) if err := verifyArgCount(cmd, 1, "-Wl,-z,defs"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-D_FORTIFY_SOURCE=1", mainCc))) if err := verifyArgCount(cmd, 1, "-D_FORTIFY_SOURCE=1"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-D_FORTIFY_SOURCE=2", mainCc))) if err := verifyArgCount(cmd, 1, "-D_FORTIFY_SOURCE=2"); err != nil { t.Error(err) @@ -64,7 +63,7 @@ func TestKeepSanitizerFlagsIfNoSanitizeGiven(t *testing.T) { func TestKeepSanitizerFlagsIfSanitizeGivenInCommonFlags(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.commonFlags = []string{"-fsanitize=kernel-address"} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-Wl,--no-undefined", mainCc))) if err := verifyArgCount(cmd, 1, "-Wl,--no-undefined"); err != nil { t.Error(err) diff --git a/compiler_wrapper/stackprotector_flags.go b/compiler_wrapper/stackprotector_flags.go index caadb6ed..fe1e6dc1 100644 --- a/compiler_wrapper/stackprotector_flags.go +++ b/compiler_wrapper/stackprotector_flags.go @@ -7,7 +7,7 @@ func processStackProtectorFlags(builder *commandBuilder) { fstack := false if builder.target.abi != "eabi" { for _, arg := range builder.args { - if arg.FromUser && fstackMap[arg.Value] { + if arg.fromUser && fstackMap[arg.value] { fstack = true break } @@ -16,10 +16,10 @@ func processStackProtectorFlags(builder *commandBuilder) { if fstack { builder.addPreUserArgs("-fno-stack-protector") builder.transformArgs(func(arg builderArg) string { - if !arg.FromUser && arg.Value == "-fstack-protector-strong" { + if !arg.fromUser && arg.value == "-fstack-protector-strong" { return "" } - return arg.Value + return arg.value }) } } diff --git a/compiler_wrapper/stackprotector_flags_test.go b/compiler_wrapper/stackprotector_flags_test.go index 1a50a9b0..ab838732 100644 --- a/compiler_wrapper/stackprotector_flags_test.go +++ b/compiler_wrapper/stackprotector_flags_test.go @@ -7,7 +7,7 @@ import ( func TestAddStrongStackProtectorFlag(t *testing.T) { withTestContext(t, func(ctx *testContext) { initStackProtectorStrongConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgOrder(cmd, "-fstack-protector-strong", mainCc); err != nil { t.Error(err) @@ -18,7 +18,7 @@ func TestAddStrongStackProtectorFlag(t *testing.T) { func TestAddNoStackProtectorFlagWhenKernelDefined(t *testing.T) { withTestContext(t, func(ctx *testContext) { initStackProtectorStrongConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, "-D__KERNEL__", mainCc))) if err := verifyArgOrder(cmd, "-fno-stack-protector", mainCc); err != nil { t.Error(err) @@ -29,7 +29,7 @@ func TestAddNoStackProtectorFlagWhenKernelDefined(t *testing.T) { func TestOmitNoStackProtectorFlagWhenAlreadyInCommonFlags(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.commonFlags = []string{"-fno-stack-protector"} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgCount(cmd, 1, "-fno-stack-protector"); err != nil { t.Error(err) @@ -40,7 +40,7 @@ func TestOmitNoStackProtectorFlagWhenAlreadyInCommonFlags(t *testing.T) { func TestAddStrongStackProtectorFlagForEabiEvenIfNoStackProtectorGiven(t *testing.T) { withTestContext(t, func(ctx *testContext) { initStackProtectorStrongConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64Eabi, "-fno-stack-protector", mainCc))) if err := verifyArgCount(cmd, 1, "-fstack-protector-strong"); err != nil { t.Error(err) diff --git a/compiler_wrapper/sysroot_flag.go b/compiler_wrapper/sysroot_flag.go index fa8a3aef..e2303046 100644 --- a/compiler_wrapper/sysroot_flag.go +++ b/compiler_wrapper/sysroot_flag.go @@ -8,7 +8,7 @@ import ( func processSysrootFlag(rootPath string, builder *commandBuilder) string { fromUser := false for _, arg := range builder.args { - if arg.FromUser && strings.HasPrefix(arg.Value, "--sysroot=") { + if arg.fromUser && strings.HasPrefix(arg.value, "--sysroot=") { fromUser = true break } diff --git a/compiler_wrapper/sysroot_flag_test.go b/compiler_wrapper/sysroot_flag_test.go index 93d995d9..295a1ab9 100644 --- a/compiler_wrapper/sysroot_flag_test.go +++ b/compiler_wrapper/sysroot_flag_test.go @@ -8,7 +8,7 @@ import ( func TestOmitSysrootGivenUserDefinedSysroot(t *testing.T) { withTestContext(t, func(ctx *testContext) { runWithCompiler := func(compiler string) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(compiler, "--sysroot=/somepath", mainCc))) if err := verifyArgOrder(cmd, "--sysroot=/somepath", mainCc); err != nil { t.Error(err) @@ -26,7 +26,7 @@ func TestOmitSysrootGivenUserDefinedSysroot(t *testing.T) { func TestSetSysrootFlagFromEnv(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.env = []string{"SYSROOT=/envpath"} - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgOrder(cmd, "--sysroot=/envpath", mainCc); err != nil { t.Error(err) @@ -37,7 +37,7 @@ func TestSetSysrootFlagFromEnv(t *testing.T) { func TestSetSysrootRelativeToWrapperPath(t *testing.T) { withTestContext(t, func(ctx *testContext) { ctx.cfg.rootRelPath = "somepath" - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgOrder(cmd, "--sysroot="+ctx.tempDir+"/somepath/usr/x86_64-cros-linux-gnu", mainCc); err != nil { @@ -52,7 +52,7 @@ func TestSetSysrootRelativeToSymlinkedWrapperPath(t *testing.T) { linkedWrapperPath := path.Join(ctx.tempDir, "a/linked/path/x86_64-cros-linux-gnu-gcc") ctx.symlink(path.Join(ctx.tempDir, gccX86_64), linkedWrapperPath) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(linkedWrapperPath, mainCc))) if err := verifyArgOrder(cmd, "--sysroot="+ctx.tempDir+"/somepath/usr/x86_64-cros-linux-gnu", mainCc); err != nil { diff --git a/compiler_wrapper/testutil_test.go b/compiler_wrapper/testutil_test.go index 5ebe0084..fe9ab99c 100644 --- a/compiler_wrapper/testutil_test.go +++ b/compiler_wrapper/testutil_test.go @@ -1,10 +1,13 @@ package main import ( + "bytes" "flag" "fmt" + "io" "io/ioutil" "os" + "os/exec" "path/filepath" "regexp" "strings" @@ -26,10 +29,16 @@ const oldHardenedWrapperPathForTest = "/usr/x86_64-pc-linux-gnu/x86_64-cros-linu const oldNonHardenedWrapperPathForTest = "/usr/x86_64-pc-linux-gnu/arm-none-eabi/gcc-bin/4.9.x/sysroot_wrapper" type testContext struct { - t *testing.T - tempDir string - env []string - cfg *config + t *testing.T + tempDir string + env []string + cfg *config + inputCmd *command + lastCmd *command + cmdCount int + cmdMock func(cmd *command, stdout io.Writer, stderr io.Writer) error + stdoutBuffer bytes.Buffer + stderrBuffer bytes.Buffer } func withTestContext(t *testing.T, work func(ctx *testContext)) { @@ -44,14 +53,11 @@ func withTestContext(t *testing.T, work func(ctx *testContext)) { t: t, tempDir: tempDir, env: nil, - cfg: &config{ - oldWrapperPath: "FilledLater", - overrideOldWrapperConfig: true, - }, + cfg: &config{}, } // Note: It's ok to use the hardened wrapper here, as we replace its config // on each run. - ctx.setOldWrapperPath(oldHardenedWrapperPathForTest) + ctx.updateConfig(oldHardenedWrapperPathForTest, &config{}) work(&ctx) } @@ -76,16 +82,69 @@ func (ctx *testContext) getwd() string { return ctx.tempDir } -func (ctx *testContext) must(cmd *command, err error) *command { - if err != nil { - ctx.t.Fatalf("Expected no error, but got %s", err) +func (ctx *testContext) stdout() io.Writer { + return &ctx.stdoutBuffer +} + +func (ctx *testContext) stdoutString() string { + return ctx.stdoutBuffer.String() +} + +func (ctx *testContext) stderr() io.Writer { + return &ctx.stderrBuffer +} + +func (ctx *testContext) stderrString() string { + return ctx.stderrBuffer.String() +} + +func (ctx *testContext) run(cmd *command, stdout io.Writer, stderr io.Writer) error { + // Keep calling the old wrapper when we are comparing the output of the + // old wrapper to the new wrapper. + if isCompareToOldWrapperCmd(cmd) { + execCmd := newExecCmd(ctx, cmd) + execCmd.Stdout = stdout + execCmd.Stderr = stderr + return execCmd.Run() + } + ctx.cmdCount++ + ctx.lastCmd = cmd + if ctx.cmdMock != nil { + return ctx.cmdMock(cmd, stdout, stderr) + } + return nil +} + +func (ctx *testContext) exec(cmd *command) error { + ctx.cmdCount++ + ctx.lastCmd = cmd + if ctx.cmdMock != nil { + return ctx.cmdMock(cmd, ctx.stdout(), ctx.stderr()) } - return cmd + return nil } -func (ctx *testContext) setOldWrapperPath(chrootPath string) { +func (ctx *testContext) must(exitCode int) *command { + if exitCode != 0 { + ctx.t.Fatalf("expected no error, but got %d. Stderr: %s", + exitCode, ctx.stderrString()) + } + return ctx.lastCmd +} + +func (ctx *testContext) mustFail(exitCode int) string { + if exitCode == 0 { + ctx.t.Fatalf("expected an error, but got none") + } + return ctx.stderrString() +} + +func (ctx *testContext) updateConfig(wrapperChrootPath string, cfg *config) { + *ctx.cfg = *cfg + ctx.cfg.overwriteOldWrapperCfg = true + ctx.cfg.mockOldWrapperCmds = true if *crosRootDirFlag != "" { - ctx.cfg.oldWrapperPath = filepath.Join(*crosRootDirFlag, chrootPath) + ctx.cfg.oldWrapperPath = filepath.Join(*crosRootDirFlag, wrapperChrootPath) } else { ctx.cfg.oldWrapperPath = "" } @@ -188,6 +247,54 @@ func verifyNoEnvUpdate(cmd *command, expectedRegex string) error { return nil } +func verifyInternalError(stderr string) error { + if !strings.Contains(stderr, "Internal error") { + return fmt.Errorf("expected an internal error. Got: %s", stderr) + } + if ok, _ := regexp.MatchString(`\w+.go:\d+`, stderr); !ok { + return fmt.Errorf("expected a source line reference. Got: %s", stderr) + } + return nil +} + +func verifyNonInternalError(stderr string, expectedRegex string) error { + if strings.Contains(stderr, "Internal error") { + return fmt.Errorf("expected a non internal error. Got: %s", stderr) + } + if ok, _ := regexp.MatchString(`\w+.go:\d+`, stderr); ok { + return fmt.Errorf("expected no source line reference. Got: %s", stderr) + } + if ok, _ := regexp.MatchString(matchFullString(expectedRegex), strings.TrimSpace(stderr)); !ok { + return fmt.Errorf("expected stderr matching %s. Got: %s", expectedRegex, stderr) + } + return nil +} + func matchFullString(regex string) string { return "^" + regex + "$" } + +func newExitCodeError(exitCode int) error { + // It's actually hard to create an error that represents a command + // with exit code. Using a real command instead. + tmpCmd := exec.Command("/usr/bin/sh", "-c", fmt.Sprintf("exit %d", exitCode)) + return tmpCmd.Run() +} + +func isForwardToOldWrapperCmd(cmd *command) bool { + for _, arg := range cmd.args { + if strings.Contains(arg, forwardToOldWrapperFilePattern) { + return true + } + } + return false +} + +func isCompareToOldWrapperCmd(cmd *command) bool { + for _, arg := range cmd.args { + if strings.Contains(arg, compareToOldWrapperFilePattern) { + return true + } + } + return false +} diff --git a/compiler_wrapper/thumb_flags.go b/compiler_wrapper/thumb_flags.go index fa65875e..7c26d0b3 100644 --- a/compiler_wrapper/thumb_flags.go +++ b/compiler_wrapper/thumb_flags.go @@ -14,10 +14,10 @@ func processThumbCodeFlags(builder *commandBuilder) { // 2. Do not force frame pointers on ARM32 (https://crbug.com/693137). builder.addPreUserArgs("-mthumb") builder.transformArgs(func(arg builderArg) string { - if !arg.FromUser && arg.Value == "-fno-omit-frame-pointer" { + if !arg.fromUser && arg.value == "-fno-omit-frame-pointer" { return "" } - return arg.Value + return arg.value }) } } diff --git a/compiler_wrapper/thumb_flags_test.go b/compiler_wrapper/thumb_flags_test.go index 463ce260..1a1187ac 100644 --- a/compiler_wrapper/thumb_flags_test.go +++ b/compiler_wrapper/thumb_flags_test.go @@ -6,13 +6,13 @@ import ( func TestAddThumbFlagForArm(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV7, mainCc))) if err := verifyArgOrder(cmd, "-mthumb", mainCc); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV8, mainCc))) if err := verifyArgOrder(cmd, "-mthumb", mainCc); err != nil { t.Error(err) @@ -22,7 +22,7 @@ func TestAddThumbFlagForArm(t *testing.T) { func TestOmitThumbFlagForNonArm(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgCount(cmd, 0, "-mthumb"); err != nil { t.Error(err) @@ -32,13 +32,13 @@ func TestOmitThumbFlagForNonArm(t *testing.T) { func TestOmitThumbFlagForEabiArm(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV7Eabi, mainCc))) if err := verifyArgCount(cmd, 0, "-mthumb"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV8Eabi, mainCc))) if err := verifyArgCount(cmd, 0, "-mthumb"); err != nil { t.Error(err) @@ -50,13 +50,13 @@ func TestRemoveNoOmitFramePointerFlagForArm(t *testing.T) { withTestContext(t, func(ctx *testContext) { initNoOmitFramePointerConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV7, mainCc))) if err := verifyArgCount(cmd, 0, "-fno-omit-frame-pointer"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV8, mainCc))) if err := verifyArgCount(cmd, 0, "-fno-omit-frame-pointer"); err != nil { t.Error(err) @@ -68,7 +68,7 @@ func TestKeepNoOmitFramePointerFlagForNonArm(t *testing.T) { withTestContext(t, func(ctx *testContext) { initNoOmitFramePointerConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgCount(cmd, 1, "-fno-omit-frame-pointer"); err != nil { t.Error(err) @@ -80,13 +80,13 @@ func TestKeepNoOmitFramePointerFlagForEabiArm(t *testing.T) { withTestContext(t, func(ctx *testContext) { initNoOmitFramePointerConfig(ctx.cfg) - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV7Eabi, mainCc))) if err := verifyArgCount(cmd, 1, "-fno-omit-frame-pointer"); err != nil { t.Error(err) } - cmd = ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd = ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV8Eabi, mainCc))) if err := verifyArgCount(cmd, 1, "-fno-omit-frame-pointer"); err != nil { t.Error(err) @@ -96,7 +96,7 @@ func TestKeepNoOmitFramePointerFlagForEabiArm(t *testing.T) { func TestKeepNoOmitFramePointIfGivenByUser(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV7, "-fno-omit-frame-pointer", mainCc))) if err := verifyArgCount(cmd, 1, "-fno-omit-frame-pointer"); err != nil { t.Error(err) diff --git a/compiler_wrapper/unsupported_flags.go b/compiler_wrapper/unsupported_flags.go index 604f94ce..3e3e8cb5 100644 --- a/compiler_wrapper/unsupported_flags.go +++ b/compiler_wrapper/unsupported_flags.go @@ -1,13 +1,9 @@ package main -import ( - "errors" -) - func checkUnsupportedFlags(cmd *command) error { for _, arg := range cmd.args { if arg == "-fstack-check" { - return errors.New(`option "-fstack-check" is not supported; crbug/485492`) + return newUserErrorf(`option %q is not supported; crbug/485492`, arg) } } return nil diff --git a/compiler_wrapper/unsupported_flags_test.go b/compiler_wrapper/unsupported_flags_test.go index 0b174a9b..d54fcf09 100644 --- a/compiler_wrapper/unsupported_flags_test.go +++ b/compiler_wrapper/unsupported_flags_test.go @@ -6,10 +6,11 @@ import ( func TestErrorOnFstatCheckFlag(t *testing.T) { withTestContext(t, func(ctx *testContext) { - _, err := calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, - ctx.newCommand(gccX86_64, "-fstack-check", mainCc)) - if err == nil || err.Error() != `option "-fstack-check" is not supported; crbug/485492` { - t.Errorf("Expected error not found. Got: %s", err) + stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg, + ctx.newCommand(gccX86_64, "-fstack-check", mainCc))) + if err := verifyNonInternalError(stderr, + `option "-fstack-check" is not supported; crbug/485492`); err != nil { + t.Fatal(err) } }) } diff --git a/compiler_wrapper/x64_flags_test.go b/compiler_wrapper/x64_flags_test.go index 90bad4e7..125d2e7b 100644 --- a/compiler_wrapper/x64_flags_test.go +++ b/compiler_wrapper/x64_flags_test.go @@ -6,7 +6,7 @@ import ( func TestAddNoMovbeFlagOnX86(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccX86_64, mainCc))) if err := verifyArgOrder(cmd, mainCc, "-mno-movbe"); err != nil { t.Error(err) @@ -16,7 +16,7 @@ func TestAddNoMovbeFlagOnX86(t *testing.T) { func TestAddNoMovbeFlagOnI686(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand("./i686-cros-linux-gnu-gcc", mainCc))) if err := verifyArgOrder(cmd, mainCc, "-mno-movbe"); err != nil { t.Error(err) @@ -26,7 +26,7 @@ func TestAddNoMovbeFlagOnI686(t *testing.T) { func TestDoNotAddNoMovbeFlagOnArm(t *testing.T) { withTestContext(t, func(ctx *testContext) { - cmd := ctx.must(calcCompilerCommandAndCompareToOld(ctx, ctx.cfg, + cmd := ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(gccArmV7, mainCc))) if err := verifyArgCount(cmd, 0, "-mno-movbe"); err != nil { t.Error(err) |