Support second execution of the compiler with -Wno-error

BUG=chromium:773875
TEST=unit test

Change-Id: Ib77fd7c166a13acb733a1dbdfd88129141c4227a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1692969
Reviewed-by: Tobias Bosch <tbosch@google.com>
Tested-by: Tobias Bosch <tbosch@google.com>

9 files changed, 488 insertions, 123 deletions

diff --git a/compiler_wrapper/clang_tidy_flag_test.go b/compiler_wrapper/clang_tidy_flag_test.go
index 5fc29318..1cfd40ba 100644
--- a/compiler_wrapper/clang_tidy_flag_test.go
+++ b/compiler_wrapper/clang_tidy_flag_test.go
@@ -9,7 +9,7 @@ import (
 )
 
 func TestClangTidyBasename(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		testData := []struct {
 			in  string
 			out string
@@ -42,7 +42,7 @@ func TestClangTidyBasename(t *testing.T) {
 }
 
 func TestClangTidyClangResourceDir(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
 			switch ctx.cmdCount {
 			case 1:
@@ -81,7 +81,7 @@ func TestClangTidyClangResourceDir(t *testing.T) {
 }
 
 func TestClangTidyArgOrder(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
 			if ctx.cmdCount == 2 {
 				if err := verifyArgOrder(cmd, "-checks=.*", mainCc, "--", "-resource-dir=.*", mainCc, "--some_arg"); err != nil {
@@ -99,7 +99,7 @@ func TestClangTidyArgOrder(t *testing.T) {
 }
 
 func TestForwardStdOutAndStderrFromClangTidyCall(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
 			if ctx.cmdCount == 2 {
 				fmt.Fprintf(stdout, "somemessage")
@@ -119,7 +119,7 @@ func TestForwardStdOutAndStderrFromClangTidyCall(t *testing.T) {
 }
 
 func TestIgnoreNonZeroExitCodeFromClangTidy(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
 			if ctx.cmdCount == 2 {
 				return newExitCodeError(23)
@@ -136,7 +136,7 @@ func TestIgnoreNonZeroExitCodeFromClangTidy(t *testing.T) {
 }
 
 func TestReportGeneralErrorsFromClangTidy(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
 			if ctx.cmdCount == 2 {
 				return errors.New("someerror")
@@ -155,7 +155,7 @@ func TestReportGeneralErrorsFromClangTidy(t *testing.T) {
 }
 
 func TestOmitClangTidyForGcc(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		ctx.must(callCompiler(ctx, ctx.cfg,
 			ctx.newCommand(gccX86_64, mainCc)))
 		if ctx.cmdCount > 1 {
@@ -165,7 +165,7 @@ func TestOmitClangTidyForGcc(t *testing.T) {
 }
 
 func TestOmitClangTidyForGccWithClangSyntax(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		ctx.must(callCompiler(ctx, ctx.cfg,
 			ctx.newCommand(gccX86_64, "-clang-syntax", mainCc)))
 		if ctx.cmdCount > 2 {
@@ -175,7 +175,7 @@ func TestOmitClangTidyForGccWithClangSyntax(t *testing.T) {
 }
 
 func TestUseClangTidyBasedOnFileExtension(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		testData := []struct {
 			args      []string
 			clangTidy bool
@@ -204,7 +204,7 @@ func TestUseClangTidyBasedOnFileExtension(t *testing.T) {
 }
 
 func TestOmitCCacheWithClangTidy(t *testing.T) {
-	withClangSyntaxTestContext(t, func(ctx *testContext) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
 		ctx.cfg.useCCache = true
 
 		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
@@ -234,7 +234,7 @@ func TestOmitCCacheWithClangTidy(t *testing.T) {
 	})
 }
 
-func withClangSyntaxTestContext(t *testing.T, work func(ctx *testContext)) {
+func withClangTidyTestContext(t *testing.T, work func(ctx *testContext)) {
 	withTestContext(t, func(ctx *testContext) {
 		ctx.env = []string{"WITH_TIDY=1"}
 		work(ctx)
diff --git a/compiler_wrapper/compiler_wrapper.go b/compiler_wrapper/compiler_wrapper.go
index 1eacb578..3f94e185 100644
--- a/compiler_wrapper/compiler_wrapper.go
+++ b/compiler_wrapper/compiler_wrapper.go
@@ -18,7 +18,7 @@ func callCompiler(env env, cfg *config, inputCmd *command) int {
 	} else if cfg.oldWrapperPath != "" {
 		exitCode, compilerErr = callCompilerWithRunAndCompareToOldWrapper(env, cfg, inputCmd)
 	} else {
-		exitCode, compilerErr = callCompilerWithExec(env, cfg, inputCmd)
+		exitCode, compilerErr = callCompilerInternal(env, cfg, inputCmd)
 	}
 	if compilerErr != nil {
 		printCompilerError(env.stderr(), compilerErr)
@@ -31,62 +31,35 @@ func callCompilerWithRunAndCompareToOldWrapper(env env, cfg *config, inputCmd *c
 	recordingEnv := &commandRecordingEnv{
 		env: env,
 	}
-	compilerCmd, exitCode, err := calcCompilerCommand(recordingEnv, cfg, inputCmd)
-	if err != nil || exitCode != 0 {
-		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 %#v", compilerCmd)
-		}
+	// Note: this won't do a real exec as recordingEnv redirects exec to run.
+	if exitCode, err = callCompilerInternal(recordingEnv, cfg, inputCmd); err != nil {
+		return 0, err
 	}
-	if err := compareToOldWrapper(env, cfg, inputCmd, recordingEnv.cmdResults); err != nil {
+	if err = compareToOldWrapper(env, cfg, inputCmd, recordingEnv.cmdResults, exitCode); err != nil {
 		return exitCode, err
 	}
 	return exitCode, nil
 }
 
-func callCompilerWithExec(env env, cfg *config, inputCmd *command) (exitCode int, err error) {
-	compilerCmd, exitCode, err := calcCompilerCommand(env, cfg, inputCmd)
-	if err != nil || exitCode != 0 {
-		return exitCode, 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 exitCode, userErr
-		}
-		return exitCode, wrapErrorwithSourceLocf(err, "failed to execute %#v", compilerCmd)
-	}
-	return exitCode, nil
-}
-
-func calcCompilerCommand(env env, cfg *config, inputCmd *command) (compilerCmd *command, exitCode int, err error) {
+func callCompilerInternal(env env, cfg *config, inputCmd *command) (exitCode int, err error) {
 	if err := checkUnsupportedFlags(inputCmd); err != nil {
-		return nil, exitCode, err
+		return 0, err
 	}
 	mainBuilder, err := newCommandBuilder(env, cfg, inputCmd)
 	if err != nil {
-		return nil, exitCode, err
+		return 0, err
 	}
+	var compilerCmd *command
 	clangSyntax := processClangSyntaxFlag(mainBuilder)
 	if mainBuilder.target.compilerType == clangType {
 		cSrcFile, useClangTidy := processClangTidyFlags(mainBuilder)
 		compilerCmd, err = calcClangCommand(useClangTidy, mainBuilder)
 		if err != nil {
-			return nil, exitCode, err
+			return 0, err
 		}
 		if useClangTidy {
 			if err := runClangTidy(env, compilerCmd, cSrcFile); err != nil {
-				return nil, exitCode, err
+				return 0, err
 			}
 		}
 	} else {
@@ -94,17 +67,30 @@ func calcCompilerCommand(env env, cfg *config, inputCmd *command) (compilerCmd *
 			forceLocal := false
 			clangCmd, err := calcClangCommand(forceLocal, mainBuilder.clone())
 			if err != nil {
-				return nil, 0, err
+				return 0, err
 			}
 			exitCode, err = checkClangSyntax(env, clangCmd)
 			if err != nil || exitCode != 0 {
-				return nil, exitCode, err
+				return exitCode, err
 			}
 		}
 		compilerCmd = calcGccCommand(mainBuilder)
 	}
-
-	return compilerCmd, exitCode, nil
+	if shouldForceDisableWError(env) {
+		return doubleBuildWithWNoError(env, cfg, compilerCmd)
+	}
+	if err := env.exec(compilerCmd); err != nil {
+		if userErr, ok := getCCacheError(compilerCmd, err); ok {
+			return 0, userErr
+		}
+		// Note: This case only happens when the underlying env is not
+		// really doing an exec, e.g. commandRecordingEnv.
+		if exitCode, ok := getExitCode(err); ok {
+			return exitCode, nil
+		}
+		return exitCode, wrapErrorwithSourceLocf(err, "failed to execute %#v", compilerCmd)
+	}
+	return 0, err
 }
 
 func calcClangCommand(forceLocal bool, builder *commandBuilder) (*command, error) {
diff --git a/compiler_wrapper/config.go b/compiler_wrapper/config.go
index 34bfc122..5e594cc3 100644
--- a/compiler_wrapper/config.go
+++ b/compiler_wrapper/config.go
@@ -21,6 +21,8 @@ type config struct {
 	mockOldWrapperCmds bool
 	// Whether to overwrite the config in the old wrapper.
 	overwriteOldWrapperCfg bool
+	// Directory to store errors that were prevented with -Wno-error.
+	newWarningsDir string
 }
 
 // UseCCache can be set via a linker flag.
@@ -90,6 +92,7 @@ func getCrosHardenedConfig(useCCache bool) *config {
 			"-fno-addrsig",
 			"-Wno-tautological-constant-compare",
 		},
+		newWarningsDir: "/tmp/fatal_clang_warnings",
 	}
 }
 
@@ -116,5 +119,6 @@ func getCrosNonHardenedConfig(useCCache bool) *config {
 			"-Wno-tautological-unsigned-enum-zero-compare",
 			"-Wno-tautological-constant-compare",
 		},
+		newWarningsDir: "/tmp/fatal_clang_warnings",
 	}
 }
diff --git a/compiler_wrapper/disable_werror_flag.go b/compiler_wrapper/disable_werror_flag.go
new file mode 100644
index 00000000..3115e692
--- /dev/null
+++ b/compiler_wrapper/disable_werror_flag.go
@@ -0,0 +1,113 @@
+package main
+
+import (
+	"bytes"
+	"encoding/json"
+	"io/ioutil"
+	"os"
+	"strings"
+)
+
+func shouldForceDisableWError(env env) bool {
+	return env.getenv("FORCE_DISABLE_WERROR") != ""
+}
+
+func doubleBuildWithWNoError(env env, cfg *config, originalCmd *command) (exitCode int, err error) {
+	originalStdoutBuffer := &bytes.Buffer{}
+	originalStderrBuffer := &bytes.Buffer{}
+	err = env.run(originalCmd, originalStdoutBuffer, originalStderrBuffer)
+	originalExitCode, ok := getExitCode(err)
+	if !ok {
+		return 0, wrapErrorwithSourceLocf(err, "failed to execute %#v", originalCmd)
+	}
+	// The only way we can do anything useful is if it looks like the failure
+	// was -Werror-related.
+	if originalExitCode == 0 || !strings.Contains(originalStderrBuffer.String(), "-Werror") {
+		originalStdoutBuffer.WriteTo(env.stdout())
+		originalStderrBuffer.WriteTo(env.stderr())
+		return originalExitCode, nil
+	}
+
+	retryStdoutBuffer := &bytes.Buffer{}
+	retryStderrBuffer := &bytes.Buffer{}
+	retryCommand := &command{
+		path:       originalCmd.path,
+		args:       append(originalCmd.args, "-Wno-error"),
+		envUpdates: originalCmd.envUpdates,
+	}
+	err = env.run(retryCommand, retryStdoutBuffer, retryStderrBuffer)
+	retryExitCode, ok := getExitCode(err)
+	if !ok {
+		return 0, wrapErrorwithSourceLocf(err, "failed to execute %#v", retryCommand)
+	}
+	// If -Wno-error fixed us, pretend that we never ran without -Wno-error.
+	// Otherwise, pretend that we never ran the second invocation. Since -Werror
+	// is an issue, log in either case.
+	if retryExitCode == 0 {
+		retryStdoutBuffer.WriteTo(env.stdout())
+		retryStderrBuffer.WriteTo(env.stderr())
+	} else {
+		originalStdoutBuffer.WriteTo(env.stdout())
+		originalStderrBuffer.WriteTo(env.stderr())
+	}
+
+	// All of the below is basically logging. If we fail at any point, it's
+	// reasonable for that to fail the build. This is all meant for FYI-like
+	// builders in the first place.
+
+	// Allow root and regular users to write to this without issue.
+	if err := os.MkdirAll(cfg.newWarningsDir, 0777); err != nil {
+		return 0, wrapErrorwithSourceLocf(err, "error creating warnings directory %s", cfg.newWarningsDir)
+	}
+
+	// Have some tag to show that files aren't fully written. It would be sad if
+	// an interrupted build (or out of disk space, or similar) caused tools to
+	// have to be overly-defensive.
+	incompleteSuffix := ".incomplete"
+
+	// Coming up with a consistent name for this is difficult (compiler command's
+	// SHA can clash in the case of identically named files in different
+	// directories, or similar); let's use a random one.
+	tmpFile, err := ioutil.TempFile(cfg.newWarningsDir, "warnings_report*.json"+incompleteSuffix)
+	if err != nil {
+		return 0, wrapErrorwithSourceLocf(err, "error creating warnings file")
+	}
+
+	lines := []string{}
+	if originalStderrBuffer.Len() > 0 {
+		lines = append(lines, originalStderrBuffer.String())
+	}
+	if originalStdoutBuffer.Len() > 0 {
+		lines = append(lines, originalStdoutBuffer.String())
+	}
+	outputToLog := strings.Join(lines, "\n")
+
+	jsonData := warningsJSONData{
+		Cwd:     env.getwd(),
+		Command: append([]string{originalCmd.path}, originalCmd.args...),
+		Stdout:  outputToLog,
+	}
+	enc := json.NewEncoder(tmpFile)
+	if err := enc.Encode(jsonData); err != nil {
+		_ = tmpFile.Close()
+		return 0, wrapErrorwithSourceLocf(err, "error writing warnings data")
+	}
+
+	if err := tmpFile.Close(); err != nil {
+		return 0, wrapErrorwithSourceLocf(err, "error closing warnings file")
+	}
+
+	if err := os.Rename(tmpFile.Name(), tmpFile.Name()[:len(tmpFile.Name())-len(incompleteSuffix)]); err != nil {
+		return 0, wrapErrorwithSourceLocf(err, "error removing incomplete suffix from warnings file")
+	}
+
+	return retryExitCode, nil
+}
+
+// Struct used to write JSON. Fileds have to be uppercase for the json
+// encoder to read them.
+type warningsJSONData struct {
+	Cwd     string   `json:"cwd"`
+	Command []string `json:"command"`
+	Stdout  string   `json:"stdout"`
+}
diff --git a/compiler_wrapper/disable_werror_flag_test.go b/compiler_wrapper/disable_werror_flag_test.go
new file mode 100644
index 00000000..d9f1758b
--- /dev/null
+++ b/compiler_wrapper/disable_werror_flag_test.go
@@ -0,0 +1,252 @@
+package main
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestOmitDoubleBuildForSuccessfulCall(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc)))
+		if ctx.cmdCount != 1 {
+			t.Errorf("expected 1 call. Got: %d", ctx.cmdCount)
+		}
+	})
+}
+
+func TestOmitDoubleBuildForGeneralError(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		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(clangX86_64, mainCc)))
+		if err := verifyInternalError(stderr); err != nil {
+			t.Fatal(err)
+		}
+		if !strings.Contains(stderr, "someerror") {
+			t.Errorf("unexpected error. Got: %s", stderr)
+		}
+		if ctx.cmdCount != 1 {
+			t.Errorf("expected 1 call. Got: %d", ctx.cmdCount)
+		}
+	})
+}
+
+func TestDoubleBuildWithWNoErrorFlag(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
+			switch ctx.cmdCount {
+			case 1:
+				if err := verifyArgCount(cmd, 0, "-Wno-error"); err != nil {
+					return err
+				}
+				fmt.Fprint(stderr, "-Werror originalerror")
+				return newExitCodeError(1)
+			case 2:
+				if err := verifyArgCount(cmd, 1, "-Wno-error"); err != nil {
+					return err
+				}
+				return nil
+			default:
+				t.Fatalf("unexpected command: %#v", cmd)
+				return nil
+			}
+		}
+		ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc)))
+		if ctx.cmdCount != 2 {
+			t.Errorf("expected 2 calls. Got: %d", ctx.cmdCount)
+		}
+	})
+}
+
+func TestForwardStdoutAndStderrWhenDoubleBuildSucceeds(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
+			switch ctx.cmdCount {
+			case 1:
+				fmt.Fprint(stdout, "originalmessage")
+				fmt.Fprint(stderr, "-Werror originalerror")
+				return newExitCodeError(1)
+			case 2:
+				fmt.Fprint(stdout, "retrymessage")
+				fmt.Fprint(stderr, "retryerror")
+				return nil
+			default:
+				t.Fatalf("unexpected command: %#v", cmd)
+				return nil
+			}
+		}
+		ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc)))
+		if err := verifyNonInternalError(ctx.stderrString(), "retryerror"); err != nil {
+			t.Error(err)
+		}
+		if !strings.Contains(ctx.stdoutString(), "retrymessage") {
+			t.Errorf("unexpected stdout. Got: %s", ctx.stdoutString())
+		}
+	})
+}
+
+func TestForwardStdoutAndStderrWhenDoubleBuildFails(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
+			switch ctx.cmdCount {
+			case 1:
+				fmt.Fprint(stdout, "originalmessage")
+				fmt.Fprint(stderr, "-Werror originalerror")
+				return newExitCodeError(3)
+			case 2:
+				fmt.Fprint(stdout, "retrymessage")
+				fmt.Fprint(stderr, "retryerror")
+				return newExitCodeError(5)
+			default:
+				t.Fatalf("unexpected command: %#v", cmd)
+				return nil
+			}
+		}
+		exitCode := callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc))
+		if exitCode != 5 {
+			t.Errorf("unexpected exitcode. Got: %d", exitCode)
+		}
+		if err := verifyNonInternalError(ctx.stderrString(), "-Werror originalerror"); err != nil {
+			t.Error(err)
+		}
+		if !strings.Contains(ctx.stdoutString(), "originalmessage") {
+			t.Errorf("unexpected stdout. Got: %s", ctx.stdoutString())
+		}
+	})
+}
+
+func TestForwardGeneralErrorWhenDoubleBuildFails(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
+			switch ctx.cmdCount {
+			case 1:
+				fmt.Fprint(stderr, "-Werror originalerror")
+				return newExitCodeError(3)
+			case 2:
+				return errors.New("someerror")
+			default:
+				t.Fatalf("unexpected command: %#v", cmd)
+				return nil
+			}
+		}
+		stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc)))
+		if err := verifyInternalError(stderr); err != nil {
+			t.Error(err)
+		}
+		if !strings.Contains(stderr, "someerror") {
+			t.Errorf("unexpected stderr. Got: %s", stderr)
+		}
+	})
+}
+
+func TestOmitLogWarningsIfNoDoubleBuild(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc)))
+		if ctx.cmdCount != 1 {
+			t.Errorf("expected 1 call. Got: %d", ctx.cmdCount)
+		}
+		if loggedWarnings := readLoggedWarnings(ctx); loggedWarnings != nil {
+			t.Errorf("expected no logged warnings. Got: %#v", loggedWarnings)
+		}
+	})
+}
+
+func TestLogWarningsWhenDoubleBuildSucceeds(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
+			switch ctx.cmdCount {
+			case 1:
+				fmt.Fprint(stdout, "originalmessage")
+				fmt.Fprint(stderr, "-Werror originalerror")
+				return newExitCodeError(1)
+			case 2:
+				fmt.Fprint(stdout, "retrymessage")
+				fmt.Fprint(stderr, "retryerror")
+				return nil
+			default:
+				t.Fatalf("unexpected command: %#v", cmd)
+				return nil
+			}
+		}
+		ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc)))
+		loggedWarnings := readLoggedWarnings(ctx)
+		if loggedWarnings == nil {
+			t.Fatal("expected logged warnings")
+		}
+		if loggedWarnings.Cwd != ctx.getwd() {
+			t.Fatalf("unexpected cwd. Got: %s", loggedWarnings.Cwd)
+		}
+		loggedCmd := &command{
+			path: loggedWarnings.Command[0],
+			args: loggedWarnings.Command[1:],
+		}
+		if err := verifyPath(loggedCmd, "usr/bin/clang"); err != nil {
+			t.Error(err)
+		}
+		if err := verifyArgOrder(loggedCmd, "--sysroot=.*", mainCc); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
+func TestLogWarningsWhenDoubleBuildFails(t *testing.T) {
+	withForceDisableWErrorTestContext(t, func(ctx *testContext) {
+		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
+			switch ctx.cmdCount {
+			case 1:
+				fmt.Fprint(stdout, "originalmessage")
+				fmt.Fprint(stderr, "-Werror originalerror")
+				return newExitCodeError(1)
+			case 2:
+				fmt.Fprint(stdout, "retrymessage")
+				fmt.Fprint(stderr, "retryerror")
+				return newExitCodeError(1)
+			default:
+				t.Fatalf("unexpected command: %#v", cmd)
+				return nil
+			}
+		}
+		ctx.mustFail(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangX86_64, mainCc)))
+		loggedWarnings := readLoggedWarnings(ctx)
+		if loggedWarnings == nil {
+			t.Fatal("expected logged warnings")
+		}
+	})
+}
+
+func withForceDisableWErrorTestContext(t *testing.T, work func(ctx *testContext)) {
+	withTestContext(t, func(ctx *testContext) {
+		ctx.env = []string{"FORCE_DISABLE_WERROR=1"}
+		work(ctx)
+	})
+}
+
+func readLoggedWarnings(ctx *testContext) *warningsJSONData {
+	files, err := ioutil.ReadDir(ctx.cfg.newWarningsDir)
+	if err != nil {
+		if _, ok := err.(*os.PathError); ok {
+			return nil
+		}
+		ctx.t.Fatal(err)
+	}
+	if len(files) != 1 {
+		ctx.t.Fatalf("expected 1 warning log file. Got: %s", files)
+	}
+	data, err := ioutil.ReadFile(filepath.Join(ctx.cfg.newWarningsDir, files[0].Name()))
+	if err != nil {
+		ctx.t.Fatal(err)
+	}
+	jsonData := warningsJSONData{}
+	if err := json.Unmarshal(data, &jsonData); err != nil {
+		ctx.t.Fatal(err)
+	}
+	return &jsonData
+}
diff --git a/compiler_wrapper/env.go b/compiler_wrapper/env.go
index 6cf1a1b9..871648cd 100644
--- a/compiler_wrapper/env.go
+++ b/compiler_wrapper/env.go
@@ -77,10 +77,9 @@ type commandResult struct {
 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)
+	// Note: We treat exec the same as run so that we can do work
+	// after the call.
+	return env.run(cmd, env.stdout(), env.stderr())
 }
 
 func (env *commandRecordingEnv) run(cmd *command, stdout io.Writer, stderr io.Writer) error {
diff --git a/compiler_wrapper/oldwrapper.go b/compiler_wrapper/oldwrapper.go
index 9751547f..56559a31 100644
--- a/compiler_wrapper/oldwrapper.go
+++ b/compiler_wrapper/oldwrapper.go
@@ -21,8 +21,6 @@ const compareToOldWrapperFilePattern = "old_wrapper_compare"
 // support it yet.
 func shouldForwardToOldWrapper(env env, inputCmd *command) bool {
 	switch {
-	case env.getenv("FORCE_DISABLE_WERROR") != "":
-		fallthrough
 	case env.getenv("GETRUSAGE") != "":
 		fallthrough
 	case env.getenv("BISECT_STAGE") != "":
@@ -39,36 +37,49 @@ func forwardToOldWrapper(env env, cfg *config, inputCmd *command) (exitCode int,
 	return callOldWrapper(env, oldWrapperCfg, inputCmd, forwardToOldWrapperFilePattern, env.stdout(), env.stderr())
 }
 
-func compareToOldWrapper(env env, cfg *config, inputCmd *command, newCmdResults []*commandResult) error {
+func compareToOldWrapper(env env, cfg *config, inputCmd *command, newCmdResults []*commandResult, newExitCode int) error {
 	oldWrapperCfg, err := newOldWrapperConfig(env, cfg, inputCmd)
 	if err != nil {
 		return err
 	}
 	oldWrapperCfg.LogCmds = true
 	oldWrapperCfg.MockCmds = cfg.mockOldWrapperCmds
+	newCmds := []*command{}
 	for _, cmdResult := range newCmdResults {
 		oldWrapperCfg.CmdResults = append(oldWrapperCfg.CmdResults, oldWrapperCmdResult{
 			Stdout:   cmdResult.stdout,
 			Stderr:   cmdResult.stderr,
 			Exitcode: cmdResult.exitCode,
 		})
+		newCmds = append(newCmds, cmdResult.cmd)
 	}
 	oldWrapperCfg.OverwriteConfig = cfg.overwriteOldWrapperCfg
 
-	stdoutBuffer := bytes.Buffer{}
 	stderrBuffer := bytes.Buffer{}
-	exitCode, err := callOldWrapper(env, oldWrapperCfg, inputCmd, compareToOldWrapperFilePattern, &stdoutBuffer, &stderrBuffer)
+	oldExitCode, err := callOldWrapper(env, oldWrapperCfg, inputCmd, compareToOldWrapperFilePattern, &bytes.Buffer{}, &stderrBuffer)
 	if err != nil {
 		return err
 	}
-	oldCmdResults := parseOldWrapperCommands(stdoutBuffer.String(), stderrBuffer.String(), exitCode)
-	return diffCommandResults(oldCmdResults, newCmdResults)
+	differences := []string{}
+	if oldExitCode != newExitCode {
+		differences = append(differences, fmt.Sprintf("exit codes differ: old %d, new %d", oldExitCode, newExitCode))
+	}
+	oldCmds, stderr := parseOldWrapperCommands(stderrBuffer.String())
+	if cmdDifferences := diffCommands(oldCmds, newCmds); cmdDifferences != "" {
+		differences = append(differences, cmdDifferences)
+	}
+	if len(differences) > 0 {
+		return newErrorwithSourceLocf("wrappers differ:\n%s\nOld stderr:%s",
+			strings.Join(differences, "\n"),
+			stderr,
+		)
+	}
+	return nil
 }
 
-func parseOldWrapperCommands(stdout string, stderr string, exitCode int) []*commandResult {
+func parseOldWrapperCommands(stderr string) (cmds []*command, remainingStderr string) {
 	allStderrLines := strings.Split(stderr, "\n")
 	remainingStderrLines := []string{}
-	cmdResults := []*commandResult{}
 	for _, line := range allStderrLines {
 		const commandPrefix = "command:"
 		const envupdatePrefix = ".EnvUpdate:"
@@ -85,63 +96,50 @@ func parseOldWrapperCommands(stdout string, stderr string, exitCode int) []*comm
 				// simpler.
 				envUpdate = nil
 			}
-			command := &command{
+			cmd := &command{
 				path:       args[0],
 				args:       args[1:],
 				envUpdates: envUpdate,
 			}
-			cmdResults = append(cmdResults, &commandResult{cmd: command})
+			cmds = append(cmds, cmd)
 		} else {
 			remainingStderrLines = append(remainingStderrLines, line)
 		}
 	}
-	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)
-	}
-
-	return cmdResults
+	remainingStderr = strings.TrimSpace(strings.Join(remainingStderrLines, "\n"))
+	return cmds, remainingStderr
 }
 
-func diffCommandResults(oldCmdResults []*commandResult, newCmdResults []*commandResult) error {
-	maxLen := len(newCmdResults)
-	if maxLen < len(oldCmdResults) {
-		maxLen = len(oldCmdResults)
+func diffCommands(oldCmds []*command, newCmds []*command) string {
+	maxLen := len(newCmds)
+	if maxLen < len(oldCmds) {
+		maxLen = len(oldCmds)
 	}
 	hasDifferences := false
 	var cmdDifferences []string
 	for i := 0; i < maxLen; i++ {
 		var differences []string
-		if i >= len(newCmdResults) {
+		if i >= len(newCmds) {
 			differences = append(differences, "missing command")
-		} else if i >= len(oldCmdResults) {
+		} else if i >= len(oldCmds) {
 			differences = append(differences, "extra command")
 		} else {
-			newCmdResult := newCmdResults[i]
-			oldCmdResult := oldCmdResults[i]
+			newCmd := newCmds[i]
+			oldCmd := oldCmds[i]
 
-			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 {
+			if newCmd.path != oldCmd.path {
 				differences = append(differences, "path")
 			}
 
-			if !reflect.DeepEqual(newCmdResult.cmd.args, oldCmdResult.cmd.args) {
+			if !reflect.DeepEqual(newCmd.args, oldCmd.args) {
 				differences = append(differences, "args")
 			}
 
 			// Sort the environment as we don't care in which order
 			// it was modified.
-			newEnvUpdates := newCmdResult.cmd.envUpdates
+			newEnvUpdates := newCmd.envUpdates
 			sort.Strings(newEnvUpdates)
-			oldEnvUpdates := oldCmdResult.cmd.envUpdates
+			oldEnvUpdates := oldCmd.envUpdates
 			sort.Strings(oldEnvUpdates)
 
 			if !reflect.DeepEqual(newEnvUpdates, oldEnvUpdates) {
@@ -157,19 +155,18 @@ func diffCommandResults(oldCmdResults []*commandResult, newCmdResults []*command
 			fmt.Sprintf("Index %d: %s", i, strings.Join(differences, ",")))
 	}
 	if hasDifferences {
-		return newErrorwithSourceLocf("commands differ:\n%s\nOld commands:\n%s\nNew commands:\n%s",
+		return fmt.Sprintf("commands differ:\n%s\nOld:%#v\nNew:%#v",
 			strings.Join(cmdDifferences, "\n"),
-			dumpCommandResults(oldCmdResults),
-			dumpCommandResults(newCmdResults),
-		)
+			dumpCommands(oldCmds),
+			dumpCommands(newCmds))
 	}
-	return nil
+	return ""
 }
 
-func dumpCommandResults(results []*commandResult) string {
+func dumpCommands(cmds []*command) string {
 	lines := []string{}
-	for _, result := range results {
-		lines = append(lines, fmt.Sprintf("cmd: %#v; result: %#v", result.cmd, result))
+	for _, cmd := range cmds {
+		lines = append(lines, fmt.Sprintf("%#v", cmd))
 	}
 	return strings.Join(lines, "\n")
 }
@@ -266,19 +263,22 @@ def check_output_mock(args):
 old_check_output = subprocess.check_output
 subprocess.check_output = check_output_mock
 
-def popen_mock(args):
+def popen_mock(args, stdout=None, stderr=None):
 	serialize_cmd(args)
 	{{if .MockCmds}}
 	result = mockResults.pop(0)
-	print(result['stdout'], file=sys.stdout)
-	print(result['stderr'], file=sys.stderr)
+	if stdout is None:
+		print(result['stdout'], file=sys.stdout)
+	if stderr is None:
+		print(result['stderr'], file=sys.stderr)
 
 	class MockResult:
 		def __init__(self, returncode):
 			self.returncode = returncode
-
 		def wait(self):
-			return None
+			return self.returncode
+		def communicate(self):
+			return (result['stdout'], result['stderr'])
 
 	return MockResult(result['exitcode'])
 	{{else}}
diff --git a/compiler_wrapper/oldwrapper_test.go b/compiler_wrapper/oldwrapper_test.go
index adfa3bde..7c623d6d 100644
--- a/compiler_wrapper/oldwrapper_test.go
+++ b/compiler_wrapper/oldwrapper_test.go
@@ -34,7 +34,6 @@ func TestNoForwardToOldWrapperBecauseOfEnv(t *testing.T) {
 func TestForwardToOldWrapperBecauseOfEnv(t *testing.T) {
 	withForwardToOldWrapperTestContext(t, func(ctx *testContext) {
 		testEnvs := []string{
-			"FORCE_DISABLE_WERROR=abc",
 			"GETRUSAGE=abc",
 			"BISECT_STAGE=abc",
 		}
@@ -122,6 +121,15 @@ func TestCompareToOldWrapperCompilerCommand(t *testing.T) {
 		pathSuffix := ""
 		extraArgs := []string{}
 		exitCode := 0
+		newWrapperExitCode := 0
+
+		reset := func() {
+			ctx.stderrBuffer.Reset()
+			pathSuffix = ""
+			extraArgs = []string{}
+			exitCode = 0
+			newWrapperExitCode = 0
+		}
 
 		ctx.cmdMock = func(cmd *command, stdout io.Writer, stderr io.Writer) error {
 			writeMockWrapper(ctx, &mockWrapperConfig{
@@ -133,42 +141,44 @@ func TestCompareToOldWrapperCompilerCommand(t *testing.T) {
 					},
 				},
 			})
+			if newWrapperExitCode != 0 {
+				return newExitCodeError(newWrapperExitCode)
+			}
 			return nil
 		}
 
 		// Note: This will cause only the compiler command.
 		inputCmd := ctx.newCommand(gccX86_64)
 
-		ctx.stderrBuffer.Reset()
+		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 = ""
+		reset()
 		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
+		reset()
 		exitCode = 1
 		stderr = ctx.mustFail(callCompiler(ctx, ctx.cfg, inputCmd))
-		if !strings.Contains(stderr, "Index 0: exit code") {
+		if !strings.Contains(stderr, "exit codes differ: old 1, new 0") {
 			t.Errorf("expected exit code difference error. Got: %s", stderr)
 		}
 
-		pathSuffix = ""
-		extraArgs = nil
-		exitCode = 0
+		reset()
+		newWrapperExitCode = 1
+		stderr = ctx.mustFail(callCompiler(ctx, ctx.cfg, inputCmd))
+		if !strings.Contains(stderr, "exit codes differ: old 0, new 1") {
+			t.Errorf("expected exit code difference error. Got: %s", stderr)
+		}
+
+		reset()
 		ctx.must(callCompiler(ctx, ctx.cfg, inputCmd))
 	})
 }
diff --git a/compiler_wrapper/testutil_test.go b/compiler_wrapper/testutil_test.go
index fe9ab99c..06ec934d 100644
--- a/compiler_wrapper/testutil_test.go
+++ b/compiler_wrapper/testutil_test.go
@@ -126,7 +126,7 @@ func (ctx *testContext) exec(cmd *command) error {
 
 func (ctx *testContext) must(exitCode int) *command {
 	if exitCode != 0 {
-		ctx.t.Fatalf("expected no error, but got %d. Stderr: %s",
+		ctx.t.Fatalf("expected no error, but got exit code %d. Stderr: %s",
 			exitCode, ctx.stderrString())
 	}
 	return ctx.lastCmd
@@ -143,6 +143,7 @@ func (ctx *testContext) updateConfig(wrapperChrootPath string, cfg *config) {
 	*ctx.cfg = *cfg
 	ctx.cfg.overwriteOldWrapperCfg = true
 	ctx.cfg.mockOldWrapperCmds = true
+	ctx.cfg.newWarningsDir = filepath.Join(ctx.tempDir, "fatal_clang_warnings")
 	if *crosRootDirFlag != "" {
 		ctx.cfg.oldWrapperPath = filepath.Join(*crosRootDirFlag, wrapperChrootPath)
 	} else {