executor, pkg/ipc: simplify retry handling

Remove kRetryStatus, it's effectively the same as exiting with 0.
Remove ipc.ExecutorFailure, nobody uses it.
Simplify few other minor things around exit status handling.

4 files changed, 24 insertions, 62 deletions

diff --git a/executor/common.h b/executor/common.h
index cf4a204fc..07cb7532c 100644
--- a/executor/common.h
+++ b/executor/common.h
@@ -611,9 +611,10 @@ static void loop(void)
 			break;
 		}
 #if SYZ_EXECUTOR
-		status = WEXITSTATUS(status);
-		if (status == kFailStatus)
+		if (WEXITSTATUS(status) == kFailStatus) {
+			errno = 0;
 			fail("child failed");
+		}
 		reply_execute(0);
 #endif
 #if SYZ_EXECUTOR || SYZ_USE_TMP_DIR
diff --git a/executor/executor.cc b/executor/executor.cc
index 798cae71e..4d60b10cc 100644
--- a/executor/executor.cc
+++ b/executor/executor.cc
@@ -59,7 +59,6 @@ const int kCoverFd = kOutPipeFd - kMaxThreads;
 const int kMaxArgs = 9;
 const int kCoverSize = 256 << 10;
 const int kFailStatus = 67;
-const int kRetryStatus = 69;
 
 // Logical error (e.g. invalid input program), use as an assert() alternative.
 static NORETURN PRINTF(1, 2) void fail(const char* msg, ...);
@@ -400,22 +399,17 @@ int main(int argc, char** argv)
 		fail("unknown sandbox type");
 	}
 #if SYZ_EXECUTOR_USES_FORK_SERVER
-	// Other statuses happen when fuzzer processes manages to kill loop.
+	fprintf(stderr, "loop exited with status %d\n", status);
+	// Other statuses happen when fuzzer processes manages to kill loop, e.g. with:
+	// ptrace(PTRACE_SEIZE, 1, 0, 0x100040)
 	if (status != kFailStatus)
-		status = kRetryStatus;
+		status = 0;
 	// If an external sandbox process wraps executor, the out pipe will be closed
 	// before the sandbox process exits this will make ipc package kill the sandbox.
 	// As the result sandbox process will exit with exit status 9 instead of the executor
-	// exit status (notably kRetryStatus). Consequently, ipc will treat it as hard
-	// failure rather than a temporal failure. So we duplicate the exit status on the pipe.
+	// exit status (notably kFailStatus). So we duplicate the exit status on the pipe.
 	reply_execute(status);
-	errno = 0;
-	if (status == kFailStatus)
-		fail("loop failed");
-	// Loop can be killed by a test process with e.g.:
-	// ptrace(PTRACE_SEIZE, 1, 0, 0x100040)
-	// This is unfortunate, but I don't have a better solution than ignoring it for now.
-	exitf("loop exited with status %d", status);
+	doexit(status);
 	// Unreachable.
 	return 1;
 #else
@@ -1343,9 +1337,7 @@ void fail(const char* msg, ...)
 	vfprintf(stderr, msg, args);
 	va_end(args);
 	fprintf(stderr, " (errno %d)\n", e);
-	// ENOMEM/EAGAIN is frequent cause of failures in fuzzing context,
-	// so handle it here as non-fatal error.
-	doexit((e == ENOMEM || e == EAGAIN) ? kRetryStatus : kFailStatus);
+	doexit(kFailStatus);
 }
 
 void exitf(const char* msg, ...)
@@ -1356,7 +1348,7 @@ void exitf(const char* msg, ...)
 	vfprintf(stderr, msg, args);
 	va_end(args);
 	fprintf(stderr, " (errno %d)\n", e);
-	doexit(kRetryStatus);
+	doexit(0);
 }
 
 void debug(const char* msg, ...)
diff --git a/pkg/csource/generated.go b/pkg/csource/generated.go
index 5a402b66f..bc238b0b0 100644
--- a/pkg/csource/generated.go
+++ b/pkg/csource/generated.go
@@ -4663,9 +4663,10 @@ static void loop(void)
 			break;
 		}
 #if SYZ_EXECUTOR
-		status = WEXITSTATUS(status);
-		if (status == kFailStatus)
+		if (WEXITSTATUS(status) == kFailStatus) {
+			errno = 0;
 			fail("child failed");
+		}
 		reply_execute(0);
 #endif
 #if SYZ_EXECUTOR || SYZ_USE_TMP_DIR
diff --git a/pkg/ipc/ipc.go b/pkg/ipc/ipc.go
index 877fe5c06..ca740219e 100644
--- a/pkg/ipc/ipc.go
+++ b/pkg/ipc/ipc.go
@@ -58,14 +58,6 @@ type ExecOpts struct {
 	FaultNth  int // fault n-th operation in the call (0-based)
 }
 
-// ExecutorFailure is returned from MakeEnv or from env.Exec when executor terminates
-// by calling fail function. This is considered a logical error (a failed assert).
-type ExecutorFailure string
-
-func (err ExecutorFailure) Error() string {
-	return string(err)
-}
-
 // Config is the configuration for Env.
 type Config struct {
 	// Path to executor binary.
@@ -120,8 +112,7 @@ type Env struct {
 const (
 	outputSize = 16 << 20
 
-	statusFail  = 67
-	statusRetry = 69
+	statusFail = 67
 
 	// Comparison types masks taken from KCOV headers.
 	compSizeMask  = 6
@@ -279,8 +270,7 @@ func (env *Env) Exec(opts *ExecOpts, p *prog.Prog) (output []byte, info *ProgInf
 			return
 		}
 	}
-	var restart bool
-	output, hanged, restart, err0 = env.cmd.exec(opts, progData)
+	output, hanged, err0 = env.cmd.exec(opts, progData)
 	if err0 != nil {
 		env.cmd.close()
 		env.cmd = nil
@@ -291,7 +281,7 @@ func (env *Env) Exec(opts *ExecOpts, p *prog.Prog) (output []byte, info *ProgInf
 	if info != nil && env.config.Flags&FlagSignal == 0 {
 		addFallbackSignal(p, info)
 	}
-	if restart {
+	if env.config.Flags&FlagUseForkServer == 0 {
 		env.cmd.close()
 		env.cmd = nil
 	}
@@ -695,12 +685,6 @@ func (c *command) handshakeError(err error) error {
 	output := <-c.readDone
 	err = fmt.Errorf("executor %v: %v\n%s", c.pid, err, output)
 	c.wait()
-	if c.cmd.ProcessState != nil {
-		// Magic values returned by executor.
-		if osutil.ProcessExitStatus(c.cmd.ProcessState) == statusFail {
-			err = ExecutorFailure(err.Error())
-		}
-	}
 	return err
 }
 
@@ -715,7 +699,7 @@ func (c *command) wait() error {
 	return err
 }
 
-func (c *command) exec(opts *ExecOpts, progData []byte) (output []byte, hanged, restart bool, err0 error) {
+func (c *command) exec(opts *ExecOpts, progData []byte) (output []byte, hanged bool, err0 error) {
 	req := &executeReq{
 		magic:     inMagic,
 		envFlags:  uint64(c.config.Flags),
@@ -753,7 +737,6 @@ func (c *command) exec(opts *ExecOpts, progData []byte) (output []byte, hanged,
 			hang <- false
 		}
 	}()
-	restart = c.config.Flags&FlagUseForkServer == 0
 	exitStatus := -1
 	completedCalls := (*uint32)(unsafe.Pointer(&c.outmem[0]))
 	outmem := c.outmem[4:]
@@ -801,27 +784,12 @@ func (c *command) exec(opts *ExecOpts, progData []byte) (output []byte, hanged,
 	}
 	if exitStatus == -1 {
 		exitStatus = osutil.ProcessExitStatus(c.cmd.ProcessState)
-		if exitStatus == 0 {
-			exitStatus = statusRetry // fuchsia always returns wrong exit status 0
-		}
-	}
-	// Handle magic values returned by executor.
-	switch exitStatus {
-	case statusFail:
-		err0 = ExecutorFailure(fmt.Sprintf("executor %v: failed: %s", c.pid, output))
-	case statusRetry:
-		// This is a temporal error (ENOMEM) or an unfortunate
-		// program that messes with testing setup (e.g. kills executor
-		// loop process). Pretend that nothing happened.
-		// It's better than a false crash report.
-		err0 = nil
-		hanged = false
-		restart = true
-	default:
-		// Consider this as no error.
-		// Without fork server executor can legitimately exit (program contains exit_group),
-		// with fork server the top process can exit with a special status if it wants special handling.
-		restart = true
+	}
+	// Ignore all other errors.
+	// Without fork server executor can legitimately exit (program contains exit_group),
+	// with fork server the top process can exit with statusFail if it wants special handling.
+	if exitStatus == statusFail {
+		err0 = fmt.Errorf("executor %v: exit status %d\n%s", c.pid, exitStatus, output)
 	}
 	return
 }