syz-verifier: fix stats access, remove races

Removed atomic operations.
Added object level mutex.

6 files changed, 232 insertions, 117 deletions

diff --git a/syz-verifier/monitoring_api.go b/syz-verifier/monitoring_api.go
index 1c95f9d26..3b082c264 100644
--- a/syz-verifier/monitoring_api.go
+++ b/syz-verifier/monitoring_api.go
@@ -44,25 +44,26 @@ func (monitor *Monitor) initHTTPHandlers() {
 // statsJSON provides information for the "/api/stats.json" render.
 type statsJSON struct {
 	StartTime           time.Time
-	TotalCallMismatches int64
-	TotalProgs          int64
-	ExecErrorProgs      int64
-	FlakyProgs          int64
-	MismatchingProgs    int64
-	AverExecSpeed       int64
+	TotalCallMismatches uint64
+	TotalProgs          uint64
+	ExecErrorProgs      uint64
+	FlakyProgs          uint64
+	MismatchingProgs    uint64
+	AverExecSpeed       uint64
 }
 
 // handleStats renders the statsJSON object.
 func (monitor *Monitor) renderStats() interface{} {
 	stats := monitor.externalStats
+
 	return &statsJSON{
-		StartTime:           stats.StartTime,
-		TotalCallMismatches: stats.TotalCallMismatches,
-		TotalProgs:          stats.TotalProgs,
-		ExecErrorProgs:      stats.ExecErrorProgs,
-		FlakyProgs:          stats.FlakyProgs,
-		MismatchingProgs:    stats.MismatchingProgs,
-		AverExecSpeed:       60 * stats.TotalProgs / int64(1+time.Since(stats.StartTime).Seconds()),
+		StartTime:           stats.StartTime.Get(),
+		TotalCallMismatches: stats.TotalCallMismatches.Get(),
+		TotalProgs:          stats.TotalProgs.Get(),
+		ExecErrorProgs:      stats.ExecErrorProgs.Get(),
+		FlakyProgs:          stats.FlakyProgs.Get(),
+		MismatchingProgs:    stats.MismatchingProgs.Get(),
+		AverExecSpeed:       60 * stats.TotalProgs.Get() / uint64(1+time.Since(stats.StartTime.Get()).Seconds()),
 	}
 }
 
diff --git a/syz-verifier/stats.go b/syz-verifier/stats.go
index 91fd7538b..3815e3898 100644
--- a/syz-verifier/stats.go
+++ b/syz-verifier/stats.go
@@ -7,22 +7,133 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/google/syzkaller/prog"
 )
 
+type StatUint64 struct {
+	uint64
+	mu *sync.Mutex
+}
+
+func (s *StatUint64) Inc() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.uint64++
+}
+
+func (s *StatUint64) Get() uint64 {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.uint64
+}
+
+type StatTime struct {
+	time.Time
+	mu *sync.Mutex
+}
+
+func (s *StatTime) Set(t time.Time) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.Time = t
+}
+
+func (s *StatTime) Get() time.Time {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.Time
+}
+
+type mapStringToCallStats map[string]*CallStats
+
+type StatMapStringToCallStats struct {
+	mapStringToCallStats
+	mu *sync.Mutex
+}
+
+func (stat *StatMapStringToCallStats) IncCallOccurrenceCount(key string) {
+	stat.mu.Lock()
+	stat.mapStringToCallStats[key].Occurrences++
+	stat.mu.Unlock()
+}
+
+func (stat *StatMapStringToCallStats) IncMismatches(key string) {
+	stat.mu.Lock()
+	stat.mapStringToCallStats[key].Mismatches++
+	stat.mu.Unlock()
+}
+
+func (stat *StatMapStringToCallStats) AddState(key string, state ReturnState) {
+	stat.mu.Lock()
+	stat.mapStringToCallStats[key].States[state] = true
+	stat.mu.Unlock()
+}
+
+func (stat *StatMapStringToCallStats) SetCallInfo(key string, info *CallStats) {
+	stat.mu.Lock()
+	stat.mapStringToCallStats[key] = info
+	stat.mu.Unlock()
+}
+
+func (stat *StatMapStringToCallStats) totalExecuted() uint64 {
+	stat.mu.Lock()
+	defer stat.mu.Unlock()
+
+	var t uint64
+	for _, cs := range stat.mapStringToCallStats {
+		t += cs.Occurrences
+	}
+	return t
+}
+
+func (stat *StatMapStringToCallStats) orderedStats() []*CallStats {
+	stat.mu.Lock()
+	defer stat.mu.Unlock()
+
+	css := make([]*CallStats, 0)
+	for _, cs := range stat.mapStringToCallStats {
+		if cs.Mismatches > 0 {
+			css = append(css, cs)
+		}
+	}
+
+	sort.Slice(css, func(i, j int) bool {
+		return getPercentage(css[i].Mismatches, css[i].Occurrences) > getPercentage(css[j].Mismatches, css[j].Occurrences)
+	})
+
+	return css
+}
+
 // Stats encapsulates data for creating statistics related to the results
 // of the verification process.
 type Stats struct {
+	mu                  sync.Mutex
+	TotalCallMismatches StatUint64
+	TotalProgs          StatUint64
+	ExecErrorProgs      StatUint64
+	FlakyProgs          StatUint64
+	MismatchingProgs    StatUint64
+	StartTime           StatTime
 	// Calls stores statistics for all supported system calls.
-	Calls               map[string]*CallStats
-	TotalCallMismatches int64
-	TotalProgs          int64
-	ExecErrorProgs      int64
-	FlakyProgs          int64
-	MismatchingProgs    int64
-	StartTime           time.Time
+	Calls StatMapStringToCallStats
+}
+
+func (stats *Stats) Init() *Stats {
+	stats.TotalCallMismatches.mu = &stats.mu
+	stats.TotalProgs.mu = &stats.mu
+	stats.ExecErrorProgs.mu = &stats.mu
+	stats.FlakyProgs.mu = &stats.mu
+	stats.MismatchingProgs.mu = &stats.mu
+	stats.StartTime.mu = &stats.mu
+	stats.Calls.mu = &stats.mu
+	return stats
+}
+
+func (stats *Stats) MismatchesFound() bool {
+	return stats.TotalCallMismatches.Get() != 0
 }
 
 // CallStats stores information used to generate statistics for the
@@ -32,29 +143,45 @@ type CallStats struct {
 	Name string
 	// Mismatches stores the number of errno mismatches identified in the
 	// verified programs for this system call.
-	Mismatches int64
+	Mismatches uint64
 	// Occurrences is the number of times the system call appeared in a
 	// verified program.
-	Occurrences int64
+	Occurrences uint64
 	// States stores the kernel return state that caused mismatches.
 	States map[ReturnState]bool
 }
 
+func (stats *CallStats) orderedStates() []string {
+	states := stats.States
+	ss := make([]string, 0, len(states))
+	for s := range states {
+		ss = append(ss, fmt.Sprintf("%q", s))
+	}
+	sort.Strings(ss)
+	return ss
+}
+
 // MakeStats creates a stats object.
 func MakeStats() *Stats {
-	return &Stats{
-		Calls: make(map[string]*CallStats),
-	}
+	return (&Stats{
+		Calls: StatMapStringToCallStats{
+			mapStringToCallStats: make(mapStringToCallStats),
+		},
+	}).Init()
+}
+
+func (stats *Stats) SetCallInfo(key string, info *CallStats) {
+	stats.Calls.SetCallInfo(key, info)
 }
 
 // SetSyscallMask initializes the allowed syscall list.
 func (stats *Stats) SetSyscallMask(calls map[*prog.Syscall]bool) {
-	stats.StartTime = time.Now()
+	stats.StartTime.Set(time.Now())
 
 	for syscall := range calls {
-		stats.Calls[syscall.Name] = &CallStats{
+		stats.SetCallInfo(syscall.Name, &CallStats{
 			Name:   syscall.Name,
-			States: make(map[ReturnState]bool)}
+			States: make(map[ReturnState]bool)})
 	}
 }
 
@@ -64,15 +191,17 @@ func (stats *Stats) SetSyscallMask(calls map[*prog.Syscall]bool) {
 func (stats *Stats) GetTextDescription(deltaTime float64) string {
 	var result strings.Builder
 
-	tc := stats.totalCallsExecuted()
+	tc := stats.Calls.totalExecuted()
 	fmt.Fprintf(&result, "total number of mismatches / total number of calls "+
-		"executed: %d / %d (%0.2f %%)\n\n", stats.TotalCallMismatches, tc, getPercentage(stats.TotalCallMismatches, tc))
-	fmt.Fprintf(&result, "programs / minute: %0.2f\n\n", float64(stats.TotalProgs)/deltaTime)
+		"executed: %d / %d (%0.2f %%)\n\n",
+		stats.TotalCallMismatches.Get(), tc, getPercentage(stats.TotalCallMismatches.Get(), tc))
+	fmt.Fprintf(&result, "programs / minute: %0.2f\n\n", float64(stats.TotalProgs.Get())/deltaTime)
 	fmt.Fprintf(&result, "true mismatching programs: %d / total number of programs: %d (%0.2f %%)\n",
-		stats.MismatchingProgs, stats.TotalProgs, getPercentage(stats.MismatchingProgs, stats.TotalProgs))
+		stats.MismatchingProgs.Get(), stats.TotalProgs.Get(),
+		getPercentage(stats.MismatchingProgs.Get(), stats.TotalProgs.Get()))
 	fmt.Fprintf(&result, "flaky programs: %d / total number of programs: %d (%0.2f %%)\n\n",
-		stats.FlakyProgs, stats.TotalProgs, getPercentage(stats.FlakyProgs, stats.TotalProgs))
-	cs := stats.getOrderedStats()
+		stats.FlakyProgs.Get(), stats.TotalProgs.Get(), getPercentage(stats.FlakyProgs.Get(), stats.TotalProgs.Get()))
+	cs := stats.Calls.orderedStats()
 	for _, c := range cs {
 		fmt.Fprintf(&result, "%s\n", stats.getCallStatsTextDescription(c.Name))
 	}
@@ -82,7 +211,10 @@ func (stats *Stats) GetTextDescription(deltaTime float64) string {
 
 // getCallStatsTextDescription creates a report with the current statistics for call.
 func (stats *Stats) getCallStatsTextDescription(call string) string {
-	syscallStat, ok := stats.Calls[call]
+	totalCallMismatches := stats.TotalCallMismatches.Get()
+	stats.mu.Lock()
+	defer stats.mu.Unlock()
+	syscallStat, ok := stats.Calls.mapStringToCallStats[call]
 	if !ok {
 		return ""
 	}
@@ -92,43 +224,11 @@ func (stats *Stats) getCallStatsTextDescription(call string) string {
 		"\t↳ mismatches of %s / total number of mismatches: "+
 		"%d / %d (%0.2f %%)\n"+
 		"\t↳ %d distinct states identified: %v\n", syscallName, syscallName, syscallName, mismatches, occurrences,
-		getPercentage(mismatches, occurrences), syscallName, mismatches, stats.TotalCallMismatches,
-		getPercentage(mismatches, stats.TotalCallMismatches), len(syscallStat.States), stats.getOrderedStates(syscallName))
-}
-
-func (stats *Stats) totalCallsExecuted() int64 {
-	var t int64
-	for _, cs := range stats.Calls {
-		t += cs.Occurrences
-	}
-	return t
-}
-
-func (stats *Stats) getOrderedStats() []*CallStats {
-	css := make([]*CallStats, 0)
-	for _, cs := range stats.Calls {
-		if cs.Mismatches > 0 {
-			css = append(css, cs)
-		}
-	}
-
-	sort.Slice(css, func(i, j int) bool {
-		return getPercentage(css[i].Mismatches, css[i].Occurrences) > getPercentage(css[j].Mismatches, css[j].Occurrences)
-	})
-
-	return css
-}
-
-func (stats *Stats) getOrderedStates(call string) []string {
-	states := stats.Calls[call].States
-	ss := make([]string, 0, len(states))
-	for s := range states {
-		ss = append(ss, fmt.Sprintf("%q", s))
-	}
-	sort.Strings(ss)
-	return ss
+		getPercentage(mismatches, occurrences), syscallName, mismatches, totalCallMismatches,
+		getPercentage(mismatches, totalCallMismatches),
+		len(syscallStat.States), syscallStat.orderedStates())
 }
 
-func getPercentage(value, total int64) float64 {
+func getPercentage(value, total uint64) float64 {
 	return float64(value) / float64(total) * 100
 }
diff --git a/syz-verifier/stats_test.go b/syz-verifier/stats_test.go
index a7f8cdec8..762f9092c 100644
--- a/syz-verifier/stats_test.go
+++ b/syz-verifier/stats_test.go
@@ -9,26 +9,28 @@ import (
 )
 
 func dummyStats() *Stats {
-	return &Stats{
-		TotalProgs:          24,
-		TotalCallMismatches: 10,
-		FlakyProgs:          4,
-		MismatchingProgs:    6,
-		Calls: map[string]*CallStats{
-			"foo": {"foo", 2, 8, map[ReturnState]bool{
-				returnState(1, 7): true,
-				returnState(3, 7): true}},
-			"bar": {"bar", 5, 6, map[ReturnState]bool{
-				crashedReturnState(): true,
-				returnState(10, 7):   true,
-				returnState(22, 7):   true}},
-			"tar": {"tar", 3, 4, map[ReturnState]bool{
-				returnState(31, 7): true,
-				returnState(17, 7): true,
-				returnState(5, 7):  true}},
-			"biz": {"biz", 0, 2, map[ReturnState]bool{}},
+	return (&Stats{
+		TotalProgs:          StatUint64{uint64: 24},
+		TotalCallMismatches: StatUint64{uint64: 10},
+		FlakyProgs:          StatUint64{uint64: 4},
+		MismatchingProgs:    StatUint64{uint64: 6},
+		Calls: StatMapStringToCallStats{
+			mapStringToCallStats: mapStringToCallStats{
+				"foo": {"foo", 2, 8, map[ReturnState]bool{
+					returnState(1, 7): true,
+					returnState(3, 7): true}},
+				"bar": {"bar", 5, 6, map[ReturnState]bool{
+					crashedReturnState(): true,
+					returnState(10, 7):   true,
+					returnState(22, 7):   true}},
+				"tar": {"tar", 3, 4, map[ReturnState]bool{
+					returnState(31, 7): true,
+					returnState(17, 7): true,
+					returnState(5, 7):  true}},
+				"biz": {"biz", 0, 2, map[ReturnState]bool{}},
+			},
 		},
-	}
+	}).Init()
 }
 
 func TestGetCallStatsTextDescription(t *testing.T) {
diff --git a/syz-verifier/utils_test.go b/syz-verifier/utils_test.go
index 04888ec5e..36bec3f74 100644
--- a/syz-verifier/utils_test.go
+++ b/syz-verifier/utils_test.go
@@ -64,16 +64,18 @@ func makeExecResultCrashed(pool int) *ExecResult {
 }
 
 func emptyTestStats() *Stats {
-	return &Stats{
-		Calls: map[string]*CallStats{
-			"breaks_returns": {Name: "breaks_returns", States: map[ReturnState]bool{}},
-			"minimize$0":     {Name: "minimize$0", States: map[ReturnState]bool{}},
-			"test$res0":      {Name: "test$res0", States: map[ReturnState]bool{}},
+	return (&Stats{
+		Calls: StatMapStringToCallStats{
+			mapStringToCallStats: mapStringToCallStats{
+				"breaks_returns": {Name: "breaks_returns", States: map[ReturnState]bool{}},
+				"minimize$0":     {Name: "minimize$0", States: map[ReturnState]bool{}},
+				"test$res0":      {Name: "test$res0", States: map[ReturnState]bool{}},
+			},
 		},
-	}
+	}).Init()
 }
 
-func makeCallStats(name string, occurrences, mismatches int64, states map[ReturnState]bool) *CallStats {
+func makeCallStats(name string, occurrences, mismatches uint64, states map[ReturnState]bool) *CallStats {
 	return &CallStats{Name: name,
 		Occurrences: occurrences,
 		Mismatches:  mismatches,
diff --git a/syz-verifier/verifier.go b/syz-verifier/verifier.go
index 4fcb73735..19182fcba 100644
--- a/syz-verifier/verifier.go
+++ b/syz-verifier/verifier.go
@@ -13,7 +13,6 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/google/syzkaller/pkg/instance"
@@ -141,23 +140,23 @@ func (vrf *Verifier) TestProgram(prog *prog.Prog) (result []*ExecResult) {
 		NewEnvironment,
 	}
 
-	defer atomic.AddInt64(&vrf.stats.TotalProgs, 1)
+	defer vrf.stats.TotalProgs.Inc()
 
 	for i, env := range steps {
 		stepRes, err := vrf.Run(prog, env)
 		if err != nil {
-			atomic.AddInt64(&vrf.stats.ExecErrorProgs, 1)
+			vrf.stats.ExecErrorProgs.Inc()
 			return
 		}
 		vrf.AddCallsExecutionStat(stepRes, prog)
 		if stepRes[0].IsEqual(stepRes[1]) {
 			if i != 0 {
-				atomic.AddInt64(&vrf.stats.FlakyProgs, 1)
+				vrf.stats.FlakyProgs.Inc()
 			}
 			return
 		}
 		if i == len(steps)-1 {
-			atomic.AddInt64(&vrf.stats.MismatchingProgs, 1)
+			vrf.stats.MismatchingProgs.Inc()
 			return stepRes
 		}
 	}
@@ -220,8 +219,8 @@ func (vrf *Verifier) SetPrintStatAtSIGINT() error {
 		<-osSignalChannel
 		defer os.Exit(0)
 
-		totalExecutionTime := time.Since(vrf.stats.StartTime).Minutes()
-		if vrf.stats.TotalCallMismatches < 0 {
+		totalExecutionTime := time.Since(vrf.stats.StartTime.Get()).Minutes()
+		if !vrf.stats.MismatchesFound() {
 			fmt.Fprint(vrf.statsWrite, "No mismatches occurred until syz-verifier was stopped.")
 		} else {
 			fmt.Fprintf(vrf.statsWrite, "%s", vrf.stats.GetTextDescription(totalExecutionTime))
@@ -313,17 +312,17 @@ func (vrf *Verifier) finalizeCallSet(w io.Writer) {
 func (vrf *Verifier) AddCallsExecutionStat(results []*ExecResult, program *prog.Prog) {
 	rr := CompareResults(results, program)
 	for _, cr := range rr.Reports {
-		atomic.AddInt64(&vrf.stats.Calls[cr.Call].Occurrences, 1)
+		vrf.stats.Calls.IncCallOccurrenceCount(cr.Call)
 
 		if !cr.Mismatch {
 			continue
 		}
-		atomic.AddInt64(&vrf.stats.Calls[cr.Call].Mismatches, 1)
-		atomic.AddInt64(&vrf.stats.TotalCallMismatches, 1)
+		vrf.stats.Calls.IncMismatches(cr.Call)
+		vrf.stats.TotalCallMismatches.Inc()
 		for _, state := range cr.States {
 			if state0 := cr.States[0]; state0 != state {
-				vrf.stats.Calls[cr.Call].States[state] = true
-				vrf.stats.Calls[cr.Call].States[state0] = true
+				vrf.stats.Calls.AddState(cr.Call, state)
+				vrf.stats.Calls.AddState(cr.Call, state0)
 			}
 		}
 	}
diff --git a/syz-verifier/verifier_test.go b/syz-verifier/verifier_test.go
index fd2167b1e..bf3ba719d 100644
--- a/syz-verifier/verifier_test.go
+++ b/syz-verifier/verifier_test.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -204,14 +205,16 @@ func TestSaveDiffResults(t *testing.T) {
 				makeExecResult(1, []int{1, 3, 5}),
 			},
 			wantExist: true,
-			wantStats: &Stats{
-				TotalCallMismatches: 1,
-				Calls: map[string]*CallStats{
-					"breaks_returns": makeCallStats("breaks_returns", 1, 0, map[ReturnState]bool{}),
-					"test$res0":      makeCallStats("test$res0", 1, 1, map[ReturnState]bool{{Errno: 2}: true, {Errno: 5}: true}),
-					"minimize$0":     makeCallStats("minimize$0", 1, 0, map[ReturnState]bool{}),
+			wantStats: (&Stats{
+				TotalCallMismatches: StatUint64{1, nil},
+				Calls: StatMapStringToCallStats{
+					mapStringToCallStats: mapStringToCallStats{
+						"breaks_returns": makeCallStats("breaks_returns", 1, 0, map[ReturnState]bool{}),
+						"test$res0":      makeCallStats("test$res0", 1, 1, map[ReturnState]bool{{Errno: 2}: true, {Errno: 5}: true}),
+						"minimize$0":     makeCallStats("minimize$0", 1, 0, map[ReturnState]bool{}),
+					},
 				},
-			},
+			}).Init(),
 		},
 	}
 	for _, test := range tests {
@@ -226,7 +229,15 @@ func TestSaveDiffResults(t *testing.T) {
 			vrf.AddCallsExecutionStat(test.res, prog)
 			vrf.SaveDiffResults(test.res, prog)
 
-			if diff := cmp.Diff(test.wantStats, vrf.stats); diff != "" {
+			if diff := cmp.Diff(test.wantStats,
+				vrf.stats,
+				cmp.AllowUnexported(
+					Stats{},
+					StatUint64{},
+					StatTime{},
+					sync.Mutex{},
+					StatMapStringToCallStats{},
+				)); diff != "" {
 				t.Errorf("vrf.stats mismatch (-want +got):\n%s", diff)
 			}