Only tee stdin if a "-" was passed the compiler.

This allows to remove the workaround in env.Run,
and fixes the bug in the workaround that it didn't
return exit code errors correctly.

BUG=chromium:773875
TEST=manually tested in the chroot.

Change-Id: I486b30b65ba3ad6249aa89e82e292c66378187a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1762345
Tested-by: Tobias Bosch <tbosch@google.com>
Reviewed-by: George Burgess <gbiv@chromium.org>

4 files changed, 17 insertions, 23 deletions

diff --git a/compiler_wrapper/clang_syntax_flag.go b/compiler_wrapper/clang_syntax_flag.go
index 8f38c613..90d2327a 100644
--- a/compiler_wrapper/clang_syntax_flag.go
+++ b/compiler_wrapper/clang_syntax_flag.go
@@ -6,7 +6,6 @@ package main
 
 import (
 	"bytes"
-	"io"
 )
 
 func processClangSyntaxFlag(builder *commandBuilder) (clangSyntax bool) {
@@ -29,7 +28,7 @@ func checkClangSyntax(env env, clangCmd *command, gccCmd *command) (exitCode int
 
 	stdinBuffer := &bytes.Buffer{}
 	exitCode, err = wrapSubprocessErrorWithSourceLoc(clangSyntaxCmd,
-		env.run(clangSyntaxCmd, io.TeeReader(env.stdin(), stdinBuffer), env.stdout(), env.stderr()))
+		env.run(clangSyntaxCmd, teeStdinIfNeeded(env, clangCmd, stdinBuffer), env.stdout(), env.stderr()))
 	if err != nil || exitCode != 0 {
 		return exitCode, err
 	}
diff --git a/compiler_wrapper/compiler_wrapper.go b/compiler_wrapper/compiler_wrapper.go
index ad66f990..bbb7434c 100644
--- a/compiler_wrapper/compiler_wrapper.go
+++ b/compiler_wrapper/compiler_wrapper.go
@@ -44,7 +44,7 @@ func callCompilerWithRunAndCompareToOldWrapper(env env, cfg *config, inputCmd *c
 	stdinBuffer := &bytes.Buffer{}
 	recordingEnv := &commandRecordingEnv{
 		env:         env,
-		stdinReader: io.TeeReader(env.stdin(), stdinBuffer),
+		stdinReader: teeStdinIfNeeded(env, inputCmd, stdinBuffer),
 	}
 	// Note: this won't do a real exec as recordingEnv redirects exec to run.
 	if exitCode, err = callCompilerInternal(recordingEnv, cfg, inputCmd); err != nil {
@@ -200,3 +200,17 @@ func printCompilerError(writer io.Writer, compilerErr error) {
 			compilerErr)
 	}
 }
+
+func teeStdinIfNeeded(env env, inputCmd *command, dest io.Writer) io.Reader {
+	// We can't use io.TeeReader unconditionally, as that would block
+	// calls to exec.Cmd.Run(), even if the underlying process has already
+	// terminated. See https://github.com/golang/go/issues/7990 for more details.
+	lastArg := ""
+	for _, arg := range inputCmd.Args {
+		if arg == "-" && lastArg != "-o" {
+			return io.TeeReader(env.stdin(), dest)
+		}
+		lastArg = arg
+	}
+	return env.stdin()
+}
diff --git a/compiler_wrapper/disable_werror_flag.go b/compiler_wrapper/disable_werror_flag.go
index 809b8b6f..a5d22f52 100644
--- a/compiler_wrapper/disable_werror_flag.go
+++ b/compiler_wrapper/disable_werror_flag.go
@@ -7,7 +7,6 @@ package main
 import (
 	"bytes"
 	"encoding/json"
-	"io"
 	"io/ioutil"
 	"os"
 	"strings"
@@ -27,7 +26,7 @@ func doubleBuildWithWNoError(env env, cfg *config, originalCmd *command) (exitCo
 	}
 	originalStdinBuffer := &bytes.Buffer{}
 	originalExitCode, err := wrapSubprocessErrorWithSourceLoc(originalCmd,
-		env.run(originalCmd, io.TeeReader(env.stdin(), originalStdinBuffer), originalStdoutBuffer, originalStderrBuffer))
+		env.run(originalCmd, teeStdinIfNeeded(env, originalCmd, originalStdinBuffer), originalStdoutBuffer, originalStderrBuffer))
 	if err != nil {
 		return 0, err
 	}
diff --git a/compiler_wrapper/env.go b/compiler_wrapper/env.go
index bbf3ec45..5f5cde38 100644
--- a/compiler_wrapper/env.go
+++ b/compiler_wrapper/env.go
@@ -72,24 +72,6 @@ func (env *processEnv) run(cmd *command, stdin io.Reader, stdout io.Writer, stde
 	execCmd.Stdin = stdin
 	execCmd.Stdout = stdout
 	execCmd.Stderr = stderr
-	_, stdinIsFile := stdin.(*os.File)
-	_, stdinIsBytesReader := stdin.(*bytes.Reader)
-	if stdin != nil && !stdinIsFile && !stdinIsBytesReader {
-		// We can't use execCmd.Run() here as that blocks if stdin blocks,
-		// even if the underlying process has already terminated. We care
-		// especially about the case when stdin is an io.TeeReader for os.Stdin.
-		// See https://github.com/golang/go/issues/7990 for more details.
-		if err := execCmd.Start(); err != nil {
-			return err
-		}
-		if _, err := execCmd.Process.Wait(); err != nil {
-			return err
-		}
-		// Closing Stdin here as we didn't wait for the read to finish via
-		// execCmd.Wait to prevent race conditions.
-		os.Stdin.Close()
-		return nil
-	}
 	return execCmd.Run()
 }