executor: use exitf instead of fail outside of setup sequence (#3959)

We have a long history of executor managing to corrupt itself in various
interesting ways (e.g. using read with a pointer pointing to some
global/stack variable and then kernel overwrites it). Or rt_sigreturn
can corrupt other registers which won't cause immediate SIGSEGV, but
rather some random behavior later.  This is the race we can't win.

We can't rely on memory consistency when the test already started, so we
should use exitf instead of fail outside of setup sequence (and relying
more on unit testing to ensure that executor works as expected for sane
programs).

Suggested-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Andrei Vagin <avagin@google.com>

5 files changed, 8 insertions, 8 deletions

diff --git a/executor/common.h b/executor/common.h
index efdb4496f..32df69001 100644
--- a/executor/common.h
+++ b/executor/common.h
@@ -408,7 +408,7 @@ static void event_set(event_t* ev)
 {
 	pthread_mutex_lock(&ev->mu);
 	if (ev->state)
-		fail("event already set");
+		exitf("event already set");
 	ev->state = 1;
 	pthread_mutex_unlock(&ev->mu);
 	pthread_cond_broadcast(&ev->cv);
diff --git a/executor/common_fuchsia.h b/executor/common_fuchsia.h
index cb29f715e..a19e3f827 100644
--- a/executor/common_fuchsia.h
+++ b/executor/common_fuchsia.h
@@ -160,7 +160,7 @@ static void event_reset(event_t* ev)
 static void event_set(event_t* ev)
 {
 	if (ev->state)
-		fail("event already set");
+		exitf("event already set");
 	__atomic_store_n(&ev->state, 1, __ATOMIC_RELEASE);
 }
 
diff --git a/executor/common_linux.h b/executor/common_linux.h
index c5fc7f6ab..621f0b2d8 100644
--- a/executor/common_linux.h
+++ b/executor/common_linux.h
@@ -35,7 +35,7 @@ static void event_reset(event_t* ev)
 static void event_set(event_t* ev)
 {
 	if (ev->state)
-		fail("event already set");
+		exitf("event already set");
 	__atomic_store_n(&ev->state, 1, __ATOMIC_RELEASE);
 	syscall(SYS_futex, &ev->state, FUTEX_WAKE | FUTEX_PRIVATE_FLAG, 1000000);
 }
diff --git a/executor/common_windows.h b/executor/common_windows.h
index a29d437b8..75020ef03 100644
--- a/executor/common_windows.h
+++ b/executor/common_windows.h
@@ -60,7 +60,7 @@ static void event_set(event_t* ev)
 {
 	EnterCriticalSection(&ev->cs);
 	if (ev->state)
-		fail("event already set");
+		exitf("event already set");
 	ev->state = 1;
 	LeaveCriticalSection(&ev->cs);
 	WakeAllConditionVariable(&ev->cv);
diff --git a/executor/executor.cc b/executor/executor.cc
index f6b09f30a..b2e24f46b 100644
--- a/executor/executor.cc
+++ b/executor/executor.cc
@@ -980,8 +980,8 @@ thread_t* schedule_call(int call_index, int call_num, uint64 copyout_index, uint
 		exitf("out of threads");
 	thread_t* th = &threads[i];
 	if (event_isset(&th->ready) || !event_isset(&th->done) || th->executing)
-		failmsg("bad thread state in schedule", "ready=%d done=%d executing=%d",
-			event_isset(&th->ready), event_isset(&th->done), th->executing);
+		exitf("bad thread state in schedule: ready=%d done=%d executing=%d",
+		      event_isset(&th->ready), event_isset(&th->done), th->executing);
 	last_scheduled = th;
 	th->copyout_pos = pos;
 	th->copyout_index = copyout_index;
@@ -1054,8 +1054,8 @@ void write_coverage_signal(cover_t* cov, uint32* signal_count_pos, uint32* cover
 void handle_completion(thread_t* th)
 {
 	if (event_isset(&th->ready) || !event_isset(&th->done) || !th->executing)
-		failmsg("bad thread state in completion", "ready=%d done=%d executing=%d",
-			event_isset(&th->ready), event_isset(&th->done), th->executing);
+		exitf("bad thread state in completion: ready=%d done=%d executing=%d",
+		      event_isset(&th->ready), event_isset(&th->done), th->executing);
 	if (th->res != (intptr_t)-1)
 		copyout_call_results(th);