From 44f9a3566ce564f9a21b1b92940a520ea241e065 Mon Sep 17 00:00:00 2001 From: Daniel Theophanes Date: Mon, 21 Jun 2021 11:11:20 -0500 Subject: [PATCH 01/21] database/sql: fix deadlock test in prepare statement The issue go#46783 correctly diagnosed the context timeout caused an intermittent failure when the context was canceled prior to the BeginTx call. However due to the asynchronous nature of canceling a Tx through a context on fast systems, the tx.Prepare also succeeded. On slower systems or if a time.Sleep was inserted between the BeginTx and Prepare, the Prepare would fail. Resolve this by moving the context cancel after the Prepare. This will still trigger the deadlock which I tested locally. In addition, I interspersed multiple time.Sleep calls and the test still functioned. Fixes #46852 Change-Id: I9cbf90d3c12b2555493a37799738772b615ae39d Reviewed-on: https://go-review.googlesource.com/c/go/+/329830 Run-TryBot: Daniel Theophanes Reviewed-by: Ian Lance Taylor Trust: Bryan C. Mills --- src/database/sql/sql_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index 7d1cb9b85a..f771dee4a9 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -2841,7 +2841,6 @@ func TestTxStmtDeadlock(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() tx, err := db.BeginTx(ctx, nil) - cancel() if err != nil { t.Fatal(err) } @@ -2850,6 +2849,7 @@ func TestTxStmtDeadlock(t *testing.T) { if err != nil { t.Fatal(err) } + cancel() // Run number of stmt queries to reproduce deadlock from context cancel for i := 0; i < 1e3; i++ { // Encounter any close related errors (e.g. ErrTxDone, stmt is closed) From 20bdfba32590c3dcce8885df875dc56a84b2d269 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 21 Jun 2021 12:52:17 -0400 Subject: [PATCH 02/21] go/scanner: fall back to next() when encountering 0 bytes in parseIdentifier CL 308611 optimized parseIdentifier for ASCII, but inadvertently skipped error handling for 0 bytes. Don't take the optimized path when encountering 0. Fixes #46855 Change-Id: Ic584e077eb74c012611fefa20eb71ca09c81b3c7 Reviewed-on: https://go-review.googlesource.com/c/go/+/329790 Trust: Robert Findley Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/go/scanner/scanner.go | 2 +- src/go/scanner/scanner_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/go/scanner/scanner.go b/src/go/scanner/scanner.go index 29cbf39721..f08e28cdd6 100644 --- a/src/go/scanner/scanner.go +++ b/src/go/scanner/scanner.go @@ -373,7 +373,7 @@ func (s *Scanner) scanIdentifier() string { continue } s.rdOffset += rdOffset - if b < utf8.RuneSelf { + if 0 < b && b < utf8.RuneSelf { // Optimization: we've encountered an ASCII character that's not a letter // or number. Avoid the call into s.next() and corresponding set up. // diff --git a/src/go/scanner/scanner_test.go b/src/go/scanner/scanner_test.go index ac8d257716..db123c32e0 100644 --- a/src/go/scanner/scanner_test.go +++ b/src/go/scanner/scanner_test.go @@ -812,6 +812,8 @@ var errors = []struct { {"//\ufeff", token.COMMENT, 2, "//\ufeff", "illegal byte order mark"}, // only first BOM is ignored {"'\ufeff" + `'`, token.CHAR, 1, "'\ufeff" + `'`, "illegal byte order mark"}, // only first BOM is ignored {`"` + "abc\ufeffdef" + `"`, token.STRING, 4, `"` + "abc\ufeffdef" + `"`, "illegal byte order mark"}, // only first BOM is ignored + {"abc\x00def", token.IDENT, 3, "abc", "illegal character NUL"}, + {"abc\x00", token.IDENT, 3, "abc", "illegal character NUL"}, } func TestScanErrors(t *testing.T) { From 3f9ec83b10c6de2a992b7458eb3be279f48eb6f8 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Sun, 20 Jun 2021 12:22:16 -0700 Subject: [PATCH 03/21] cmd/go: document GOPPC64 environment variable Change-Id: I2d2c02eec4ac6eca218fa5334d32650c1620692c Reviewed-on: https://go-review.googlesource.com/c/go/+/329689 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Carlos Eduardo Seo --- src/cmd/go/alldocs.go | 3 +++ src/cmd/go/internal/help/helpdoc.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 3febe880cd..27f993aeb3 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -1887,6 +1887,9 @@ // GOMIPS64 // For GOARCH=mips64{,le}, whether to use floating point instructions. // Valid values are hardfloat (default), softfloat. +// GOPPC64 +// For GOARCH=ppc64{,le}, the target ISA (Instruction Set Architecture). +// Valid values are power8 (default), power9. // GOWASM // For GOARCH=wasm, comma-separated list of experimental WebAssembly features to use. // Valid values are satconv, signext. diff --git a/src/cmd/go/internal/help/helpdoc.go b/src/cmd/go/internal/help/helpdoc.go index 9ec6501892..b552777e3e 100644 --- a/src/cmd/go/internal/help/helpdoc.go +++ b/src/cmd/go/internal/help/helpdoc.go @@ -598,6 +598,9 @@ Architecture-specific environment variables: GOMIPS64 For GOARCH=mips64{,le}, whether to use floating point instructions. Valid values are hardfloat (default), softfloat. + GOPPC64 + For GOARCH=ppc64{,le}, the target ISA (Instruction Set Architecture). + Valid values are power8 (default), power9. GOWASM For GOARCH=wasm, comma-separated list of experimental WebAssembly features to use. Valid values are satconv, signext. From a0400420ade001265f656c5dd9be1b48d7c8e6fe Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 16 Jun 2021 14:34:54 -0400 Subject: [PATCH 04/21] cmd/internal/moddeps: use -mod=readonly instead of -mod=mod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestAllDependencies is attempting to check that the modules in GOROOT satisfy certain properties; it should not modify those modules itself. The “quick” part of the test checks that vendored packages are present and complete, without constructing a parallel GOROOT. It shouldn't resolve new dependencies or change formatting in any way. The longer version of the test already constructs a parallel GOROOT and tidies the modules within it. That part of the test will flag any modifications needed to the go.mod and go.sum files, without modifying the original GOROOT. From what I can tell, the failure mode in #46695 is caused by running the test on a module rooted in $GOROOT proper. There is no such module in the mainline Go repo, but it may have been introduced in the fork and could also be introduced by stray edits in contributor CLs. It should be diagnosed clearly. For #46695 Change-Id: I62b90ccbd54cb3e3b413017021c952a7b1d455e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/328770 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- src/cmd/internal/moddeps/moddeps_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/internal/moddeps/moddeps_test.go b/src/cmd/internal/moddeps/moddeps_test.go index 7723250468..8d01b913c3 100644 --- a/src/cmd/internal/moddeps/moddeps_test.go +++ b/src/cmd/internal/moddeps/moddeps_test.go @@ -68,7 +68,7 @@ func TestAllDependencies(t *testing.T) { // There is no vendor directory, so the module must have no dependencies. // Check that the list of active modules contains only the main module. - cmd := exec.Command(goBin, "list", "-mod=mod", "-m", "all") + cmd := exec.Command(goBin, "list", "-mod=readonly", "-m", "all") cmd.Env = append(os.Environ(), "GO111MODULE=on") cmd.Dir = m.Dir cmd.Stderr = new(strings.Builder) From 761edf71f64bb2ef949ceb588822c47d2e1cc6ac Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 16 Jun 2021 16:33:29 -0400 Subject: [PATCH 05/21] cmd/internal/moddeps: use a temporary directory for GOMODCACHE if needed CL 328770 should be sufficient to fix the specific failure in the report, but when attempting to reproduce it I noticed a related failure mode, triggered by the environment variables set in src/run.bash. The failure mode is currently masked on the Go project builders due to the lack of any 'longtest' builder running as a non-root user (#10719). It is also masked from Go contributors running 'run.bash' locally because 'run.bash' does not actually run all of the tests unless GO_TEST_SHORT=0 is set in the environment (#29266, #46054). Fixes #46695 Change-Id: I272c09dae462734590dce59b3d3c5b6d3f733c92 Reviewed-on: https://go-review.googlesource.com/c/go/+/328771 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- src/cmd/internal/moddeps/moddeps_test.go | 33 ++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/cmd/internal/moddeps/moddeps_test.go b/src/cmd/internal/moddeps/moddeps_test.go index 8d01b913c3..56c3b2585c 100644 --- a/src/cmd/internal/moddeps/moddeps_test.go +++ b/src/cmd/internal/moddeps/moddeps_test.go @@ -5,6 +5,7 @@ package moddeps_test import ( + "bytes" "encoding/json" "fmt" "internal/testenv" @@ -123,10 +124,38 @@ func TestAllDependencies(t *testing.T) { t.Skip("skipping because a diff command with support for --recursive and --unified flags is unavailable") } + // We're going to check the standard modules for tidiness, so we need a usable + // GOMODCACHE. If the default directory doesn't exist, use a temporary + // directory instead. (That can occur, for example, when running under + // run.bash with GO_TEST_SHORT=0: run.bash sets GOPATH=/nonexist-gopath, and + // GO_TEST_SHORT=0 causes it to run this portion of the test.) + var modcacheEnv []string + { + out, err := exec.Command(goBin, "env", "GOMODCACHE").Output() + if err != nil { + t.Fatalf("%s env GOMODCACHE: %v", goBin, err) + } + modcacheOk := false + if gomodcache := string(bytes.TrimSpace(out)); gomodcache != "" { + if _, err := os.Stat(gomodcache); err == nil { + modcacheOk = true + } + } + if !modcacheOk { + modcacheEnv = []string{ + "GOMODCACHE=" + t.TempDir(), + "GOFLAGS=" + os.Getenv("GOFLAGS") + " -modcacherw", // Allow t.TempDir() to clean up subdirectories. + } + } + } + // Build the bundle binary at the golang.org/x/tools // module version specified in GOROOT/src/cmd/go.mod. bundleDir := t.TempDir() - r := runner{Dir: filepath.Join(runtime.GOROOT(), "src/cmd")} + r := runner{ + Dir: filepath.Join(runtime.GOROOT(), "src/cmd"), + Env: append(os.Environ(), modcacheEnv...), + } r.run(t, goBin, "build", "-mod=readonly", "-o", bundleDir, "golang.org/x/tools/cmd/bundle") var gorootCopyDir string @@ -160,7 +189,7 @@ func TestAllDependencies(t *testing.T) { } r := runner{ Dir: filepath.Join(gorootCopyDir, rel), - Env: append(os.Environ(), + Env: append(append(os.Environ(), modcacheEnv...), // Set GOROOT. "GOROOT="+gorootCopyDir, // Explicitly override PWD and clear GOROOT_FINAL so that GOROOT=gorootCopyDir is definitely used. From 1bd5a20e3c8a3a82e87487c381b76c97b720cd52 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 18 Jun 2021 17:28:29 -0400 Subject: [PATCH 06/21] cmd/go: add a -go flag to 'go mod graph' For #46366 Change-Id: I8417e6e4dbb8cb56ff7afc16893a01b7bb938217 Reviewed-on: https://go-review.googlesource.com/c/go/+/329529 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- doc/go1.17.html | 7 ++ src/cmd/go/alldocs.go | 6 +- src/cmd/go/internal/modcmd/graph.go | 13 ++- src/cmd/go/internal/modcmd/verify.go | 3 +- src/cmd/go/internal/modget/get.go | 6 +- src/cmd/go/internal/modload/buildlist.go | 26 ++++- .../go/testdata/script/mod_graph_version.txt | 101 ++++++++++++++++++ 7 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_graph_version.txt diff --git a/doc/go1.17.html b/doc/go1.17.html index 02cd18d037..22896c8c27 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -187,6 +187,13 @@ Do not send CLs removing the interior tags from such phrases. features.

+

+ The go mod graph subcommand also + supports the -go flag, which causes it to report the graph as + seen by the indicated Go version, showing dependencies that may otherwise be + pruned out by lazy loading. +

+

Module deprecation comments

diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 27f993aeb3..fd95da23eb 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -1186,13 +1186,17 @@ // // Usage: // -// go mod graph +// go mod graph [-go=version] // // Graph prints the module requirement graph (with replacements applied) // in text form. Each line in the output has two space-separated fields: a module // and one of its requirements. Each module is identified as a string of the form // path@version, except for the main module, which has no @version suffix. // +// The -go flag causes graph to report the module graph as loaded by by the +// given Go version, instead of the version indicated by the 'go' directive +// in the go.mod file. +// // See https://golang.org/ref/mod#go-mod-graph for more about 'go mod graph'. // // diff --git a/src/cmd/go/internal/modcmd/graph.go b/src/cmd/go/internal/modcmd/graph.go index 77853304e9..903bd9970f 100644 --- a/src/cmd/go/internal/modcmd/graph.go +++ b/src/cmd/go/internal/modcmd/graph.go @@ -18,7 +18,7 @@ import ( ) var cmdGraph = &base.Command{ - UsageLine: "go mod graph", + UsageLine: "go mod graph [-go=version]", Short: "print module requirement graph", Long: ` Graph prints the module requirement graph (with replacements applied) @@ -26,12 +26,21 @@ in text form. Each line in the output has two space-separated fields: a module and one of its requirements. Each module is identified as a string of the form path@version, except for the main module, which has no @version suffix. +The -go flag causes graph to report the module graph as loaded by by the +given Go version, instead of the version indicated by the 'go' directive +in the go.mod file. + See https://golang.org/ref/mod#go-mod-graph for more about 'go mod graph'. `, Run: runGraph, } +var ( + graphGo goVersionFlag +) + func init() { + cmdGraph.Flag.Var(&graphGo, "go", "") base.AddModCommonFlags(&cmdGraph.Flag) } @@ -41,7 +50,7 @@ func runGraph(ctx context.Context, cmd *base.Command, args []string) { } modload.ForceUseModules = true modload.RootMode = modload.NeedRoot - mg := modload.LoadModGraph(ctx) + mg := modload.LoadModGraph(ctx, graphGo.String()) w := bufio.NewWriter(os.Stdout) defer w.Flush() diff --git a/src/cmd/go/internal/modcmd/verify.go b/src/cmd/go/internal/modcmd/verify.go index 5c321c783a..5a6eca32cf 100644 --- a/src/cmd/go/internal/modcmd/verify.go +++ b/src/cmd/go/internal/modcmd/verify.go @@ -54,7 +54,8 @@ func runVerify(ctx context.Context, cmd *base.Command, args []string) { sem := make(chan token, runtime.GOMAXPROCS(0)) // Use a slice of result channels, so that the output is deterministic. - mods := modload.LoadModGraph(ctx).BuildList()[1:] + const defaultGoVersion = "" + mods := modload.LoadModGraph(ctx, defaultGoVersion).BuildList()[1:] errsChans := make([]<-chan []error, len(mods)) for i, mod := range mods { diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go index ea5c4e229a..9672e5598e 100644 --- a/src/cmd/go/internal/modget/get.go +++ b/src/cmd/go/internal/modget/get.go @@ -506,7 +506,8 @@ type versionReason struct { func newResolver(ctx context.Context, queries []*query) *resolver { // LoadModGraph also sets modload.Target, which is needed by various resolver // methods. - mg := modload.LoadModGraph(ctx) + const defaultGoVersion = "" + mg := modload.LoadModGraph(ctx, defaultGoVersion) buildList := mg.BuildList() initialVersion := make(map[string]string, len(buildList)) @@ -1803,7 +1804,8 @@ func (r *resolver) updateBuildList(ctx context.Context, additions []module.Versi return false } - r.buildList = modload.LoadModGraph(ctx).BuildList() + const defaultGoVersion = "" + r.buildList = modload.LoadModGraph(ctx, defaultGoVersion).BuildList() r.buildListVersion = make(map[string]string, len(r.buildList)) for _, m := range r.buildList { r.buildListVersion[m.Path] = m.Version diff --git a/src/cmd/go/internal/modload/buildlist.go b/src/cmd/go/internal/modload/buildlist.go index 64eaa16e8b..604a57b437 100644 --- a/src/cmd/go/internal/modload/buildlist.go +++ b/src/cmd/go/internal/modload/buildlist.go @@ -403,11 +403,33 @@ func (mg *ModuleGraph) allRootsSelected() bool { // LoadModGraph loads and returns the graph of module dependencies of the main module, // without loading any packages. // +// If the goVersion string is non-empty, the returned graph is the graph +// as interpreted by the given Go version (instead of the version indicated +// in the go.mod file). +// // Modules are loaded automatically (and lazily) in LoadPackages: // LoadModGraph need only be called if LoadPackages is not, // typically in commands that care about modules but no particular package. -func LoadModGraph(ctx context.Context) *ModuleGraph { - rs, mg, err := expandGraph(ctx, LoadModFile(ctx)) +func LoadModGraph(ctx context.Context, goVersion string) *ModuleGraph { + rs := LoadModFile(ctx) + + if goVersion != "" { + depth := modDepthFromGoVersion(goVersion) + if depth == eager && rs.depth != eager { + // Use newRequirements instead of convertDepth because convertDepth + // also updates roots; here, we want to report the unmodified roots + // even though they may seem inconsistent. + rs = newRequirements(eager, rs.rootModules, rs.direct) + } + + mg, err := rs.Graph(ctx) + if err != nil { + base.Fatalf("go: %v", err) + } + return mg + } + + rs, mg, err := expandGraph(ctx, rs) if err != nil { base.Fatalf("go: %v", err) } diff --git a/src/cmd/go/testdata/script/mod_graph_version.txt b/src/cmd/go/testdata/script/mod_graph_version.txt new file mode 100644 index 0000000000..f9a73f4617 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_graph_version.txt @@ -0,0 +1,101 @@ +# For this module, Go 1.17 prunes out a (transitive and otherwise-irrelevant) +# requirement on a retracted higher version of a dependency. +# However, when Go 1.16 reads the same requirements from the go.mod file, +# it does not prune out that requirement, and selects the retracted version. +# +# The Go 1.16 module graph looks like: +# +# m ---- lazy v0.1.0 ---- requireincompatible v0.1.0 ---- incompatible v2.0.0+incompatible +# | | +# + -------+------------- incompatible v1.0.0 +# +# The Go 1.17 module graph is the same except that the dependencies of +# requireincompatible are pruned out (because the module that requires +# it — lazy v0.1.0 — specifies 'go 1.17', and it is not otherwise relevant to +# the main module). + +cp go.mod go.mod.orig + +go mod graph +cp stdout graph-1.17.txt +stdout '^example\.com/m example\.com/retract/incompatible@v1\.0\.0$' +stdout '^example\.net/lazy@v0\.1\.0 example\.com/retract/incompatible@v1\.0\.0$' +! stdout 'example\.com/retract/incompatible@v2\.0\.0\+incompatible' + +go mod graph -go=1.17 +cmp stdout graph-1.17.txt + +cmp go.mod go.mod.orig + + +# Setting -go=1.16 should report the graph as viewed by Go 1.16, +# but should not edit the go.mod file. + +go mod graph -go=1.16 +cp stdout graph-1.16.txt +stdout '^example\.com/m example\.com/retract/incompatible@v1\.0\.0$' +stdout '^example\.net/lazy@v0\.1\.0 example.com/retract/incompatible@v1\.0\.0$' +stdout '^example.net/requireincompatible@v0.1.0 example.com/retract/incompatible@v2\.0\.0\+incompatible$' + +cmp go.mod go.mod.orig + + +# If we actually update the go.mod file to the requested go version, +# we should get the same selected versions, but the roots of the graph +# may be updated. +# +# TODO(#45551): The roots should not be updated. + +go mod edit -go=1.16 +go mod graph +! stdout '^example\.com/m example\.com/retract/incompatible@v1\.0\.0$' +stdout '^example\.net/lazy@v0.1.0 example.com/retract/incompatible@v1\.0\.0$' +stdout '^example.net/requireincompatible@v0.1.0 example.com/retract/incompatible@v2\.0\.0\+incompatible$' + # TODO(#45551): cmp stdout graph-1.16.txt + + +# Unsupported go versions should be rejected, since we don't know +# what versions they would report. +! go mod graph -go=1.99999999999 +stderr '^invalid value "1\.99999999999" for flag -go: maximum supported Go version is '$goversion'\nusage: go mod graph \[-go=version\]\nRun ''go help mod graph'' for details.$' + + +-- go.mod -- +// Module m indirectly imports a package from +// example.com/retract/incompatible. Its selected version of +// that module is lower under Go 1.17 semantics than under Go 1.16. +module example.com/m + +go 1.17 + +replace ( + example.net/lazy v0.1.0 => ./lazy + example.net/requireincompatible v0.1.0 => ./requireincompatible +) + +require ( + example.com/retract/incompatible v1.0.0 // indirect + example.net/lazy v0.1.0 +) +-- lazy/go.mod -- +// Module lazy requires example.com/retract/incompatible v1.0.0. +// +// When viewed from the outside it also has a transitive dependency +// on v2.0.0+incompatible, but in lazy mode that transitive dependency +// is pruned out. +module example.net/lazy + +go 1.17 + +exclude example.com/retract/incompatible v2.0.0+incompatible + +require ( + example.com/retract/incompatible v1.0.0 + example.net/requireincompatible v0.1.0 +) +-- requireincompatible/go.mod -- +module example.net/requireincompatible + +go 1.15 + +require example.com/retract/incompatible v2.0.0+incompatible From 9afd158eb228fbe191ffa27b1a334a8837c45ef7 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 21 Jun 2021 16:10:14 -0400 Subject: [PATCH 07/21] go/parser: parse an ast.IndexExpr for a[] To be consistent with Go 1.16, and to preserve as much information in the AST as possible, parse an ast.IndexExpr with BadExpr Index for the invalid expression a[]. A go/types test had to be adjusted to account for an additional error resulting from this change. We don't have a lot of test coverage for parser error recovery, so rather than write an ad-hoc test for this issue, add a new go/types test that checks that the indexed operand is used. Updates #46403 Change-Id: I21e6ff4179746aaa50e530d4091fded450e69824 Reviewed-on: https://go-review.googlesource.com/c/go/+/329791 Trust: Robert Findley Trust: Robert Griesemer Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/go/parser/parser.go | 7 ++++++- src/go/types/testdata/examples/functions.go2 | 2 +- src/go/types/testdata/fixedbugs/issue46403.src | 11 +++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 src/go/types/testdata/fixedbugs/issue46403.src diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go index 3965641713..f10c8650af 100644 --- a/src/go/parser/parser.go +++ b/src/go/parser/parser.go @@ -1302,7 +1302,12 @@ func (p *parser) parseIndexOrSliceOrInstance(x ast.Expr) ast.Expr { p.errorExpected(p.pos, "operand") rbrack := p.pos p.next() - return &ast.BadExpr{From: x.Pos(), To: rbrack} + return &ast.IndexExpr{ + X: x, + Lbrack: lbrack, + Index: &ast.BadExpr{From: rbrack, To: rbrack}, + Rbrack: rbrack, + } } p.exprLev++ diff --git a/src/go/types/testdata/examples/functions.go2 b/src/go/types/testdata/examples/functions.go2 index fb74ae7ae2..a053471202 100644 --- a/src/go/types/testdata/examples/functions.go2 +++ b/src/go/types/testdata/examples/functions.go2 @@ -210,5 +210,5 @@ func _() { func h[] /* ERROR empty type parameter list */ () func _() { - h[] /* ERROR operand */ () + h /* ERROR cannot index */ [] /* ERROR operand */ () } diff --git a/src/go/types/testdata/fixedbugs/issue46403.src b/src/go/types/testdata/fixedbugs/issue46403.src new file mode 100644 index 0000000000..9d475222ad --- /dev/null +++ b/src/go/types/testdata/fixedbugs/issue46403.src @@ -0,0 +1,11 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package issue46403 + +func _() { + // a should be used, despite the parser error below. + var a []int + var _ = a[] // ERROR expected operand +} From 197a5ee2ab8b64c687c74b986bf92139057366b6 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 21 Jun 2021 20:14:21 -0400 Subject: [PATCH 08/21] cmd/gofmt: remove stale documentation for the -G flag This documentation remained from the original dev.typeparams merge. This flag no longer exists. Change-Id: Ic9a82071c512614dc1382780d69ef13253fca21d Reviewed-on: https://go-review.googlesource.com/c/go/+/329792 Trust: Robert Findley Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/cmd/gofmt/doc.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cmd/gofmt/doc.go b/src/cmd/gofmt/doc.go index 68476e7d44..e340665594 100644 --- a/src/cmd/gofmt/doc.go +++ b/src/cmd/gofmt/doc.go @@ -26,9 +26,6 @@ The flags are: Do not print reformatted sources to standard output. If a file's formatting is different from gofmt's, print its name to standard output. - -G - Allow generic code, using type parameters. - See golang.org/issues/43651 for more information. -r rule Apply the rewrite rule to the source before reformatting. -s From 63daa774b566d7fe58b3aa82cda9e595929bb777 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 21 Jun 2021 20:16:37 -0400 Subject: [PATCH 09/21] go/types: guard against checking instantiation when generics is disabled When type checking t[_], where t is a type name, it was possible to leak an error message related to generics. Fix this by guarding on typeparams.Enabled. In order to test this fix, we need to be able to run the new go/types test only if type parameters are disabled. Introduce the .go1 test data suffix (similar to .go2) to control this behavior. Originally found via fuzzing, though the test case was manually simplified. Updates #46404 Change-Id: Ib1e2c27cf974c2a5ca5b9d6d01b84a30ba4d583b Reviewed-on: https://go-review.googlesource.com/c/go/+/329793 Trust: Robert Findley Trust: Robert Griesemer Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/go/types/check_test.go | 13 ++++++++----- src/go/types/testdata/fixedbugs/issue46404.go1 | 8 ++++++++ src/go/types/typexpr.go | 8 ++++++-- 3 files changed, 22 insertions(+), 7 deletions(-) create mode 100644 src/go/types/testdata/fixedbugs/issue46404.go1 diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go index 6c3b630a1b..5a3e57ba44 100644 --- a/src/go/types/check_test.go +++ b/src/go/types/check_test.go @@ -207,12 +207,15 @@ func checkFiles(t *testing.T, sizes Sizes, goVersion string, filenames []string, t.Fatal("no source files") } + if strings.HasSuffix(filenames[0], ".go2") && !typeparams.Enabled { + t.Skip("type params are not enabled") + } + if strings.HasSuffix(filenames[0], ".go1") && typeparams.Enabled { + t.Skip("type params are enabled") + } + mode := parser.AllErrors - if strings.HasSuffix(filenames[0], ".go2") { - if !typeparams.Enabled { - t.Skip("type params are not enabled") - } - } else { + if !strings.HasSuffix(filenames[0], ".go2") { mode |= typeparams.DisallowParsing } diff --git a/src/go/types/testdata/fixedbugs/issue46404.go1 b/src/go/types/testdata/fixedbugs/issue46404.go1 new file mode 100644 index 0000000000..db604bc1ac --- /dev/null +++ b/src/go/types/testdata/fixedbugs/issue46404.go1 @@ -0,0 +1,8 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package issue46404 + +// Check that we don't type check t[_] as an instantiation. +type t [t /* ERROR not a type */ [_]]_ // ERROR cannot use diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index 5185c33fcb..1738d864a6 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -463,8 +463,12 @@ func (check *Checker) typInternal(e0 ast.Expr, def *Named) (T Type) { } case *ast.IndexExpr: - exprs := typeparams.UnpackExpr(e.Index) - return check.instantiatedType(e.X, exprs, def) + if typeparams.Enabled { + exprs := typeparams.UnpackExpr(e.Index) + return check.instantiatedType(e.X, exprs, def) + } + check.errorf(e0, _NotAType, "%s is not a type", e0) + check.use(e.X) case *ast.ParenExpr: // Generic types must be instantiated before they can be used in any form. From 666315b4d38b99931bb9fd158a76e59928fd2852 Mon Sep 17 00:00:00 2001 From: Xing Gao <18340825824@163.com> Date: Tue, 22 Jun 2021 02:12:29 +0000 Subject: [PATCH 10/21] runtime/internal/atomic: remove incorrect pointer indirection in comment Change-Id: I9d743b7f6b001158299bea4af4aede678654bc8e GitHub-Last-Rev: 7e07834abc861e21030fe4a8eb323bac01e18f7a GitHub-Pull-Request: golang/go#46851 Reviewed-on: https://go-review.googlesource.com/c/go/+/329730 Reviewed-by: Ian Lance Taylor Reviewed-by: Ben Shi --- src/runtime/internal/atomic/atomic_386.s | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/internal/atomic/atomic_386.s b/src/runtime/internal/atomic/atomic_386.s index 37318e0ad7..724d515231 100644 --- a/src/runtime/internal/atomic/atomic_386.s +++ b/src/runtime/internal/atomic/atomic_386.s @@ -65,7 +65,7 @@ TEXT ·Xaddint64(SB), NOSPLIT, $0-20 // bool ·Cas64(uint64 *val, uint64 old, uint64 new) // Atomically: -// if(*val == *old){ +// if(*val == old){ // *val = new; // return 1; // } else { From 5bd09e5efccf0d3df89085c9f214f94017f6e969 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 21 Jun 2021 22:20:11 -0700 Subject: [PATCH 11/21] spec: unsafe.Add/Slice are not permitted in statement context Add unsafe.Add and unsafe.Slice to the list of built-in functions which are not permitted in statement context. The compiler and type checker already enforce this restriction, this just fixes a documentation oversight. For #19367. For #40481. Change-Id: Iabc63a8db048eaf40a5f5b5573fdf00b79d54119 Reviewed-on: https://go-review.googlesource.com/c/go/+/329925 Trust: Robert Griesemer Run-TryBot: Robert Griesemer Reviewed-by: Matthew Dempsky Reviewed-by: Rob Pike TryBot-Result: Go Bot --- doc/go_spec.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/go_spec.html b/doc/go_spec.html index 561d44271a..b59b37fd55 100644 --- a/doc/go_spec.html +++ b/doc/go_spec.html @@ -1,6 +1,6 @@ @@ -4670,7 +4670,7 @@ The following built-in functions are not permitted in statement context: 

 append cap complex imag len make new real
-unsafe.Alignof unsafe.Offsetof unsafe.Sizeof
+unsafe.Add unsafe.Alignof unsafe.Offsetof unsafe.Sizeof unsafe.Slice
 

From 0ebd5a8de05823109263bef31b38be8c29d2cd54 Mon Sep 17 00:00:00 2001
From: Ian Lance Taylor 
Date: Mon, 21 Jun 2021 14:48:54 -0700
Subject: [PATCH 12/21] cmd/go: update ToolTags based on GOARCH value

The build.Context ToolTags value is set based on the set of enabled
experiments, which in turn depends on GOARCH. Before this CL the set
of experiments was being set based on GOARCH in the environment.
That is normally fine, but fails with cmd/go when somebody has run
"go env -w GOARCH=val"; in that case cmd/go changes its GOARCH value
after initialization. The new GOARCH value was affect the set of
enabled experiments, which can affect the ToolTags value. With this
CL, we update ToolTags in cmd/go based on the GOARCH value it is using.

This is a pretty ugly fix. We should do something cleaner for 1.18.

Fixes #46815

Change-Id: Ie9416781a168248813c3da8afdc257acdd3fef7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/329930
Trust: Ian Lance Taylor 
Run-TryBot: Ian Lance Taylor 
TryBot-Result: Go Bot 
Reviewed-by: Matthew Dempsky 
---
 src/cmd/go/internal/cfg/cfg.go                |  8 +++++
 .../go/testdata/script/env_cross_build.txt    | 29 +++++++++++++++++++
 src/internal/buildcfg/exp.go                  | 13 +++++++--
 3 files changed, 47 insertions(+), 3 deletions(-)
 create mode 100644 src/cmd/go/testdata/script/env_cross_build.txt

diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go
index b47eb812b5..fc6989097e 100644
--- a/src/cmd/go/internal/cfg/cfg.go
+++ b/src/cmd/go/internal/cfg/cfg.go
@@ -77,6 +77,14 @@ func defaultContext() build.Context {
 	ctxt.GOOS = envOr("GOOS", ctxt.GOOS)
 	ctxt.GOARCH = envOr("GOARCH", ctxt.GOARCH)
 
+	// The experiments flags are based on GOARCH, so they may
+	// need to change.  TODO: This should be cleaned up.
+	buildcfg.UpdateExperiments(ctxt.GOARCH)
+	ctxt.ToolTags = nil
+	for _, exp := range buildcfg.EnabledExperiments() {
+		ctxt.ToolTags = append(ctxt.ToolTags, "goexperiment."+exp)
+	}
+
 	// The go/build rule for whether cgo is enabled is:
 	//	1. If $CGO_ENABLED is set, respect it.
 	//	2. Otherwise, if this is a cross-compile, disable cgo.
diff --git a/src/cmd/go/testdata/script/env_cross_build.txt b/src/cmd/go/testdata/script/env_cross_build.txt
new file mode 100644
index 0000000000..3feeba6b14
--- /dev/null
+++ b/src/cmd/go/testdata/script/env_cross_build.txt
@@ -0,0 +1,29 @@
+# Test that the corect default GOEXPERIMENT is used when cross
+# building with GOENV (#46815).
+
+# Unset variables set by the TestScript harness. Users typically won't
+# explicitly configure these, and #46815 doesn't repro if they are.
+env GOOS=
+env GOARCH=
+env GOEXPERIMENT=
+
+env GOENV=windows-amd64
+go build internal/abi
+
+env GOENV=ios-arm64
+go build internal/abi
+
+env GOENV=linux-mips
+go build internal/abi
+
+-- windows-amd64 --
+GOOS=windows
+GOARCH=amd64
+
+-- ios-arm64 --
+GOOS=ios
+GOARCH=arm64
+
+-- linux-mips --
+GOOS=linux
+GOARCH=mips
diff --git a/src/internal/buildcfg/exp.go b/src/internal/buildcfg/exp.go
index 2435a79dce..640aa1934d 100644
--- a/src/internal/buildcfg/exp.go
+++ b/src/internal/buildcfg/exp.go
@@ -18,7 +18,7 @@ import (
 //
 // (This is not necessarily the set of experiments the compiler itself
 // was built with.)
-var Experiment goexperiment.Flags = parseExperiments()
+var Experiment goexperiment.Flags = parseExperiments(GOARCH)
 
 var regabiSupported = GOARCH == "amd64" && (GOOS == "android" || GOOS == "linux" || GOOS == "darwin" || GOOS == "windows")
 
@@ -42,7 +42,7 @@ var experimentBaseline = goexperiment.Flags{
 // Note: must agree with runtime.framepointer_enabled.
 var FramePointerEnabled = GOARCH == "amd64" || GOARCH == "arm64"
 
-func parseExperiments() goexperiment.Flags {
+func parseExperiments(goarch string) goexperiment.Flags {
 	// Start with the statically enabled set of experiments.
 	flags := experimentBaseline
 
@@ -99,7 +99,7 @@ func parseExperiments() goexperiment.Flags {
 	}
 
 	// regabi is only supported on amd64.
-	if GOARCH != "amd64" {
+	if goarch != "amd64" {
 		flags.RegabiWrappers = false
 		flags.RegabiG = false
 		flags.RegabiReflect = false
@@ -165,3 +165,10 @@ func EnabledExperiments() []string {
 func AllExperiments() []string {
 	return expList(&Experiment, nil, true)
 }
+
+// UpdateExperiments updates the Experiment global based on a new GOARCH value.
+// This is only required for cmd/go, which can change GOARCH after
+// program startup due to use of "go env -w".
+func UpdateExperiments(goarch string) {
+	Experiment = parseExperiments(goarch)
+}

From 222ed1b38af0fb6f83f80062092a267dcbd354df Mon Sep 17 00:00:00 2001
From: siddharth 
Date: Mon, 21 Jun 2021 21:50:09 -0400
Subject: [PATCH 13/21] os: enable TestFifoEOF on openbsd

The test successfully runs on currently supported versions (6.8 and
6.9) of openbsd.

Fixes #25877

Change-Id: I2694f08c5596b486453c2ac829f17b8bc455f828
Reviewed-on: https://go-review.googlesource.com/c/go/+/329732
Run-TryBot: Ian Lance Taylor 
TryBot-Result: Go Bot 
Reviewed-by: Ian Lance Taylor 
Trust: Tobias Klauser 
---
 src/os/fifo_test.go | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/os/fifo_test.go b/src/os/fifo_test.go
index 9b262f8205..007ed29129 100644
--- a/src/os/fifo_test.go
+++ b/src/os/fifo_test.go
@@ -26,9 +26,6 @@ func TestFifoEOF(t *testing.T) {
 	switch runtime.GOOS {
 	case "android":
 		t.Skip("skipping on Android; mkfifo syscall not available")
-	case "openbsd":
-		// On OpenBSD 6.2 this test just hangs for some reason.
-		t.Skip("skipping on OpenBSD; issue 25877")
 	}
 
 	dir := t.TempDir()

From 73496e0df0ba4284f460d1955ddf6bb096957c9f Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" 
Date: Tue, 22 Jun 2021 21:24:57 -0400
Subject: [PATCH 14/21] net: use absDomainName in the Windows lookupPTR test
 helper

The real net code uses subtle heuristics to transform a domain name
to its absolute form. Since lookupPTR isn't checking that
transformation specifically, it should use the real code instead of
using a different heuristic.

Fixes #46882

Change-Id: I503357e0f62059c37c359cd54b44d343c7d5ab2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/330249
Trust: Bryan C. Mills 
Trust: Alex Brainman 
Run-TryBot: Bryan C. Mills 
TryBot-Result: Go Bot 
Reviewed-by: Alex Brainman 
Reviewed-by: Ian Lance Taylor 
---
 src/net/lookup_windows_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/net/lookup_windows_test.go b/src/net/lookup_windows_test.go
index 62b61ed6c2..aa95501d02 100644
--- a/src/net/lookup_windows_test.go
+++ b/src/net/lookup_windows_test.go
@@ -299,7 +299,7 @@ func lookupPTR(name string) (ptr []string, err error) {
 	ptr = make([]string, 0, 10)
 	rx := regexp.MustCompile(`(?m)^Pinging\s+([a-zA-Z0-9.\-]+)\s+\[.*$`)
 	for _, ans := range rx.FindAllStringSubmatch(r, -1) {
-		ptr = append(ptr, ans[1]+".")
+		ptr = append(ptr, absDomainName([]byte(ans[1])))
 	}
 	return
 }

From 44a12e5f33bed2189735d8466b38fe455fe9b752 Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" 
Date: Wed, 23 Jun 2021 15:28:37 -0400
Subject: [PATCH 15/21] cmd/go: search breadth-first instead of depth-first for
 test dependency cycles

When we are looking for a dependency cycle involving a specific
package, we need to keep track of visited packages in order to avoid
repeatedly traversing a cycle that does not involve that package.

If we're keeping track of all visited packages anyway, we're already
spending O(N) memory on the traversal, so we may as well use
breadth-first search. That not only keeps the bookkeeping simple, but
also guarantees that we will find a shortest path (rather than a
completely arbitrary one).

Fixes #45863

Change-Id: I810c7337857e42dcb83630abbdea75021554be45
Reviewed-on: https://go-review.googlesource.com/c/go/+/330430
Trust: Bryan C. Mills 
Reviewed-by: Jay Conrod 
---
 src/cmd/go/internal/load/test.go              | 50 +++++++++++++------
 .../testdata/script/mod_list_test_cycle.txt   | 23 +++++++++
 2 files changed, 59 insertions(+), 14 deletions(-)
 create mode 100644 src/cmd/go/testdata/script/mod_list_test_cycle.txt

diff --git a/src/cmd/go/internal/load/test.go b/src/cmd/go/internal/load/test.go
index 6baa1db14f..c828296566 100644
--- a/src/cmd/go/internal/load/test.go
+++ b/src/cmd/go/internal/load/test.go
@@ -116,7 +116,7 @@ func TestPackagesAndErrors(ctx context.Context, opts PackageOpts, p *Package, co
 			// Can't change that code, because that code is only for loading the
 			// non-test copy of a package.
 			ptestErr = &PackageError{
-				ImportStack:   testImportStack(stk[0], p1, p.ImportPath),
+				ImportStack:   importCycleStack(p1, p.ImportPath),
 				Err:           errors.New("import cycle not allowed in test"),
 				IsImportCycle: true,
 			}
@@ -375,22 +375,44 @@ func TestPackagesAndErrors(ctx context.Context, opts PackageOpts, p *Package, co
 	return pmain, ptest, pxtest
 }
 
-func testImportStack(top string, p *Package, target string) []string {
-	stk := []string{top, p.ImportPath}
-Search:
-	for p.ImportPath != target {
-		for _, p1 := range p.Internal.Imports {
-			if p1.ImportPath == target || str.Contains(p1.Deps, target) {
-				stk = append(stk, p1.ImportPath)
-				p = p1
-				continue Search
+// importCycleStack returns an import stack from p to the package whose import
+// path is target.
+func importCycleStack(p *Package, target string) []string {
+	// importerOf maps each import path to its importer nearest to p.
+	importerOf := map[string]string{p.ImportPath: ""}
+
+	// q is a breadth-first queue of packages to search for target.
+	// Every package added to q has a corresponding entry in pathTo.
+	//
+	// We search breadth-first for two reasons:
+	//
+	// 	1. We want to report the shortest cycle.
+	//
+	// 	2. If p contains multiple cycles, the first cycle we encounter might not
+	// 	   contain target. To ensure termination, we have to break all cycles
+	// 	   other than the first.
+	q := []*Package{p}
+
+	for len(q) > 0 {
+		p := q[0]
+		q = q[1:]
+		if path := p.ImportPath; path == target {
+			var stk []string
+			for path != "" {
+				stk = append(stk, path)
+				path = importerOf[path]
+			}
+			return stk
+		}
+		for _, dep := range p.Internal.Imports {
+			if _, ok := importerOf[dep.ImportPath]; !ok {
+				importerOf[dep.ImportPath] = p.ImportPath
+				q = append(q, dep)
 			}
 		}
-		// Can't happen, but in case it does...
-		stk = append(stk, "")
-		break
 	}
-	return stk
+
+	panic("lost path to cycle")
 }
 
 // recompileForTest copies and replaces certain packages in pmain's dependency
diff --git a/src/cmd/go/testdata/script/mod_list_test_cycle.txt b/src/cmd/go/testdata/script/mod_list_test_cycle.txt
new file mode 100644
index 0000000000..755e50b076
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_list_test_cycle.txt
@@ -0,0 +1,23 @@
+# https://golang.org/issue/45863: a typo in a test package leading to an
+# import cycle should be diagnosed, instead of causing an infinite loop.
+# The failure mode of this test prior to the fix was a timeout or OOM crash.
+
+go list -e -test -deps ./datastore/sql
+
+-- go.mod --
+module golang.org/issue45863
+
+go 1.17
+-- datastore/datastore_health.go --
+package datastore
+
+import (
+	"golang.org/issue45863/datastore"
+	"golang.org/issue45863/datastore/sql"
+)
+-- datastore/sql/sql.go --
+package sql
+-- datastore/sql/sql_test.go --
+package sql
+
+import _ "golang.org/issue45863/datastore"

From 86d72fa2cba51342ba5617abf43a732f9fd668ca Mon Sep 17 00:00:00 2001
From: Andy Pan 
Date: Wed, 23 Jun 2021 12:59:48 +0800
Subject: [PATCH 16/21] time: handle invalid UTF-8 byte sequences in quote to
 prevent panic

Fixes #46883
Updates CL 267017

Change-Id: I15c307bfb0aaa2877a148d32527681f79df1a650
Reviewed-on: https://go-review.googlesource.com/c/go/+/330289
Reviewed-by: Kevin Burke 
Reviewed-by: Ian Lance Taylor 
Run-TryBot: Ian Lance Taylor 
TryBot-Result: Go Bot 
Trust: Emmanuel Odeke 
---
 src/time/format.go    | 18 +++++++++++++++---
 src/time/time_test.go |  5 +++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/time/format.go b/src/time/format.go
index 6040ed5aeb..bb173a21c2 100644
--- a/src/time/format.go
+++ b/src/time/format.go
@@ -751,8 +751,11 @@ type ParseError struct {
 
 // These are borrowed from unicode/utf8 and strconv and replicate behavior in
 // that package, since we can't take a dependency on either.
-const runeSelf = 0x80
-const lowerhex = "0123456789abcdef"
+const (
+	lowerhex  = "0123456789abcdef"
+	runeSelf  = 0x80
+	runeError = '\uFFFD'
+)
 
 func quote(s string) string {
 	buf := make([]byte, 1, len(s)+2) // slice will be at least len(s) + quotes
@@ -765,7 +768,16 @@ func quote(s string) string {
 			// reproduce strconv.Quote's behavior with full fidelity but
 			// given how rarely we expect to hit these edge cases, speed and
 			// conciseness are better.
-			for j := 0; j < len(string(c)) && j < len(s); j++ {
+			var width int
+			if c == runeError {
+				width = 1
+				if i+2 < len(s) && s[i:i+3] == string(runeError) {
+					width = 3
+				}
+			} else {
+				width = len(string(c))
+			}
+			for j := 0; j < width; j++ {
 				buf = append(buf, `\x`...)
 				buf = append(buf, lowerhex[s[i+j]>>4])
 				buf = append(buf, lowerhex[s[i+j]&0xF])
diff --git a/src/time/time_test.go b/src/time/time_test.go
index f272bbd558..cea5f2d3f5 100644
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -917,6 +917,11 @@ var parseDurationErrorTests = []struct {
 	{".s", `".s"`},
 	{"+.s", `"+.s"`},
 	{"1d", `"1d"`},
+	{"\x85\x85", `"\x85\x85"`},
+	{"\xffff", `"\xffff"`},
+	{"hello \xffff world", `"hello \xffff world"`},
+	{"\uFFFD", `"\xef\xbf\xbd"`},                                             // utf8.RuneError
+	{"\uFFFD hello \uFFFD world", `"\xef\xbf\xbd hello \xef\xbf\xbd world"`}, // utf8.RuneError
 	// overflow
 	{"9223372036854775810ns", `"9223372036854775810ns"`},
 	{"9223372036854775808ns", `"9223372036854775808ns"`},

From a9bb38222a06333176cafc5637083e0322277c09 Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" 
Date: Tue, 22 Jun 2021 21:48:11 -0400
Subject: [PATCH 17/21] net: remove hard-coded timeout in dialClosedPort test
 helper

The helper function claims that dialing a closed port should be
"nearly instantaneous", but that is empirically not the case on
OpenBSD or Windows. The tests do not appear to be particularly
sensitive to the exact upper bound otherwise, so let's just
remove the arbitrary latency assumption.

Fixes #46884

Change-Id: If00c9fdc3063da6aaf60d365d4a2ee2c94dc6df1
Reviewed-on: https://go-review.googlesource.com/c/go/+/330250
Trust: Bryan C. Mills 
Run-TryBot: Bryan C. Mills 
TryBot-Result: Go Bot 
Reviewed-by: Ian Lance Taylor 
---
 src/net/dial_test.go | 51 ++++++++++++++------------------------------
 1 file changed, 16 insertions(+), 35 deletions(-)

diff --git a/src/net/dial_test.go b/src/net/dial_test.go
index f899da10cf..723038c7a2 100644
--- a/src/net/dial_test.go
+++ b/src/net/dial_test.go
@@ -155,40 +155,27 @@ func slowDialTCP(ctx context.Context, network string, laddr, raddr *TCPAddr) (*T
 	return c, err
 }
 
-func dialClosedPort(t *testing.T) (actual, expected time.Duration) {
-	// Estimate the expected time for this platform.
-	// On Windows, dialing a closed port takes roughly 1 second,
-	// but other platforms should be instantaneous.
-	if runtime.GOOS == "windows" {
-		expected = 1500 * time.Millisecond
-	} else if runtime.GOOS == "darwin" || runtime.GOOS == "ios" {
-		expected = 150 * time.Millisecond
-	} else {
-		expected = 95 * time.Millisecond
-	}
+func dialClosedPort(t *testing.T) (dialLatency time.Duration) {
+	// On most platforms, dialing a closed port should be nearly instantaneous —
+	// less than a few hundred milliseconds. However, on some platforms it may be
+	// much slower: on Windows and OpenBSD, it has been observed to take up to a
+	// few seconds.
 
 	l, err := Listen("tcp", "127.0.0.1:0")
 	if err != nil {
-		t.Logf("dialClosedPort: Listen failed: %v", err)
-		return 999 * time.Hour, expected
+		t.Fatalf("dialClosedPort: Listen failed: %v", err)
 	}
 	addr := l.Addr().String()
 	l.Close()
-	// On OpenBSD, interference from TestTCPSelfConnect is mysteriously
-	// causing the first attempt to hang for a few seconds, so we throw
-	// away the first result and keep the second.
-	for i := 1; ; i++ {
-		startTime := time.Now()
-		c, err := Dial("tcp", addr)
-		if err == nil {
-			c.Close()
-		}
-		elapsed := time.Now().Sub(startTime)
-		if i == 2 {
-			t.Logf("dialClosedPort: measured delay %v", elapsed)
-			return elapsed, expected
-		}
+
+	startTime := time.Now()
+	c, err := Dial("tcp", addr)
+	if err == nil {
+		c.Close()
 	}
+	elapsed := time.Now().Sub(startTime)
+	t.Logf("dialClosedPort: measured delay %v", elapsed)
+	return elapsed
 }
 
 func TestDialParallel(t *testing.T) {
@@ -198,10 +185,7 @@ func TestDialParallel(t *testing.T) {
 		t.Skip("both IPv4 and IPv6 are required")
 	}
 
-	closedPortDelay, expectClosedPortDelay := dialClosedPort(t)
-	if closedPortDelay > expectClosedPortDelay {
-		t.Errorf("got %v; want <= %v", closedPortDelay, expectClosedPortDelay)
-	}
+	closedPortDelay := dialClosedPort(t)
 
 	const instant time.Duration = 0
 	const fallbackDelay = 200 * time.Millisecond
@@ -675,10 +659,7 @@ func TestDialerDualStack(t *testing.T) {
 		t.Skip("both IPv4 and IPv6 are required")
 	}
 
-	closedPortDelay, expectClosedPortDelay := dialClosedPort(t)
-	if closedPortDelay > expectClosedPortDelay {
-		t.Errorf("got %v; want <= %v", closedPortDelay, expectClosedPortDelay)
-	}
+	closedPortDelay := dialClosedPort(t)
 
 	origTestHookLookupIP := testHookLookupIP
 	defer func() { testHookLookupIP = origTestHookLookupIP }()

From 600a2a4ffb9a273a3a1635b60120ffc768741aa9 Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" 
Date: Wed, 23 Jun 2021 23:29:10 -0400
Subject: [PATCH 18/21] cmd/go: don't try to add replaced versions that won't
 be selected

In Go 1.12, we added a heuristic to 'go mod tidy' to resolve packages
by adding replaced-but-not-required modules before falling back to
searching for modules from the network. Unfortunately, that heuristic
fails when the replaced version is already lower than the selected
version: adding such a module to the build list doesn't change the
selected version of that module, and so it doesn't make progress
toward resolving the missing package.

Fixes #46659

Change-Id: I75e2387d5290e769f6b0fa1231dcc4605db68597
Reviewed-on: https://go-review.googlesource.com/c/go/+/330432
Trust: Bryan C. Mills 
Run-TryBot: Bryan C. Mills 
TryBot-Result: Go Bot 
Reviewed-by: Jay Conrod 
---
 src/cmd/go/internal/modload/import.go         |  9 +++++
 .../testdata/script/mod_tidy_replace_old.txt  | 34 +++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100644 src/cmd/go/testdata/script/mod_tidy_replace_old.txt

diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go
index 60bd26fb22..d2bbe5cbe0 100644
--- a/src/cmd/go/internal/modload/import.go
+++ b/src/cmd/go/internal/modload/import.go
@@ -428,6 +428,15 @@ func queryImport(ctx context.Context, path string, rs *Requirements) (module.Ver
 					mv = module.ZeroPseudoVersion("v0")
 				}
 			}
+			mg, err := rs.Graph(ctx)
+			if err != nil {
+				return module.Version{}, err
+			}
+			if cmpVersion(mg.Selected(mp), mv) >= 0 {
+				// We can't resolve the import by adding mp@mv to the module graph,
+				// because the selected version of mp is already at least mv.
+				continue
+			}
 			mods = append(mods, module.Version{Path: mp, Version: mv})
 		}
 
diff --git a/src/cmd/go/testdata/script/mod_tidy_replace_old.txt b/src/cmd/go/testdata/script/mod_tidy_replace_old.txt
new file mode 100644
index 0000000000..cfd3792600
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_tidy_replace_old.txt
@@ -0,0 +1,34 @@
+# Regression test for https://golang.org/issue/46659.
+#
+# If a 'replace' directive specifies an older-than-selected version of a module,
+# 'go mod tidy' shouldn't try to add that version to the build list to resolve a
+# missing package: it won't be selected, and would cause the module loader to
+# loop indefinitely trying to resolve the package.
+
+cp go.mod go.mod.orig
+
+! go mod tidy
+! stderr panic
+stderr '^golang\.org/issue46659 imports\n\texample\.com/missingpkg/deprecated: package example\.com/missingpkg/deprecated provided by example\.com/missingpkg at latest version v1\.0\.0 but not at required version v1\.0\.1-beta$'
+
+go mod tidy -e
+
+cmp go.mod go.mod.orig
+
+-- go.mod --
+module golang.org/issue46659
+
+go 1.17
+
+replace example.com/missingpkg v1.0.1-alpha => example.com/missingpkg v1.0.0
+
+require example.com/usemissingpre v1.0.0
+
+require example.com/missingpkg v1.0.1-beta // indirect
+-- m.go --
+package m
+
+import (
+	_ "example.com/missingpkg/deprecated"
+	_ "example.com/usemissingpre"
+)

From cce621431a9bce86527b25898a01a7a693cc56a8 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le 
Date: Fri, 25 Jun 2021 01:45:32 +0700
Subject: [PATCH 19/21] cmd/compile: fix wrong type in SSA generation for
 OSLICE2ARRPTR

Fixes #46907

Change-Id: I6a2728d2f2159df583b32f40f6100d3e90c34dd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/330672
Trust: Cuong Manh Le 
Run-TryBot: Cuong Manh Le 
Reviewed-by: Matthew Dempsky 
TryBot-Result: Go Bot 
---
 src/cmd/compile/internal/ssagen/ssa.go |  2 +-
 test/fixedbugs/issue46907.go           | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 test/fixedbugs/issue46907.go

diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index 004e084f72..f1dc56e729 100644
--- a/src/cmd/compile/internal/ssagen/ssa.go
+++ b/src/cmd/compile/internal/ssagen/ssa.go
@@ -3174,7 +3174,7 @@ func (s *state) expr(n ir.Node) *ssa.Value {
 		arrlen := s.constInt(types.Types[types.TINT], n.Type().Elem().NumElem())
 		cap := s.newValue1(ssa.OpSliceLen, types.Types[types.TINT], v)
 		s.boundsCheck(arrlen, cap, ssa.BoundsConvert, false)
-		return s.newValue1(ssa.OpSlicePtrUnchecked, types.Types[types.TINT], v)
+		return s.newValue1(ssa.OpSlicePtrUnchecked, n.Type(), v)
 
 	case ir.OCALLFUNC:
 		n := n.(*ir.CallExpr)
diff --git a/test/fixedbugs/issue46907.go b/test/fixedbugs/issue46907.go
new file mode 100644
index 0000000000..bd82f4f2b1
--- /dev/null
+++ b/test/fixedbugs/issue46907.go
@@ -0,0 +1,11 @@
+// compile
+
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+func f(b []byte) []byte {
+	return (*[32]byte)(b[:32])[:]
+}

From c309c89db5aa15e1dad486c49ed4fd1babd23360 Mon Sep 17 00:00:00 2001
From: Matthew Dempsky 
Date: Thu, 24 Jun 2021 10:44:39 -0700
Subject: [PATCH 20/21] reflect: document that InterfaceData is a low-entropy
 RNG

Change-Id: Ie26b9060630e2e774ac23d8492eaaf785bfca6b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/330709
Reviewed-by: Ian Lance Taylor 
Trust: Matthew Dempsky 
---
 src/reflect/value.go | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/reflect/value.go b/src/reflect/value.go
index 6ba6202a1a..9dce251ac5 100644
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -1381,10 +1381,16 @@ func valueInterface(v Value, safe bool) interface{} {
 	return packEface(v)
 }
 
-// InterfaceData returns the interface v's value as a uintptr pair.
+// InterfaceData returns a pair of unspecified uintptr values.
 // It panics if v's Kind is not Interface.
+//
+// In earlier versions of Go, this function returned the interface's
+// value as a uintptr pair. As of Go 1.4, the implementation of
+// interface values precludes any defined use of InterfaceData.
+//
+// Deprecated: The memory representation of interface values is not
+// compatible with InterfaceData.
 func (v Value) InterfaceData() [2]uintptr {
-	// TODO: deprecate this
 	v.mustBe(Interface)
 	// We treat this as a read operation, so we allow
 	// it even for unexported data, because the caller

From 37f9a8f69d6299783eac8848d87e27eb563500ac Mon Sep 17 00:00:00 2001
From: Rob Findley 
Date: Thu, 24 Jun 2021 11:01:49 -0400
Subject: [PATCH 21/21] go/types: fix a bug in package qualification logic

CL 313035 had a bug, initializing pkgPathMap by walking the imported
package being considered rather than check.pkg.

Fix this, and enhance our tests to exercise this bug as well as other
edge cases.

Also fix error assertions in issues.src to not use quotation marks
inside the error regexp. The check tests only matched the error regexp
up to the first quotation mark.

Fixes #46905

Change-Id: I6aa8eae4bec6495006a5c03fc063db0d66b44cd6
Reviewed-on: https://go-review.googlesource.com/c/go/+/330629
Trust: Robert Findley 
Trust: Robert Griesemer 
Run-TryBot: Robert Findley 
TryBot-Result: Go Bot 
Reviewed-by: Robert Griesemer 
---
 src/go/types/check_test.go             | 15 +++---
 src/go/types/errors.go                 |  2 +-
 src/go/types/issues_test.go            | 72 +++++++++++++++++---------
 src/go/types/testdata/check/issues.src |  4 +-
 4 files changed, 59 insertions(+), 34 deletions(-)

diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go
index 5a3e57ba44..c85a8e46fb 100644
--- a/src/go/types/check_test.go
+++ b/src/go/types/check_test.go
@@ -202,7 +202,7 @@ func asGoVersion(s string) string {
 	return ""
 }
 
-func checkFiles(t *testing.T, sizes Sizes, goVersion string, filenames []string, srcs [][]byte, manual bool) {
+func checkFiles(t *testing.T, sizes Sizes, goVersion string, filenames []string, srcs [][]byte, manual bool, imp Importer) {
 	if len(filenames) == 0 {
 		t.Fatal("no source files")
 	}
@@ -252,7 +252,10 @@ func checkFiles(t *testing.T, sizes Sizes, goVersion string, filenames []string,
 		}
 	}
 
-	conf.Importer = importer.Default()
+	conf.Importer = imp
+	if imp == nil {
+		conf.Importer = importer.Default()
+	}
 	conf.Error = func(err error) {
 		if *haltOnError {
 			defer panic(err)
@@ -322,7 +325,7 @@ func TestManual(t *testing.T) {
 func TestLongConstants(t *testing.T) {
 	format := "package longconst\n\nconst _ = %s\nconst _ = %s // ERROR excessively long constant"
 	src := fmt.Sprintf(format, strings.Repeat("1", 9999), strings.Repeat("1", 10001))
-	checkFiles(t, nil, "", []string{"longconst.go"}, [][]byte{[]byte(src)}, false)
+	checkFiles(t, nil, "", []string{"longconst.go"}, [][]byte{[]byte(src)}, false, nil)
 }
 
 // TestIndexRepresentability tests that constant index operands must
@@ -330,7 +333,7 @@ func TestLongConstants(t *testing.T) {
 // represent larger values.
 func TestIndexRepresentability(t *testing.T) {
 	const src = "package index\n\nvar s []byte\nvar _ = s[int64 /* ERROR \"int64\\(1\\) << 40 \\(.*\\) overflows int\" */ (1) << 40]"
-	checkFiles(t, &StdSizes{4, 4}, "", []string{"index.go"}, [][]byte{[]byte(src)}, false)
+	checkFiles(t, &StdSizes{4, 4}, "", []string{"index.go"}, [][]byte{[]byte(src)}, false, nil)
 }
 
 func TestIssue46453(t *testing.T) {
@@ -338,7 +341,7 @@ func TestIssue46453(t *testing.T) {
 		t.Skip("type params are enabled")
 	}
 	const src = "package p\ntype _ comparable // ERROR \"undeclared name: comparable\""
-	checkFiles(t, nil, "", []string{"issue46453.go"}, [][]byte{[]byte(src)}, false)
+	checkFiles(t, nil, "", []string{"issue46453.go"}, [][]byte{[]byte(src)}, false, nil)
 }
 
 func TestCheck(t *testing.T)     { DefPredeclaredTestFuncs(); testDir(t, "check") }
@@ -388,5 +391,5 @@ func testPkg(t *testing.T, filenames []string, goVersion string, manual bool) {
 		}
 		srcs[i] = src
 	}
-	checkFiles(t, nil, goVersion, filenames, srcs, manual)
+	checkFiles(t, nil, goVersion, filenames, srcs, manual, nil)
 }
diff --git a/src/go/types/errors.go b/src/go/types/errors.go
index 19e9ae8d44..2263106417 100644
--- a/src/go/types/errors.go
+++ b/src/go/types/errors.go
@@ -31,7 +31,7 @@ func (check *Checker) qualifier(pkg *Package) string {
 		if check.pkgPathMap == nil {
 			check.pkgPathMap = make(map[string]map[string]bool)
 			check.seenPkgMap = make(map[*Package]bool)
-			check.markImports(pkg)
+			check.markImports(check.pkg)
 		}
 		// If the same package name was used by multiple packages, display the full path.
 		if len(check.pkgPathMap[pkg.name]) > 1 {
diff --git a/src/go/types/issues_test.go b/src/go/types/issues_test.go
index 44926919ef..519e199536 100644
--- a/src/go/types/issues_test.go
+++ b/src/go/types/issues_test.go
@@ -577,42 +577,64 @@ func TestIssue44515(t *testing.T) {
 }
 
 func TestIssue43124(t *testing.T) {
-	// TODO(rFindley) enhance the testdata tests to be able to express this type
-	//                of setup.
+	// TODO(rFindley) move this to testdata by enhancing support for importing.
 
 	// All involved packages have the same name (template). Error messages should
 	// disambiguate between text/template and html/template by printing the full
 	// path.
 	const (
 		asrc = `package a; import "text/template"; func F(template.Template) {}; func G(int) {}`
-		bsrc = `package b; import ("a"; "html/template"); func _() { a.F(template.Template{}) }`
-		csrc = `package c; import ("a"; "html/template"); func _() { a.G(template.Template{}) }`
+		bsrc = `
+package b
+
+import (
+	"a"
+	"html/template"
+)
+
+func _() {
+	// Packages should be fully qualified when there is ambiguity within the
+	// error string itself.
+	a.F(template /* ERROR cannot use.*html/template.* as .*text/template */ .Template{})
+}
+`
+		csrc = `
+package c
+
+import (
+	"a"
+	"fmt"
+	"html/template"
+)
+
+// Issue #46905: make sure template is not the first package qualified.
+var _ fmt.Stringer = 1 // ERROR cannot use 1.*as fmt\.Stringer
+
+// Packages should be fully qualified when there is ambiguity in reachable
+// packages. In this case both a (and for that matter html/template) import
+// text/template.
+func _() { a.G(template /* ERROR cannot use .*html/template.*Template */ .Template{}) }
+`
+
+		tsrc = `
+package template
+
+import "text/template"
+
+type T int
+
+// Verify that the current package name also causes disambiguation.
+var _ T = template /* ERROR cannot use.*text/template.* as T value */.Template{}
+`
 	)
 
 	a, err := pkgFor("a", asrc, nil)
 	if err != nil {
 		t.Fatalf("package a failed to typecheck: %v", err)
 	}
-	conf := Config{Importer: importHelper{pkg: a, fallback: importer.Default()}}
+	imp := importHelper{pkg: a, fallback: importer.Default()}
 
-	// Packages should be fully qualified when there is ambiguity within the
-	// error string itself.
-	bast := mustParse(t, bsrc)
-	_, err = conf.Check(bast.Name.Name, fset, []*ast.File{bast}, nil)
-	if err == nil {
-		t.Fatal("package b had no errors")
-	}
-	if !strings.Contains(err.Error(), "text/template") || !strings.Contains(err.Error(), "html/template") {
-		t.Errorf("type checking error for b does not disambiguate package template: %q", err)
-	}
-
-	// ...and also when there is any ambiguity in reachable packages.
-	cast := mustParse(t, csrc)
-	_, err = conf.Check(cast.Name.Name, fset, []*ast.File{cast}, nil)
-	if err == nil {
-		t.Fatal("package c had no errors")
-	}
-	if !strings.Contains(err.Error(), "html/template") {
-		t.Errorf("type checking error for c does not disambiguate package template: %q", err)
-	}
+	checkFiles(t, nil, "", []string{"b.go"}, [][]byte{[]byte(bsrc)}, false, imp)
+	checkFiles(t, nil, "", []string{"c.go"}, [][]byte{[]byte(csrc)}, false, imp)
+	checkFiles(t, nil, "", []string{"t.go"}, [][]byte{[]byte(tsrc)}, false, imp)
 }
diff --git a/src/go/types/testdata/check/issues.src b/src/go/types/testdata/check/issues.src
index e2ac06759b..74d185cbc3 100644
--- a/src/go/types/testdata/check/issues.src
+++ b/src/go/types/testdata/check/issues.src
@@ -332,7 +332,7 @@ func issue28281g() (... /* ERROR can only use ... with final parameter */ TT)
 func issue26234a(f *syn.File) {
 	// The error message below should refer to the actual package name (syntax)
 	// not the local package name (syn).
-	f.foo /* ERROR f.foo undefined \(type \*syntax.File has no field or method foo\) */
+	f.foo /* ERROR f\.foo undefined \(type \*syntax\.File has no field or method foo\) */
 }
 
 type T struct {
@@ -361,7 +361,7 @@ func issue35895() {
 
 	// Because both t1 and t2 have the same global package name (template),
 	// qualify packages with full path name in this case.
-	var _ t1.Template = t2 /* ERROR cannot use .* \(value of type "html/template".Template\) as "text/template".Template */ .Template{}
+	var _ t1.Template = t2 /* ERROR cannot use .* \(value of type .html/template.\.Template\) as .text/template.\.Template */ .Template{}
 }
 
 func issue42989(s uint) {