From 7df4f03f0bca2ce51d72b78e2cfe3823733810aa Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Tue, 28 Mar 2023 11:51:46 +0200
Subject: pkg/subsystem: take only always present calls from repros

We're not yet perfect at eliminating unneeded calls from reproducers, so
let's make the subsystem extraction rules stricter: only take a
subsystem from the reproducer if it's present in all reproducers.

Consider more crashes (7 instead of 5) to give more opportunities to
drop an unneeded call.
---
 dashboard/app/subsystem.go      |  2 +-
 pkg/subsystem/extractor.go      | 19 +++++++++++++++++--
 pkg/subsystem/extractor_test.go | 15 ++++++---------
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/dashboard/app/subsystem.go b/dashboard/app/subsystem.go
index 0b4300060..34cbd0547 100644
--- a/dashboard/app/subsystem.go
+++ b/dashboard/app/subsystem.go
@@ -128,7 +128,7 @@ func updateBugSubsystems(c context.Context, bugKey *db.Key,
 
 const (
 	// We load the top crashesForInference crashes to determine the bug subsystem(s).
-	crashesForInference = 5
+	crashesForInference = 7
 	// How often we update open bugs.
 	openBugsUpdateTime = time.Hour * 24 * 30
 )
diff --git a/pkg/subsystem/extractor.go b/pkg/subsystem/extractor.go
index 075c8079d..1f635c7b3 100644
--- a/pkg/subsystem/extractor.go
+++ b/pkg/subsystem/extractor.go
@@ -28,12 +28,27 @@ func MakeExtractor(list []*Subsystem) *Extractor {
 func (e *Extractor) Extract(crashes []*Crash) []*Subsystem {
 	// First put all subsystems to the same list.
 	subsystems := []*Subsystem{}
+	reproCount := 0
 	for _, crash := range crashes {
 		if crash.GuiltyPath != "" {
 			subsystems = append(subsystems, e.raw.FromPath(crash.GuiltyPath)...)
 		}
-		if len(crash.SyzRepro) > 0 {
-			subsystems = append(subsystems, e.raw.FromProg(crash.SyzRepro)...)
+		if len(crash.SyzRepro) != 0 {
+			reproCount++
+		}
+	}
+
+	// If all reproducers hint at the same subsystem, take it as well.
+	reproSubsystems := map[*Subsystem]int{}
+	for _, crash := range crashes {
+		if len(crash.SyzRepro) == 0 {
+			continue
+		}
+		for _, subsystem := range e.raw.FromProg(crash.SyzRepro) {
+			reproSubsystems[subsystem]++
+			if reproSubsystems[subsystem] == reproCount {
+				subsystems = append(subsystems, subsystem)
+			}
 		}
 	}
 
diff --git a/pkg/subsystem/extractor_test.go b/pkg/subsystem/extractor_test.go
index c2d165973..a78509e0f 100644
--- a/pkg/subsystem/extractor_test.go
+++ b/pkg/subsystem/extractor_test.go
@@ -13,7 +13,7 @@ import (
 func TestExtractor(t *testing.T) {
 	// Objects used in tests.
 	fsPath := "fs/"
-	extProg, nfsProg := []byte("ext prog"), []byte("nfs prog")
+	extProg, nfsProg, extNfsProg := []byte("ext"), []byte("nfs"), []byte("ext nfs")
 	fs := &Subsystem{Name: "fs"}
 	ext := &Subsystem{Name: "ext", Parents: []*Subsystem{fs}}
 	nfs := &Subsystem{Name: "nfs", Parents: []*Subsystem{fs}}
@@ -46,7 +46,7 @@ func TestExtractor(t *testing.T) {
 			want: []*Subsystem{ext},
 		},
 		{
-			name: `Two equally present children`,
+			name: `Reproducers hint at different subsystems`,
 			crashes: []*Crash{
 				{
 					GuiltyPath: fsPath,
@@ -60,10 +60,10 @@ func TestExtractor(t *testing.T) {
 					SyzRepro:   nfsProg,
 				},
 			},
-			want: []*Subsystem{nfs, ext},
+			want: []*Subsystem{fs},
 		},
 		{
-			name: `One child is more present than another`,
+			name: `One subsystem from reproducers is irrelevant`,
 			crashes: []*Crash{
 				{
 					GuiltyPath: fsPath,
@@ -74,11 +74,7 @@ func TestExtractor(t *testing.T) {
 				},
 				{
 					GuiltyPath: fsPath,
-					SyzRepro:   nfsProg,
-				},
-				{
-					GuiltyPath: fsPath,
-					SyzRepro:   extProg,
+					SyzRepro:   extNfsProg,
 				},
 			},
 			want: []*Subsystem{ext},
@@ -92,6 +88,7 @@ func TestExtractor(t *testing.T) {
 			perProg: []progSubsystems{
 				{extProg, []*Subsystem{ext}},
 				{nfsProg, []*Subsystem{nfs}},
+				{extNfsProg, []*Subsystem{ext, nfs}},
 			},
 		},
 	}
-- 
cgit mrf-deployment