diff options
31 files changed, 1161 insertions, 339 deletions
diff --git a/dashboard/app/aetest.go b/dashboard/app/aetest.go new file mode 100644 index 000000000..c94379bc6 --- /dev/null +++ b/dashboard/app/aetest.go @@ -0,0 +1,8 @@ +// Copyright 2019 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +// +build aetest + +package dash + +const isAppEngineTest = true diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 4f5b68be7..c37afb404 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -46,6 +46,8 @@ var apiNamespaceHandlers = map[string]APINamespaceHandler{ "report_failed_repro": apiReportFailedRepro, "need_repro": apiNeedRepro, "manager_stats": apiManagerStats, + "commit_poll": apiCommitPoll, + "upload_commits": apiUploadCommits, } type JSONHandler func(c context.Context, r *http.Request) (interface{}, error) @@ -201,20 +203,153 @@ loop: commits = append(commits, com) } sort.Strings(commits) - reportEmail := "" + resp := &dashapi.BuilderPollResp{ + PendingCommits: commits, + ReportEmail: reportEmail(c, ns), + } + return resp, nil +} + +func reportEmail(c context.Context, ns string) string { for _, reporting := range config.Namespaces[ns].Reporting { if _, ok := reporting.Config.(*EmailConfig); ok { - reportEmail = ownEmail(c) - break + return ownEmail(c) } } - resp := &dashapi.BuilderPollResp{ - PendingCommits: commits, - ReportEmail: reportEmail, + return "" +} + +func apiCommitPoll(c context.Context, ns string, r *http.Request, payload []byte) (interface{}, error) { + resp := &dashapi.CommitPollResp{ + ReportEmail: reportEmail(c, ns), + } + for _, repo := range config.Namespaces[ns].Repos { + resp.Repos = append(resp.Repos, dashapi.Repo{ + URL: repo.URL, + Branch: repo.Branch, + }) + } + var bugs []*Bug + _, err := datastore.NewQuery("Bug"). + Filter("Namespace=", ns). + Filter("NeedCommitInfo=", true). + Project("Commits"). + Limit(100). + GetAll(c, &bugs) + if err != nil { + return nil, fmt.Errorf("failed to query bugs: %v", err) + } + commits := make(map[string]bool) + for _, bug := range bugs { + for _, com := range bug.Commits { + commits[com] = true + } + } + for com := range commits { + resp.Commits = append(resp.Commits, com) } return resp, nil } +func apiUploadCommits(c context.Context, ns string, r *http.Request, payload []byte) (interface{}, error) { + req := new(dashapi.CommitPollResultReq) + if err := json.Unmarshal(payload, req); err != nil { + return nil, fmt.Errorf("failed to unmarshal request: %v", err) + } + // This adds fixing commits to bugs. + err := addCommitsToBugs(c, ns, "", nil, req.Commits) + if err != nil { + return nil, err + } + // Now add commit info to commits. + for _, com := range req.Commits { + if com.Hash == "" { + continue + } + if err := addCommitInfo(c, ns, com); err != nil { + return nil, err + } + } + return nil, nil +} + +func addCommitInfo(c context.Context, ns string, com dashapi.Commit) error { + var bugs []*Bug + keys, err := datastore.NewQuery("Bug"). + Filter("Namespace=", ns). + Filter("Commits=", com.Title). + GetAll(c, &bugs) + if err != nil { + return fmt.Errorf("failed to query bugs: %v", err) + } + for i, bug := range bugs { + if err := addCommitInfoToBug(c, bug, keys[i], com); err != nil { + return err + } + } + return nil +} + +func addCommitInfoToBug(c context.Context, bug *Bug, bugKey *datastore.Key, com dashapi.Commit) error { + if needUpdate, err := addCommitInfoToBugImpl(c, bug, com); err != nil { + return err + } else if !needUpdate { + return nil + } + tx := func(c context.Context) error { + bug := new(Bug) + if err := datastore.Get(c, bugKey, bug); err != nil { + return fmt.Errorf("failed to get bug %v: %v", bugKey.StringID(), err) + } + if needUpdate, err := addCommitInfoToBugImpl(c, bug, com); err != nil { + return err + } else if !needUpdate { + return nil + } + if _, err := datastore.Put(c, bugKey, bug); err != nil { + return fmt.Errorf("failed to put bug: %v", err) + } + return nil + } + return datastore.RunInTransaction(c, tx, nil) +} + +func addCommitInfoToBugImpl(c context.Context, bug *Bug, com dashapi.Commit) (bool, error) { + ci := -1 + for i, title := range bug.Commits { + if title == com.Title { + ci = i + break + } + } + if ci < 0 { + return false, nil + } + for len(bug.CommitInfo) < len(bug.Commits) { + bug.CommitInfo = append(bug.CommitInfo, CommitInfo{}) + } + hash0 := bug.CommitInfo[ci].Hash + date0 := bug.CommitInfo[ci].Date + author0 := bug.CommitInfo[ci].Author + needCommitInfo0 := bug.NeedCommitInfo + + bug.CommitInfo[ci].Hash = com.Hash + bug.CommitInfo[ci].Date = com.Date + bug.CommitInfo[ci].Author = com.Author + bug.NeedCommitInfo = false + for i := range bug.CommitInfo { + if bug.CommitInfo[i].Hash == "" { + bug.NeedCommitInfo = true + break + } + } + changed := hash0 != bug.CommitInfo[ci].Hash || + date0 != bug.CommitInfo[ci].Date || + author0 != bug.CommitInfo[ci].Author || + needCommitInfo0 != bug.NeedCommitInfo + return changed, nil +} + func apiJobPoll(c context.Context, r *http.Request, payload []byte) (interface{}, error) { req := new(dashapi.JobPollReq) if err := json.Unmarshal(payload, req); err != nil { @@ -255,6 +390,11 @@ func apiUploadBuild(c context.Context, ns string, r *http.Request, payload []byt } } if len(req.Commits) != 0 || len(req.FixCommits) != 0 { + for i := range req.FixCommits { + // Reset hashes just to make sure, + // the build does not necessary come from the master repo, so we must not remember hashes. + req.FixCommits[i].Hash = "" + } if err := addCommitsToBugs(c, ns, req.Manager, req.Commits, req.FixCommits); err != nil { // We've already uploaded the build successfully and manager can use it. // Moreover, addCommitsToBugs scans all bugs and can take long time. @@ -329,8 +469,7 @@ func uploadBuild(c context.Context, now time.Time, ns string, req *dashapi.Build return build, true, nil } -func addCommitsToBugs(c context.Context, ns, manager string, - titles []string, fixCommits []dashapi.FixCommit) error { +func addCommitsToBugs(c context.Context, ns, manager string, titles []string, fixCommits []dashapi.Commit) error { presentCommits := make(map[string]bool) bugFixedBy := make(map[string][]string) for _, com := range titles { @@ -338,7 +477,9 @@ func addCommitsToBugs(c context.Context, ns, manager string, } for _, com := range fixCommits { presentCommits[com.Title] = true - bugFixedBy[com.BugID] = append(bugFixedBy[com.BugID], com.Title) + for _, bugID := range com.BugIDs { + bugFixedBy[bugID] = append(bugFixedBy[bugID], com.Title) + } } managers, err := managerList(c, ns) if err != nil { @@ -347,10 +488,9 @@ func addCommitsToBugs(c context.Context, ns, manager string, // Fetching all bugs in a namespace can be slow, and there is no way to filter only Open/Dup statuses. // So we run a separate query for each status, this both avoids fetching unnecessary data // and splits a long query into two (two smaller queries have lower chances of trigerring - // timeouts that one huge). + // timeouts than one huge). for _, status := range []int{BugStatusOpen, BugStatusDup} { - err := addCommitsToBugsInStatus(c, status, ns, manager, managers, - fixCommits, presentCommits, bugFixedBy) + err := addCommitsToBugsInStatus(c, status, ns, manager, managers, presentCommits, bugFixedBy) if err != nil { return err } @@ -359,7 +499,7 @@ func addCommitsToBugs(c context.Context, ns, manager string, } func addCommitsToBugsInStatus(c context.Context, status int, ns, manager string, managers []string, - fixCommits []dashapi.FixCommit, presentCommits map[string]bool, bugFixedBy map[string][]string) error { + presentCommits map[string]bool, bugFixedBy map[string][]string) error { var bugs []*Bug _, err := datastore.NewQuery("Bug"). Filter("Namespace=", ns). @@ -409,22 +549,22 @@ func addCommitsToBug(c context.Context, bug *Bug, manager string, managers []str return nil } if len(fixCommits) != 0 && !reflect.DeepEqual(bug.Commits, fixCommits) { - bug.Commits = fixCommits - bug.FixTime = now - bug.PatchedOn = nil - } - bug.PatchedOn = append(bug.PatchedOn, manager) - if bug.Status == BugStatusOpen { - fixed := true - for _, mgr := range managers { - if !stringInList(bug.PatchedOn, mgr) { - fixed = false - break + bug.updateCommits(fixCommits, now) + } + if manager != "" { + bug.PatchedOn = append(bug.PatchedOn, manager) + if bug.Status == BugStatusOpen { + fixed := true + for _, mgr := range managers { + if !stringInList(bug.PatchedOn, mgr) { + fixed = false + break + } + } + if fixed { + bug.Status = BugStatusFixed + bug.Closed = now } - } - if fixed { - bug.Status = BugStatusFixed - bug.Closed = now } } if _, err := datastore.Put(c, bugKey, bug); err != nil { @@ -435,6 +575,25 @@ func addCommitsToBug(c context.Context, bug *Bug, manager string, managers []str return datastore.RunInTransaction(c, tx, nil) } +func bugNeedsCommitUpdate(c context.Context, bug *Bug, manager string, fixCommits []string, + presentCommits map[string]bool, dolog bool) bool { + if len(fixCommits) != 0 && !reflect.DeepEqual(bug.Commits, fixCommits) { + if dolog { + log.Infof(c, "bug %q is fixed with %q", bug.Title, fixCommits) + } + return true + } + if len(bug.Commits) == 0 || manager == "" || stringInList(bug.PatchedOn, manager) { + return false + } + for _, com := range bug.Commits { + if !presentCommits[com] { + return false + } + } + return true +} + func managerList(c context.Context, ns string) ([]string, error) { var builds []*Build _, err := datastore.NewQuery("Build"). @@ -456,25 +615,6 @@ func managerList(c context.Context, ns string) ([]string, error) { return managers, nil } -func bugNeedsCommitUpdate(c context.Context, bug *Bug, manager string, fixCommits []string, - presentCommits map[string]bool, dolog bool) bool { - if len(fixCommits) != 0 && !reflect.DeepEqual(bug.Commits, fixCommits) { - if dolog { - log.Infof(c, "bug %q is fixed with %q", bug.Title, fixCommits) - } - return true - } - if len(bug.Commits) == 0 || stringInList(bug.PatchedOn, manager) { - return false - } - for _, com := range bug.Commits { - if !presentCommits[com] { - return false - } - } - return true -} - func stringInList(list []string, str string) bool { for _, s := range list { if s == str { diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index e030ddaa4..a63f6e8d9 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -37,6 +37,20 @@ var testConfig = &GlobalConfig{ Clients: map[string]string{ client1: key1, }, + Repos: []KernelRepo{ + { + URL: "git://syzkaller.org", + Branch: "branch10", + Alias: "repo10alias", + CC: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + }, + { + URL: "git://github.com/google/syzkaller", + Branch: "master", + Alias: "repo10alias", + CC: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + }, + }, Reporting: []Reporting{ { Name: "reporting1", @@ -67,6 +81,14 @@ var testConfig = &GlobalConfig{ Clients: map[string]string{ client2: key2, }, + Repos: []KernelRepo{ + { + URL: "git://syzkaller.org", + Branch: "branch10", + Alias: "repo10alias", + CC: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + }, + }, Managers: map[string]ConfigManager{ "restricted-manager": { RestrictedTestingRepo: "git://restricted.git/restricted.git", @@ -109,6 +131,13 @@ var testConfig = &GlobalConfig{ Clients: map[string]string{ clientAdmin: keyAdmin, }, + Repos: []KernelRepo{ + { + URL: "git://syzkaller.org/access-admin.git", + Branch: "access-admin", + Alias: "access-admin", + }, + }, Reporting: []Reporting{ { Name: "access-admin-reporting1", @@ -126,6 +155,13 @@ var testConfig = &GlobalConfig{ Clients: map[string]string{ clientUser: keyUser, }, + Repos: []KernelRepo{ + { + URL: "git://syzkaller.org/access-user.git", + Branch: "access-user", + Alias: "access-user", + }, + }, Reporting: []Reporting{ { AccessLevel: AccessAdmin, @@ -144,6 +180,13 @@ var testConfig = &GlobalConfig{ Clients: map[string]string{ clientPublic: keyPublic, }, + Repos: []KernelRepo{ + { + URL: "git://syzkaller.org/access-public.git", + Branch: "access-public", + Alias: "access-public", + }, + }, Reporting: []Reporting{ { AccessLevel: AccessUser, @@ -157,12 +200,6 @@ var testConfig = &GlobalConfig{ }, }, }, - KernelRepos: map[string]KernelRepo{ - "repo10/branch10": { - Alias: "repo10alias", - CC: []string{"maintainers@repo10.org", "bugs@repo10.org"}, - }, - }, } const ( diff --git a/dashboard/app/bug.html b/dashboard/app/bug.html index fa59f14bc..8b3afaf3d 100644 --- a/dashboard/app/bug.html +++ b/dashboard/app/bug.html @@ -18,12 +18,12 @@ Page with details about a single bug. Status: {{if .Bug.ExternalLink}}<a href="{{.Bug.ExternalLink}}">{{.Bug.Status}}</a>{{else}}{{.Bug.Status}}{{end}}<br> Reported-by: {{.Bug.CreditEmail}}<br> {{if .Bug.Commits}} - Commits: {{.Bug.Commits}}<br> + Fix commit: {{template "fix_commits" .Bug.Commits}}<br> {{if .Bug.ClosedTime.IsZero}} Patched on: {{.Bug.PatchedOn}}, missing on: {{.Bug.MissingOn}}<br> {{end}} {{end}} - First: {{formatLateness $.Now $.Bug.FirstTime}}, last: {{formatLateness $.Now $.Bug.LastTime}}<br> + First crash: {{formatLateness $.Now $.Bug.FirstTime}}, last: {{formatLateness $.Now $.Bug.LastTime}}<br> {{template "bug_list" .DupOf}} {{template "bug_list" .Dups}} @@ -57,8 +57,8 @@ Page with details about a single bug. <td class="time">{{formatTime $c.Time}}</td> <td class="kernel" title="{{$c.KernelAlias}}">{{$c.KernelAlias}}</td> <td class="tag" title="{{$c.KernelCommit}} -{{formatTime $c.KernelCommitDate}}">{{formatShortHash $c.KernelCommit}}</td> - <td class="tag" title="{{$c.SyzkallerCommit}}">{{formatShortHash $c.SyzkallerCommit}}</td> +{{formatTime $c.KernelCommitDate}}">{{link $c.KernelCommitLink (formatShortHash $c.KernelCommit)}}</td> + <td class="tag">{{link $c.SyzkallerCommitLink (formatShortHash $c.SyzkallerCommit)}}</td> <td class="config">{{if $c.KernelConfigLink}}<a href="{{$c.KernelConfigLink}}">.config</a>{{end}}</td> <td class="repro">{{if $c.LogLink}}<a href="{{$c.LogLink}}">log</a>{{end}}</td> <td class="repro">{{if $c.ReportLink}}<a href="{{$c.ReportLink}}">report</a>{{end}}</td> diff --git a/dashboard/app/commit_poll_test.go b/dashboard/app/commit_poll_test.go new file mode 100644 index 000000000..48c86292f --- /dev/null +++ b/dashboard/app/commit_poll_test.go @@ -0,0 +1,94 @@ +// Copyright 2019 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +// +build aetest + +package dash + +import ( + "sort" + "testing" + + "github.com/google/syzkaller/dashboard/dashapi" +) + +func TestCommitPoll(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build1 := testBuild(1) + c.client.UploadBuild(build1) + + crash1 := testCrash(build1, 1) + c.client.ReportCrash(crash1) + rep1 := c.client.pollBug() + + crash2 := testCrash(build1, 2) + c.client.ReportCrash(crash2) + rep2 := c.client.pollBug() + + // No commits in commit poll. + commitPollResp, err := c.client.CommitPoll() + c.expectOK(err) + c.expectEQ(len(commitPollResp.Repos), 2) + c.expectEQ(commitPollResp.Repos[0].URL, testConfig.Namespaces["test1"].Repos[0].URL) + c.expectEQ(commitPollResp.Repos[0].Branch, testConfig.Namespaces["test1"].Repos[0].Branch) + c.expectEQ(commitPollResp.Repos[1].URL, testConfig.Namespaces["test1"].Repos[1].URL) + c.expectEQ(commitPollResp.Repos[1].Branch, testConfig.Namespaces["test1"].Repos[1].Branch) + c.expectEQ(len(commitPollResp.Commits), 0) + + // Specify fixing commit for the bug. + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ + ID: rep1.ID, + Status: dashapi.BugStatusOpen, + FixCommits: []string{"foo: fix1", "foo: fix2"}, + }) + c.expectEQ(reply.OK, true) + + // The commit should appear in commit poll. + for i := 0; i < 2; i++ { + commitPollResp, err = c.client.CommitPoll() + c.expectOK(err) + c.expectEQ(len(commitPollResp.Commits), 2) + sort.Strings(commitPollResp.Commits) + c.expectEQ(commitPollResp.Commits[0], "foo: fix1") + c.expectEQ(commitPollResp.Commits[1], "foo: fix2") + } + + // Upload hash for the first commit and fixing commit for the second bug. + c.expectOK(c.client.UploadCommits([]dashapi.Commit{ + {Hash: "hash1", Title: "foo: fix1"}, + {Hash: "hash2", Title: "bar: fix3", BugIDs: []string{rep2.ID}}, + {Hash: "hash3", Title: "some unrelated commit", BugIDs: []string{"does not exist"}}, + {Hash: "hash4", Title: "another unrelated commit"}, + })) + + commitPollResp, err = c.client.CommitPoll() + c.expectOK(err) + c.expectEQ(len(commitPollResp.Commits), 2) + sort.Strings(commitPollResp.Commits) + c.expectEQ(commitPollResp.Commits[0], "foo: fix1") + c.expectEQ(commitPollResp.Commits[1], "foo: fix2") + + // Upload hash for the second commit and a new fixing commit for the second bug. + c.expectOK(c.client.UploadCommits([]dashapi.Commit{ + {Hash: "hash5", Title: "foo: fix2"}, + {Title: "bar: fix4", BugIDs: []string{rep2.ID}}, + })) + + commitPollResp, err = c.client.CommitPoll() + c.expectOK(err) + c.expectEQ(len(commitPollResp.Commits), 1) + c.expectEQ(commitPollResp.Commits[0], "bar: fix4") + + // Upload hash for the second commit and a new fixing commit for the second bug. + c.expectOK(c.client.UploadCommits([]dashapi.Commit{ + {Hash: "hash1", Title: "foo: fix1"}, + {Hash: "hash5", Title: "foo: fix2"}, + {Hash: "hash6", Title: "bar: fix4"}, + })) + + commitPollResp, err = c.client.CommitPoll() + c.expectOK(err) + c.expectEQ(len(commitPollResp.Commits), 0) +} diff --git a/dashboard/app/config.go b/dashboard/app/config.go index 26487ae6b..f0f3d168d 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -12,6 +12,7 @@ import ( "github.com/google/syzkaller/dashboard/dashapi" "github.com/google/syzkaller/pkg/email" + "github.com/google/syzkaller/pkg/vcs" ) // There are multiple configurable aspects of the app (namespaces, reporting, API clients, etc). @@ -38,8 +39,6 @@ type GlobalConfig struct { // Each namespace has own reporting config, own API clients // and bugs are not merged across namespaces. Namespaces map[string]*Config - // Maps full repository address/branch to description of this repo. - KernelRepos map[string]KernelRepo } // Per-namespace config. @@ -70,6 +69,11 @@ type Config struct { TransformCrash func(build *Build, crash *dashapi.Crash) bool // NeedRepro hook can be used to prevent reproduction of some bugs. NeedRepro func(bug *Bug) bool + // List of kernel repositories for this namespace. + // The first repo considered the "main" repo (e.g. fixing commit info is shown against this repo). + // Other repos are secondary repos, they may be tested or not. + // If not tested they are used to poll for fixing commits. + Repos []KernelRepo } // ConfigManager describes a single syz-manager instance. @@ -114,6 +118,8 @@ type ReportingType interface { } type KernelRepo struct { + URL string + Branch string // Alias is a short, readable name of a kernel repository. Alias string // ReportingPriority says if we need to prefer to report crashes in this @@ -164,7 +170,14 @@ func installConfig(cfg *GlobalConfig) { if config != nil { panic("another config is already installed") } - // Validate the global cfg. + checkConfig(cfg) + config = cfg + initEmailReporting() + initHTTPHandlers() + initAPIHandlers() +} + +func checkConfig(cfg *GlobalConfig) { if len(cfg.Namespaces) == 0 { panic("no namespaces found") } @@ -178,23 +191,6 @@ func installConfig(cfg *GlobalConfig) { for ns, cfg := range cfg.Namespaces { checkNamespace(ns, cfg, namespaces, clientNames) } - for repo, info := range cfg.KernelRepos { - if info.Alias == "" { - panic(fmt.Sprintf("empty kernel repo alias for %q", repo)) - } - if prio := info.ReportingPriority; prio < 0 || prio > 9 { - panic(fmt.Sprintf("bad kernel repo reporting priority %v for %q", prio, repo)) - } - for _, email := range info.CC { - if _, err := mail.ParseAddress(email); err != nil { - panic(fmt.Sprintf("bad email address %q: %v", email, err)) - } - } - } - config = cfg - initEmailReporting() - initHTTPHandlers() - initAPIHandlers() } func checkNamespace(ns string, cfg *Config, namespaces, clientNames map[string]bool) { @@ -231,6 +227,36 @@ func checkNamespace(ns string, cfg *Config, namespaces, clientNames map[string]b return true } } + checkKernelRepos(ns, cfg) + checkNamespaceReporting(ns, cfg) +} + +func checkKernelRepos(ns string, cfg *Config) { + if len(cfg.Repos) == 0 { + panic(fmt.Sprintf("no repos in namespace %q", ns)) + } + for _, repo := range cfg.Repos { + if !vcs.CheckRepoAddress(repo.URL) { + panic(fmt.Sprintf("%v: bad repo URL %q", ns, repo.URL)) + } + if !vcs.CheckBranch(repo.Branch) { + panic(fmt.Sprintf("%v: bad repo branch %q", ns, repo.Branch)) + } + if repo.Alias == "" { + panic(fmt.Sprintf("%v: empty repo alias for %q", ns, repo.Alias)) + } + if prio := repo.ReportingPriority; prio < 0 || prio > 9 { + panic(fmt.Sprintf("%v: bad kernel repo reporting priority %v for %q", ns, prio, repo.Alias)) + } + for _, email := range repo.CC { + if _, err := mail.ParseAddress(email); err != nil { + panic(fmt.Sprintf("bad email address %q: %v", email, err)) + } + } + } +} + +func checkNamespaceReporting(ns string, cfg *Config) { checkConfigAccessLevel(&cfg.AccessLevel, cfg.AccessLevel, fmt.Sprintf("namespace %q", ns)) parentAccessLevel := cfg.AccessLevel reportingNames := make(map[string]bool) diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index 560dc54ca..557c4cccf 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -118,6 +118,8 @@ For more options, visit https://groups.google.com/d/optout. // Now report syz reproducer and check updated email. build2 := testBuild(10) + build2.KernelRepo = testConfig.Namespaces["test2"].Repos[0].URL + build2.KernelBranch = testConfig.Namespaces["test2"].Repos[0].Branch build2.KernelCommitTitle = "a really long title, longer than 80 chars, really long-long-long-long-long-long title" c.client2.UploadBuild(build2) crash.BuildID = build2.ID diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go index 3ffe09b7d..705f4a26d 100644 --- a/dashboard/app/entities.go +++ b/dashboard/app/entities.go @@ -74,6 +74,7 @@ type Bug struct { NumRepro int64 ReproLevel dashapi.ReproLevel HasReport bool + NeedCommitInfo bool FirstTime time.Time LastTime time.Time LastSavedCrash time.Time @@ -82,9 +83,16 @@ type Bug struct { LastActivity time.Time // last time we observed any activity related to the bug Closed time.Time Reporting []BugReporting - Commits []string - HappenedOn []string `datastore:",noindex"` // list of managers - PatchedOn []string `datastore:",noindex"` // list of managers + Commits []string // titles of fixing commmits + CommitInfo []CommitInfo // git hashes of fixing commits (parallel array to Commits) + HappenedOn []string `datastore:",noindex"` // list of managers + PatchedOn []string `datastore:",noindex"` // list of managers +} + +type CommitInfo struct { + Hash string + Author string + Date time.Time } type BugReporting struct { @@ -362,18 +370,35 @@ func bugReportingHash(bugHash, reporting string) string { return hash.String([]byte(fmt.Sprintf("%v-%v", bugHash, reporting)))[:hashLen] } +func (bug *Bug) updateCommits(commits []string, now time.Time) { + bug.Commits = commits + bug.CommitInfo = nil + bug.NeedCommitInfo = true + bug.FixTime = now + bug.PatchedOn = nil +} + +func (bug *Bug) getCommitInfo(i int) CommitInfo { + if i < len(bug.CommitInfo) { + return bug.CommitInfo[i] + } + return CommitInfo{} +} + func kernelRepoInfo(build *Build) KernelRepo { - return kernelRepoInfoRaw(build.KernelRepo, build.KernelBranch) + return kernelRepoInfoRaw(build.Namespace, build.KernelRepo, build.KernelBranch) } -func kernelRepoInfoRaw(repo, branch string) KernelRepo { - repoID := repo - if branch != "" { - repoID += "/" + branch +func kernelRepoInfoRaw(ns, url, branch string) KernelRepo { + var info KernelRepo + for _, repo := range config.Namespaces[ns].Repos { + if repo.URL == url && repo.Branch == branch { + info = repo + break + } } - info := config.KernelRepos[repoID] if info.Alias == "" { - info.Alias = repo + info.Alias = url if branch != "" { info.Alias += " " + branch } diff --git a/dashboard/app/fix_test.go b/dashboard/app/fix_test.go index b0c7bee2b..448ee73a7 100644 --- a/dashboard/app/fix_test.go +++ b/dashboard/app/fix_test.go @@ -418,7 +418,10 @@ func TestFixedWithCommitTags(t *testing.T) { rep := c.client.pollBug() // Upload build with 2 fixing commits for this bug. - build1.FixCommits = []dashapi.FixCommit{{"fix commit 1", rep.ID}, {"fix commit 2", rep.ID}} + build1.FixCommits = []dashapi.Commit{ + {Title: "fix commit 1", BugIDs: []string{rep.ID}}, + {Title: "fix commit 2", BugIDs: []string{rep.ID}}, + } c.client.UploadBuild(build1) // Now the commits must be associated with the bug and the second @@ -472,7 +475,9 @@ func TestFixedDup(t *testing.T) { c.client.updateBug(rep2.ID, dashapi.BugStatusDup, rep1.ID) // Upload build that fixes rep2. - build.FixCommits = []dashapi.FixCommit{{"fix commit 1", rep2.ID}} + build.FixCommits = []dashapi.Commit{ + {Title: "fix commit 1", BugIDs: []string{rep2.ID}}, + } c.client.UploadBuild(build) // This must fix rep1. @@ -505,16 +510,11 @@ func TestFixedDup2(t *testing.T) { c.client.updateBug(rep2.ID, dashapi.BugStatusDup, rep1.ID) // Upload build that fixes rep2. - build1.FixCommits = []dashapi.FixCommit{{"fix commit 1", rep2.ID}} + build1.FixCommits = []dashapi.Commit{ + {Title: "fix commit 1", BugIDs: []string{rep2.ID}}, + } c.client.UploadBuild(build1) - /* - dbBug1, _, _ := c.loadBug(rep1.ID) - t.Logf("BUG1: status=%v, commits: %+v, patched: %+v", dbBug1.Status, dbBug1.Commits, dbBug1.PatchedOn) - dbBug2, _, _ := c.loadBug(rep2.ID) - t.Logf("BUG2: status=%v, commits: %+v, patched: %+v", dbBug2.Status, dbBug2.Commits, dbBug2.PatchedOn) - */ - // Now undup the bugs. They are still unfixed as only 1 manager uploaded the commit. c.client.updateBug(rep2.ID, dashapi.BugStatusOpen, "") @@ -557,7 +557,10 @@ func TestFixedDup3(t *testing.T) { // Upload builds that fix rep1 and rep2 with different commits. // This must fix rep1 eventually and we must not livelock in such scenario. - build1.FixCommits = []dashapi.FixCommit{{"fix commit 1", rep1.ID}, {"fix commit 2", rep2.ID}} + build1.FixCommits = []dashapi.Commit{ + {Title: "fix commit 1", BugIDs: []string{rep1.ID}}, + {Title: "fix commit 2", BugIDs: []string{rep2.ID}}, + } build2.FixCommits = build1.FixCommits c.client.UploadBuild(build1) c.client.UploadBuild(build2) diff --git a/dashboard/app/index.yaml b/dashboard/app/index.yaml index 4adbf1527..56f82c937 100644 --- a/dashboard/app/index.yaml +++ b/dashboard/app/index.yaml @@ -22,6 +22,12 @@ indexes: - name: Seq direction: desc +- kind: Bug + properties: + - name: Namespace + - name: NeedCommitInfo + - name: Commits + - kind: Build properties: - name: Namespace diff --git a/dashboard/app/main.go b/dashboard/app/main.go index 044b4f767..8c3f624c8 100644 --- a/dashboard/app/main.go +++ b/dashboard/app/main.go @@ -15,6 +15,7 @@ import ( "github.com/google/syzkaller/dashboard/dashapi" "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/pkg/html" + "github.com/google/syzkaller/pkg/vcs" "golang.org/x/net/context" "google.golang.org/appengine/datastore" "google.golang.org/appengine/log" @@ -62,12 +63,21 @@ type uiManager struct { } type uiBuild struct { - Time time.Time - SyzkallerCommit string - KernelAlias string - KernelCommit string - KernelCommitDate time.Time - KernelConfigLink string + Time time.Time + SyzkallerCommit string + SyzkallerCommitLink string + KernelAlias string + KernelCommit string + KernelCommitLink string + KernelCommitDate time.Time + KernelConfigLink string +} + +type uiCommit struct { + Hash string + Title string + Link string + Date time.Time } type uiBugPage struct { @@ -119,7 +129,7 @@ type uiBug struct { Link string ExternalLink string CreditEmail string - Commits string + Commits []uiCommit PatchedOn []string MissingOn []string NumManagers int @@ -165,6 +175,7 @@ func handleMain(c context.Context, w http.ResponseWriter, r *http.Request) error var managers []*uiManager var jobs []*uiJob accessLevel := accessLevel(c, r) + if r.FormValue("fixed") == "" { var err error managers, err = loadManagers(c, accessLevel) @@ -422,7 +433,7 @@ func fetchNamespaceBugs(c context.Context, accessLevel AccessLevel, ns string, id := uiBug.ReportingIndex if bug.Status == BugStatusFixed { id = -1 - } else if uiBug.Commits != "" { + } else if len(uiBug.Commits) != 0 { id = -2 } groups[id] = append(groups[id], uiBug) @@ -622,9 +633,14 @@ func createUIBug(c context.Context, bug *Bug, state *ReportingState, managers [] } updateBugBadness(c, uiBug) if len(bug.Commits) != 0 { - uiBug.Commits = bug.Commits[0] - if len(bug.Commits) > 1 { - uiBug.Commits = fmt.Sprintf("%q", bug.Commits) + for i, com := range bug.Commits { + cfg := config.Namespaces[bug.Namespace] + info := bug.getCommitInfo(i) + uiBug.Commits = append(uiBug.Commits, uiCommit{ + Hash: info.Hash, + Title: com, + Link: vcs.CommitLink(cfg.Repos[0].URL, info.Hash), + }) } for _, mgr := range managers { found := false @@ -700,12 +716,14 @@ func loadCrashesForBug(c context.Context, bug *Bug) ([]*uiCrash, []byte, error) func makeUIBuild(build *Build) *uiBuild { return &uiBuild{ - Time: build.Time, - SyzkallerCommit: build.SyzkallerCommit, - KernelAlias: kernelRepoInfo(build).Alias, - KernelCommit: build.KernelCommit, - KernelCommitDate: build.KernelCommitDate, - KernelConfigLink: textLink(textKernelConfig, build.KernelConfig), + Time: build.Time, + SyzkallerCommit: build.SyzkallerCommit, + SyzkallerCommitLink: vcs.LogLink(vcs.SyzkallerRepo, build.SyzkallerCommit), + KernelAlias: kernelRepoInfo(build).Alias, + KernelCommit: build.KernelCommit, + KernelCommitLink: vcs.LogLink(build.KernelRepo, build.KernelCommit), + KernelCommitDate: build.KernelCommitDate, + KernelConfigLink: textLink(textKernelConfig, build.KernelConfig), } } @@ -813,7 +831,7 @@ func loadRecentJobs(c context.Context) ([]*uiJob, error) { Namespace: job.Namespace, Manager: job.Manager, BugTitle: job.BugTitle, - KernelAlias: kernelRepoInfoRaw(job.KernelRepo, job.KernelBranch).Alias, + KernelAlias: kernelRepoInfoRaw(job.Namespace, job.KernelRepo, job.KernelBranch).Alias, PatchLink: textLink(textPatch, job.Patch), Attempts: job.Attempts, Started: job.Started, diff --git a/dashboard/app/noaetest.go b/dashboard/app/noaetest.go new file mode 100644 index 000000000..5c461b779 --- /dev/null +++ b/dashboard/app/noaetest.go @@ -0,0 +1,8 @@ +// Copyright 2019 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +// +build !aetest + +package dash + +const isAppEngineTest = false diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index 32d3fc8e6..c038c67cb 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -511,9 +511,7 @@ func incomingCommandTx(c context.Context, now time.Time, cmd *dashapi.BugUpdate, if len(cmd.FixCommits) != 0 && (bug.Status == BugStatusOpen || bug.Status == BugStatusDup) { sort.Strings(cmd.FixCommits) if !reflect.DeepEqual(bug.Commits, cmd.FixCommits) { - bug.Commits = cmd.FixCommits - bug.FixTime = now - bug.PatchedOn = nil + bug.updateCommits(cmd.FixCommits, now) } } if cmd.CrashID != 0 { diff --git a/dashboard/app/static/style.css b/dashboard/app/static/style.css index 6ac90a5f9..3a42bf93e 100644 --- a/dashboard/app/static/style.css +++ b/dashboard/app/static/style.css @@ -77,6 +77,11 @@ table td, table th { max-width: 350pt; } +.list_table .commit_list { + width: 500pt; + max-width: 500pt; +} + .list_table .tag { font-family: monospace; font-size: 8pt; @@ -101,8 +106,8 @@ table td, table th { } .list_table .kernel { - width: 60pt; - max-width: 60pt; + width: 80pt; + max-width: 80pt; } .list_table .maintainers { @@ -151,3 +156,7 @@ textarea { width:100%; font-family: monospace; } + +.mono { + font-family: monospace; +} diff --git a/dashboard/app/templates.html b/dashboard/app/templates.html index 2047db570..4c81af27e 100644 --- a/dashboard/app/templates.html +++ b/dashboard/app/templates.html @@ -82,10 +82,10 @@ Use of this source code is governed by Apache 2 LICENSE that can be found in the </td> {{if $.ShowPatch}} <td class="stat">{{formatLateness $.Now $b.ClosedTime}}</td> - <td class="title" title="{{$b.Commits}}">{{$b.Commits}}</td> + <td class="commit_list">{{template "fix_commits" $b.Commits}}</td> {{end}} {{if $.ShowPatched}} - <td class="patched" title="{{$b.Commits}}">{{if $b.Commits}}{{len $b.PatchedOn}}/{{$b.NumManagers}}{{end}}</td> + <td class="patched" {{if $b.Commits}}title="{{with $com := index $b.Commits 0}}{{$com.Title}}{{end}}"{{end}}>{{len $b.PatchedOn}}/{{$b.NumManagers}}</td> {{end}} {{if $.ShowStatus}} <td class="status"> @@ -130,11 +130,11 @@ Use of this source code is governed by Apache 2 LICENSE that can be found in the </td> <td class="stat {{if $mgr.LastActiveBad}}bad{{end}}">{{formatLateness $mgr.Now $mgr.LastActive}}</td> <td class="stat">{{formatDuration $mgr.CurrentUpTime}}</td> - {{if $mgr.CurrentBuild}} - <td class="stat">{{formatLateness $mgr.Now $mgr.CurrentBuild.Time}}</td> - <td class="stat" title="{{$mgr.CurrentBuild.KernelAlias}} {{$mgr.CurrentBuild.KernelCommit}} -{{formatTime $mgr.CurrentBuild.KernelCommitDate}}">{{formatShortHash $mgr.CurrentBuild.KernelCommit}}</td> - <td class="stat" title="{{$mgr.CurrentBuild.SyzkallerCommit}}">{{formatShortHash $mgr.CurrentBuild.SyzkallerCommit}}</td> + {{with $build := $mgr.CurrentBuild}} + <td class="stat">{{formatLateness $mgr.Now $build.Time}}</td> + <td class="stat" title="{{$build.KernelAlias}} {{$build.KernelCommit}} +{{formatTime $build.KernelCommitDate}}">{{link $build.KernelCommitLink (formatShortHash $mgr.CurrentBuild.KernelCommit)}}</td> + <td class="stat">{{link $mgr.CurrentBuild.SyzkallerCommitLink (formatShortHash $mgr.CurrentBuild.SyzkallerCommit)}}</td> {{else}} <td></td> <td></td> @@ -158,3 +158,15 @@ Use of this source code is governed by Apache 2 LICENSE that can be found in the </table> {{end}} {{end}} + +{{/* List of fixing commits, invoked with []*uiCommit */}} +{{define "fix_commits"}} +{{range $com := .}} + <span class="mono"> + {{if $com.Hash}} + {{formatShortHash $com.Hash}} + {{end}} + {{link $com.Link $com.Title}} + </span> +{{end}} +{{end}} diff --git a/dashboard/dashapi/dashapi.go b/dashboard/dashapi/dashapi.go index bec406d78..02d7ea132 100644 --- a/dashboard/dashapi/dashapi.go +++ b/dashboard/dashapi/dashapi.go @@ -68,12 +68,15 @@ type Build struct { KernelCommitDate time.Time KernelConfig []byte Commits []string // see BuilderPoll - FixCommits []FixCommit + FixCommits []Commit } -type FixCommit struct { - Title string - BugID string +type Commit struct { + Hash string + Title string + Author string + BugIDs []string // ID's extracted from Reported-by tags + Date time.Time } func (dash *Dashboard) UploadBuild(build *Build) error { @@ -161,6 +164,34 @@ func (dash *Dashboard) ReportBuildError(req *BuildErrorReq) error { return dash.Query("report_build_error", req, nil) } +type CommitPollResp struct { + ReportEmail string + Repos []Repo + Commits []string +} + +type CommitPollResultReq struct { + Commits []Commit +} + +type Repo struct { + URL string + Branch string +} + +func (dash *Dashboard) CommitPoll() (*CommitPollResp, error) { + resp := new(CommitPollResp) + err := dash.Query("commit_poll", nil, resp) + return resp, err +} + +func (dash *Dashboard) UploadCommits(commits []Commit) error { + if len(commits) == 0 { + return nil + } + return dash.Query("upload_commits", &CommitPollResultReq{commits}, nil) +} + // Crash describes a single kernel crash (potentially with repro). type Crash struct { BuildID string // refers to Build.ID diff --git a/pkg/html/generated.go b/pkg/html/generated.go index 644cbddd9..732ed507a 100644 --- a/pkg/html/generated.go +++ b/pkg/html/generated.go @@ -80,6 +80,11 @@ table td, table th { max-width: 350pt; } +.list_table .commit_list { + width: 500pt; + max-width: 500pt; +} + .list_table .tag { font-family: monospace; font-size: 8pt; @@ -104,8 +109,8 @@ table td, table th { } .list_table .kernel { - width: 60pt; - max-width: 60pt; + width: 80pt; + max-width: 80pt; } .list_table .maintainers { @@ -154,6 +159,10 @@ textarea { width:100%; font-family: monospace; } + +.mono { + font-family: monospace; +} ` const js = ` // Copyright 2018 syzkaller project authors. All rights reserved. diff --git a/pkg/html/html.go b/pkg/html/html.go index c7f85da88..e985ae977 100644 --- a/pkg/html/html.go +++ b/pkg/html/html.go @@ -32,6 +32,7 @@ func CreateGlob(glob string) *template.Template { } var funcs = template.FuncMap{ + "link": link, "formatTime": FormatTime, "formatClock": formatClock, "formatDuration": formatDuration, @@ -41,6 +42,14 @@ var funcs = template.FuncMap{ "formatShortHash": formatShortHash, } +func link(url, text string) template.HTML { + text = template.HTMLEscapeString(text) + if url != "" { + text = fmt.Sprintf(`<a href="%v">%v</a>`, url, text) + } + return template.HTML(text) +} + func FormatTime(t time.Time) string { if t.IsZero() { return "" diff --git a/pkg/vcs/akaros.go b/pkg/vcs/akaros.go index aaf9d985e..4f476542a 100644 --- a/pkg/vcs/akaros.go +++ b/pkg/vcs/akaros.go @@ -10,7 +10,7 @@ import ( ) type akaros struct { - git *git + *git dropbear *git } @@ -40,15 +40,7 @@ func (ctx *akaros) SwitchCommit(commit string) (*Commit, error) { return nil, fmt.Errorf("not implemented for akaros") } -func (ctx *akaros) HeadCommit() (*Commit, error) { - return nil, fmt.Errorf("not implemented for akaros") -} - -func (ctx *akaros) ListRecentCommits(baseCommit string) ([]string, error) { - return ctx.git.ListRecentCommits(baseCommit) -} - -func (ctx *akaros) ExtractFixTagsFromCommits(baseCommit, email string) ([]FixCommit, error) { +func (ctx *akaros) ExtractFixTagsFromCommits(baseCommit, email string) ([]*Commit, error) { return ctx.git.ExtractFixTagsFromCommits(baseCommit, email) } diff --git a/pkg/vcs/fuchsia.go b/pkg/vcs/fuchsia.go index f9a027f67..04d8e5d36 100644 --- a/pkg/vcs/fuchsia.go +++ b/pkg/vcs/fuchsia.go @@ -75,11 +75,19 @@ func (ctx *fuchsia) HeadCommit() (*Commit, error) { return nil, fmt.Errorf("not implemented for fuchsia") } +func (ctx *fuchsia) GetCommitByTitle(title string) (*Commit, error) { + return ctx.zircon.GetCommitByTitle(title) +} + +func (ctx *fuchsia) GetCommitsByTitles(titles []string) ([]*Commit, []string, error) { + return ctx.zircon.GetCommitsByTitles(titles) +} + func (ctx *fuchsia) ListRecentCommits(baseCommit string) ([]string, error) { return ctx.zircon.ListRecentCommits(baseCommit) } -func (ctx *fuchsia) ExtractFixTagsFromCommits(baseCommit, email string) ([]FixCommit, error) { +func (ctx *fuchsia) ExtractFixTagsFromCommits(baseCommit, email string) ([]*Commit, error) { return ctx.zircon.ExtractFixTagsFromCommits(baseCommit, email) } diff --git a/pkg/vcs/git.go b/pkg/vcs/git.go index 45a7e340c..4f23bb16c 100644 --- a/pkg/vcs/git.go +++ b/pkg/vcs/git.go @@ -148,10 +148,10 @@ func (git *git) getCommit(commit string) (*Commit, error) { if err != nil { return nil, err } - return gitParseCommit(output) + return gitParseCommit(output, nil, nil) } -func gitParseCommit(output []byte) (*Commit, error) { +func gitParseCommit(output, user, domain []byte) (*Commit, error) { lines := bytes.Split(output, []byte{'\n'}) if len(lines) < 4 || len(lines[0]) != 40 { return nil, fmt.Errorf("unexpected git log output: %q", output) @@ -163,7 +163,29 @@ func gitParseCommit(output []byte) (*Commit, error) { } cc := make(map[string]bool) cc[strings.ToLower(string(lines[2]))] = true + var tags []string for _, line := range lines[4:] { + if user != nil { + userPos := bytes.Index(line, user) + if userPos != -1 { + domainPos := bytes.Index(line[userPos+len(user)+1:], domain) + if domainPos != -1 { + startPos := userPos + len(user) + endPos := userPos + len(user) + domainPos + 1 + tag := string(line[startPos:endPos]) + present := false + for _, tag1 := range tags { + if tag1 == tag { + present = true + break + } + } + if !present { + tags = append(tags, tag) + } + } + } + } for _, re := range ccRes { matches := re.FindSubmatchIndex(line) if matches == nil { @@ -187,26 +209,82 @@ func gitParseCommit(output []byte) (*Commit, error) { Title: string(lines[1]), Author: string(lines[2]), CC: sortedCC, + Tags: tags, Date: date, } return com, nil } +func (git *git) GetCommitByTitle(title string) (*Commit, error) { + commits, _, err := git.GetCommitsByTitles([]string{title}) + if err != nil || len(commits) == 0 { + return nil, err + } + return commits[0], nil +} + +func (git *git) GetCommitsByTitles(titles []string) ([]*Commit, []string, error) { + var greps []string + m := make(map[string]string) + for _, title := range titles { + canonical := CanonicalizeCommit(title) + greps = append(greps, canonical) + m[canonical] = title + } + since := time.Now().Add(-time.Hour * 24 * 365 * 2).Format("01-02-2006") + commits, err := git.fetchCommits(since, "HEAD", "", "", greps, true) + if err != nil { + return nil, nil, err + } + var results []*Commit + for _, com := range commits { + canonical := CanonicalizeCommit(com.Title) + if orig := m[canonical]; orig != "" { + delete(m, canonical) + results = append(results, com) + com.Title = orig + } + } + var missing []string + for _, orig := range m { + missing = append(missing, orig) + } + return results, missing, nil +} + func (git *git) ListRecentCommits(baseCommit string) ([]string, error) { // On upstream kernel this produces ~11MB of output. // Somewhat inefficient to collect whole output in a slice // and then convert to string, but should be bearable. output, err := runSandboxed(git.dir, "git", "log", - "--pretty=format:%s", "--no-merges", "-n", "200000", baseCommit) + "--pretty=format:%s", "-n", "200000", baseCommit) if err != nil { return nil, err } return strings.Split(string(output), "\n"), nil } -func (git *git) ExtractFixTagsFromCommits(baseCommit, email string) ([]FixCommit, error) { +func (git *git) ExtractFixTagsFromCommits(baseCommit, email string) ([]*Commit, error) { + user, domain, err := splitEmail(email) + if err != nil { + return nil, fmt.Errorf("failed to parse email %q: %v", email, err) + } + grep := user + "+.*" + domain since := time.Now().Add(-time.Hour * 24 * 365).Format("01-02-2006") - cmd := exec.Command("git", "log", "--no-merges", "--since", since, baseCommit) + return git.fetchCommits(since, baseCommit, user, domain, []string{grep}, false) +} + +func (git *git) fetchCommits(since, base, user, domain string, greps []string, fixedStrings bool) ([]*Commit, error) { + const commitSeparator = "---===syzkaller-commit-separator===---" + args := []string{"log", "--since", since, "--format=%H%n%s%n%ae%n%ad%n%b%n" + commitSeparator} + if fixedStrings { + args = append(args, "--fixed-strings") + } + for _, grep := range greps { + args = append(args, "--grep", grep) + } + args = append(args, base) + cmd := exec.Command("git", args...) cmd.Dir = git.dir stdout, err := cmd.StdoutPipe() if err != nil { @@ -217,52 +295,33 @@ func (git *git) ExtractFixTagsFromCommits(baseCommit, email string) ([]FixCommit } defer cmd.Wait() defer cmd.Process.Kill() - return gitExtractFixTags(stdout, email) -} - -func gitExtractFixTags(r io.Reader, email string) ([]FixCommit, error) { - user, domain, err := splitEmail(email) - if err != nil { - return nil, fmt.Errorf("failed to parse email %q: %v", email, err) - } var ( - s = bufio.NewScanner(r) - commits []FixCommit - commitTitle = "" - commitStart = []byte("commit ") - bodyPrefix = []byte(" ") - userBytes = []byte(user + "+") - domainBytes = []byte(domain) + s = bufio.NewScanner(stdout) + buf = new(bytes.Buffer) + separator = []byte(commitSeparator) + commits []*Commit + userBytes []byte + domainBytes []byte ) + if user != "" { + userBytes = []byte(user + "+") + domainBytes = []byte(domain) + } for s.Scan() { ln := s.Bytes() - if bytes.HasPrefix(ln, commitStart) { - commitTitle = "" + if !bytes.Equal(ln, separator) { + buf.Write(ln) + buf.WriteByte('\n') continue } - if !bytes.HasPrefix(ln, bodyPrefix) { - continue - } - ln = ln[len(bodyPrefix):] - if len(ln) == 0 { - continue - } - if commitTitle == "" { - commitTitle = string(ln) - continue - } - userPos := bytes.Index(ln, userBytes) - if userPos == -1 { - continue + com, err := gitParseCommit(buf.Bytes(), userBytes, domainBytes) + if err != nil { + return nil, err } - domainPos := bytes.Index(ln[userPos+len(userBytes)+1:], domainBytes) - if domainPos == -1 { - continue + if user == "" || len(com.Tags) != 0 { + commits = append(commits, com) } - startPos := userPos + len(userBytes) - endPos := userPos + len(userBytes) + domainPos + 1 - tag := string(ln[startPos:endPos]) - commits = append(commits, FixCommit{tag, commitTitle}) + buf.Reset() } return commits, s.Err() } diff --git a/pkg/vcs/git_repo_test.go b/pkg/vcs/git_repo_test.go index 848e369c9..f60bfc737 100644 --- a/pkg/vcs/git_repo_test.go +++ b/pkg/vcs/git_repo_test.go @@ -20,7 +20,13 @@ func init() { os.Setenv("SYZ_DISABLE_SANDBOXING", "yes") } +const ( + userEmail = `test@syzkaller.com` + extractFixTagsEmail = `"syzbot" <syzbot@my.mail.com>` +) + func TestGitRepo(t *testing.T) { + t.Parallel() baseDir, err := ioutil.TempDir("", "syz-git-test") if err != nil { t.Fatal(err) @@ -108,6 +114,170 @@ func TestGitRepo(t *testing.T) { } } +func TestMetadata(t *testing.T) { + t.Parallel() + repoDir, err := ioutil.TempDir("", "syz-git-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(repoDir) + repo := makeTestRepo(t, repoDir) + for i, test := range metadataTests { + repo.commitChange(test.description) + com, err := repo.repo.HeadCommit() + if err != nil { + t.Fatal(err) + } + checkCommit(t, i, test, com, false) + } + commits, err := repo.repo.ExtractFixTagsFromCommits("HEAD", extractFixTagsEmail) + if err != nil { + t.Fatal(err) + } + if len(metadataTests) != len(commits) { + t.Fatalf("want %v commits, got %v", len(metadataTests), len(commits)) + } + for i, test := range metadataTests { + checkCommit(t, i, test, commits[len(commits)-i-1], true) + for _, title := range []string{test.title, test.title2} { + if title == "" { + continue + } + com, err := repo.repo.GetCommitByTitle(title) + if err != nil { + t.Error(err) + } else if com == nil { + t.Errorf("no commits found by title %q", title) + } else if com.Title != title { + t.Errorf("wrong commit %q found by title %q", com.Title, title) + } + } + } +} + +func checkCommit(t *testing.T, idx int, test testCommit, com *Commit, checkTags bool) { + if !checkTags { + return + } + if test.title != com.Title { + t.Errorf("#%v: want title %q, got %q", idx, test.title, com.Title) + } + if test.author != com.Author { + t.Errorf("#%v: want author %q, got %q", idx, test.author, com.Author) + } + if diff := cmp.Diff(test.cc, com.CC); diff != "" { + t.Logf("%#v", com.CC) + t.Error(diff) + } + if diff := cmp.Diff(test.tags, com.Tags); checkTags && diff != "" { + t.Error(diff) + } +} + +type testCommit struct { + description string + title string + title2 string + author string + cc []string + tags []string +} + +// nolint: lll +var metadataTests = []testCommit{ + { + description: `dashboard/app: bump max repros per bug to 10 + +Reported-by: syzbot+8e4090902540da8c6e8f@my.mail.com +`, + title: "dashboard/app: bump max repros per bug to 10", + author: userEmail, + cc: []string{userEmail}, + tags: []string{"8e4090902540da8c6e8f"}, + }, + { + description: `executor: remove dead code + +Reported-by: syzbot+8e4090902540da8c6e8f@my.mail.com +Reported-by: syzbot <syzbot+a640a0fc325c29c3efcb@my.mail.com> +`, + title: "executor: remove dead code", + author: userEmail, + cc: []string{userEmail}, + tags: []string{"8e4090902540da8c6e8f", "a640a0fc325c29c3efcb"}, + }, + { + description: `pkg/csource: fix string escaping bug + +Reported-and-tested-by: syzbot+8e4090902540da8c6e8fa640a0fc325c29c3efcb@my.mail.com +Tested-by: syzbot+4234987263748623784623758235@my.mail.com +`, + title: "pkg/csource: fix string escaping bug", + author: userEmail, + cc: []string{"syzbot+4234987263748623784623758235@my.mail.com", "syzbot+8e4090902540da8c6e8fa640a0fc325c29c3efcb@my.mail.com", userEmail}, + tags: []string{"8e4090902540da8c6e8fa640a0fc325c29c3efcb", "4234987263748623784623758235"}, + }, + { + description: `When freeing a lockf struct that already is part of a linked list, make sure to update the next pointer for the preceding lock. Prevents a double free panic. + +ok millert@ +Reported-by: syzbot+6dd701dc797b23b8c761@my.mail.com +`, + title: "When freeing a lockf struct that already is part of a linked list, make sure to update the next pointer for the preceding lock. Prevents a double free panic.", + author: userEmail, + cc: []string{userEmail}, + tags: []string{"6dd701dc797b23b8c761"}, + }, + { + description: `ipmr: properly check rhltable_init() return value + +commit 8fb472c09b9d ("ipmr: improve hash scalability") +added a call to rhltable_init() without checking its return value. + +This problem was then later copied to IPv6 and factorized in commit +0bbbf0e7d0e7 ("ipmr, ip6mr: Unite creation of new mr_table") + +Fixes: 8fb472c09b9d ("ipmr: improve hash scalability") +Fixes: 0bbbf0e7d0e7 ("ipmr, ip6mr: Unite creation of new mr_table") +Reported-by: syzbot+6dd701dc797b23b8c761@my.mail.com +`, + title: "ipmr: properly check rhltable_init() return value", + title2: "net-backports: ipmr: properly check rhltable_init() return value", + author: userEmail, + cc: []string{userEmail}, + tags: []string{"6dd701dc797b23b8c761"}, + }, + { + description: `f2fs: sanity check for total valid node blocks + +Reported-by: syzbot+bf9253040425feb155ad@my.mail.com +Reported-by: syzbot+bf9253040425feb155ad@my.mail.com +`, + title: "f2fs: sanity check for total valid node blocks", + author: userEmail, + cc: []string{userEmail}, + tags: []string{"bf9253040425feb155ad"}, + }, + { + description: `USB: fix usbmon BUG trigger + +Automated tests triggered this by opening usbmon and accessing the +mmap while simultaneously resizing the buffers. This bug was with +us since 2006, because typically applications only size the buffers +once and thus avoid racing. Reported by Kirill A. Shutemov. + +Reported-by: <syzbot+f9831b881b3e849829fc@my.mail.com> +Signed-off-by: Pete Zaitcev <zaitcev@redhat.com> +Cc: stable <stable@vger.kernel.org> +Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> +`, + title: "USB: fix usbmon BUG trigger", + author: userEmail, + cc: []string{userEmail}, + tags: []string{"f9831b881b3e849829fc"}, + }, +} + func createTestRepo(t *testing.T, baseDir, name string) *testRepo { repo := makeTestRepo(t, filepath.Join(baseDir, name)) repo.git("checkout", "-b", "master") @@ -127,6 +297,7 @@ type testRepo struct { dir string name string commits map[string]map[string]*Commit + repo *git } func makeTestRepo(t *testing.T, dir string) *testRepo { @@ -138,8 +309,11 @@ func makeTestRepo(t *testing.T, dir string) *testRepo { dir: dir, name: filepath.Base(dir), commits: make(map[string]map[string]*Commit), + repo: newGit(dir), } repo.git("init") + repo.git("config", "--add", "user.email", userEmail) + repo.git("config", "--add", "user.name", "Test Syzkaller") return repo } @@ -160,9 +334,13 @@ func (repo *testRepo) commitFileChange(branch, change string) { if repo.commits[branch] == nil { repo.commits[branch] = make(map[string]*Commit) } - com, err := newGit(repo.dir).HeadCommit() + com, err := repo.repo.HeadCommit() if err != nil { repo.t.Fatal(err) } repo.commits[branch][change] = com } + +func (repo *testRepo) commitChange(description string) { + repo.git("commit", "--allow-empty", "-m", description) +} diff --git a/pkg/vcs/git_test.go b/pkg/vcs/git_test.go index 5c036e14c..a22835a69 100644 --- a/pkg/vcs/git_test.go +++ b/pkg/vcs/git_test.go @@ -5,11 +5,8 @@ package vcs import ( "reflect" - "strings" "testing" "time" - - "github.com/google/go-cmp/cmp" ) func TestGitParseCommit(t *testing.T) { @@ -48,7 +45,7 @@ Signed-off-by: Linux Master <linux@linux-foundation.org> }, } for input, com := range tests { - res, err := gitParseCommit([]byte(input)) + res, err := gitParseCommit([]byte(input), nil, nil) if err != nil && com != nil { t.Fatalf("want %+v, got error: %v", com, err) } @@ -120,62 +117,3 @@ v1. t.Fatalf("got bad tags\ngot: %+v\nwant: %+v", got, want) } } - -func TestGitExtractFixTags(t *testing.T) { - commits, err := gitExtractFixTags(strings.NewReader(extractFixTagsInput), extractFixTagsEmail) - if err != nil { - t.Fatal(err) - } - if diff := cmp.Diff(extractFixTagsOutput, commits); diff != "" { - t.Fatal(diff) - } -} - -const extractFixTagsEmail = "\"syzbot\" <syzbot@my.mail.com>" - -var extractFixTagsOutput = []FixCommit{ - {"8e4090902540da8c6e8f", "dashboard/app: bump max repros per bug to 10"}, - {"8e4090902540da8c6e8f", "executor: remove dead code"}, - {"a640a0fc325c29c3efcb", "executor: remove dead code"}, - {"8e4090902540da8c6e8fa640a0fc325c29c3efcb", "pkg/csource: fix string escaping bug"}, - {"4234987263748623784623758235", "pkg/csource: fix string escaping bug"}, - {"6dd701dc797b23b8c761", "When freeing a lockf struct that already is part of a linked list, make sure to"}, -} - -var extractFixTagsInput = ` -commit 73aba437a774237b1130837b856f3b40b3ec3bf0 (HEAD -> master, origin/master) -Author: me <foo@bar.com> -Date: Fri Dec 22 19:59:56 2017 +0100 - - dashboard/app: bump max repros per bug to 10 - - Reported-by: syzbot+8e4090902540da8c6e8f@my.mail.com - -commit 26cd53f078db858a6ccca338e13e7f4d1d291c22 -Author: me <foo@bar.com> -Date: Fri Dec 22 13:42:27 2017 +0100 - - executor: remove dead code - - Reported-by: syzbot+8e4090902540da8c6e8f@my.mail.com - Reported-by: syzbot <syzbot+a640a0fc325c29c3efcb@my.mail.com> - -commit 7b62abdb0abadbaf7b3f3a23ab4d78485fbf9059 -Author: Dmitry Vyukov <dvyukov@google.com> -Date: Fri Dec 22 11:59:09 2017 +0100 - - pkg/csource: fix string escaping bug - - Reported-and-tested-by: syzbot+8e4090902540da8c6e8fa640a0fc325c29c3efcb@my.mail.com - Tested-by: syzbot+4234987263748623784623758235@my.mail.com - -commit 47546510aa98d3fbff3291a5dc3cefe712e70394 -Author: anton <openbsd@openbsd.org> -Date: Sat Oct 6 21:12:23 2018 +0000 - - When freeing a lockf struct that already is part of a linked list, make sure to - update the next pointer for the preceding lock. Prevents a double free panic. - - ok millert@ - Reported-by: syzbot+6dd701dc797b23b8c761@my.mail.com -` diff --git a/pkg/vcs/netbsd.go b/pkg/vcs/netbsd.go index c210e4878..b43523756 100644 --- a/pkg/vcs/netbsd.go +++ b/pkg/vcs/netbsd.go @@ -9,7 +9,7 @@ import ( ) type netbsd struct { - git *git + *git } func newNetBSD(vm, dir string) *netbsd { @@ -18,31 +18,7 @@ func newNetBSD(vm, dir string) *netbsd { } } -func (ctx *netbsd) Poll(repo, branch string) (*Commit, error) { - return ctx.git.Poll(repo, branch) -} - -func (ctx *netbsd) CheckoutBranch(repo, branch string) (*Commit, error) { - return nil, fmt.Errorf("not implemented for netbsd") -} - -func (ctx *netbsd) CheckoutCommit(repo, commit string) (*Commit, error) { - return nil, fmt.Errorf("not implemented for netbsd") -} - -func (ctx *netbsd) SwitchCommit(commit string) (*Commit, error) { - return nil, fmt.Errorf("not implemented for netbsd") -} - -func (ctx *netbsd) HeadCommit() (*Commit, error) { - return nil, fmt.Errorf("not implemented for netbsd") -} - -func (ctx *netbsd) ListRecentCommits(baseCommit string) ([]string, error) { - return ctx.git.ListRecentCommits(baseCommit) -} - -func (ctx *netbsd) ExtractFixTagsFromCommits(baseCommit, email string) ([]FixCommit, error) { +func (ctx *netbsd) ExtractFixTagsFromCommits(baseCommit, email string) ([]*Commit, error) { return ctx.git.ExtractFixTagsFromCommits(baseCommit, email) } diff --git a/pkg/vcs/openbsd.go b/pkg/vcs/openbsd.go index fdcc648f7..3998ce58a 100644 --- a/pkg/vcs/openbsd.go +++ b/pkg/vcs/openbsd.go @@ -9,7 +9,7 @@ import ( ) type openbsd struct { - git *git + *git } func newOpenBSD(vm, dir string) *openbsd { @@ -18,31 +18,7 @@ func newOpenBSD(vm, dir string) *openbsd { } } -func (ctx *openbsd) Poll(repo, branch string) (*Commit, error) { - return ctx.git.Poll(repo, branch) -} - -func (ctx *openbsd) CheckoutBranch(repo, branch string) (*Commit, error) { - return nil, fmt.Errorf("not implemented for openbsd") -} - -func (ctx *openbsd) CheckoutCommit(repo, commit string) (*Commit, error) { - return nil, fmt.Errorf("not implemented for openbsd") -} - -func (ctx *openbsd) SwitchCommit(commit string) (*Commit, error) { - return nil, fmt.Errorf("not implemented for openbsd") -} - -func (ctx *openbsd) HeadCommit() (*Commit, error) { - return nil, fmt.Errorf("not implemented for openbsd") -} - -func (ctx *openbsd) ListRecentCommits(baseCommit string) ([]string, error) { - return ctx.git.ListRecentCommits(baseCommit) -} - -func (ctx *openbsd) ExtractFixTagsFromCommits(baseCommit, email string) ([]FixCommit, error) { +func (ctx *openbsd) ExtractFixTagsFromCommits(baseCommit, email string) ([]*Commit, error) { return ctx.git.ExtractFixTagsFromCommits(baseCommit, email) } diff --git a/pkg/vcs/vcs.go b/pkg/vcs/vcs.go index b315a83ba..def15153c 100644 --- a/pkg/vcs/vcs.go +++ b/pkg/vcs/vcs.go @@ -33,13 +33,22 @@ type Repo interface { // HeadCommit returns info about the HEAD commit of the current branch of git repository. HeadCommit() (*Commit, error) + // GetCommitByTitle finds commit info by the title. + // Remote is not fetched, the commit needs to be reachable from the current repo state + // (e.g. do CheckoutBranch before). If the commit is not found, nil is returned. + GetCommitByTitle(title string) (*Commit, error) + + // GetCommitsByTitles is a batch version of GetCommitByTitle. + // Returns list of commits and titles of commits that are not found. + GetCommitsByTitles(titles []string) ([]*Commit, []string, error) + // ListRecentCommits returns list of recent commit titles starting from baseCommit. ListRecentCommits(baseCommit string) ([]string, error) // ExtractFixTagsFromCommits extracts fixing tags for bugs from git log. // Given email = "user@domain.com", it searches for tags of the form "user+tag@domain.com" - // and return pairs {tag, commit title}. - ExtractFixTagsFromCommits(baseCommit, email string) ([]FixCommit, error) + // and returns commits with these tags. + ExtractFixTagsFromCommits(baseCommit, email string) ([]*Commit, error) // PreviousReleaseTags returns list of preceding release tags that are reachable from the given commit. PreviousReleaseTags(commit string) ([]string, error) @@ -57,14 +66,10 @@ type Commit struct { Title string Author string CC []string + Tags []string Date time.Time } -type FixCommit struct { - Tag string - Title string -} - type BisectResult int const ( @@ -153,7 +158,7 @@ func runSandboxed(dir, command string, args ...string) ([]byte, error) { var ( // nolint: lll - gitRepoRe = regexp.MustCompile(`^(git|ssh|http|https|ftp|ftps)://[a-zA-Z0-9-_]+(\.[a-zA-Z0-9-_]+)+(:[0-9]+)?/[a-zA-Z0-9-_./]+\.git(/)?$`) + gitRepoRe = regexp.MustCompile(`^(git|ssh|http|https|ftp|ftps)://[a-zA-Z0-9-_]+(\.[a-zA-Z0-9-_]+)+(:[0-9]+)?(/[a-zA-Z0-9-_./]+)?(/)?$`) gitBranchRe = regexp.MustCompile("^[a-zA-Z0-9-_/.]{2,200}$") gitHashRe = regexp.MustCompile("^[a-f0-9]{8,40}$") releaseTagRe = regexp.MustCompile(`^v([0-9]+).([0-9]+)(?:\.([0-9]+))?$`) @@ -189,3 +194,63 @@ var commitPrefixes = []string{ "FROMGIT:", "net-backports:", } + +const SyzkallerRepo = "https://github.com/google/syzkaller" + +func CommitLink(url, hash string) string { + return link(url, hash, 0) +} + +func TreeLink(url, hash string) string { + return link(url, hash, 1) +} + +func LogLink(url, hash string) string { + return link(url, hash, 2) +} + +func link(url, hash string, typ int) string { + if url == "" || hash == "" { + return "" + } + switch url { + case "https://fuchsia.googlesource.com": + // We collect hashes from zircon repo. + return link(url+"/zircon", hash, typ) + } + if strings.HasPrefix(url, "https://github.com/") { + url = strings.TrimSuffix(url, ".git") + switch typ { + case 1: + return url + "/tree/" + hash + case 2: + return url + "/commits/" + hash + default: + return url + "/commit/" + hash + } + } + if strings.HasPrefix(url, "https://git.kernel.org/pub/scm/") || + strings.HasPrefix(url, "git://git.kernel.org/pub/scm/") { + url = strings.TrimPrefix(url, "git") + url = strings.TrimPrefix(url, "https") + switch typ { + case 1: + return "https" + url + "/tree/?id=" + hash + case 2: + return "https" + url + "/log/?id=" + hash + default: + return "https" + url + "/commit/?id=" + hash + } + } + if strings.HasPrefix(url, "https://") && strings.Contains(url, ".googlesource.com") { + switch typ { + case 1: + return url + "/+/" + hash + "/" + case 2: + return url + "/+log/" + hash + default: + return url + "/+/" + hash + "^!" + } + } + return "" +} diff --git a/pkg/vcs/vcs_test.go b/pkg/vcs/vcs_test.go index 9e90090a4..457430989 100644 --- a/pkg/vcs/vcs_test.go +++ b/pkg/vcs/vcs_test.go @@ -31,12 +31,13 @@ func TestCheckRepoAddress(t *testing.T) { "https://anonscm.debian.org/git/kernel/linux.git": true, "git://kernel.ubuntu.com/ubuntu/ubuntu-zesty.git": true, "http://host.xz:123/path/to/repo.git/": true, + "https://chromium.googlesource.com/chromiumos/third_party/kernel": true, + "https://fuchsia.googlesource.com": true, "": false, "foobar": false, "linux-next": false, "foo://kernel.ubuntu.com/ubuntu/ubuntu-zesty.git": false, "git://kernel/ubuntu.git": false, - "git://kernel.com/ubuntu": false, "gitgit://kernel.ubuntu.com/ubuntu/ubuntu-zesty.git": false, }) } @@ -83,3 +84,70 @@ func testPredicate(t *testing.T, fn func(string) bool, tests map[string]bool) { } } } + +func TestCommitLink(t *testing.T) { + type Test struct { + URL string + Hash string + CommitLink string + } + tests := []Test{ + { + "https://github.com/google/syzkaller", + "76dd003f1b102b791d8b342a1f92a6486ff56a1e", + "https://github.com/google/syzkaller/commit/76dd003f1b102b791d8b342a1f92a6486ff56a1e", + }, + { + "https://github.com/google/syzkaller.git", + "76dd003f1b", + "https://github.com/google/syzkaller/commit/76dd003f1b", + }, + { + "https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git", + "8fe28cb58bcb", + "https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8fe28cb58bcb", + }, + { + "git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git", + "8fe28cb58b", + "https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8fe28cb58b", + }, + + { + "https://android.googlesource.com/kernel/common", + "d0c3914ffbe4c00f0a131bae83f811d5606699bc", + "https://android.googlesource.com/kernel/common/+/d0c3914ffbe4c00f0a131bae83f811d5606699bc^!", + }, + { + "https://gvisor.googlesource.com/gvisor", + "5301cbf8430e5436211bc142c0886d8c11cc71ab", + "https://gvisor.googlesource.com/gvisor/+/5301cbf8430e5436211bc142c0886d8c11cc71ab^!", + }, + { + "https://fuchsia.googlesource.com", + "13ee3dc5e4c46bf127977ad28645c47442ec517d", + "https://fuchsia.googlesource.com/zircon/+/13ee3dc5e4c46bf127977ad28645c47442ec517d^!", + }, + { + "git://git.cmpxchg.org/linux-mmots.git", + "8fe28cb58b", + "", + }, + { + "", + "8fe28cb58b", + "", + }, + { + "https://android.googlesource.com/kernel/common", + "", + "", + }, + } + for _, test := range tests { + link := CommitLink(test.URL, test.Hash) + if link != test.CommitLink { + t.Errorf("URL: %v\nhash: %v\nwant: %v\ngot: %v", test.URL, test.Hash, test.CommitLink, link) + } + } +} diff --git a/syz-ci/jobs.go b/syz-ci/jobs.go index a8dfdd824..c74e7b2be 100644 --- a/syz-ci/jobs.go +++ b/syz-ci/jobs.go @@ -8,6 +8,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "time" "github.com/google/syzkaller/dashboard/dashapi" @@ -20,9 +21,15 @@ import ( "github.com/google/syzkaller/vm" ) +const ( + commitPollPeriod = time.Hour +) + type JobProcessor struct { + cfg *Config name string managers []*Manager + knownCommits map[string]bool stop chan struct{} dash *dashapi.Dashboard syzkallerRepo string @@ -31,28 +38,37 @@ type JobProcessor struct { func newJobProcessor(cfg *Config, managers []*Manager, stop chan struct{}) *JobProcessor { jp := &JobProcessor{ + cfg: cfg, name: fmt.Sprintf("%v-job", cfg.Name), managers: managers, + knownCommits: make(map[string]bool), stop: stop, syzkallerRepo: cfg.SyzkallerRepo, syzkallerBranch: cfg.SyzkallerBranch, } - if cfg.DashboardAddr != "" && cfg.DashboardClient != "" { + if cfg.EnableJobs { + if cfg.DashboardAddr == "" || cfg.DashboardClient == "" { + panic("enabled_jobs is set but no dashboard info") + } jp.dash = dashapi.New(cfg.DashboardClient, cfg.DashboardAddr, cfg.DashboardKey) } return jp } func (jp *JobProcessor) loop() { - if jp.dash == nil { - return - } ticker := time.NewTicker(time.Minute) defer ticker.Stop() + var lastCommitPoll time.Time for { select { case <-ticker.C: - jp.poll() + if jp.cfg.EnableJobs { + jp.pollJobs() + } + if time.Since(lastCommitPoll) > commitPollPeriod { + jp.pollCommits() + lastCommitPoll = time.Now() + } case <-jp.stop: log.Logf(0, "job loop stopped") return @@ -60,7 +76,115 @@ func (jp *JobProcessor) loop() { } } -func (jp *JobProcessor) poll() { +func (jp *JobProcessor) pollCommits() { + for _, mgr := range jp.managers { + if !mgr.mgrcfg.PollCommits { + continue + } + if err := jp.pollManagerCommits(mgr); err != nil { + jp.Errorf("failed to poll commits on %v: %v", mgr.name, err) + } + } +} + +func brokenRepo(url string) bool { + // TODO(dvyukov): mmots contains weird squashed commits titled "linux-next" or "origin", + // which contain hundreds of other commits. This makes fix attribution totally broken. + return strings.Contains(url, "git.cmpxchg.org/linux-mmots") +} + +func (jp *JobProcessor) pollManagerCommits(mgr *Manager) error { + resp, err := mgr.dash.CommitPoll() + if err != nil { + return err + } + log.Logf(0, "polling commits for %v: repos %v, commits %v", mgr.name, len(resp.Repos), len(resp.Commits)) + if len(resp.Repos) == 0 { + return fmt.Errorf("no repos") + } + commits := make(map[string]*vcs.Commit) + for i, repo := range resp.Repos { + if brokenRepo(repo.URL) { + continue + } + commits1, err := jp.pollRepo(mgr, repo.URL, repo.Branch, resp.ReportEmail) + if err != nil { + jp.Errorf("failed to poll %v %v: %v", repo.URL, repo.Branch, err) + continue + } + log.Logf(1, "got %v commits from %v/%v repo", len(commits1), repo.URL, repo.Branch) + for _, com := range commits1 { + // Only the "main" repo is the source of true hashes. + if i != 0 { + com.Hash = "" + } + // Not overwrite existing commits, in particular commit from the main repo with hash. + if _, ok := commits[com.Title]; !ok && !jp.knownCommits[com.Title] && len(commits) < 100 { + commits[com.Title] = com + jp.knownCommits[com.Title] = true + } + } + if i == 0 && len(resp.Commits) != 0 { + commits1, err := jp.getCommitInfo(mgr, repo.URL, repo.Branch, resp.Commits) + if err != nil { + jp.Errorf("failed to poll %v %v: %v", repo.URL, repo.Branch, err) + continue + } + log.Logf(1, "got %v commit infos from %v/%v repo", len(commits1), repo.URL, repo.Branch) + for _, com := range commits1 { + // GetCommitByTitle does not accept ReportEmail and does not return tags, + // so don't replace the existing commit. + if _, ok := commits[com.Title]; !ok { + commits[com.Title] = com + } + } + } + } + results := make([]dashapi.Commit, 0, len(commits)) + for _, com := range commits { + results = append(results, dashapi.Commit{ + Hash: com.Hash, + Title: com.Title, + Author: com.Author, + BugIDs: com.Tags, + Date: com.Date, + }) + } + return mgr.dash.UploadCommits(results) +} + +func (jp *JobProcessor) pollRepo(mgr *Manager, URL, branch, reportEmail string) ([]*vcs.Commit, error) { + dir := osutil.Abs(filepath.Join("jobs", mgr.managercfg.TargetOS, "kernel")) + repo, err := vcs.NewRepo(mgr.managercfg.TargetOS, mgr.managercfg.Type, dir) + if err != nil { + return nil, fmt.Errorf("failed to create kernel repo: %v", err) + } + if _, err = repo.CheckoutBranch(URL, branch); err != nil { + return nil, fmt.Errorf("failed to checkout kernel repo %v/%v: %v", URL, branch, err) + } + return repo.ExtractFixTagsFromCommits("HEAD", reportEmail) +} + +func (jp *JobProcessor) getCommitInfo(mgr *Manager, URL, branch string, commits []string) ([]*vcs.Commit, error) { + dir := osutil.Abs(filepath.Join("jobs", mgr.managercfg.TargetOS, "kernel")) + repo, err := vcs.NewRepo(mgr.managercfg.TargetOS, mgr.managercfg.Type, dir) + if err != nil { + return nil, fmt.Errorf("failed to create kernel repo: %v", err) + } + if _, err = repo.CheckoutBranch(URL, branch); err != nil { + return nil, fmt.Errorf("failed to checkout kernel repo %v/%v: %v", URL, branch, err) + } + results, missing, err := repo.GetCommitsByTitles(commits) + if err != nil { + return nil, err + } + for _, title := range missing { + log.Logf(0, "did not find commit %q", title) + } + return results, nil +} + +func (jp *JobProcessor) pollJobs() { var names []string for _, mgr := range jp.managers { names = append(names, mgr.name) diff --git a/syz-ci/manager.go b/syz-ci/manager.go index 68dd7636d..4867216ca 100644 --- a/syz-ci/manager.go +++ b/syz-ci/manager.go @@ -545,7 +545,7 @@ func (mgr *Manager) createDashboardBuild(info *BuildInfo, imageDir, typ string) // pollCommits asks dashboard what commits it is interested in (i.e. fixes for // open bugs) and returns subset of these commits that are present in a build // on commit buildCommit. -func (mgr *Manager) pollCommits(buildCommit string) ([]string, []dashapi.FixCommit, error) { +func (mgr *Manager) pollCommits(buildCommit string) ([]string, []dashapi.Commit, error) { resp, err := mgr.dash.BuilderPoll(mgr.name) if err != nil || len(resp.PendingCommits) == 0 && resp.ReportEmail == "" { return nil, nil, err @@ -566,19 +566,18 @@ func (mgr *Manager) pollCommits(buildCommit string) ([]string, []dashapi.FixComm } } } - var fixCommits []dashapi.FixCommit + var fixCommits []dashapi.Commit if resp.ReportEmail != "" { - // TODO(dvyukov): mmots contains weird squashed commits titled "linux-next" or "origin", - // which contain hundreds of other commits. This makes fix attribution totally broken. - if mgr.mgrcfg.Repo != "git://git.cmpxchg.org/linux-mmots.git" { + if !brokenRepo(mgr.mgrcfg.Repo) { commits, err := mgr.repo.ExtractFixTagsFromCommits(buildCommit, resp.ReportEmail) if err != nil { return nil, nil, err } for _, com := range commits { - fixCommits = append(fixCommits, dashapi.FixCommit{ - Title: com.Title, - BugID: com.Tag, + fixCommits = append(fixCommits, dashapi.Commit{ + Title: com.Title, + BugIDs: com.Tags, + Date: com.Date, }) } } diff --git a/syz-ci/syz-ci.go b/syz-ci/syz-ci.go index 738b9d599..ef4648bf6 100644 --- a/syz-ci/syz-ci.go +++ b/syz-ci/syz-ci.go @@ -108,6 +108,7 @@ type ManagerConfig struct { KernelCmdline string `json:"kernel_cmdline"` // File with sysctl values (e.g. output of sysctl -a, optional). KernelSysctl string `json:"kernel_sysctl"` + PollCommits bool `json:"poll_commits"` ManagerConfig json.RawMessage `json:"manager_config"` managercfg *mgrconfig.Config } @@ -158,14 +159,13 @@ func main() { mgr.loop() }() } - if cfg.EnableJobs { - jp := newJobProcessor(cfg, managers, stop) - wg.Add(1) - go func() { - defer wg.Done() - jp.loop() - }() - } + + jp := newJobProcessor(cfg, managers, stop) + wg.Add(1) + go func() { + defer wg.Done() + jp.loop() + }() // For testing. Racy. Use with care. http.HandleFunc("/upload_cover", func(w http.ResponseWriter, r *http.Request) { @@ -218,6 +218,9 @@ func loadConfig(filename string) (*Config, error) { if len(cfg.Managers) == 0 { return nil, fmt.Errorf("no managers specified") } + if cfg.EnableJobs && (cfg.DashboardAddr == "" || cfg.DashboardClient == "") { + return nil, fmt.Errorf("enabled_jobs is set but no dashboard info") + } for i, mgr := range cfg.Managers { if mgr.Name == "" { return nil, fmt.Errorf("param 'managers[%v].name' is empty", i) @@ -229,6 +232,9 @@ func loadConfig(filename string) (*Config, error) { if err != nil { return nil, fmt.Errorf("manager %v: %v", mgr.Name, err) } + if mgr.PollCommits && (cfg.DashboardAddr == "" || mgr.DashboardClient == "") { + return nil, fmt.Errorf("manager %v: commit_poll is set but no dashboard info", mgr.Name) + } mgr.managercfg = managercfg managercfg.Name = cfg.Name + "-" + mgr.Name managercfg.Syzkaller = filepath.FromSlash("syzkaller/current") diff --git a/syz-manager/html.go b/syz-manager/html.go index 5f8d5a9cb..bead85a04 100644 --- a/syz-manager/html.go +++ b/syz-manager/html.go @@ -25,6 +25,7 @@ import ( "github.com/google/syzkaller/pkg/log" "github.com/google/syzkaller/pkg/osutil" "github.com/google/syzkaller/pkg/signal" + "github.com/google/syzkaller/pkg/vcs" "github.com/google/syzkaller/prog" "github.com/google/syzkaller/sys" ) @@ -117,11 +118,8 @@ func (mgr *Manager) collectStats() []UIStat { rawStats := mgr.stats.all() head := strings.Replace(sys.GitRevision, "+", "", -1) - if head == "" { - head = "master" - } stats := []UIStat{ - {Name: "revision", Value: fmt.Sprint(head[:8]), Link: "https://github.com/google/syzkaller/commit/" + head}, + {Name: "revision", Value: fmt.Sprint(head[:8]), Link: vcs.LogLink(vcs.SyzkallerRepo, head)}, {Name: "config", Value: mgr.cfg.Name, Link: "/config"}, {Name: "uptime", Value: fmt.Sprint(time.Since(mgr.startTime) / 1e9 * 1e9)}, {Name: "fuzzing", Value: fmt.Sprint(mgr.fuzzingTime / 60e9 * 60e9)}, |