From f53c2fac46e2e193493045eef08591ded1496b74 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 16 Apr 2021 00:47:35 -0400 Subject: [PATCH] cmd/go/internal/modload: in newRequirements, verify that rootModules is sorted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment for the Requirements.rootModules field requires that it be "sorted and capped to length". I noticed that we were not capping it correctly — we were capping the local variable (the rorotModules argument itself) but not the struct field. That prompted me to question whether we were also at some point failing to sort it correctly, so I decided to add an explicit check. With the explicit check, all tests continue to pass. For #36460 Change-Id: I6687de8ef8ecc5129fa8810d678e5673752fd27b Reviewed-on: https://go-review.googlesource.com/c/go/+/310790 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod Reviewed-by: Michael Matloob --- src/cmd/go/internal/modload/buildlist.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/cmd/go/internal/modload/buildlist.go b/src/cmd/go/internal/modload/buildlist.go index 07d9fdfc541..8be5cc2a1be 100644 --- a/src/cmd/go/internal/modload/buildlist.go +++ b/src/cmd/go/internal/modload/buildlist.go @@ -19,6 +19,7 @@ import ( "sync/atomic" "golang.org/x/mod/module" + "golang.org/x/mod/semver" ) // capVersionSlice returns s with its cap reduced to its length. @@ -89,6 +90,7 @@ var requirements *Requirements // The dependencies of the roots will be loaded lazily at the first call to the // Graph method. // +// The rootModules slice must be sorted according to module.Sort. // The caller must not modify the rootModules slice or direct map after passing // them to newRequirements. // @@ -102,15 +104,20 @@ func newRequirements(depth modDepth, rootModules []module.Version, direct map[st if m.Path == "" || m.Version == "" { panic(fmt.Sprintf("bad requirement: rootModules[%v] = %v", i, m)) } + if i > 0 { + prev := rootModules[i-1] + if prev.Path > m.Path || (prev.Path == m.Path && semver.Compare(prev.Version, m.Version) > 0) { + panic(fmt.Sprintf("newRequirements called with unsorted roots: %v", rootModules)) + } + } } rs := &Requirements{ depth: depth, - rootModules: rootModules, + rootModules: capVersionSlice(rootModules), maxRootVersion: make(map[string]string, len(rootModules)), direct: direct, } - rootModules = capVersionSlice(rootModules) for _, m := range rootModules { if v, ok := rs.maxRootVersion[m.Path]; ok && cmpVersion(v, m.Version) >= 0 {