From 909dd5e010c99d48f1dc72d7da61fd8d3fd8f030 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 7 Jun 2021 10:51:33 -0700 Subject: [PATCH 01/45] strconv: ParseFloat: always return ErrSyntax for bad syntax Previously we would sometimes return ErrRange if the parseable part of the floating point number was out of range. Fixes #46628 Change-Id: I15bbbb1e2a56fa27c19fe25ab5554d988cbfd9d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/325750 Trust: Ian Lance Taylor Trust: Robert Griesemer Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/strconv/atof.go | 2 +- src/strconv/atof_test.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/strconv/atof.go b/src/strconv/atof.go index 9010a66ca8..57556c7047 100644 --- a/src/strconv/atof.go +++ b/src/strconv/atof.go @@ -689,7 +689,7 @@ func atof64(s string) (f float64, n int, err error) { // as their respective special floating point values. It ignores case when matching. func ParseFloat(s string, bitSize int) (float64, error) { f, n, err := parseFloatPrefix(s, bitSize) - if err == nil && n != len(s) { + if n != len(s) && (err == nil || err.(*NumError).Err != ErrSyntax) { return 0, syntaxError(fnParseFloat, s) } return f, err diff --git a/src/strconv/atof_test.go b/src/strconv/atof_test.go index 3c058b9be5..aa587a473c 100644 --- a/src/strconv/atof_test.go +++ b/src/strconv/atof_test.go @@ -342,6 +342,9 @@ var atoftests = []atofTest{ {"0x12.345p-_12", "0", ErrSyntax}, {"0x12.345p+1__2", "0", ErrSyntax}, {"0x12.345p+12_", "0", ErrSyntax}, + + {"1e100x", "0", ErrSyntax}, + {"1e1000x", "0", ErrSyntax}, } var atof32tests = []atofTest{ From dc8b55895166c808b02e93ef4a778c6648c10bf3 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 3 Jun 2021 14:15:12 -0700 Subject: [PATCH 02/45] cmd/dist: pass -Wno-lto-type-mismatch in swig_callback_lto Fixes #46557 Change-Id: I95200ddd60e2879db15dd7353c2152b515c89020 Reviewed-on: https://go-review.googlesource.com/c/go/+/324909 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Damien Neil --- src/cmd/dist/test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index bc49c6d804..1ed2c0f631 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -737,9 +737,9 @@ func (t *tester) registerTests() { fn: func(dt *distTest) error { cmd := t.addCmd(dt, "misc/swig/callback", t.goTest()) cmd.Env = append(os.Environ(), - "CGO_CFLAGS=-flto", - "CGO_CXXFLAGS=-flto", - "CGO_LDFLAGS=-flto", + "CGO_CFLAGS=-flto -Wno-lto-type-mismatch", + "CGO_CXXFLAGS=-flto -Wno-lto-type-mismatch", + "CGO_LDFLAGS=-flto -Wno-lto-type-mismatch", ) return nil }, From 39c39ae52f0f35085be8ca1e004dee509abc21de Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Mon, 7 Jun 2021 14:28:14 -0700 Subject: [PATCH 03/45] doc: document Go 1.17 language changes Fixes #46020. Change-Id: Iadf9a0ac4a8863e17155d6ba1af2cc497634a634 Reviewed-on: https://go-review.googlesource.com/c/go/+/325870 Trust: Matthew Dempsky Reviewed-by: Ian Lance Taylor Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 7438d894fe..c1978ff1c1 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -25,12 +25,46 @@ Do not send CLs removing the interior tags from such phrases.

Changes to the language

-

- TODO: https://golang.org/cl/216424: allow conversion from slice to array ptr +

+ Go 1.17 includes three small enhancements to the language.

-

- TODO: https://golang.org/cl/312212: add unsafe.Add and unsafe.Slice +

    +
  • + Conversions + from slice to array pointer: An expression s of + type []T may now be converted to array pointer type + *[N]T. If a is the result of such a + conversion, then corresponding indices that are in range refer to + the same underlying elements: &a[i] == &s[i] + for 0 <= i < N. The conversion panics if + len(s) is less than N. +
    • + +
  • + unsafe.Add: + unsafe.Add(ptr, len) adds len + to ptr and returns the updated pointer + unsafe.Pointer(uintptr(ptr) + uintptr(len)). +
    • + +
  • + unsafe.Slice: + For expression ptr of type *T, + unsafe.Slice(ptr, len) returns a slice of + type []T whose underlying array starts + at ptr and whose length and capacity + are len. +
    • +
+ +

+ These enhancements were added to simplify writing code that conforms + to unsafe.Pointer's safety + rules, but the rules remain unchanged. In particular, existing + programs that correctly use unsafe.Pointer remain + valid, and new programs must still follow the rules when + using unsafe.Add or unsafe.Slice.

Ports

From c20bcb64882d1134770683d663ee9f82fea715e6 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Mon, 7 Jun 2021 16:24:40 -0700 Subject: [PATCH 04/45] runtime: remove out-of-date comments about frame skipping skipPleaseUseCallersFrames was removed in CL 152537. Change-Id: Ide47feec85a33a6fb6882e16baf9e21492521640 Reviewed-on: https://go-review.googlesource.com/c/go/+/325949 Trust: Matthew Dempsky Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Keith Randall --- src/runtime/traceback.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 89780edc1f..814c323634 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -56,8 +56,6 @@ func tracebackdefers(gp *g, callback func(*stkframe, unsafe.Pointer) bool, v uns } } -const sizeofSkipFunction = 256 - // Generic traceback. Handles runtime stack prints (pcbuf == nil), // the runtime.Callers function (pcbuf != nil), as well as the garbage // collector (callback != nil). A little clunky to merge these, but avoids @@ -65,9 +63,7 @@ const sizeofSkipFunction = 256 // // The skip argument is only valid with pcbuf != nil and counts the number // of logical frames to skip rather than physical frames (with inlining, a -// PC in pcbuf can represent multiple calls). If a PC is partially skipped -// and max > 1, pcbuf[1] will be runtime.skipPleaseUseCallersFrames+N where -// N indicates the number of logical frames to skip in pcbuf[0]. +// PC in pcbuf can represent multiple calls). func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, flags uint) int { if skip > 0 && callback != nil { throw("gentraceback callback cannot be used with non-zero skip") From 2169deb35247a80794519589e7cd845c6ebf4e5a Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sun, 30 May 2021 15:35:06 +0700 Subject: [PATCH 05/45] cmd/compile: use t.AllMethods when sorting typesByString For interface types, t.Methods contains only unexpanded method set, i.e exclusive of interface embedding. Thus, we can't use it to detect an interface contains embedding empty interface, like in: type EI interface{} func f() interface{ EI } { return nil } At the time we generate runtime types, we want to check against the full method set of interface instead. Fixes #46386 Change-Id: Idff53ad39276be6632eb5932b76e855c15cbdd2e Reviewed-on: https://go-review.googlesource.com/c/go/+/323649 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky --- .../compile/internal/reflectdata/reflect.go | 4 +-- test/fixedbugs/issue46386.go | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 test/fixedbugs/issue46386.go diff --git a/src/cmd/compile/internal/reflectdata/reflect.go b/src/cmd/compile/internal/reflectdata/reflect.go index b3688fca67..e07294be0f 100644 --- a/src/cmd/compile/internal/reflectdata/reflect.go +++ b/src/cmd/compile/internal/reflectdata/reflect.go @@ -1475,8 +1475,8 @@ func (a typesByString) Less(i, j int) bool { // will be equal for the above checks, but different in DWARF output. // Sort by source position to ensure deterministic order. // See issues 27013 and 30202. - if a[i].t.Kind() == types.TINTER && a[i].t.Methods().Len() > 0 { - return a[i].t.Methods().Index(0).Pos.Before(a[j].t.Methods().Index(0).Pos) + if a[i].t.Kind() == types.TINTER && a[i].t.AllMethods().Len() > 0 { + return a[i].t.AllMethods().Index(0).Pos.Before(a[j].t.AllMethods().Index(0).Pos) } return false } diff --git a/test/fixedbugs/issue46386.go b/test/fixedbugs/issue46386.go new file mode 100644 index 0000000000..89dea8abf3 --- /dev/null +++ b/test/fixedbugs/issue46386.go @@ -0,0 +1,32 @@ +// compile -p=main + +// 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 main + +type I interface { + M() interface{} +} + +type S1 struct{} + +func (S1) M() interface{} { + return nil +} + +type EI interface{} + +type S struct{} + +func (S) M(as interface{ I }) {} + +func f() interface{ EI } { + return &S1{} +} + +func main() { + var i interface{ I } + (&S{}).M(i) +} From 0fb3e2c18408cc8ff6cb87962fc13f2684d1df96 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 7 Jun 2021 17:04:32 -0400 Subject: [PATCH 06/45] doc/go1.17: add a release note for the '-compat' flag to 'go mod tidy' Updates #46141 Change-Id: I7a6a84f816e3db19bb492f862366a29dc46ed2ee Reviewed-on: https://go-review.googlesource.com/c/go/+/325910 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Michael Matloob --- doc/go1.17.html | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index c1978ff1c1..ba6b8baf19 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -137,8 +137,9 @@ Do not send CLs removing the interior tags from such phrases.

-

To facilitate the upgrade to lazy loading, - the go mod tidy subcommand now supports +

+ To facilitate the upgrade to lazy loading, the + go mod tidy subcommand now supports a -go flag to set or change the go version in the go.mod file. To enable lazy loading for an existing module without changing the selected versions of its dependencies, run: @@ -149,8 +150,32 @@ Do not send CLs removing the interior tags from such phrases.

- TODO: Describe the -compat flag - for go mod tidy. + By default, go mod tidy verifies that + the selected versions of dependencies relevant to the main module are the same + versions that would be used by the prior Go release (Go 1.16 for a module that + spsecifies go 1.17), and preserves + the go.sum entries needed by that release even for dependencies + that are not normally needed by other commands. +

+ +

+ The -compat flag allows that version to be overridden to support + older (or only newer) versions, up to the version specified by + the go directive in the go.mod file. To tidy + a go 1.17 module for Go 1.17 only, without saving + checksums for (or checking for consistency with) Go 1.16: +

+ +
+  go mod tidy -compat=1.17
+
+ +

+ Note that even if the main module is tidied with -compat=1.17, + users who require the module from a + go 1.16 or earlier module will still be able to + use it, provided that the packages use only compatible language and library + features.

Module deprecation comments

From 949f00cebe9a40c7454bc42acaa77fdb8bf6c4e6 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Mon, 7 Jun 2021 10:20:38 -0400 Subject: [PATCH 07/45] doc/go1.17: add release notes for crypto packages For #44513 Change-Id: I459b3a4f9936eaa2c09888177f91176140d04280 Reviewed-on: https://go-review.googlesource.com/c/go/+/325649 Trust: Filippo Valsorda Trust: Katie Hockman Reviewed-by: Roland Shoemaker Reviewed-by: Katie Hockman --- doc/go1.17.html | 136 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 108 insertions(+), 28 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index ba6b8baf19..c1b3b3cef4 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -338,30 +338,6 @@ Do not send CLs removing the interior tags from such phrases. TODO: complete the Core library section

-

crypto/tls

- -

- (*Conn).HandshakeContext was added to - allow the user to control cancellation of an in-progress TLS Handshake. - The context provided is propagated into the - ClientHelloInfo - and CertificateRequestInfo - structs and accessible through the new - (*ClientHelloInfo).Context - and - - (*CertificateRequestInfo).Context - methods respectively. Canceling the context after the handshake has finished - has no effect. -

- -

- When Config.NextProtos is set, servers now - enforce that there is an overlap between the configured protocols and the protocols - advertised by the client, if any. If there is no overlap the connection is closed - with the no_application_protocol alert, as required by RFC 7301. -

-

Cgo

@@ -424,13 +400,117 @@ Do not send CLs removing the interior tags from such phrases. -

crypto/rsa
+
crypto/ed25519
-

- TODO: https://golang.org/cl/302230: fix salt length calculation with PSSSaltLengthAuto +

+ The crypto/ed25519 package has been rewritten, and all + operations are now approximately twice as fast on amd64 and arm64. + The observable behavior has not otherwise changed.

-
+
+ +
crypto/elliptic
+
+

+ CurveParams + methods now automatically invoke faster and safer dedicated + implementations for known curves (P-224, P-256, and P-521) when + available. Note that this is a best-effort approach and applications + should avoid using the generic, not constant-time CurveParams + methods and instead use dedicated + Curve implementations + such as P256. +

+ +

+ The P521 curve + implementation has been rewritten using code generated by the + fiat-crypto project, + which is based on a formally-verified model of the arithmetic + operations. It is now constant-time and three times faster on amd64 and + arm64. The observable behavior has not otherwise changed. +

+
+
+ +
crypto/rand
+
+

+ The crypto/rand package now uses the getentropy + syscall on macOS and the getrandom syscall on Solaris, + Illumos, and DragonFlyBSD. +

+
+
+ +
crypto/tls
+
+

+ The new Conn.HandshakeContext + method allows the user to control cancellation of an in-progress TLS + handshake. The provided context is accessible from various callbacks through the new + ClientHelloInfo.Context and + CertificateRequestInfo.Context + methods. Canceling the context after the handshake has finished has no effect. +

+ +

+ When Config.NextProtos + is set, servers now enforce that there is an overlap between the + configured protocols and the protocols advertised by the client, if any. + If there is no overlap the connection is closed with the + no_application_protocol alert, as required by RFC 7301. +

+ +

+ Cipher suite ordering is now handled entirely by the + crypto/tls package. Currently, cipher suites are sorted based + on their security, performance, and hardware support taking into account + both the local and peer's hardware. The order of the + Config.CipherSuites + field is now ignored, as well as the + Config.PreferServerCipherSuites + field. Note that Config.CipherSuites still allows + applications to choose what TLS 1.0–1.2 cipher suites to enable. +

+ +

+ The 3DES cipher suites have been moved to + InsecureCipherSuites + due to fundamental block size-related + weakness. They are still enabled by default but only as a last resort, + thanks to the cipher suite ordering change above. +

+
+
+ +
crypto/x509
+
+

+ CreateCertificate + now returns an error if the provided private key doesn't match the + parent's public key, if any. The resulting certificate would have failed + to verify. +

+ +

+ The temporary GODEBUG=x509ignoreCN=0 flag has been removed. +

+ +

+ ParseCertificate + has been rewritten, and now consumes ~70% fewer resources. The observable + behavior has not otherwise changed, except for error messages. +

+ +

+ On BSD systems, /etc/ssl/certs is now searched for trusted + roots. This adds support for the new system trusted certificate store in + FreeBSD 12.2+. +

+
+
database/sql
From 9498b0155d4c38c018d00b83afaedaabbdbb9e85 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 7 Jun 2021 23:04:16 -0400 Subject: [PATCH 08/45] cmd/go: in Go 1.17+ modules, add indirect go.mod dependencies separately from direct ones Fixes #45965 Change-Id: If5c0d7b29e9f81be0763f3fa68051d4ef5419990 Reviewed-on: https://go-review.googlesource.com/c/go/+/325922 Trust: Bryan C. Mills Reviewed-by: Michael Matloob --- doc/go1.17.html | 8 + src/cmd/go.mod | 2 +- src/cmd/go.sum | 4 +- src/cmd/go/internal/modload/init.go | 6 +- src/cmd/go/internal/modload/modfile.go | 5 + .../script/mod_go_version_missing.txt | 7 +- .../script/mod_lazy_import_allmod.txt | 3 +- .../testdata/script/mod_lazy_new_import.txt | 10 +- .../script/mod_lazy_test_of_test_dep.txt | 7 +- src/cmd/go/testdata/script/mod_retention.txt | 3 +- .../testdata/script/mod_tidy_convergence.txt | 18 +- .../go/testdata/script/mod_tidy_version.txt | 22 +- .../vendor/golang.org/x/mod/modfile/read.go | 7 +- .../vendor/golang.org/x/mod/modfile/rule.go | 422 +++++++++++++----- src/cmd/vendor/modules.txt | 2 +- 15 files changed, 382 insertions(+), 144 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index c1b3b3cef4..8b0fcea29d 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -137,6 +137,14 @@ Do not send CLs removing the interior tags from such phrases.

+

+ Because the number of additional explicit requirements in the go.mod file may + be substantial, in a Go 1.17 module the newly-added requirements + on indirect dependencies are maintained in a + separate require block from the block containing direct + dependencies. +

+

To facilitate the upgrade to lazy loading, the go mod tidy subcommand now supports diff --git a/src/cmd/go.mod b/src/cmd/go.mod index 1aa0320d07..cd03968eed 100644 --- a/src/cmd/go.mod +++ b/src/cmd/go.mod @@ -7,7 +7,7 @@ require ( github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639 // indirect golang.org/x/arch v0.0.0-20210502124803-cbf565b21d1e golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e // indirect - golang.org/x/mod v0.4.3-0.20210512182355-6088ed88cecd + golang.org/x/mod v0.4.3-0.20210608190319-0f08993efd8a golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744 // indirect golang.org/x/term v0.0.0-20210503060354-a79de5458b56 golang.org/x/tools v0.1.2-0.20210519160823-49064d2332f9 diff --git a/src/cmd/go.sum b/src/cmd/go.sum index eeb625fcf8..d728acaec9 100644 --- a/src/cmd/go.sum +++ b/src/cmd/go.sum @@ -13,8 +13,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e h1:8foAy0aoO5GkqCvAEJ4VC4P3zksTg4X4aJCDpZzmgQI= golang.org/x/crypto v0.0.0-20210503195802-e9a32991a82e/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.4.3-0.20210512182355-6088ed88cecd h1:CuRnpyMrCCBulv0d/y0CswR4K0vGydgE3DZ2wYPIOo8= -golang.org/x/mod v0.4.3-0.20210512182355-6088ed88cecd/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro= +golang.org/x/mod v0.4.3-0.20210608190319-0f08993efd8a h1:e8qnjKz4EE6OjRki9wTadWSIogINvq10sMcuBRORxMY= +golang.org/x/mod v0.4.3-0.20210608190319-0f08993efd8a/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index ea404b9f78..eb9cfe629b 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -999,10 +999,14 @@ func commitRequirements(ctx context.Context, goVersion string, rs *Requirements) Indirect: !rs.direct[m.Path], }) } - modFile.SetRequire(list) if goVersion != "" { modFile.AddGoStmt(goVersion) } + if semver.Compare("v"+modFileGoVersion(), separateIndirectVersionV) < 0 { + modFile.SetRequire(list) + } else { + modFile.SetRequireSeparateIndirect(list) + } modFile.Cleanup() dirty := index.modFileIsDirty(modFile) diff --git a/src/cmd/go/internal/modload/modfile.go b/src/cmd/go/internal/modload/modfile.go index a9c3a91d35..1145ac4ba5 100644 --- a/src/cmd/go/internal/modload/modfile.go +++ b/src/cmd/go/internal/modload/modfile.go @@ -35,6 +35,11 @@ const ( // module's go.mod file is expected to list explicit requirements on every // module that provides any package transitively imported by that module. lazyLoadingVersionV = "v1.17" + + // separateIndirectVersionV is the Go version (plus leading "v") at which + // "// indirect" dependencies are added in a block separate from the direct + // ones. See https://golang.org/issue/45965. + separateIndirectVersionV = "v1.17" ) const ( diff --git a/src/cmd/go/testdata/script/mod_go_version_missing.txt b/src/cmd/go/testdata/script/mod_go_version_missing.txt index aca36a0450..d704816729 100644 --- a/src/cmd/go/testdata/script/mod_go_version_missing.txt +++ b/src/cmd/go/testdata/script/mod_go_version_missing.txt @@ -73,10 +73,9 @@ module example.com/m go $goversion -require ( - example.com/dep v0.1.0 - example.com/testdep v0.1.0 // indirect -) +require example.com/dep v0.1.0 + +require example.com/testdep v0.1.0 // indirect replace ( example.com/dep v0.1.0 => ./dep diff --git a/src/cmd/go/testdata/script/mod_lazy_import_allmod.txt b/src/cmd/go/testdata/script/mod_lazy_import_allmod.txt index 3dc1515df2..97718c4513 100644 --- a/src/cmd/go/testdata/script/mod_lazy_import_allmod.txt +++ b/src/cmd/go/testdata/script/mod_lazy_import_allmod.txt @@ -139,9 +139,10 @@ go 1.17 require ( a v0.1.0 b v0.1.0 - c v0.1.0 // indirect ) +require c v0.1.0 // indirect + replace ( a v0.1.0 => ./a1 b v0.1.0 => ./b1 diff --git a/src/cmd/go/testdata/script/mod_lazy_new_import.txt b/src/cmd/go/testdata/script/mod_lazy_new_import.txt index 86b14b64b6..4272a52de1 100644 --- a/src/cmd/go/testdata/script/mod_lazy_new_import.txt +++ b/src/cmd/go/testdata/script/mod_lazy_new_import.txt @@ -78,10 +78,9 @@ module example.com/lazy go 1.17 -require ( - example.com/a v0.1.0 - example.com/b v0.1.0 // indirect -) +require example.com/a v0.1.0 + +require example.com/b v0.1.0 // indirect replace ( example.com/a v0.1.0 => ./a @@ -94,8 +93,9 @@ module example.com/lazy go 1.17 +require example.com/a v0.1.0 + require ( - example.com/a v0.1.0 example.com/b v0.1.0 // indirect example.com/c v0.1.0 // indirect ) diff --git a/src/cmd/go/testdata/script/mod_lazy_test_of_test_dep.txt b/src/cmd/go/testdata/script/mod_lazy_test_of_test_dep.txt index 722712d1f2..68a5b6dca2 100644 --- a/src/cmd/go/testdata/script/mod_lazy_test_of_test_dep.txt +++ b/src/cmd/go/testdata/script/mod_lazy_test_of_test_dep.txt @@ -148,10 +148,9 @@ module example.com/lazy go 1.17 -require ( - example.com/a v0.1.0 - example.com/b v0.1.0 // indirect -) +require example.com/a v0.1.0 + +require example.com/b v0.1.0 // indirect replace ( example.com/a v0.1.0 => ./a diff --git a/src/cmd/go/testdata/script/mod_retention.txt b/src/cmd/go/testdata/script/mod_retention.txt index 0e639db551..7a371b1806 100644 --- a/src/cmd/go/testdata/script/mod_retention.txt +++ b/src/cmd/go/testdata/script/mod_retention.txt @@ -140,8 +140,9 @@ module m go $goversion require ( - golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c // indirect rsc.io/quote v1.5.2 rsc.io/sampler v1.3.0 // indirect rsc.io/testonly v1.0.0 // indirect ) + +require golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c // indirect diff --git a/src/cmd/go/testdata/script/mod_tidy_convergence.txt b/src/cmd/go/testdata/script/mod_tidy_convergence.txt index 22c8fc66c5..09c46f764b 100644 --- a/src/cmd/go/testdata/script/mod_tidy_convergence.txt +++ b/src/cmd/go/testdata/script/mod_tidy_convergence.txt @@ -90,7 +90,6 @@ cmp go.mod go.mod.postget cp go.mod.orig go.mod go mod edit -go=1.17 go.mod go mod edit -go=1.17 go.mod.tidye -go mod edit -go=1.17 go.mod.postget go mod tidy -e cmp go.mod go.mod.tidye @@ -99,7 +98,7 @@ stderr '^example\.net/m imports\n\texample\.net/x: package example\.net/x provid go get -d example.net/x@v0.1.0 example.net/y@v0.1.0 go mod tidy -cmp go.mod go.mod.postget +cmp go.mod go.mod.postget-117 -- go.mod -- @@ -144,6 +143,21 @@ require ( example.net/x v0.1.0 example.net/y v0.1.0 // indirect ) +-- go.mod.postget-117 -- +module example.net/m + +go 1.17 + +replace ( + example.net/x v0.1.0 => ./x1 + example.net/x v0.2.0-pre => ./x2-pre + example.net/y v0.1.0 => ./y1 + example.net/y v0.2.0 => ./y2 +) + +require example.net/x v0.1.0 + +require example.net/y v0.1.0 // indirect -- m.go -- package m diff --git a/src/cmd/go/testdata/script/mod_tidy_version.txt b/src/cmd/go/testdata/script/mod_tidy_version.txt index eaa6ee7b0d..3bc97bcb1e 100644 --- a/src/cmd/go/testdata/script/mod_tidy_version.txt +++ b/src/cmd/go/testdata/script/mod_tidy_version.txt @@ -92,8 +92,9 @@ cmpenv go.mod go.mod.latest -- go.mod -- module example.com/m +require example.net/a v0.1.0 + require ( - example.net/a v0.1.0 example.net/c v0.1.0 // indirect example.net/d v0.1.0 // indirect ) @@ -118,8 +119,9 @@ module example.com/m go 1.15 +require example.net/a v0.1.0 + require ( - example.net/a v0.1.0 example.net/c v0.1.0 // indirect example.net/d v0.1.0 // indirect ) @@ -139,8 +141,9 @@ module example.com/m go 1.15 +require example.net/a v0.1.0 + require ( - example.net/a v0.1.0 example.net/c v0.1.0 // indirect example.net/d v0.2.0 // indirect ) @@ -160,10 +163,9 @@ module example.com/m go 1.16 -require ( - example.net/a v0.1.0 - example.net/c v0.1.0 // indirect -) +require example.net/a v0.1.0 + +require example.net/c v0.1.0 // indirect replace ( example.net/a v0.1.0 => ./a @@ -180,8 +182,9 @@ module example.com/m go 1.17 +require example.net/a v0.1.0 + require ( - example.net/a v0.1.0 example.net/b v0.1.0 // indirect example.net/c v0.1.0 // indirect ) @@ -201,8 +204,9 @@ module example.com/m go $goversion +require example.net/a v0.1.0 + require ( - example.net/a v0.1.0 example.net/b v0.1.0 // indirect example.net/c v0.1.0 // indirect ) diff --git a/src/cmd/vendor/golang.org/x/mod/modfile/read.go b/src/cmd/vendor/golang.org/x/mod/modfile/read.go index 2a961ca81c..956f30cbb3 100644 --- a/src/cmd/vendor/golang.org/x/mod/modfile/read.go +++ b/src/cmd/vendor/golang.org/x/mod/modfile/read.go @@ -194,12 +194,15 @@ func (x *FileSyntax) updateLine(line *Line, tokens ...string) { line.Token = tokens } -func (x *FileSyntax) removeLine(line *Line) { +// markRemoved modifies line so that it (and its end-of-line comment, if any) +// will be dropped by (*FileSyntax).Cleanup. +func (line *Line) markRemoved() { line.Token = nil + line.Comments.Suffix = nil } // Cleanup cleans up the file syntax x after any edit operations. -// To avoid quadratic behavior, removeLine marks the line as dead +// To avoid quadratic behavior, (*Line).markRemoved marks the line as dead // by setting line.Token = nil but does not remove it from the slice // in which it appears. After edits have all been indicated, // calling Cleanup cleans out the dead lines. diff --git a/src/cmd/vendor/golang.org/x/mod/modfile/rule.go b/src/cmd/vendor/golang.org/x/mod/modfile/rule.go index 7299e15500..78f83fa714 100644 --- a/src/cmd/vendor/golang.org/x/mod/modfile/rule.go +++ b/src/cmd/vendor/golang.org/x/mod/modfile/rule.go @@ -58,13 +58,6 @@ type Go struct { Syntax *Line } -// A Require is a single require statement. -type Require struct { - Mod module.Version - Indirect bool // has "// indirect" comment - Syntax *Line -} - // An Exclude is a single exclude statement. type Exclude struct { Mod module.Version @@ -93,6 +86,93 @@ type VersionInterval struct { Low, High string } +// A Require is a single require statement. +type Require struct { + Mod module.Version + Indirect bool // has "// indirect" comment + Syntax *Line +} + +func (r *Require) markRemoved() { + r.Syntax.markRemoved() + *r = Require{} +} + +func (r *Require) setVersion(v string) { + r.Mod.Version = v + + if line := r.Syntax; len(line.Token) > 0 { + if line.InBlock { + // If the line is preceded by an empty line, remove it; see + // https://golang.org/issue/33779. + if len(line.Comments.Before) == 1 && len(line.Comments.Before[0].Token) == 0 { + line.Comments.Before = line.Comments.Before[:0] + } + if len(line.Token) >= 2 { // example.com v1.2.3 + line.Token[1] = v + } + } else { + if len(line.Token) >= 3 { // require example.com v1.2.3 + line.Token[2] = v + } + } + } +} + +// setIndirect sets line to have (or not have) a "// indirect" comment. +func (r *Require) setIndirect(indirect bool) { + r.Indirect = indirect + line := r.Syntax + if isIndirect(line) == indirect { + return + } + if indirect { + // Adding comment. + if len(line.Suffix) == 0 { + // New comment. + line.Suffix = []Comment{{Token: "// indirect", Suffix: true}} + return + } + + com := &line.Suffix[0] + text := strings.TrimSpace(strings.TrimPrefix(com.Token, string(slashSlash))) + if text == "" { + // Empty comment. + com.Token = "// indirect" + return + } + + // Insert at beginning of existing comment. + com.Token = "// indirect; " + text + return + } + + // Removing comment. + f := strings.TrimSpace(strings.TrimPrefix(line.Suffix[0].Token, string(slashSlash))) + if f == "indirect" { + // Remove whole comment. + line.Suffix = nil + return + } + + // Remove comment prefix. + com := &line.Suffix[0] + i := strings.Index(com.Token, "indirect;") + com.Token = "//" + com.Token[i+len("indirect;"):] +} + +// isIndirect reports whether line has a "// indirect" comment, +// meaning it is in go.mod only for its effect on indirect dependencies, +// so that it can be dropped entirely once the effective version of the +// indirect dependency reaches the given minimum version. +func isIndirect(line *Line) bool { + if len(line.Suffix) == 0 { + return false + } + f := strings.Fields(strings.TrimPrefix(line.Suffix[0].Token, string(slashSlash))) + return (len(f) == 1 && f[0] == "indirect" || len(f) > 1 && f[0] == "indirect;") +} + func (f *File) AddModuleStmt(path string) error { if f.Syntax == nil { f.Syntax = new(FileSyntax) @@ -476,58 +556,6 @@ func (f *File) fixRetract(fix VersionFixer, errs *ErrorList) { } } -// isIndirect reports whether line has a "// indirect" comment, -// meaning it is in go.mod only for its effect on indirect dependencies, -// so that it can be dropped entirely once the effective version of the -// indirect dependency reaches the given minimum version. -func isIndirect(line *Line) bool { - if len(line.Suffix) == 0 { - return false - } - f := strings.Fields(strings.TrimPrefix(line.Suffix[0].Token, string(slashSlash))) - return (len(f) == 1 && f[0] == "indirect" || len(f) > 1 && f[0] == "indirect;") -} - -// setIndirect sets line to have (or not have) a "// indirect" comment. -func setIndirect(line *Line, indirect bool) { - if isIndirect(line) == indirect { - return - } - if indirect { - // Adding comment. - if len(line.Suffix) == 0 { - // New comment. - line.Suffix = []Comment{{Token: "// indirect", Suffix: true}} - return - } - - com := &line.Suffix[0] - text := strings.TrimSpace(strings.TrimPrefix(com.Token, string(slashSlash))) - if text == "" { - // Empty comment. - com.Token = "// indirect" - return - } - - // Insert at beginning of existing comment. - com.Token = "// indirect; " + text - return - } - - // Removing comment. - f := strings.TrimSpace(strings.TrimPrefix(line.Suffix[0].Token, string(slashSlash))) - if f == "indirect" { - // Remove whole comment. - line.Suffix = nil - return - } - - // Remove comment prefix. - com := &line.Suffix[0] - i := strings.Index(com.Token, "indirect;") - com.Token = "//" + com.Token[i+len("indirect;"):] -} - // IsDirectoryPath reports whether the given path should be interpreted // as a directory path. Just like on the go command line, relative paths // and rooted paths are directory paths; the rest are module paths. @@ -835,6 +863,12 @@ func (f *File) AddGoStmt(version string) error { return nil } +// AddRequire sets the first require line for path to version vers, +// preserving any existing comments for that line and removing all +// other lines for path. +// +// If no line currently exists for path, AddRequire adds a new line +// at the end of the last require block. func (f *File) AddRequire(path, vers string) error { need := true for _, r := range f.Require { @@ -844,7 +878,7 @@ func (f *File) AddRequire(path, vers string) error { f.Syntax.updateLine(r.Syntax, "require", AutoQuote(path), vers) need = false } else { - f.Syntax.removeLine(r.Syntax) + r.Syntax.markRemoved() *r = Require{} } } @@ -856,69 +890,235 @@ func (f *File) AddRequire(path, vers string) error { return nil } +// AddNewRequire adds a new require line for path at version vers at the end of +// the last require block, regardless of any existing require lines for path. func (f *File) AddNewRequire(path, vers string, indirect bool) { line := f.Syntax.addLine(nil, "require", AutoQuote(path), vers) - setIndirect(line, indirect) - f.Require = append(f.Require, &Require{module.Version{Path: path, Version: vers}, indirect, line}) + r := &Require{ + Mod: module.Version{Path: path, Version: vers}, + Syntax: line, + } + r.setIndirect(indirect) + f.Require = append(f.Require, r) } +// SetRequire updates the requirements of f to contain exactly req, preserving +// the existing block structure and line comment contents (except for 'indirect' +// markings) for the first requirement on each named module path. +// +// The Syntax field is ignored for the requirements in req. +// +// Any requirements not already present in the file are added to the block +// containing the last require line. +// +// The requirements in req must specify at most one distinct version for each +// module path. +// +// If any existing requirements may be removed, the caller should call Cleanup +// after all edits are complete. func (f *File) SetRequire(req []*Require) { - need := make(map[string]string) - indirect := make(map[string]bool) + type elem struct { + version string + indirect bool + } + need := make(map[string]elem) for _, r := range req { - need[r.Mod.Path] = r.Mod.Version - indirect[r.Mod.Path] = r.Indirect - } - - for _, r := range f.Require { - if v, ok := need[r.Mod.Path]; ok { - r.Mod.Version = v - r.Indirect = indirect[r.Mod.Path] - } else { - *r = Require{} + if prev, dup := need[r.Mod.Path]; dup && prev.version != r.Mod.Version { + panic(fmt.Errorf("SetRequire called with conflicting versions for path %s (%s and %s)", r.Mod.Path, prev.version, r.Mod.Version)) } + need[r.Mod.Path] = elem{r.Mod.Version, r.Indirect} } - var newStmts []Expr + // Update or delete the existing Require entries to preserve + // only the first for each module path in req. + for _, r := range f.Require { + e, ok := need[r.Mod.Path] + if ok { + r.setVersion(e.version) + r.setIndirect(e.indirect) + } else { + r.markRemoved() + } + delete(need, r.Mod.Path) + } + + // Add new entries in the last block of the file for any paths that weren't + // already present. + // + // This step is nondeterministic, but the final result will be deterministic + // because we will sort the block. + for path, e := range need { + f.AddNewRequire(path, e.version, e.indirect) + } + + f.SortBlocks() +} + +// SetRequireSeparateIndirect updates the requirements of f to contain the given +// requirements. Comment contents (except for 'indirect' markings) are retained +// from the first existing requirement for each module path, and block structure +// is maintained as long as the indirect markings match. +// +// Any requirements on paths not already present in the file are added. Direct +// requirements are added to the last block containing *any* other direct +// requirement. Indirect requirements are added to the last block containing +// *only* other indirect requirements. If no suitable block exists, a new one is +// added, with the last block containing a direct dependency (if any) +// immediately before the first block containing only indirect dependencies. +// +// The Syntax field is ignored for requirements in the given blocks. +func (f *File) SetRequireSeparateIndirect(req []*Require) { + type modKey struct { + path string + indirect bool + } + need := make(map[modKey]string) + for _, r := range req { + need[modKey{r.Mod.Path, r.Indirect}] = r.Mod.Version + } + + comments := make(map[string]Comments) + for _, r := range f.Require { + v, ok := need[modKey{r.Mod.Path, r.Indirect}] + if !ok { + if _, ok := need[modKey{r.Mod.Path, !r.Indirect}]; ok { + if _, dup := comments[r.Mod.Path]; !dup { + comments[r.Mod.Path] = r.Syntax.Comments + } + } + r.markRemoved() + continue + } + r.setVersion(v) + delete(need, modKey{r.Mod.Path, r.Indirect}) + } + + var ( + lastDirectOrMixedBlock Expr + firstIndirectOnlyBlock Expr + lastIndirectOnlyBlock Expr + ) for _, stmt := range f.Syntax.Stmt { switch stmt := stmt.(type) { - case *LineBlock: - if len(stmt.Token) > 0 && stmt.Token[0] == "require" { - var newLines []*Line - for _, line := range stmt.Line { - if p, err := parseString(&line.Token[0]); err == nil && need[p] != "" { - if len(line.Comments.Before) == 1 && len(line.Comments.Before[0].Token) == 0 { - line.Comments.Before = line.Comments.Before[:0] - } - line.Token[1] = need[p] - delete(need, p) - setIndirect(line, indirect[p]) - newLines = append(newLines, line) - } - } - if len(newLines) == 0 { - continue // drop stmt - } - stmt.Line = newLines - } - case *Line: - if len(stmt.Token) > 0 && stmt.Token[0] == "require" { - if p, err := parseString(&stmt.Token[1]); err == nil && need[p] != "" { - stmt.Token[2] = need[p] - delete(need, p) - setIndirect(stmt, indirect[p]) - } else { - continue // drop stmt + if len(stmt.Token) == 0 || stmt.Token[0] != "require" { + continue + } + if isIndirect(stmt) { + lastIndirectOnlyBlock = stmt + } else { + lastDirectOrMixedBlock = stmt + } + case *LineBlock: + if len(stmt.Token) == 0 || stmt.Token[0] != "require" { + continue + } + indirectOnly := true + for _, line := range stmt.Line { + if len(line.Token) == 0 { + continue + } + if !isIndirect(line) { + indirectOnly = false + break + } + } + if indirectOnly { + lastIndirectOnlyBlock = stmt + if firstIndirectOnlyBlock == nil { + firstIndirectOnlyBlock = stmt + } + } else { + lastDirectOrMixedBlock = stmt + } + } + } + + isOrContainsStmt := func(stmt Expr, target Expr) bool { + if stmt == target { + return true + } + if stmt, ok := stmt.(*LineBlock); ok { + if target, ok := target.(*Line); ok { + for _, line := range stmt.Line { + if line == target { + return true + } } } } - newStmts = append(newStmts, stmt) + return false } - f.Syntax.Stmt = newStmts - for path, vers := range need { - f.AddNewRequire(path, vers, indirect[path]) + addRequire := func(path, vers string, indirect bool, comments Comments) { + var line *Line + if indirect { + if lastIndirectOnlyBlock != nil { + line = f.Syntax.addLine(lastIndirectOnlyBlock, "require", path, vers) + } else { + // Add a new require block after the last direct-only or mixed "require" + // block (if any). + // + // (f.Syntax.addLine would add the line to an existing "require" block if + // present, but here the existing "require" blocks are all direct-only, so + // we know we need to add a new block instead.) + line = &Line{Token: []string{"require", path, vers}} + lastIndirectOnlyBlock = line + firstIndirectOnlyBlock = line // only block implies first block + if lastDirectOrMixedBlock == nil { + f.Syntax.Stmt = append(f.Syntax.Stmt, line) + } else { + for i, stmt := range f.Syntax.Stmt { + if isOrContainsStmt(stmt, lastDirectOrMixedBlock) { + f.Syntax.Stmt = append(f.Syntax.Stmt, nil) // increase size + copy(f.Syntax.Stmt[i+2:], f.Syntax.Stmt[i+1:]) // shuffle elements up + f.Syntax.Stmt[i+1] = line + break + } + } + } + } + } else { + if lastDirectOrMixedBlock != nil { + line = f.Syntax.addLine(lastDirectOrMixedBlock, "require", path, vers) + } else { + // Add a new require block before the first indirect block (if any). + // + // That way if the file initially contains only indirect lines, + // the direct lines still appear before it: we preserve existing + // structure, but only to the extent that that structure already + // reflects the direct/indirect split. + line = &Line{Token: []string{"require", path, vers}} + lastDirectOrMixedBlock = line + if firstIndirectOnlyBlock == nil { + f.Syntax.Stmt = append(f.Syntax.Stmt, line) + } else { + for i, stmt := range f.Syntax.Stmt { + if isOrContainsStmt(stmt, firstIndirectOnlyBlock) { + f.Syntax.Stmt = append(f.Syntax.Stmt, nil) // increase size + copy(f.Syntax.Stmt[i+1:], f.Syntax.Stmt[i:]) // shuffle elements up + f.Syntax.Stmt[i] = line + break + } + } + } + } + } + + line.Comments.Before = commentsAdd(line.Comments.Before, comments.Before) + line.Comments.Suffix = commentsAdd(line.Comments.Suffix, comments.Suffix) + + r := &Require{ + Mod: module.Version{Path: path, Version: vers}, + Indirect: indirect, + Syntax: line, + } + r.setIndirect(indirect) + f.Require = append(f.Require, r) + } + + for k, vers := range need { + addRequire(k.path, vers, k.indirect, comments[k.path]) } f.SortBlocks() } @@ -926,7 +1126,7 @@ func (f *File) SetRequire(req []*Require) { func (f *File) DropRequire(path string) error { for _, r := range f.Require { if r.Mod.Path == path { - f.Syntax.removeLine(r.Syntax) + r.Syntax.markRemoved() *r = Require{} } } @@ -957,7 +1157,7 @@ func (f *File) AddExclude(path, vers string) error { func (f *File) DropExclude(path, vers string) error { for _, x := range f.Exclude { if x.Mod.Path == path && x.Mod.Version == vers { - f.Syntax.removeLine(x.Syntax) + x.Syntax.markRemoved() *x = Exclude{} } } @@ -988,7 +1188,7 @@ func (f *File) AddReplace(oldPath, oldVers, newPath, newVers string) error { continue } // Already added; delete other replacements for same. - f.Syntax.removeLine(r.Syntax) + r.Syntax.markRemoved() *r = Replace{} } if r.Old.Path == oldPath { @@ -1004,7 +1204,7 @@ func (f *File) AddReplace(oldPath, oldVers, newPath, newVers string) error { func (f *File) DropReplace(oldPath, oldVers string) error { for _, r := range f.Replace { if r.Old.Path == oldPath && r.Old.Version == oldVers { - f.Syntax.removeLine(r.Syntax) + r.Syntax.markRemoved() *r = Replace{} } } @@ -1045,7 +1245,7 @@ func (f *File) AddRetract(vi VersionInterval, rationale string) error { func (f *File) DropRetract(vi VersionInterval) error { for _, r := range f.Retract { if r.VersionInterval == vi { - f.Syntax.removeLine(r.Syntax) + r.Syntax.markRemoved() *r = Retract{} } } diff --git a/src/cmd/vendor/modules.txt b/src/cmd/vendor/modules.txt index 9a1723d32c..34dbdaf5dd 100644 --- a/src/cmd/vendor/modules.txt +++ b/src/cmd/vendor/modules.txt @@ -28,7 +28,7 @@ golang.org/x/arch/x86/x86asm ## explicit; go 1.17 golang.org/x/crypto/ed25519 golang.org/x/crypto/ed25519/internal/edwards25519 -# golang.org/x/mod v0.4.3-0.20210512182355-6088ed88cecd +# golang.org/x/mod v0.4.3-0.20210608190319-0f08993efd8a ## explicit; go 1.17 golang.org/x/mod/internal/lazyregexp golang.org/x/mod/modfile From f753d7223e5c10f53af8bbe5f5558292b08d5e8a Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 7 Jun 2021 13:43:55 -0400 Subject: [PATCH 09/45] doc/go1.17: resolve TODO for cmd/cover Updates #32211 Change-Id: Ie38e831fcf557534023afd552d9394fe9e055caa Reviewed-on: https://go-review.googlesource.com/c/go/+/325909 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov Reviewed-by: Emmanuel Odeke --- doc/go1.17.html | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 8b0fcea29d..ac315d4727 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -277,10 +277,6 @@ Do not send CLs removing the interior tags from such phrases. mod download all.

-

- TODO: https://golang.org/cl/249759: cmd/cover: replace code using optimized golang.org/x/tools/cover -

-

Vet

@@ -291,6 +287,14 @@ Do not send CLs removing the interior tags from such phrases. TODO: complete the Vet section

+

Cover

+ +

+ The cover tool now uses an optimized parser + from golang.org/x/tools/cover, which may be noticeably faster + when parsing large coverage profiles. +

+

Compiler

From 9afe071c605c65302066fa8ece7bdecd00730f8b Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 7 Jun 2021 17:11:32 -0400 Subject: [PATCH 10/45] doc/go1.17: remove TODO for Tools section Change-Id: I6cd7376bd051222a830cbf18cf7e887072b61f3b Reviewed-on: https://go-review.googlesource.com/c/go/+/325911 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index ac315d4727..42f3631b92 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -116,10 +116,6 @@ Do not send CLs removing the interior tags from such phrases.

Tools

-

- TODO: complete the Tools section -

-

Go command

Lazy module loading

From 689f4c7415acc8a135440574a483e0eeabba8b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisend=C3=B6rfer?= Date: Thu, 3 Jun 2021 15:33:08 +0200 Subject: [PATCH 11/45] doc/go1.17: mention block profile bias fix Change-Id: I76fd872b2d74704396f0683ffa9cec40b7027247 Reviewed-on: https://go-review.googlesource.com/c/go/+/324471 Reviewed-by: Heschi Kreinick Trust: Dmitri Shuralyov --- doc/go1.17.html | 10 ++++++++++ src/runtime/pprof/pprof_test.go | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/doc/go1.17.html b/doc/go1.17.html index 42f3631b92..1701508ea9 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -725,6 +725,16 @@ Do not send CLs removing the interior tags from such phrases.
+ +
runtime/pprof
+
+

+ Block profiles are no longer biased to favor infrequent long events over + frequent short events. +

+
+
+
strconv

diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index 7cbb4fc7ae..e139ee787d 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -106,6 +106,28 @@ func TestCPUProfileMultithreaded(t *testing.T) { }) } +func TestCPUProfileThreadBias(t *testing.T) { + cpuHogA := func(dur time.Duration) { + cpuHogger(cpuHog1, &salt2, dur) + } + + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2)) + prof := testCPUProfile(t, stackContains, []string{"runtime/pprof.cpuHog1", "runtime/pprof.cpuHog2"}, avoidFunctions(), func(dur time.Duration) { + //c := make(chan int) + //go func() { + //cpuHogger(cpuHog1, &salt1, dur) + //c <- 1 + //}() + cpuHogA(dur) + //<-c + }) + fmt.Printf("%#v\n", prof) +} + +func cpuHogA(dur time.Duration) { + cpuHogger(cpuHog1, &salt2, dur) +} + // containsInlinedCall reports whether the function body for the function f is // known to contain an inlined function call within the first maxBytes bytes. func containsInlinedCall(f interface{}, maxBytes int) bool { From da4a64014140adf83fb1434367ff68067249c267 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Wed, 26 May 2021 12:14:19 -0400 Subject: [PATCH 12/45] doc/go1.17: revise OpenBSD release notes Updates #44513. Change-Id: I64077859fa3061fee8327599875ad3870d603a81 Reviewed-on: https://go-review.googlesource.com/c/go/+/322856 Trust: Cherry Mui Reviewed-by: Dmitri Shuralyov Reviewed-by: Heschi Kreinick --- doc/go1.17.html | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 1701508ea9..3a1b43a4e5 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -95,11 +95,13 @@ Do not send CLs removing the interior tags from such phrases. In Go 1.16, on the 64-bit x86 and 64-bit ARM architectures on OpenBSD (the openbsd/amd64 and openbsd/arm64 ports) system calls are made through libc, instead - of directly using the machine instructions. In Go 1.17, this is - also done on the 32-bit x86 and 32-bit ARM architectures on OpenBSD + of directly using machine instructions. In Go 1.17, this is also + done on the 32-bit x86 and 32-bit ARM architectures on OpenBSD (the openbsd/386 and openbsd/arm ports). This ensures forward-compatibility with future versions of - OpenBSD. + OpenBSD, in particular, with OpenBSD 6.9 onwards, which requires + system calls to be made through libc for non-static + Go binaries.

ARM64

From d3e3d03666bbd8784007bbb78a75864aac786967 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 7 Jun 2021 10:21:29 -0700 Subject: [PATCH 13/45] net: reject leading zeros in IP address parsers In both net.ParseIP and net.ParseCIDR reject leading zeros in the dot-decimal notation of IPv4 addresses. Fixes #30999 Fixes #43389 Change-Id: I2b6a31fe84db89ac828cf5ed03eaa586ee96ab68 Reviewed-on: https://go-review.googlesource.com/c/go/+/325829 Trust: Roland Shoemaker Trust: Katie Hockman Run-TryBot: Roland Shoemaker Reviewed-by: Filippo Valsorda Reviewed-by: Katie Hockman TryBot-Result: Go Bot --- doc/go1.17.html | 10 ++++++++++ src/net/hosts_test.go | 4 ++-- src/net/ip.go | 4 ++++ src/net/ip_test.go | 8 ++++++-- src/net/testdata/ipv4-hosts | 8 ++------ 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 3a1b43a4e5..56f88e6724 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -639,6 +639,16 @@ Do not send CLs removing the interior tags from such phrases. ParseError error type now implement the net.Error interface.

+ +

+ The ParseIP and ParseCIDR + functions now reject IPv4 addresses which contain decimal components with leading zeros. + + These components were always interpreted as decimal, but some operating systems treat them as octal. + This mismatch could hypothetically lead to security issues if a Go application was used to validate IP addresses + which were then used in their original form with non-Go applications which interpreted components as octal. Generally, + it is advisable to always re-encoded values after validation, which avoids this class of parser misalignment issues. +

diff --git a/src/net/hosts_test.go b/src/net/hosts_test.go index f850e2fccf..19c43999f9 100644 --- a/src/net/hosts_test.go +++ b/src/net/hosts_test.go @@ -36,7 +36,7 @@ var lookupStaticHostTests = []struct { }, }, { - "testdata/ipv4-hosts", // see golang.org/issue/8996 + "testdata/ipv4-hosts", []staticHostEntry{ {"localhost", []string{"127.0.0.1", "127.0.0.2", "127.0.0.3"}}, {"localhost.localdomain", []string{"127.0.0.3"}}, @@ -102,7 +102,7 @@ var lookupStaticAddrTests = []struct { }, }, { - "testdata/ipv4-hosts", // see golang.org/issue/8996 + "testdata/ipv4-hosts", []staticHostEntry{ {"127.0.0.1", []string{"localhost"}}, {"127.0.0.2", []string{"localhost"}}, diff --git a/src/net/ip.go b/src/net/ip.go index 0477269761..38e1aa2247 100644 --- a/src/net/ip.go +++ b/src/net/ip.go @@ -574,6 +574,10 @@ func parseIPv4(s string) IP { if !ok || n > 0xFF { return nil } + if c > 1 && s[0] == '0' { + // Reject non-zero components with leading zeroes. + return nil + } s = s[c:] p[i] = byte(n) } diff --git a/src/net/ip_test.go b/src/net/ip_test.go index 3af5e41ceb..5bbda6024d 100644 --- a/src/net/ip_test.go +++ b/src/net/ip_test.go @@ -21,9 +21,7 @@ var parseIPTests = []struct { }{ {"127.0.1.2", IPv4(127, 0, 1, 2)}, {"127.0.0.1", IPv4(127, 0, 0, 1)}, - {"127.001.002.003", IPv4(127, 1, 2, 3)}, {"::ffff:127.1.2.3", IPv4(127, 1, 2, 3)}, - {"::ffff:127.001.002.003", IPv4(127, 1, 2, 3)}, {"::ffff:7f01:0203", IPv4(127, 1, 2, 3)}, {"0:0:0:0:0000:ffff:127.1.2.3", IPv4(127, 1, 2, 3)}, {"0:0:0:0:000000:ffff:127.1.2.3", IPv4(127, 1, 2, 3)}, @@ -43,6 +41,11 @@ var parseIPTests = []struct { {"fe80::1%911", nil}, {"", nil}, {"a1:a2:a3:a4::b1:b2:b3:b4", nil}, // Issue 6628 + {"127.001.002.003", nil}, + {"::ffff:127.001.002.003", nil}, + {"123.000.000.000", nil}, + {"1.2..4", nil}, + {"0123.0.0.1", nil}, } func TestParseIP(t *testing.T) { @@ -358,6 +361,7 @@ var parseCIDRTests = []struct { {"0.0.-2.0/32", nil, nil, &ParseError{Type: "CIDR address", Text: "0.0.-2.0/32"}}, {"0.0.0.-3/32", nil, nil, &ParseError{Type: "CIDR address", Text: "0.0.0.-3/32"}}, {"0.0.0.0/-0", nil, nil, &ParseError{Type: "CIDR address", Text: "0.0.0.0/-0"}}, + {"127.000.000.001/32", nil, nil, &ParseError{Type: "CIDR address", Text: "127.000.000.001/32"}}, {"", nil, nil, &ParseError{Type: "CIDR address", Text: ""}}, } diff --git a/src/net/testdata/ipv4-hosts b/src/net/testdata/ipv4-hosts index 5208bb44ac..6b99675dfc 100644 --- a/src/net/testdata/ipv4-hosts +++ b/src/net/testdata/ipv4-hosts @@ -1,12 +1,8 @@ # See https://tools.ietf.org/html/rfc1123. -# -# The literal IPv4 address parser in the net package is a relaxed -# one. It may accept a literal IPv4 address in dotted-decimal notation -# with leading zeros such as "001.2.003.4". # internet address and host name 127.0.0.1 localhost # inline comment separated by tab -127.000.000.002 localhost # inline comment separated by space +127.0.0.2 localhost # inline comment separated by space # internet address, host name and aliases -127.000.000.003 localhost localhost.localdomain +127.0.0.3 localhost localhost.localdomain From cb80937bf6b728fa56ee315d2c079f07c2f9f2a1 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Tue, 8 Jun 2021 20:38:30 +0000 Subject: [PATCH 14/45] Revert "doc/go1.17: mention block profile bias fix" This reverts CL 324471 (commit 689f4c7415acc8a135440574a483e0eeabba8b87). Reason for revert: break ~all builders. And it is not a doc-only change. Change-Id: Iadbdda34d2ca476a9f5e6c2d3a28592ed7ccb067 Reviewed-on: https://go-review.googlesource.com/c/go/+/326170 Trust: Cherry Mui Run-TryBot: Cherry Mui Reviewed-by: Heschi Kreinick --- doc/go1.17.html | 10 ---------- src/runtime/pprof/pprof_test.go | 22 ---------------------- 2 files changed, 32 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 56f88e6724..1e153377d6 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -737,16 +737,6 @@ Do not send CLs removing the interior tags from such phrases. - -
runtime/pprof
-
-

- Block profiles are no longer biased to favor infrequent long events over - frequent short events. -

-
-
-
strconv

diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index e139ee787d..7cbb4fc7ae 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -106,28 +106,6 @@ func TestCPUProfileMultithreaded(t *testing.T) { }) } -func TestCPUProfileThreadBias(t *testing.T) { - cpuHogA := func(dur time.Duration) { - cpuHogger(cpuHog1, &salt2, dur) - } - - defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2)) - prof := testCPUProfile(t, stackContains, []string{"runtime/pprof.cpuHog1", "runtime/pprof.cpuHog2"}, avoidFunctions(), func(dur time.Duration) { - //c := make(chan int) - //go func() { - //cpuHogger(cpuHog1, &salt1, dur) - //c <- 1 - //}() - cpuHogA(dur) - //<-c - }) - fmt.Printf("%#v\n", prof) -} - -func cpuHogA(dur time.Duration) { - cpuHogger(cpuHog1, &salt2, dur) -} - // containsInlinedCall reports whether the function body for the function f is // known to contain an inlined function call within the first maxBytes bytes. func containsInlinedCall(f interface{}, maxBytes int) bool { From 6551763a60ce25d171feaa69089a7f1ca60f43b6 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Tue, 8 Jun 2021 16:42:02 -0400 Subject: [PATCH 15/45] doc/go1.17: mention block profile bias fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-apply the doc part of CL 324471, originally written by Felix Geisendörfer. Change-Id: I831bead9a385bc5a5eed3058649a25ef17373bc6 Reviewed-on: https://go-review.googlesource.com/c/go/+/326171 Trust: Cherry Mui Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/go1.17.html b/doc/go1.17.html index 1e153377d6..eb7932cd67 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -737,6 +737,15 @@ Do not send CLs removing the interior tags from such phrases.

+
runtime/pprof
+
+

+ Block profiles are no longer biased to favor infrequent long events over + frequent short events. +

+
+
+
strconv

From 63dcab2e91cfa40ae6dc1f0455b1f3c2801a00ec Mon Sep 17 00:00:00 2001 From: Tim King Date: Tue, 25 May 2021 19:23:02 -0700 Subject: [PATCH 16/45] doc/go1.17: mention new vet checks sigchanyzer and stdmethods. These vet checks were added in CL 299532 and CL 321389. Also adds a TODO for buildtags. Change-Id: I516dc77729f6d2dc147318260fe452831b115dfa Reviewed-on: https://go-review.googlesource.com/c/go/+/322769 Trust: Tim King Run-TryBot: Tim King TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 49 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index eb7932cd67..cc3bcdf180 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -277,14 +277,55 @@ Do not send CLs removing the interior tags from such phrases.

Vet

-

- TODO: https://golang.org/cl/299532: cmd/vet: bring in sigchanyzer to report unbuffered channels to signal.Notify +

New warning within buildtags

+ +

+ TODO(rsc): Describe changes to buildtags https://golang.org/cl/240609

-

- TODO: complete the Vet section +

New warning for calling signal.Notify on unbuffered channels

+ +

+ The vet tool now warns about calls to signal.Notify + with incoming signals being sent to an unbuffered channel. Using an unbuffered channel + risks missing signals sent on them as signal.Notify does not block when + sending to a channel. For example:

+
+c := make(chan os.Signal)
+// signals are sent on c before the channel is read from.
+// This signal may be dropped as c is unbuffered.
+signal.Notify(c, os.Interrupt)
+
+ +

+ Users of signal.Notify should use channels with sufficient buffer space to keep up with the + expected signal rate. +

+ +

New warnings for Is, As and Unwrap methods

+ +

+ The vet tool now warns about methods named As, Is or Unwrap + on types implementing the error interface that have a different signature than the + one expected by the errors package. The errors.{As,Is,Unwrap} functions + expect such methods to implement either Is(error) bool, + As(interface{}) bool, or Unwrap() error + respectively. The functions errors.{As,Is,Unwrap} will ignore methods with the same + names but a different signature. For example: +

+ +
+type MyError struct { hint string }
+func (m MyError) Error() string { ... } // MyError implements error.
+func (MyError) Is(target interface{}) bool { ... } // target is interface{} instead of error.
+func Foo() bool {
+	x, y := MyError{"A"}, MyError{"B"}
+	return errors.Is(x, y) // returns false as x != y and MyError does not have an `Is(error) bool` function.
+}
+
+

Cover

From bcecae2af6ee43abebf84411385d538ec4e7d0ea Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 8 Jun 2021 12:42:02 -0700 Subject: [PATCH 17/45] doc/go1.17: mention new possibility of type conversion panicking For #44513 For #46020 Change-Id: I07c7a4268465c536d1866cc6bb1fad76b2b88b15 Reviewed-on: https://go-review.googlesource.com/c/go/+/326149 Trust: Ian Lance Taylor Reviewed-by: Matthew Dempsky --- doc/go1.17.html | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/go1.17.html b/doc/go1.17.html index cc3bcdf180..011377a84e 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -67,6 +67,14 @@ Do not send CLs removing the interior tags from such phrases. using unsafe.Add or unsafe.Slice.

+ +

+ Note that the new conversion from slice to array pointer is the + first case in which a type conversion can panic at run time. + Analysis tools that assume type conversions can never panic + should be updated to consider this possibility. +

+

Ports

Darwin

From 07ca28d529e1afb64a9f6f068214c05ee9772d34 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 8 Jun 2021 19:45:41 -0400 Subject: [PATCH 18/45] cmd/link: fix bug in -strictdups checking of BSS symbols The linker's -strictdups debugging option was not properly checking for cases where you have two dupok BSS symbols with different length (the check examined data length and content, but not symbol size). Updates #46653. Change-Id: I3844f25ef76dd6e4a84ffd5caed5d19a1b1a57c9 Reviewed-on: https://go-review.googlesource.com/c/go/+/326210 Trust: Than McIntosh Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: Cherry Mui --- src/cmd/link/internal/loader/loader.go | 12 +++++-- src/cmd/link/link_test.go | 43 +++++++++++++++++++------- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 1b71a66c6f..efca824d98 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -699,12 +699,18 @@ func (l *Loader) checkdup(name string, r *oReader, li uint32, dup Sym) { p := r.Data(li) rdup, ldup := l.toLocal(dup) pdup := rdup.Data(ldup) - if bytes.Equal(p, pdup) { - return - } reason := "same length but different contents" if len(p) != len(pdup) { reason = fmt.Sprintf("new length %d != old length %d", len(p), len(pdup)) + } else if bytes.Equal(p, pdup) { + // For BSS symbols, we need to check size as well, see issue 46653. + szdup := l.SymSize(dup) + sz := int64(r.Sym(li).Siz()) + if szdup == sz { + return + } + reason = fmt.Sprintf("different sizes: new size %d != old size %d", + sz, szdup) } fmt.Fprintf(os.Stderr, "cmd/link: while reading object for '%v': duplicate symbol '%s', previous def at '%v', with mismatched payload: %s\n", r.unit.Lib, name, rdup.unit.Lib, reason) diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go index 4d6bc76aca..7230054bed 100644 --- a/src/cmd/link/link_test.go +++ b/src/cmd/link/link_test.go @@ -470,10 +470,30 @@ TEXT ·f(SB), NOSPLIT|DUPOK, $0-0 JMP 0(PC) ` +const testStrictDupAsmSrc3 = ` +#include "textflag.h" +GLOBL ·rcon(SB), RODATA|DUPOK, $64 +` + +const testStrictDupAsmSrc4 = ` +#include "textflag.h" +GLOBL ·rcon(SB), RODATA|DUPOK, $32 +` + func TestStrictDup(t *testing.T) { // Check that -strictdups flag works. testenv.MustHaveGoBuild(t) + asmfiles := []struct { + fname string + payload string + }{ + {"a", testStrictDupAsmSrc1}, + {"b", testStrictDupAsmSrc2}, + {"c", testStrictDupAsmSrc3}, + {"d", testStrictDupAsmSrc4}, + } + t.Parallel() tmpdir := t.TempDir() @@ -483,15 +503,12 @@ func TestStrictDup(t *testing.T) { if err != nil { t.Fatal(err) } - src = filepath.Join(tmpdir, "a.s") - err = ioutil.WriteFile(src, []byte(testStrictDupAsmSrc1), 0666) - if err != nil { - t.Fatal(err) - } - src = filepath.Join(tmpdir, "b.s") - err = ioutil.WriteFile(src, []byte(testStrictDupAsmSrc2), 0666) - if err != nil { - t.Fatal(err) + for _, af := range asmfiles { + src = filepath.Join(tmpdir, af.fname+".s") + err = ioutil.WriteFile(src, []byte(af.payload), 0666) + if err != nil { + t.Fatal(err) + } } src = filepath.Join(tmpdir, "go.mod") err = ioutil.WriteFile(src, []byte("module teststrictdup\n"), 0666) @@ -503,7 +520,7 @@ func TestStrictDup(t *testing.T) { cmd.Dir = tmpdir out, err := cmd.CombinedOutput() if err != nil { - t.Errorf("linking with -strictdups=1 failed: %v", err) + t.Errorf("linking with -strictdups=1 failed: %v\n%s", err, string(out)) } if !bytes.Contains(out, []byte("mismatched payload")) { t.Errorf("unexpected output:\n%s", out) @@ -515,7 +532,11 @@ func TestStrictDup(t *testing.T) { if err == nil { t.Errorf("linking with -strictdups=2 did not fail") } - if !bytes.Contains(out, []byte("mismatched payload")) { + // NB: on amd64 we get the 'new length' error, on arm64 the 'different + // contents' error. + if !(bytes.Contains(out, []byte("mismatched payload: new length")) || + bytes.Contains(out, []byte("mismatched payload: same length but different contents"))) || + !bytes.Contains(out, []byte("mismatched payload: different sizes")) { t.Errorf("unexpected output:\n%s", out) } } From aa5540cd82170f82c6fe11511e12de96aa58cbc1 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 8 Jun 2021 20:09:49 -0400 Subject: [PATCH 19/45] cmd/compile: make map.zero symbol content-addressable The compiler machinery that generates "map.zero" symbols marks them as RODATA and DUPOK, which is problematic when a given application has multiple map zero symbols (from different packages) with varying sizes: the dupok path in the loader assumes that if two symbols have the same name, it is safe to pick any of the versions. In the case of map.zero, the link needs to select the largest symbol, not an arbitrary sym. To fix this problem, mark map.zero symbols as content-addressable, since the loader's content addressability processing path already supports selection of the larger symbol in cases where there are dups. Fixes #46653. Change-Id: Iabd2feef01d448670ba795c7eaddc48c191ea276 Reviewed-on: https://go-review.googlesource.com/c/go/+/326211 Trust: Than McIntosh Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: Cherry Mui --- src/cmd/compile/internal/gc/obj.go | 1 + test/fixedbugs/issue46653.dir/bad/bad.go | 64 ++++++++++++++++++++++++ test/fixedbugs/issue46653.dir/main.go | 27 ++++++++++ test/fixedbugs/issue46653.go | 10 ++++ 4 files changed, 102 insertions(+) create mode 100644 test/fixedbugs/issue46653.dir/bad/bad.go create mode 100644 test/fixedbugs/issue46653.dir/main.go create mode 100644 test/fixedbugs/issue46653.go diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index 0b10cb8a9e..55a0ab7da7 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -148,6 +148,7 @@ func dumpdata() { if reflectdata.ZeroSize > 0 { zero := base.PkgLinksym("go.map", "zero", obj.ABI0) objw.Global(zero, int32(reflectdata.ZeroSize), obj.DUPOK|obj.RODATA) + zero.Set(obj.AttrContentAddressable, true) } staticdata.WriteFuncSyms() diff --git a/test/fixedbugs/issue46653.dir/bad/bad.go b/test/fixedbugs/issue46653.dir/bad/bad.go new file mode 100644 index 0000000000..c1611b8347 --- /dev/null +++ b/test/fixedbugs/issue46653.dir/bad/bad.go @@ -0,0 +1,64 @@ +// 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 a + +func Bad() { + m := make(map[int64]A) + a := m[0] + if len(a.B.C1.D2.E2.F1) != 0 || + len(a.B.C1.D2.E2.F2) != 0 || + len(a.B.C1.D2.E2.F3) != 0 || + len(a.B.C1.D2.E2.F4) != 0 || + len(a.B.C1.D2.E2.F5) != 0 || + len(a.B.C1.D2.E2.F6) != 0 || + len(a.B.C1.D2.E2.F7) != 0 || + len(a.B.C1.D2.E2.F8) != 0 || + len(a.B.C1.D2.E2.F9) != 0 || + len(a.B.C1.D2.E2.F10) != 0 || + len(a.B.C1.D2.E2.F11) != 0 || + len(a.B.C1.D2.E2.F16) != 0 { + panic("bad") + } +} + +type A struct { + B +} + +type B struct { + C1 C + C2 C +} + +type C struct { + D1 D + D2 D +} + +type D struct { + E1 E + E2 E + E3 E + E4 E +} + +type E struct { + F1 string + F2 string + F3 string + F4 string + F5 string + F6 string + F7 string + F8 string + F9 string + F10 string + F11 string + F12 string + F13 string + F14 string + F15 string + F16 string +} diff --git a/test/fixedbugs/issue46653.dir/main.go b/test/fixedbugs/issue46653.dir/main.go new file mode 100644 index 0000000000..e2a96e54ec --- /dev/null +++ b/test/fixedbugs/issue46653.dir/main.go @@ -0,0 +1,27 @@ +// 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 main + +import ( + bad "issue46653.dir/bad" +) + +func main() { + bad.Bad() +} + +func neverCalled() L { + m := make(map[string]L) + return m[""] +} + +type L struct { + A Data + B Data +} + +type Data struct { + F1 [22][]string +} diff --git a/test/fixedbugs/issue46653.go b/test/fixedbugs/issue46653.go new file mode 100644 index 0000000000..e6283b1de5 --- /dev/null +++ b/test/fixedbugs/issue46653.go @@ -0,0 +1,10 @@ +// runindir + +// 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. + +// Test to verify compiler and linker handling of multiple +// competing map.zero symbol definitions. + +package ignored From 139e935d3cc8d38c9adc7ff7de8a87c28fe339c6 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 13 May 2021 10:46:58 -0400 Subject: [PATCH 20/45] math/big: comment division The comments in the code refer to Knuth and to Burnikel and Ziegler, but Knuth's presentation is inscrutable, and our recursive division code does not bear much resemblance to Burnikel and Ziegler's paper (which is fine, ours is nicer). Add a standalone explanation of division instead of referring to difficult or not-directly-used references. Change-Id: Ic1b35dc167fb29a69ee00e0b4a768ac9cc9e1324 Reviewed-on: https://go-review.googlesource.com/c/go/+/321078 Trust: Russ Cox Trust: Katie Hockman Run-TryBot: Russ Cox Reviewed-by: Katie Hockman Reviewed-by: Filippo Valsorda --- src/math/big/natdiv.go | 684 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 611 insertions(+), 73 deletions(-) diff --git a/src/math/big/natdiv.go b/src/math/big/natdiv.go index 1330990c2c..882bb6d3ba 100644 --- a/src/math/big/natdiv.go +++ b/src/math/big/natdiv.go @@ -2,10 +2,506 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +/* + +Multi-precision division. Here be dragons. + +Given u and v, where u is n+m digits, and v is n digits (with no leading zeros), +the goal is to return quo, rem such that u = quo*v + rem, where 0 ≤ rem < v. +That is, quo = ⌊u/v⌋ where ⌊x⌋ denotes the floor (truncation to integer) of x, +and rem = u - quo·v. + + +Long Division + +Division in a computer proceeds the same as long division in elementary school, +but computers are not as good as schoolchildren at following vague directions, +so we have to be much more precise about the actual steps and what can happen. + +We work from most to least significant digit of the quotient, doing: + + • Guess a digit q, the number of v to subtract from the current + section of u to zero out the topmost digit. + • Check the guess by multiplying q·v and comparing it against + the current section of u, adjusting the guess as needed. + • Subtract q·v from the current section of u. + • Add q to the corresponding section of the result quo. + +When all digits have been processed, the final remainder is left in u +and returned as rem. + +For example, here is a sketch of dividing 5 digits by 3 digits (n=3, m=2). + + q₂ q₁ q₀ + _________________ + v₂ v₁ v₀ ) u₄ u₃ u₂ u₁ u₀ + ↓ ↓ ↓ | | + [u₄ u₃ u₂]| | + - [ q₂·v ]| | + ----------- ↓ | + [ rem | u₁]| + - [ q₁·v ]| + ----------- ↓ + [ rem | u₀] + - [ q₀·v ] + ------------ + [ rem ] + +Instead of creating new storage for the remainders and copying digits from u +as indicated by the arrows, we use u's storage directly as both the source +and destination of the subtractions, so that the remainders overwrite +successive overlapping sections of u as the division proceeds, using a slice +of u to identify the current section. This avoids all the copying as well as +shifting of remainders. + +Division of u with n+m digits by v with n digits (in base B) can in general +produce at most m+1 digits, because: + + • u < B^(n+m) [B^(n+m) has n+m+1 digits] + • v ≥ B^(n-1) [B^(n-1) is the smallest n-digit number] + • u/v < B^(n+m) / B^(n-1) [divide bounds for u, v] + • u/v < B^(m+1) [simplify] + +The first step is special: it takes the top n digits of u and divides them by +the n digits of v, producing the first quotient digit and an n-digit remainder. +In the example, q₂ = ⌊u₄u₃u₂ / v⌋. + +The first step divides n digits by n digits to ensure that it produces only a +single digit. + +Each subsequent step appends the next digit from u to the remainder and divides +those n+1 digits by the n digits of v, producing another quotient digit and a +new n-digit remainder. + +Subsequent steps divide n+1 digits by n digits, an operation that in general +might produce two digits. However, as used in the algorithm, that division is +guaranteed to produce only a single digit. The dividend is of the form +rem·B + d, where rem is a remainder from the previous step and d is a single +digit, so: + + • rem ≤ v - 1 [rem is a remainder from dividing by v] + • rem·B ≤ v·B - B [multiply by B] + • d ≤ B - 1 [d is a single digit] + • rem·B + d ≤ v·B - 1 [add] + • rem·B + d < v·B [change ≤ to <] + • (rem·B + d)/v < B [divide by v] + + +Guess and Check + +At each step we need to divide n+1 digits by n digits, but this is for the +implementation of division by n digits, so we can't just invoke a division +routine: we _are_ the division routine. Instead, we guess at the answer and +then check it using multiplication. If the guess is wrong, we correct it. + +How can this guessing possibly be efficient? It turns out that the following +statement (let's call it the Good Guess Guarantee) is true. + +If + + • q = ⌊u/v⌋ where u is n+1 digits and v is n digits, + • q < B, and + • the topmost digit of v = vₙ₋₁ ≥ B/2, + +then q̂ = ⌊uₙuₙ₋₁ / vₙ₋₁⌋ satisfies q ≤ q̂ ≤ q+2. (Proof below.) + +That is, if we know the answer has only a single digit and we guess an answer +by ignoring the bottom n-1 digits of u and v, using a 2-by-1-digit division, +then that guess is at least as large as the correct answer. It is also not +too much larger: it is off by at most two from the correct answer. + +Note that in the first step of the overall division, which is an n-by-n-digit +division, the 2-by-1 guess uses an implicit uₙ = 0. + +Note that using a 2-by-1-digit division here does not mean calling ourselves +recursively. Instead, we use an efficient direct hardware implementation of +that operation. + +Note that because q is u/v rounded down, q·v must not exceed u: u ≥ q·v. +If a guess q̂ is too big, it will not satisfy this test. Viewed a different way, +the remainder r̂ for a given q̂ is u - q̂·v, which must be positive. If it is +negative, then the guess q̂ is too big. + +This gives us a way to compute q. First compute q̂ with 2-by-1-digit division. +Then, while u < q̂·v, decrement q̂; this loop executes at most twice, because +q̂ ≤ q+2. + + +Scaling Inputs + +The Good Guess Guarantee requires that the top digit of v (vₙ₋₁) be at least B/2. +For example in base 10, ⌊172/19⌋ = 9, but ⌊18/1⌋ = 18: the guess is wildly off +because the first digit 1 is smaller than B/2 = 5. + +We can ensure that v has a large top digit by multiplying both u and v by the +right amount. Continuing the example, if we multiply both 172 and 19 by 3, we +now have ⌊516/57⌋, the leading digit of v is now ≥ 5, and sure enough +⌊51/5⌋ = 10 is much closer to the correct answer 9. It would be easier here +to multiply by 4, because that can be done with a shift. Specifically, we can +always count the number of leading zeros i in the first digit of v and then +shift both u and v left by i bits. + +Having scaled u and v, the value ⌊u/v⌋ is unchanged, but the remainder will +be scaled: 172 mod 19 is 1, but 516 mod 57 is 3. We have to divide the remainder +by the scaling factor (shifting right i bits) when we finish. + +Note that these shifts happen before and after the entire division algorithm, +not at each step in the per-digit iteration. + +Note the effect of scaling inputs on the size of the possible quotient. +In the scaled u/v, u can gain a digit from scaling; v never does, because we +pick the scaling factor to make v's top digit larger but without overflowing. +If u and v have n+m and n digits after scaling, then: + + • u < B^(n+m) [B^(n+m) has n+m+1 digits] + • v ≥ B^n / 2 [vₙ₋₁ ≥ B/2, so vₙ₋₁·B^(n-1) ≥ B^n/2] + • u/v < B^(n+m) / (B^n / 2) [divide bounds for u, v] + • u/v < 2 B^m [simplify] + +The quotient can still have m+1 significant digits, but if so the top digit +must be a 1. This provides a different way to handle the first digit of the +result: compare the top n digits of u against v and fill in either a 0 or a 1. + + +Refining Guesses + +Before we check whether u < q̂·v, we can adjust our guess to change it from +q̂ = ⌊uₙuₙ₋₁ / vₙ₋₁⌋ into the refined guess ⌊uₙuₙ₋₁uₙ₋₂ / vₙ₋₁vₙ₋₂⌋. +Although not mentioned above, the Good Guess Guarantee also promises that this +3-by-2-digit division guess is more precise and at most one away from the real +answer q. The improvement from the 2-by-1 to the 3-by-2 guess can also be done +without n-digit math. + +If we have a guess q̂ = ⌊uₙuₙ₋₁ / vₙ₋₁⌋ and we want to see if it also equal to +⌊uₙuₙ₋₁uₙ₋₂ / vₙ₋₁vₙ₋₂⌋, we can use the same check we would for the full division: +if uₙuₙ₋₁uₙ₋₂ < q̂·vₙ₋₁vₙ₋₂, then the guess is too large and should be reduced. + +Checking uₙuₙ₋₁uₙ₋₂ < q̂·vₙ₋₁vₙ₋₂ is the same as uₙuₙ₋₁uₙ₋₂ - q̂·vₙ₋₁vₙ₋₂ < 0, +and + + uₙuₙ₋₁uₙ₋₂ - q̂·vₙ₋₁vₙ₋₂ = (uₙuₙ₋₁·B + uₙ₋₂) - q̂·(vₙ₋₁·B + vₙ₋₂) + [splitting off the bottom digit] + = (uₙuₙ₋₁ - q̂·vₙ₋₁)·B + uₙ₋₂ - q̂·vₙ₋₂ + [regrouping] + +The expression (uₙuₙ₋₁ - q̂·vₙ₋₁) is the remainder of uₙuₙ₋₁ / vₙ₋₁. +If the initial guess returns both q̂ and its remainder r̂, then checking +whether uₙuₙ₋₁uₙ₋₂ < q̂·vₙ₋₁vₙ₋₂ is the same as checking r̂·B + uₙ₋₂ < q̂·vₙ₋₂. + +If we find that r̂·B + uₙ₋₂ < q̂·vₙ₋₂, then we can adjust the guess by +decrementing q̂ and adding vₙ₋₁ to r̂. We repeat until r̂·B + uₙ₋₂ ≥ q̂·vₙ₋₂. +(As before, this fixup is only needed at most twice.) + +Now that q̂ = ⌊uₙuₙ₋₁uₙ₋₂ / vₙ₋₁vₙ₋₂⌋, as mentioned above it is at most one +away from the correct q, and we've avoided doing any n-digit math. +(If we need the new remainder, it can be computed as r̂·B + uₙ₋₂ - q̂·vₙ₋₂.) + +The final check u < q̂·v and the possible fixup must be done at full precision. +For random inputs, a fixup at this step is exceedingly rare: the 3-by-2 guess +is not often wrong at all. But still we must do the check. Note that since the +3-by-2 guess is off by at most 1, it can be convenient to perform the final +u < q̂·v as part of the computation of the remainder r = u - q̂·v. If the +subtraction underflows, decremeting q̂ and adding one v back to r is enough to +arrive at the final q, r. + +That's the entirety of long division: scale the inputs, and then loop over +each output position, guessing, checking, and correcting the next output digit. + +For a 2n-digit number divided by an n-digit number (the worst size-n case for +division complexity), this algorithm uses n+1 iterations, each of which must do +at least the 1-by-n-digit multiplication q̂·v. That's O(n) iterations of +O(n) time each, so O(n²) time overall. + + +Recursive Division + +For very large inputs, it is possible to improve on the O(n²) algorithm. +Let's call a group of n/2 real digits a (very) “wide digit”. We can run the +standard long division algorithm explained above over the wide digits instead of +the actual digits. This will result in many fewer steps, but the math involved in +each step is more work. + +Where basic long division uses a 2-by-1-digit division to guess the initial q̂, +the new algorithm must use a 2-by-1-wide-digit division, which is of course +really an n-by-n/2-digit division. That's OK: if we implement n-digit division +in terms of n/2-digit division, the recursion will terminate when the divisor +becomes small enough to handle with standard long division or even with the +2-by-1 hardware instruction. + +For example, here is a sketch of dividing 10 digits by 4, proceeding with +wide digits corresponding to two regular digits. The first step, still special, +must leave off a (regular) digit, dividing 5 by 4 and producing a 4-digit +remainder less than v. The middle steps divide 6 digits by 4, guaranteed to +produce two output digits each (one wide digit) with 4-digit remainders. +The final step must use what it has: the 4-digit remainder plus one more, +5 digits to divide by 4. + + q₆ q₅ q₄ q₃ q₂ q₁ q₀ + _______________________________ + v₃ v₂ v₁ v₀ ) u₉ u₈ u₇ u₆ u₅ u₄ u₃ u₂ u₁ u₀ + ↓ ↓ ↓ ↓ ↓ | | | | | + [u₉ u₈ u₇ u₆ u₅]| | | | | + - [ q₆q₅·v ]| | | | | + ----------------- ↓ ↓ | | | + [ rem |u₄ u₃]| | | + - [ q₄q₃·v ]| | | + -------------------- ↓ ↓ | + [ rem |u₂ u₁]| + - [ q₂q₁·v ]| + -------------------- ↓ + [ rem |u₀] + - [ q₀·v ] + ------------------ + [ rem ] + +An alternative would be to look ahead to how well n/2 divides into n+m and +adjust the first step to use fewer digits as needed, making the first step +more special to make the last step not special at all. For example, using the +same input, we could choose to use only 4 digits in the first step, leaving +a full wide digit for the last step: + + q₆ q₅ q₄ q₃ q₂ q₁ q₀ + _______________________________ + v₃ v₂ v₁ v₀ ) u₉ u₈ u₇ u₆ u₅ u₄ u₃ u₂ u₁ u₀ + ↓ ↓ ↓ ↓ | | | | | | + [u₉ u₈ u₇ u₆]| | | | | | + - [ q₆·v ]| | | | | | + -------------- ↓ ↓ | | | | + [ rem |u₅ u₄]| | | | + - [ q₅q₄·v ]| | | | + -------------------- ↓ ↓ | | + [ rem |u₃ u₂]| | + - [ q₃q₂·v ]| | + -------------------- ↓ ↓ + [ rem |u₁ u₀] + - [ q₁q₀·v ] + --------------------- + [ rem ] + +Today, the code in divRecursiveStep works like the first example. Perhaps in +the future we will make it work like the alternative, to avoid a special case +in the final iteration. + +Either way, each step is a 3-by-2-wide-digit division approximated first by +a 2-by-1-wide-digit division, just as we did for regular digits in long division. +Because the actual answer we want is a 3-by-2-wide-digit division, instead of +multiplying q̂·v directly during the fixup, we can use the quick refinement +from long division (an n/2-by-n/2 multiply) to correct q to its actual value +and also compute the remainder (as mentioned above), and then stop after that, +never doing a full n-by-n multiply. + +Instead of using an n-by-n/2-digit division to produce n/2 digits, we can add +(not discard) one more real digit, doing an (n+1)-by-(n/2+1)-digit division that +produces n/2+1 digits. That single extra digit tightens the Good Guess Guarantee +to q ≤ q̂ ≤ q+1 and lets us drop long division's special treatment of the first +digit. These benefits are discussed more after the Good Guess Guarantee proof +below. + + +How Fast is Recursive Division? + +For a 2n-by-n-digit division, this algorithm runs a 4-by-2 long division over +wide digits, producing two wide digits plus a possible leading regular digit 1, +which can be handled without a recursive call. That is, the algorithm uses two +full iterations, each using an n-by-n/2-digit division and an n/2-by-n/2-digit +multiplication, along with a few n-digit additions and subtractions. The standard +n-by-n-digit multiplication algorithm requires O(n²) time, making the overall +algorithm require time T(n) where + + T(n) = 2T(n/2) + O(n) + O(n²) + +which, by the Bentley-Haken-Saxe theorem, ends up reducing to T(n) = O(n²). +This is not an improvement over regular long division. + +When the number of digits n becomes large enough, Karatsuba's algorithm for +multiplication can be used instead, which takes O(n^log₂3) = O(n^1.6) time. +(Karatsuba multiplication is implemented in func karatsuba in nat.go.) +That makes the overall recursive division algorithm take O(n^1.6) time as well, +which is an improvement, but again only for large enough numbers. + +It is not critical to make sure that every recursion does only two recursive +calls. While in general the number of recursive calls can change the time +analysis, in this case doing three calls does not change the analysis: + + T(n) = 3T(n/2) + O(n) + O(n^log₂3) + +ends up being T(n) = O(n^log₂3). Because the Karatsuba multiplication taking +time O(n^log₂3) is itself doing 3 half-sized recursions, doing three for the +division does not hurt the asymptotic performance. Of course, it is likely +still faster in practice to do two. + + +Proof of the Good Guess Guarantee + +Given numbers x, y, let us break them into the quotients and remainders when +divided by some scaling factor S, with the added constraints that the quotient +x/y and the high part of y are both less than some limit T, and that the high +part of y is at least half as big as T. + + x₁ = ⌊x/S⌋ y₁ = ⌊y/S⌋ + x₀ = x mod S y₀ = y mod S + + x = x₁·S + x₀ 0 ≤ x₀ < S x/y < T + y = y₁·S + y₀ 0 ≤ y₀ < S T/2 ≤ y₁ < T + +And consider the two truncated quotients: + + q = ⌊x/y⌋ + q̂ = ⌊x₁/y₁⌋ + +We will prove that q ≤ q̂ ≤ q+2. + +The guarantee makes no real demands on the scaling factor S: it is simply the +magnitude of the digits cut from both x and y to produce x₁ and y₁. +The guarantee makes only limited demands on T: it must be large enough to hold +the quotient x/y, and y₁ must have roughly the same size. + +To apply to the earlier discussion of 2-by-1 guesses in long division, +we would choose: + + S = Bⁿ⁻¹ + T = B + x = u + x₁ = uₙuₙ₋₁ + x₀ = uₙ₋₂...u₀ + y = v + y₁ = vₙ₋₁ + y₀ = vₙ₋₂...u₀ + +These simpler variables avoid repeating those longer expressions in the proof. + +Note also that, by definition, truncating division ⌊x/y⌋ satisfies + + x/y - 1 < ⌊x/y⌋ ≤ x/y. + +This fact will be used a few times in the proofs. + +Proof that q ≤ q̂: + + q̂·y₁ = ⌊x₁/y₁⌋·y₁ [by definition, q̂ = ⌊x₁/y₁⌋] + > (x₁/y₁ - 1)·y₁ [x₁/y₁ - 1 < ⌊x₁/y₁⌋] + = x₁ - y₁ [distribute y₁] + + So q̂·y₁ > x₁ - y₁. + Since q̂·y₁ is an integer, q̂·y₁ ≥ x₁ - y₁ + 1. + + q̂ - q = q̂ - ⌊x/y⌋ [by definition, q = ⌊x/y⌋] + ≥ q̂ - x/y [⌊x/y⌋ < x/y] + = (1/y)·(q̂·y - x) [factor out 1/y] + ≥ (1/y)·(q̂·y₁·S - x) [y = y₁·S + y₀ ≥ y₁·S] + ≥ (1/y)·((x₁ - y₁ + 1)·S - x) [above: q̂·y₁ ≥ x₁ - y₁ + 1] + = (1/y)·(x₁·S - y₁·S + S - x) [distribute S] + = (1/y)·(S - x₀ - y₁·S) [-x = -x₁·S - x₀] + > -y₁·S / y [x₀ < S, so S - x₀ < 0; drop it] + ≥ -1 [y₁·S ≤ y] + + So q̂ - q > -1. + Since q̂ - q is an integer, q̂ - q ≥ 0, or equivalently q ≤ q̂. + +Proof that q̂ ≤ q+2: + + x₁/y₁ - x/y = x₁·S/y₁·S - x/y [multiply left term by S/S] + ≤ x/y₁·S - x/y [x₁S ≤ x] + = (x/y)·(y/y₁·S - 1) [factor out x/y] + = (x/y)·((y - y₁·S)/y₁·S) [move -1 into y/y₁·S fraction] + = (x/y)·(y₀/y₁·S) [y - y₁·S = y₀] + = (x/y)·(1/y₁)·(y₀/S) [factor out 1/y₁] + < (x/y)·(1/y₁) [y₀ < S, so y₀/S < 1] + ≤ (x/y)·(2/T) [y₁ ≥ T/2, so 1/y₁ ≤ 2/T] + < T·(2/T) [x/y < T] + = 2 [T·(2/T) = 2] + + So x₁/y₁ - x/y < 2. + + q̂ - q = ⌊x₁/y₁⌋ - q [by definition, q̂ = ⌊x₁/y₁⌋] + = ⌊x₁/y₁⌋ - ⌊x/y⌋ [by definition, q = ⌊x/y⌋] + ≤ x₁/y₁ - ⌊x/y⌋ [⌊x₁/y₁⌋ ≤ x₁/y₁] + < x₁/y₁ - (x/y - 1) [⌊x/y⌋ > x/y - 1] + = (x₁/y₁ - x/y) + 1 [regrouping] + < 2 + 1 [above: x₁/y₁ - x/y < 2] + = 3 + + So q̂ - q < 3. + Since q̂ - q is an integer, q̂ - q ≤ 2. + +Note that when x/y < T/2, the bounds tighten to x₁/y₁ - x/y < 1 and therefore +q̂ - q ≤ 1. + +Note also that in the general case 2n-by-n division where we don't know that +x/y < T, we do know that x/y < 2T, yielding the bound q̂ - q ≤ 4. So we could +remove the special case first step of long division as long as we allow the +first fixup loop to run up to four times. (Using a simple comparison to decide +whether the first digit is 0 or 1 is still more efficient, though.) + +Finally, note that when dividing three leading base-B digits by two (scaled), +we have T = B² and x/y < B = T/B, a much tighter bound than x/y < T. +This in turn yields the much tighter bound x₁/y₁ - x/y < 2/B. This means that +⌊x₁/y₁⌋ and ⌊x/y⌋ can only differ when x/y is less than 2/B greater than an +integer. For random x and y, the chance of this is 2/B, or, for large B, +approximately zero. This means that after we produce the 3-by-2 guess in the +long division algorithm, the fixup loop essentially never runs. + +In the recursive algorithm, the extra digit in (2·⌊n/2⌋+1)-by-(⌊n/2⌋+1)-digit +division has exactly the same effect: the probability of needing a fixup is the +same 2/B. Even better, we can allow the general case x/y < 2T and the fixup +probability only grows to 4/B, still essentially zero. + + +References + +There are no great references for implementing long division; thus this comment. +Here are some notes about what to expect from the obvious references. + +Knuth Volume 2 (Seminumerical Algorithms) section 4.3.1 is the usual canonical +reference for long division, but that entire series is highly compressed, never +repeating a necessary fact and leaving important insights to the exercises. +For example, no rationale whatsoever is given for the calculation that extends +q̂ from a 2-by-1 to a 3-by-2 guess, nor why it reduces the error bound. +The proof that the calculation even has the desired effect is left to exercises. +The solutions to those exercises provided at the back of the book are entirely +calculations, still with no explanation as to what is going on or how you would +arrive at the idea of doing those exact calculations. Nowhere is it mentioned +that this test extends the 2-by-1 guess into a 3-by-2 guess. The proof of the +Good Guess Guarantee is only for the 2-by-1 guess and argues by contradiction, +making it difficult to understand how modifications like adding another digit +or adjusting the quotient range affects the overall bound. + +All that said, Knuth remains the canonical reference. It is dense but packed +full of information and references, and the proofs are simpler than many other +presentations. The proofs above are reworkings of Knuth's to remove the +arguments by contradiction and add explanations or steps that Knuth omitted. +But beware of errors in older printings. Take the published errata with you. + +Brinch Hansen's “Multiple-length Division Revisited: a Tour of the Minefield” +starts with a blunt critique of Knuth's presentation (among others) and then +presents a more detailed and easier to follow treatment of long division, +including an implementation in Pascal. But the algorithm and implementation +work entirely in terms of 3-by-2 division, which is much less useful on modern +hardware than an algorithm using 2-by-1 division. The proofs are a bit too +focused on digit counting and seem needlessly complex, especially compared to +the ones given above. + +Burnikel and Ziegler's “Fast Recursive Division” introduced the key insight of +implementing division by an n-digit divisor using recursive calls to division +by an n/2-digit divisor, relying on Karatsuba multiplication to yield a +sub-quadratic run time. However, the presentation decisions are made almost +entirely for the purpose of simplifying the run-time analysis, rather than +simplifying the presentation. Instead of a single algorithm that loops over +quotient digits, the paper presents two mutually-recursive algorithms, for +2n-by-n and 3n-by-2n. The paper also does not present any general (n+m)-by-n +algorithm. + +The proofs in the paper are remarkably complex, especially considering that +the algorithm is at its core just long division on wide digits, so that the +usual long division proofs apply essentially unaltered. +*/ + package big import "math/bits" +// div returns q, r such that q = ⌊u/v⌋ and r = u%v = u - q·v. +// It uses z and z2 as the storage for q and r. func (z nat) div(z2, u, v nat) (q, r nat) { if len(v) == 0 { panic("division by zero") @@ -18,6 +514,8 @@ func (z nat) div(z2, u, v nat) (q, r nat) { } if len(v) == 1 { + // Short division: long optimized for a single-word divisor. + // In that case, the 2-by-1 guess is all we need at each step. var r2 Word q, r2 = z.divW(u, v[0]) r = z2.setWord(r2) @@ -28,7 +526,9 @@ func (z nat) div(z2, u, v nat) (q, r nat) { return } -// q = (x-r)/y, with 0 <= r < y +// divW returns q, r such that q = ⌊x/y⌋ and r = x%y = x - q·y. +// It uses z as the storage for q. +// Note that y is a single digit (Word), not a big number. func (z nat) divW(x nat, y Word) (q nat, r Word) { m := len(x) switch { @@ -56,6 +556,8 @@ func (x nat) modW(d Word) (r Word) { return divWVW(q, 0, x, d) } +// divWVW overwrites z with ⌊x/y⌋, returning the remainder r. +// The caller must ensure that len(z) = len(x). func divWVW(z []Word, xn Word, x []Word, y Word) (r Word) { r = xn if len(x) == 1 { @@ -70,34 +572,33 @@ func divWVW(z []Word, xn Word, x []Word, y Word) (r Word) { return r } -// q = (uIn-r)/vIn, with 0 <= r < vIn -// Uses z as storage for q, and u as storage for r if possible. -// See Knuth, Volume 2, section 4.3.1, Algorithm D. -// Preconditions: -// len(vIn) >= 2 -// len(uIn) >= len(vIn) -// u must not alias z +// div returns q, r such that q = ⌊uIn/vIn⌋ and r = uIn%vIn = uIn - q·vIn. +// It uses z and u as the storage for q and r. +// The caller must ensure that len(vIn) ≥ 2 (use divW otherwise) +// and that len(uIn) ≥ len(vIn) (the answer is 0, uIn otherwise). func (z nat) divLarge(u, uIn, vIn nat) (q, r nat) { n := len(vIn) m := len(uIn) - n - // D1. + // Scale the inputs so vIn's top bit is 1 (see “Scaling Inputs” above). + // vIn is treated as a read-only input (it may be in use by another + // goroutine), so we must make a copy. + // uIn is copied to u. shift := nlz(vIn[n-1]) - // do not modify vIn, it may be used by another goroutine simultaneously vp := getNat(n) v := *vp shlVU(v, vIn, shift) - - // u may safely alias uIn or vIn, the value of uIn is used to set u and vIn was already used u = u.make(len(uIn) + 1) u[len(uIn)] = shlVU(u[0:len(uIn)], uIn, shift) - // z may safely alias uIn or vIn, both values were used already + // The caller should not pass aliased z and u, since those are + // the two different outputs, but correct just in case. if alias(z, u) { - z = nil // z is an alias for u - cannot reuse + z = nil } q = z.make(m + 1) + // Use basic or recursive long division depending on size. if n < divRecursiveThreshold { q.divBasic(u, v) } else { @@ -106,19 +607,17 @@ func (z nat) divLarge(u, uIn, vIn nat) (q, r nat) { putNat(vp) q = q.norm() + + // Undo scaling of remainder. shrVU(u, u, shift) r = u.norm() return q, r } -// divBasic performs word-by-word division of u by v. -// The quotient is written in pre-allocated q. -// The remainder overwrites input u. -// -// Precondition: -// - q is large enough to hold the quotient u / v -// which has a maximum length of len(u)-len(v)+1. +// divBasic implements long division as described above. +// It overwrites q with ⌊u/v⌋ and overwrites u with the remainder r. +// q must be large enough to hold ⌊u/v⌋. func (q nat) divBasic(u, v nat) { n := len(v) m := len(u) - n @@ -126,45 +625,56 @@ func (q nat) divBasic(u, v nat) { qhatvp := getNat(n + 1) qhatv := *qhatvp - // D2. + // Set up for divWW below, precomputing reciprocal argument. vn1 := v[n-1] rec := reciprocalWord(vn1) + + // Compute each digit of quotient. for j := m; j >= 0; j-- { - // D3. + // Compute the 2-by-1 guess q̂. + // The first iteration must invent a leading 0 for u. qhat := Word(_M) var ujn Word if j+n < len(u) { ujn = u[j+n] } + + // ujn ≤ vn1, or else q̂ would be more than one digit. + // For ujn == vn1, we set q̂ to the max digit M above. + // Otherwise, we compute the 2-by-1 guess. if ujn != vn1 { var rhat Word qhat, rhat = divWW(ujn, u[j+n-1], vn1, rec) - // x1 | x2 = q̂v_{n-2} + // Refine q̂ to a 3-by-2 guess. See “Refining Guesses” above. vn2 := v[n-2] x1, x2 := mulWW(qhat, vn2) - // test if q̂v_{n-2} > br̂ + u_{j+n-2} ujn2 := u[j+n-2] - for greaterThan(x1, x2, rhat, ujn2) { + for greaterThan(x1, x2, rhat, ujn2) { // x1x2 > r̂ u[j+n-2] qhat-- prevRhat := rhat rhat += vn1 - // v[n-1] >= 0, so this tests for overflow. + // If r̂ overflows, then + // r̂ u[j+n-2]v[n-1] is now definitely > x1 x2. if rhat < prevRhat { break } + // TODO(rsc): No need for a full mulWW. + // x2 += vn2; if x2 overflows, x1++ x1, x2 = mulWW(qhat, vn2) } } - // D4. - // Compute the remainder u - (q̂*v) << (_W*j). - // The subtraction may overflow if q̂ estimate was off by one. + // Compute q̂·v. qhatv[n] = mulAddVWW(qhatv[0:n], v, qhat, 0) qhl := len(qhatv) if j+qhl > len(u) && qhatv[n] == 0 { qhl-- } + + // Subtract q̂·v from the current section of u. + // If it underflows, q̂·v > u, which we fix up + // by decrementing q̂ and adding v back. c := subVV(u[j:j+qhl], u[j:], qhatv) if c != 0 { c := addVV(u[j:j+n], u[j:], v) @@ -176,6 +686,8 @@ func (q nat) divBasic(u, v nat) { qhat-- } + // Save quotient digit. + // Caller may know the top digit is zero and not leave room for it. if j == m && m == len(q) && qhat == 0 { continue } @@ -185,30 +697,34 @@ func (q nat) divBasic(u, v nat) { putNat(qhatvp) } -// greaterThan reports whether (x1<<_W + x2) > (y1<<_W + y2) +// greaterThan reports whether the two digit numbers x1 x2 > y1 y2. +// TODO(rsc): In contradiction to most of this file, x1 is the high +// digit and x2 is the low digit. This should be fixed. func greaterThan(x1, x2, y1, y2 Word) bool { return x1 > y1 || x1 == y1 && x2 > y2 } +// divRecursiveThreshold is the number of divisor digits +// at which point divRecursive is faster than divBasic. const divRecursiveThreshold = 100 -// divRecursive performs word-by-word division of u by v. -// The quotient is written in pre-allocated z. -// The remainder overwrites input u. -// -// Precondition: -// - len(z) >= len(u)-len(v) -// -// See Burnikel, Ziegler, "Fast Recursive Division", Algorithm 1 and 2. +// divRecursive implements recursive division as described above. +// It overwrites z with ⌊u/v⌋ and overwrites u with the remainder r. +// z must be large enough to hold ⌊u/v⌋. +// This function is just for allocating and freeing temporaries +// around divRecursiveStep, the real implementation. func (z nat) divRecursive(u, v nat) { - // Recursion depth is less than 2 log2(len(v)) - // Allocate a slice of temporaries to be reused across recursion. + // Recursion depth is (much) less than 2 log₂(len(v)). + // Allocate a slice of temporaries to be reused across recursion, + // plus one extra temporary not live across the recursion. recDepth := 2 * bits.Len(uint(len(v))) - // large enough to perform Karatsuba on operands as large as v tmp := getNat(3 * len(v)) temps := make([]*nat, recDepth) + z.clear() z.divRecursiveStep(u, v, 0, tmp, temps) + + // Free temporaries. for _, n := range temps { if n != nil { putNat(n) @@ -217,72 +733,92 @@ func (z nat) divRecursive(u, v nat) { putNat(tmp) } -// divRecursiveStep computes the division of u by v. -// - z must be large enough to hold the quotient -// - the quotient will overwrite z -// - the remainder will overwrite u +// divRecursiveStep is the actual implementation of recursive division. +// It adds ⌊u/v⌋ to z and overwrites u with the remainder r. +// z must be large enough to hold ⌊u/v⌋. +// It uses temps[depth] (allocating if needed) as a temporary live across +// the recursive call. It also uses tmp, but not live across the recursion. func (z nat) divRecursiveStep(u, v nat, depth int, tmp *nat, temps []*nat) { + // u is a subsection of the original and may have leading zeros. + // TODO(rsc): The v = v.norm() is useless and should be removed. + // We know (and require) that v's top digit is ≥ B/2. u = u.norm() v = v.norm() - if len(u) == 0 { z.clear() return } + + // Fall back to basic division if the problem is now small enough. n := len(v) if n < divRecursiveThreshold { z.divBasic(u, v) return } + + // Nothing to do if u is shorter than v (implies u < v). m := len(u) - n if m < 0 { return } - // Produce the quotient by blocks of B words. - // Division by v (length n) is done using a length n/2 division - // and a length n/2 multiplication for each block. The final - // complexity is driven by multiplication complexity. + // We consider B digits in a row as a single wide digit. + // (See “Recursive Division” above.) + // + // TODO(rsc): rename B to Wide, to avoid confusion with _B, + // which is something entirely different. + // TODO(rsc): Look into whether using ⌈n/2⌉ is better than ⌊n/2⌋. B := n / 2 // Allocate a nat for qhat below. if temps[depth] == nil { - temps[depth] = getNat(n) + temps[depth] = getNat(n) // TODO(rsc): Can be just B+1. } else { *temps[depth] = temps[depth].make(B + 1) } + // Compute each wide digit of the quotient. + // + // TODO(rsc): Change the loop to be + // for j := (m+B-1)/B*B; j > 0; j -= B { + // which will make the final step a regular step, letting us + // delete what amounts to an extra copy of the loop body below. j := m for j > B { - // Divide u[j-B:j+n] by vIn. Keep remainder in u - // for next block. + // Divide u[j-B:j+n] (3 wide digits) by v (2 wide digits). + // First make the 2-by-1-wide-digit guess using a recursive call. + // Then extend the guess to the full 3-by-2 (see “Refining Guesses”). // - // The following property will be used (Lemma 2): - // if u = u1 << s + u0 - // v = v1 << s + v0 - // then floor(u1/v1) >= floor(u/v) - // - // Moreover, the difference is at most 2 if len(v1) >= len(u/v) - // We choose s = B-1 since len(v)-s >= B+1 >= len(u/v) + // For the 2-by-1-wide-digit guess, instead of doing 2B-by-B-digit, + // we use a (2B+1)-by-(B+1) digit, which handles the possibility that + // the result has an extra leading 1 digit as well as guaranteeing + // that the computed q̂ will be off by at most 1 instead of 2. + + // s is the number of digits to drop from the 3B- and 2B-digit chunks. + // We drop B-1 to be left with 2B+1 and B+1. s := (B - 1) - // Except for the first step, the top bits are always - // a division remainder, so the quotient length is <= n. + + // uu is the up-to-3B-digit section of u we are working on. uu := u[j-B:] + // Compute the 2-by-1 guess q̂, leaving r̂ in uu[s:B+n]. qhat := *temps[depth] qhat.clear() qhat.divRecursiveStep(uu[s:B+n], v[s:], depth+1, tmp, temps) qhat = qhat.norm() - // Adjust the quotient: - // u = u_h << s + u_l - // v = v_h << s + v_l - // u_h = q̂ v_h + rh - // u = q̂ (v - v_l) + rh << s + u_l - // After the above step, u contains a remainder: - // u = rh << s + u_l - // and we need to subtract q̂ v_l - // - // But it may be a bit too large, in which case q̂ needs to be smaller. + + // Extend to a 3-by-2 quotient and remainder. + // Because divRecursiveStep overwrote the top part of uu with + // the remainder r̂, the full uu already contains the equivalent + // of r̂·B + uₙ₋₂ from the “Refining Guesses” discussion. + // Subtracting q̂·vₙ₋₂ from it will compute the full-length remainder. + // If that subtraction underflows, q̂·v > u, which we fix up + // by decrementing q̂ and adding v back, same as in long division. + + // TODO(rsc): Instead of subtract and fix-up, this code is computing + // q̂·vₙ₋₂ and decrementing q̂ until that product is ≤ u. + // But we can do the subtraction directly, as in the comment above + // and in long division, because we know that q̂ is wrong by at most one. qhatv := tmp.make(3 * n) qhatv.clear() qhatv = qhatv.mul(qhat, v[:s]) @@ -309,6 +845,8 @@ func (z nat) divRecursiveStep(u, v nat, depth int, tmp *nat, temps []*nat) { j -= B } + // TODO(rsc): Rewrite loop as described above and delete all this code. + // Now u < (v< Date: Mon, 7 Jun 2021 14:29:43 -0400 Subject: [PATCH 21/45] net/url: reject query values with semicolons Semicolons are no longer valid separators, so net/url.ParseQuery will now return an error if any part of the query contains a semicolon. net/http.(*Request).ParseMultipartForm has been changed to fall through and continue parsing even if the call to (*Request).ParseForm fails. This change also includes a few minor refactors to existing tests. Fixes #25192 Change-Id: Iba3f108950fb99b9288e402c41fe71ca3a2ababd Reviewed-on: https://go-review.googlesource.com/c/go/+/325697 Trust: Katie Hockman Run-TryBot: Katie Hockman TryBot-Result: Go Bot Reviewed-by: Filippo Valsorda --- src/net/http/request.go | 12 ++-- src/net/http/request_test.go | 31 +++++++++- src/net/http/server.go | 5 ++ src/net/url/example_test.go | 4 +- src/net/url/url.go | 13 ++-- src/net/url/url_test.go | 116 +++++++++++++++++++++++++++-------- 6 files changed, 145 insertions(+), 36 deletions(-) diff --git a/src/net/http/request.go b/src/net/http/request.go index 7895417af5..09cb0c7f56 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -1293,16 +1293,18 @@ func (r *Request) ParseForm() error { // its file parts are stored in memory, with the remainder stored on // disk in temporary files. // ParseMultipartForm calls ParseForm if necessary. +// If ParseForm returns an error, ParseMultipartForm returns it but also +// continues parsing the request body. // After one call to ParseMultipartForm, subsequent calls have no effect. func (r *Request) ParseMultipartForm(maxMemory int64) error { if r.MultipartForm == multipartByReader { return errors.New("http: multipart handled by MultipartReader") } + var parseFormErr error if r.Form == nil { - err := r.ParseForm() - if err != nil { - return err - } + // Let errors in ParseForm fall through, and just + // return it at the end. + parseFormErr = r.ParseForm() } if r.MultipartForm != nil { return nil @@ -1329,7 +1331,7 @@ func (r *Request) ParseMultipartForm(maxMemory int64) error { r.MultipartForm = f - return nil + return parseFormErr } // FormValue returns the first value for the named component of the query. diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 952828b395..4e0c4ba207 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -32,9 +32,26 @@ func TestQuery(t *testing.T) { } } +// Issue #25192: Test that ParseForm fails but still parses the form when an URL +// containing a semicolon is provided. +func TestParseFormSemicolonSeparator(t *testing.T) { + for _, method := range []string{"POST", "PATCH", "PUT", "GET"} { + req, _ := NewRequest(method, "http://www.google.com/search?q=foo;q=bar&a=1", + strings.NewReader("q")) + err := req.ParseForm() + if err == nil { + t.Fatalf(`for method %s, ParseForm expected an error, got success`, method) + } + wantForm := url.Values{"a": []string{"1"}} + if !reflect.DeepEqual(req.Form, wantForm) { + t.Fatalf("for method %s, ParseForm expected req.Form = %v, want %v", method, req.Form, wantForm) + } + } +} + func TestParseFormQuery(t *testing.T) { req, _ := NewRequest("POST", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&orphan=nope&empty=not", - strings.NewReader("z=post&both=y&prio=2&=nokey&orphan;empty=&")) + strings.NewReader("z=post&both=y&prio=2&=nokey&orphan&empty=&")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") if q := req.FormValue("q"); q != "foo" { @@ -365,6 +382,18 @@ func TestMultipartRequest(t *testing.T) { validateTestMultipartContents(t, req, false) } +// Issue #25192: Test that ParseMultipartForm fails but still parses the +// multi-part form when an URL containing a semicolon is provided. +func TestParseMultipartFormSemicolonSeparator(t *testing.T) { + req := newTestMultipartRequest(t) + req.URL = &url.URL{RawQuery: "q=foo;q=bar"} + if err := req.ParseMultipartForm(25); err == nil { + t.Fatal("ParseMultipartForm expected error due to invalid semicolon, got nil") + } + defer req.MultipartForm.RemoveAll() + validateTestMultipartContents(t, req, false) +} + func TestMultipartRequestAuto(t *testing.T) { // Test that FormValue and FormFile automatically invoke // ParseMultipartForm and return the right values. diff --git a/src/net/http/server.go b/src/net/http/server.go index 430019de50..8a1847e67a 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -2863,6 +2863,11 @@ func (sh serverHandler) ServeHTTP(rw ResponseWriter, req *Request) { handler = globalOptionsHandler{} } handler.ServeHTTP(rw, req) + if req.URL != nil && strings.Contains(req.URL.RawQuery, ";") { + // TODO(filippo): update this not to log if the special + // semicolon handler was called. + sh.srv.logf("http: URL query contains semicolon, which is no longer a supported separator; parts of the query may be stripped when parsed; see golang.org/issue/25192") + } } // ListenAndServe listens on the TCP network address srv.Addr and then diff --git a/src/net/url/example_test.go b/src/net/url/example_test.go index cb9e8922a2..476132a1c9 100644 --- a/src/net/url/example_test.go +++ b/src/net/url/example_test.go @@ -72,13 +72,13 @@ func ExampleURL_ResolveReference() { } func ExampleParseQuery() { - m, err := url.ParseQuery(`x=1&y=2&y=3;z`) + m, err := url.ParseQuery(`x=1&y=2&y=3`) if err != nil { log.Fatal(err) } fmt.Println(toJSON(m)) // Output: - // {"x":["1"], "y":["2", "3"], "z":[""]} + // {"x":["1"], "y":["2", "3"]} } func ExampleURL_EscapedPath() { diff --git a/src/net/url/url.go b/src/net/url/url.go index 73bef22e45..20de0f6f51 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -921,9 +921,10 @@ func (v Values) Has(key string) bool { // valid query parameters found; err describes the first decoding error // encountered, if any. // -// Query is expected to be a list of key=value settings separated by -// ampersands or semicolons. A setting without an equals sign is -// interpreted as a key set to an empty value. +// Query is expected to be a list of key=value settings separated by ampersands. +// A setting without an equals sign is interpreted as a key set to an empty +// value. +// Settings containing a non-URL-encoded semicolon are considered invalid. func ParseQuery(query string) (Values, error) { m := make(Values) err := parseQuery(m, query) @@ -933,11 +934,15 @@ func ParseQuery(query string) (Values, error) { func parseQuery(m Values, query string) (err error) { for query != "" { key := query - if i := strings.IndexAny(key, "&;"); i >= 0 { + if i := strings.IndexAny(key, "&"); i >= 0 { key, query = key[:i], key[i+1:] } else { query = "" } + if strings.Contains(key, ";") { + err = fmt.Errorf("invalid semicolon separator in query") + continue + } if key == "" { continue } diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index 55348c4a7d..63c8e695af 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -1334,57 +1334,125 @@ func TestQueryValues(t *testing.T) { type parseTest struct { query string out Values + ok bool } var parseTests = []parseTest{ + { + query: "a=1", + out: Values{"a": []string{"1"}}, + ok: true, + }, { query: "a=1&b=2", out: Values{"a": []string{"1"}, "b": []string{"2"}}, + ok: true, }, { query: "a=1&a=2&a=banana", out: Values{"a": []string{"1", "2", "banana"}}, + ok: true, }, { query: "ascii=%3Ckey%3A+0x90%3E", out: Values{"ascii": []string{""}}, + ok: true, + }, { + query: "a=1;b=2", + out: Values{}, + ok: false, + }, { + query: "a;b=1", + out: Values{}, + ok: false, + }, { + query: "a=%3B", // hex encoding for semicolon + out: Values{"a": []string{";"}}, + ok: true, }, { - query: "a=1;b=2", - out: Values{"a": []string{"1"}, "b": []string{"2"}}, + query: "a%3Bb=1", + out: Values{"a;b": []string{"1"}}, + ok: true, }, { query: "a=1&a=2;a=banana", - out: Values{"a": []string{"1", "2", "banana"}}, + out: Values{"a": []string{"1"}}, + ok: false, + }, + { + query: "a;b&c=1", + out: Values{"c": []string{"1"}}, + ok: false, + }, + { + query: "a=1&b=2;a=3&c=4", + out: Values{"a": []string{"1"}, "c": []string{"4"}}, + ok: false, + }, + { + query: "a=1&b=2;c=3", + out: Values{"a": []string{"1"}}, + ok: false, + }, + { + query: ";", + out: Values{}, + ok: false, + }, + { + query: "a=1;", + out: Values{}, + ok: false, + }, + { + query: "a=1&;", + out: Values{"a": []string{"1"}}, + ok: false, + }, + { + query: ";a=1&b=2", + out: Values{"b": []string{"2"}}, + ok: false, + }, + { + query: "a=1&b=2;", + out: Values{"a": []string{"1"}}, + ok: false, }, } func TestParseQuery(t *testing.T) { - for i, test := range parseTests { - form, err := ParseQuery(test.query) - if err != nil { - t.Errorf("test %d: Unexpected error: %v", i, err) - continue - } - if len(form) != len(test.out) { - t.Errorf("test %d: len(form) = %d, want %d", i, len(form), len(test.out)) - } - for k, evs := range test.out { - vs, ok := form[k] - if !ok { - t.Errorf("test %d: Missing key %q", i, k) - continue + for _, test := range parseTests { + t.Run(test.query, func(t *testing.T) { + form, err := ParseQuery(test.query) + if test.ok != (err == nil) { + want := "" + if test.ok { + want = "" + } + t.Errorf("Unexpected error: %v, want %v", err, want) } - if len(vs) != len(evs) { - t.Errorf("test %d: len(form[%q]) = %d, want %d", i, k, len(vs), len(evs)) - continue + if len(form) != len(test.out) { + t.Errorf("len(form) = %d, want %d", len(form), len(test.out)) } - for j, ev := range evs { - if v := vs[j]; v != ev { - t.Errorf("test %d: form[%q][%d] = %q, want %q", i, k, j, v, ev) + for k, evs := range test.out { + vs, ok := form[k] + if !ok { + t.Errorf("Missing key %q", k) + continue + } + if len(vs) != len(evs) { + t.Errorf("len(form[%q]) = %d, want %d", k, len(vs), len(evs)) + continue + } + for j, ev := range evs { + if v := vs[j]; v != ev { + t.Errorf("form[%q][%d] = %q, want %q", k, j, v, ev) + } } } - } + }) } } From ec3026d032be065fb26c5826b9abb7b6b806d7ef Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Wed, 9 Jun 2021 11:04:58 -0400 Subject: [PATCH 22/45] doc/go1.17: remove TODO for ports section I'm not aware of anything more to mention for ports. Change-Id: I686df8a230a55ad7f4c5eae43ca27f85fad9dd84 Reviewed-on: https://go-review.googlesource.com/c/go/+/326409 Trust: Cherry Mui Reviewed-by: Jeremy Faller Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 011377a84e..2a56b6d270 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -120,10 +120,6 @@ Do not send CLs removing the interior tags from such phrases. stack frame pointers only on Linux, macOS, and iOS.

-

- TODO: complete the Ports section -

-

Tools

Go command

From e4e7807d240eb62e1d4a73eec2706975c8cc847b Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Wed, 9 Jun 2021 07:43:57 -0400 Subject: [PATCH 23/45] net/http: add AllowQuerySemicolons Fixes #45973 Change-Id: I6cbe05f5d1d3c324900c74314b0ea0e12524d7f2 Reviewed-on: https://go-review.googlesource.com/c/go/+/326309 Run-TryBot: Filippo Valsorda Reviewed-by: Katie Hockman Trust: Katie Hockman Trust: Filippo Valsorda TryBot-Result: Go Bot --- src/net/http/serve_test.go | 84 ++++++++++++++++++++++++++++++++++++++ src/net/http/server.go | 45 ++++++++++++++++++-- 2 files changed, 125 insertions(+), 4 deletions(-) diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index a9714682c7..c2f8811469 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -6524,3 +6524,87 @@ func TestMuxRedirectRelative(t *testing.T) { t.Errorf("Expected response code %d; got %d", want, got) } } + +// TestQuerySemicolon tests the behavior of semicolons in queries. See Issue 25192. +func TestQuerySemicolon(t *testing.T) { + t.Cleanup(func() { afterTest(t) }) + + tests := []struct { + query string + xNoSemicolons string + xWithSemicolons string + warning bool + }{ + {"?a=1;x=bad&x=good", "good", "bad", true}, + {"?a=1;b=bad&x=good", "good", "good", true}, + {"?a=1%3Bx=bad&x=good%3B", "good;", "good;", false}, + {"?a=1;x=good;x=bad", "", "good", true}, + } + + for _, tt := range tests { + t.Run(tt.query+"/allow=false", func(t *testing.T) { + allowSemicolons := false + testQuerySemicolon(t, tt.query, tt.xNoSemicolons, allowSemicolons, tt.warning) + }) + t.Run(tt.query+"/allow=true", func(t *testing.T) { + allowSemicolons, expectWarning := true, false + testQuerySemicolon(t, tt.query, tt.xWithSemicolons, allowSemicolons, expectWarning) + }) + } +} + +func testQuerySemicolon(t *testing.T, query string, wantX string, allowSemicolons, expectWarning bool) { + setParallel(t) + + writeBackX := func(w ResponseWriter, r *Request) { + x := r.URL.Query().Get("x") + if expectWarning { + if err := r.ParseForm(); err == nil || !strings.Contains(err.Error(), "semicolon") { + t.Errorf("expected error mentioning semicolons from ParseForm, got %v", err) + } + } else { + if err := r.ParseForm(); err != nil { + t.Errorf("expected no error from ParseForm, got %v", err) + } + } + if got := r.FormValue("x"); x != got { + t.Errorf("got %q from FormValue, want %q", got, x) + } + fmt.Fprintf(w, "%s", x) + } + + h := Handler(HandlerFunc(writeBackX)) + if allowSemicolons { + h = AllowQuerySemicolons(h) + } + + ts := httptest.NewUnstartedServer(h) + logBuf := &bytes.Buffer{} + ts.Config.ErrorLog = log.New(logBuf, "", 0) + ts.Start() + defer ts.Close() + + req, _ := NewRequest("GET", ts.URL+query, nil) + res, err := ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + slurp, _ := io.ReadAll(res.Body) + res.Body.Close() + if got, want := res.StatusCode, 200; got != want { + t.Errorf("Status = %d; want = %d", got, want) + } + if got, want := string(slurp), wantX; got != want { + t.Errorf("Body = %q; want = %q", got, want) + } + + if expectWarning { + if !strings.Contains(logBuf.String(), "semicolon") { + t.Errorf("got %q from ErrorLog, expected a mention of semicolons", logBuf.String()) + } + } else { + if strings.Contains(logBuf.String(), "semicolon") { + t.Errorf("got %q from ErrorLog, expected no mention of semicolons", logBuf.String()) + } + } +} diff --git a/src/net/http/server.go b/src/net/http/server.go index 8a1847e67a..50fab4520d 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -2862,12 +2862,49 @@ func (sh serverHandler) ServeHTTP(rw ResponseWriter, req *Request) { if req.RequestURI == "*" && req.Method == "OPTIONS" { handler = globalOptionsHandler{} } - handler.ServeHTTP(rw, req) + if req.URL != nil && strings.Contains(req.URL.RawQuery, ";") { - // TODO(filippo): update this not to log if the special - // semicolon handler was called. - sh.srv.logf("http: URL query contains semicolon, which is no longer a supported separator; parts of the query may be stripped when parsed; see golang.org/issue/25192") + var allowQuerySemicolonsInUse int32 + req = req.WithContext(context.WithValue(req.Context(), silenceSemWarnContextKey, func() { + atomic.StoreInt32(&allowQuerySemicolonsInUse, 1) + })) + defer func() { + if atomic.LoadInt32(&allowQuerySemicolonsInUse) == 0 { + sh.srv.logf("http: URL query contains semicolon, which is no longer a supported separator; parts of the query may be stripped when parsed; see golang.org/issue/25192") + } + }() } + + handler.ServeHTTP(rw, req) +} + +var silenceSemWarnContextKey = &contextKey{"silence-semicolons"} + +// AllowQuerySemicolons returns a handler that serves requests by converting any +// unescaped semicolons in the URL query to ampersands, and invoking the handler h. +// +// This restores the pre-Go 1.17 behavior of splitting query parameters on both +// semicolons and ampersands. (See golang.org/issue/25192). Note that this +// behavior doesn't match that of many proxies, and the mismatch can lead to +// security issues. +// +// AllowQuerySemicolons should be invoked before Request.ParseForm is called. +func AllowQuerySemicolons(h Handler) Handler { + return HandlerFunc(func(w ResponseWriter, r *Request) { + if silenceSemicolonsWarning, ok := r.Context().Value(silenceSemWarnContextKey).(func()); ok { + silenceSemicolonsWarning() + } + if strings.Contains(r.URL.RawQuery, ";") { + r2 := new(Request) + *r2 = *r + r2.URL = new(url.URL) + *r2.URL = *r.URL + r2.URL.RawQuery = strings.ReplaceAll(r.URL.RawQuery, ";", "&") + h.ServeHTTP(w, r2) + } else { + h.ServeHTTP(w, r) + } + }) } // ListenAndServe listens on the TCP network address srv.Addr and then From df35ade067f22ef1f3aad3c2f3576997ff9646b4 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Tue, 8 Jun 2021 20:34:16 -0400 Subject: [PATCH 24/45] doc/go1.17: document //go:build lines In 1.17, //go:build lines are fully supported. This entails changes to the go command, vet, and gofmt. Document all of them. I'm not Russ, but this is a significant change, it slipped under the radar, and we're trying to get the release out. So here's what I got. I wasn't sure where to put the go command change. On the one hand, it's pretty significant. On the other, it certainly affects fewer people than lazy loading. So it probably shouldn't be first, but I also didn't want to bury it the middle of all the other module changes. Open to suggestions. Change-Id: Ia1a96bcfb1977973c5b0b0a6b18a9242a745af12 Reviewed-on: https://go-review.googlesource.com/c/go/+/326209 Trust: Heschi Kreinick Run-TryBot: Heschi Kreinick TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 2a56b6d270..6c53aaaa88 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -279,12 +279,41 @@ Do not send CLs removing the interior tags from such phrases. mod download all.

+

//go:build lines

+ +

+ The go command now understands //go:build lines + and prefers them over // +build lines. The new syntax uses + boolean expressions, just like Go, and should be less error-prone. + As of this release, the new syntax is fully supported, and all Go files + should be updated to have both forms with the same meaning. To aid in + migration, gofmt now automatically + synchronizes the two forms. For more details on the syntax and migration plan, + see + https://golang.org/design/draft-gobuild. +

+ +

gofmt

+ gofmt (and go fmt) now synchronizes + //go:build lines with // +build lines. If a file + only has // +build lines, they will be moved to the appropriate + location in the file, and matching //go:build lines will be + added. Otherwise, // +build lines will be overwritten based on + any existing //go:build lines. For more information, see + https://golang.org/design/draft-gobuild. + +

Vet

-

New warning within buildtags

+

New warning for mismatched //go:build and // +build lines

- TODO(rsc): Describe changes to buildtags https://golang.org/cl/240609 + The vet tool now verifies that //go:build and + // +build lines are in the correct part of the file and + synchronized with each other. If they aren't, + gofmt can be used to fix them. For more + information, see + https://golang.org/design/draft-gobuild.

New warning for calling signal.Notify on unbuffered channels

@@ -638,6 +667,22 @@ func Foo() bool {
+
go/format
+
+

+ The Source and + Node functions now + synchronize //go:build lines with // +build + lines. If a file only has // +build lines, they will be + moved to the appropriate location in the file, and matching + //go:build lines will be added. Otherwise, + // +build lines will be overwritten based on any existing + //go:build lines. For more information, see + https://golang.org/design/draft-gobuild. +

+
+
+
io/fs

From 1402b27d465d9949027a048ea2c86a3583400b4c Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Mon, 7 Jun 2021 16:30:03 -0700 Subject: [PATCH 25/45] strconv: document parsing of leading +/- Explicitly document the handling of a sign prefix, and the interaction between the sign and base prefixes. Fixes #46641. Change-Id: I3cd6773e3f074fe671a944a05a79d2408137fcd4 Reviewed-on: https://go-review.googlesource.com/c/go/+/325875 Trust: Damien Neil Run-TryBot: Damien Neil TryBot-Result: Go Bot Reviewed-by: Rob Pike --- src/strconv/atoi.go | 11 ++++++++--- src/strconv/atoi_test.go | 10 ++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/strconv/atoi.go b/src/strconv/atoi.go index c9ba0383b3..631b487d97 100644 --- a/src/strconv/atoi.go +++ b/src/strconv/atoi.go @@ -57,6 +57,8 @@ const IntSize = intSize const maxUint64 = 1<<64 - 1 // ParseUint is like ParseInt but for unsigned numbers. +// +// A sign prefix is not permitted. func ParseUint(s string, base int, bitSize int) (uint64, error) { const fnParseUint = "ParseUint" @@ -159,10 +161,13 @@ func ParseUint(s string, base int, bitSize int) (uint64, error) { // ParseInt interprets a string s in the given base (0, 2 to 36) and // bit size (0 to 64) and returns the corresponding value i. // +// The string may begin with a leading sign: "+" or "-". +// // If the base argument is 0, the true base is implied by the string's -// prefix: 2 for "0b", 8 for "0" or "0o", 16 for "0x", and 10 otherwise. -// Also, for argument base 0 only, underscore characters are permitted -// as defined by the Go syntax for integer literals. +// prefix following the sign (if present): 2 for "0b", 8 for "0" or "0o", +// 16 for "0x", and 10 otherwise. Also, for argument base 0 only, +// underscore characters are permitted as defined by the Go syntax for +// integer literals. // // The bitSize argument specifies the integer type // that the result must fit into. Bit sizes 0, 8, 16, 32, and 64 diff --git a/src/strconv/atoi_test.go b/src/strconv/atoi_test.go index 178fb01ea7..867fa66a14 100644 --- a/src/strconv/atoi_test.go +++ b/src/strconv/atoi_test.go @@ -33,6 +33,9 @@ var parseUint64Tests = []parseUint64Test{ {"_12345", 0, ErrSyntax}, {"1__2345", 0, ErrSyntax}, {"12345_", 0, ErrSyntax}, + {"-0", 0, ErrSyntax}, + {"-1", 0, ErrSyntax}, + {"+1", 0, ErrSyntax}, } type parseUint64BaseTest struct { @@ -140,8 +143,10 @@ var parseInt64Tests = []parseInt64Test{ {"", 0, ErrSyntax}, {"0", 0, nil}, {"-0", 0, nil}, + {"+0", 0, nil}, {"1", 1, nil}, {"-1", -1, nil}, + {"+1", 1, nil}, {"12345", 12345, nil}, {"-12345", -12345, nil}, {"012345", 12345, nil}, @@ -236,6 +241,11 @@ var parseInt64BaseTests = []parseInt64BaseTest{ {"0__12345", 0, 0, ErrSyntax}, {"01234__5", 0, 0, ErrSyntax}, {"012345_", 0, 0, ErrSyntax}, + + {"+0xf", 0, 0xf, nil}, + {"-0xf", 0, -0xf, nil}, + {"0x+f", 0, 0, ErrSyntax}, + {"0x-f", 0, 0, ErrSyntax}, } type parseUint32Test struct { From a5bc060b42fe1bee8910a1081eff0a1047b15869 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 8 Jun 2021 14:26:13 -0700 Subject: [PATCH 26/45] doc/go1.17: document strconv changes for Go 1.17 For #44513. Fixes #46021. Change-Id: I40a4645fedfae24f67e249743c6a143e71b9f507 Reviewed-on: https://go-review.googlesource.com/c/go/+/326150 Trust: Damien Neil Run-TryBot: Damien Neil TryBot-Result: Go Bot Reviewed-by: Heschi Kreinick Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 6c53aaaa88..988026f44d 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -838,12 +838,9 @@ func Foo() bool {

strconv
-

- TODO: https://golang.org/cl/170079: implement Ryū-like algorithm for fixed precision ftoa -

- -

- TODO: https://golang.org/cl/170080: Implement Ryū algorithm for ftoa shortest mode +

+ The strconv package now uses Ulf Adams's Ryū algorithm for formatting floating-point numbers. + This algorithm improves performance on most inputs, and is more than 99% faster on worst-case inputs.

From 182157c81ad5a147ba13c6e80f844b2242598aed Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Wed, 9 Jun 2021 12:59:30 -0400 Subject: [PATCH 27/45] doc/go1.17: remove lingering TODO As far as I can tell the Core Library section is complete. Remove its TODO. Change-Id: Ia84c6656fac045e25fae1a7ce8b488a3a26fd250 Reviewed-on: https://go-review.googlesource.com/c/go/+/326469 Trust: Heschi Kreinick Run-TryBot: Heschi Kreinick TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov Reviewed-by: Damien Neil --- doc/go1.17.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 988026f44d..49fbabdc3f 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -418,10 +418,6 @@ func Foo() bool {

Core library

-

- TODO: complete the Core library section -

-

Cgo

From 27f83723e98d8e3795a07bdca2b3a8155b0d72b3 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Wed, 9 Jun 2021 15:21:39 -0400 Subject: [PATCH 28/45] api: promote next to go1.17 Change-Id: If631878a2f6ec0317b4fad614f98ab102810ed47 Reviewed-on: https://go-review.googlesource.com/c/go/+/326410 Trust: Heschi Kreinick Run-TryBot: Heschi Kreinick TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- api/go1.17.txt | 159 +++++++++++++++++++++++++++++++++++++++++++++++++ api/next.txt | 99 ------------------------------ 2 files changed, 159 insertions(+), 99 deletions(-) create mode 100644 api/go1.17.txt diff --git a/api/go1.17.txt b/api/go1.17.txt new file mode 100644 index 0000000000..f054458715 --- /dev/null +++ b/api/go1.17.txt @@ -0,0 +1,159 @@ +pkg archive/zip, method (*File) OpenRaw() (io.Reader, error) +pkg archive/zip, method (*Writer) Copy(*File) error +pkg archive/zip, method (*Writer) CreateRaw(*FileHeader) (io.Writer, error) +pkg compress/lzw, method (*Reader) Close() error +pkg compress/lzw, method (*Reader) Read([]uint8) (int, error) +pkg compress/lzw, method (*Reader) Reset(io.Reader, Order, int) +pkg compress/lzw, method (*Writer) Close() error +pkg compress/lzw, method (*Writer) Reset(io.Writer, Order, int) +pkg compress/lzw, method (*Writer) Write([]uint8) (int, error) +pkg compress/lzw, type Reader struct +pkg compress/lzw, type Writer struct +pkg crypto/tls, method (*CertificateRequestInfo) Context() context.Context +pkg crypto/tls, method (*ClientHelloInfo) Context() context.Context +pkg crypto/tls, method (*Conn) HandshakeContext(context.Context) error +pkg database/sql, method (*NullByte) Scan(interface{}) error +pkg database/sql, method (*NullInt16) Scan(interface{}) error +pkg database/sql, method (NullByte) Value() (driver.Value, error) +pkg database/sql, method (NullInt16) Value() (driver.Value, error) +pkg database/sql, type NullByte struct +pkg database/sql, type NullByte struct, Byte uint8 +pkg database/sql, type NullByte struct, Valid bool +pkg database/sql, type NullInt16 struct +pkg database/sql, type NullInt16 struct, Int16 int16 +pkg database/sql, type NullInt16 struct, Valid bool +pkg debug/elf, const SHT_MIPS_ABIFLAGS = 1879048234 +pkg debug/elf, const SHT_MIPS_ABIFLAGS SectionType +pkg encoding/csv, method (*Reader) FieldPos(int) (int, int) +pkg go/build, type Context struct, ToolTags []string +pkg go/parser, const SkipObjectResolution = 64 +pkg go/parser, const SkipObjectResolution Mode +pkg io/fs, func FileInfoToDirEntry(FileInfo) DirEntry +pkg math, const MaxFloat64 = 1.79769e+308 // 179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368 +pkg math, const MaxInt = 9223372036854775807 +pkg math, const MaxInt ideal-int +pkg math, const MaxUint = 18446744073709551615 +pkg math, const MaxUint ideal-int +pkg math, const MinInt = -9223372036854775808 +pkg math, const MinInt ideal-int +pkg math, const SmallestNonzeroFloat32 = 1.4013e-45 // 1/713623846352979940529142984724747568191373312 +pkg math, const SmallestNonzeroFloat64 = 4.94066e-324 // 1/202402253307310618352495346718917307049556649764142118356901358027430339567995346891960383701437124495187077864316811911389808737385793476867013399940738509921517424276566361364466907742093216341239767678472745068562007483424692698618103355649159556340810056512358769552333414615230502532186327508646006263307707741093494784 +pkg net, method (*ParseError) Temporary() bool +pkg net, method (*ParseError) Timeout() bool +pkg net, method (IP) IsPrivate() bool +pkg net/http, func AllowQuerySemicolons(Handler) Handler +pkg net/url, method (Values) Has(string) bool +pkg reflect, func VisibleFields(Type) []StructField +pkg reflect, method (Method) IsExported() bool +pkg reflect, method (StructField) IsExported() bool +pkg runtime/cgo (darwin-amd64-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (darwin-amd64-cgo), method (Handle) Delete() +pkg runtime/cgo (darwin-amd64-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (darwin-amd64-cgo), type Handle uintptr +pkg runtime/cgo (freebsd-386-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (freebsd-386-cgo), method (Handle) Delete() +pkg runtime/cgo (freebsd-386-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (freebsd-386-cgo), type Handle uintptr +pkg runtime/cgo (freebsd-amd64-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (freebsd-amd64-cgo), method (Handle) Delete() +pkg runtime/cgo (freebsd-amd64-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (freebsd-amd64-cgo), type Handle uintptr +pkg runtime/cgo (freebsd-arm-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (freebsd-arm-cgo), method (Handle) Delete() +pkg runtime/cgo (freebsd-arm-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (freebsd-arm-cgo), type Handle uintptr +pkg runtime/cgo (linux-386-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (linux-386-cgo), method (Handle) Delete() +pkg runtime/cgo (linux-386-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (linux-386-cgo), type Handle uintptr +pkg runtime/cgo (linux-amd64-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (linux-amd64-cgo), method (Handle) Delete() +pkg runtime/cgo (linux-amd64-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (linux-amd64-cgo), type Handle uintptr +pkg runtime/cgo (linux-arm-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (linux-arm-cgo), method (Handle) Delete() +pkg runtime/cgo (linux-arm-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (linux-arm-cgo), type Handle uintptr +pkg runtime/cgo (netbsd-386-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (netbsd-386-cgo), method (Handle) Delete() +pkg runtime/cgo (netbsd-386-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (netbsd-386-cgo), type Handle uintptr +pkg runtime/cgo (netbsd-amd64-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (netbsd-amd64-cgo), method (Handle) Delete() +pkg runtime/cgo (netbsd-amd64-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (netbsd-amd64-cgo), type Handle uintptr +pkg runtime/cgo (netbsd-arm-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (netbsd-arm-cgo), method (Handle) Delete() +pkg runtime/cgo (netbsd-arm-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (netbsd-arm-cgo), type Handle uintptr +pkg runtime/cgo (netbsd-arm64-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (netbsd-arm64-cgo), method (Handle) Delete() +pkg runtime/cgo (netbsd-arm64-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (netbsd-arm64-cgo), type Handle uintptr +pkg runtime/cgo (openbsd-386-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (openbsd-386-cgo), method (Handle) Delete() +pkg runtime/cgo (openbsd-386-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (openbsd-386-cgo), type Handle uintptr +pkg runtime/cgo (openbsd-amd64-cgo), func NewHandle(interface{}) Handle +pkg runtime/cgo (openbsd-amd64-cgo), method (Handle) Delete() +pkg runtime/cgo (openbsd-amd64-cgo), method (Handle) Value() interface{} +pkg runtime/cgo (openbsd-amd64-cgo), type Handle uintptr +pkg strconv, func QuotedPrefix(string) (string, error) +pkg sync/atomic, method (*Value) CompareAndSwap(interface{}, interface{}) bool +pkg sync/atomic, method (*Value) Swap(interface{}) interface{} +pkg syscall (netbsd-386), const SYS_WAIT6 = 481 +pkg syscall (netbsd-386), const SYS_WAIT6 ideal-int +pkg syscall (netbsd-386), const WEXITED = 32 +pkg syscall (netbsd-386), const WEXITED ideal-int +pkg syscall (netbsd-386-cgo), const SYS_WAIT6 = 481 +pkg syscall (netbsd-386-cgo), const SYS_WAIT6 ideal-int +pkg syscall (netbsd-386-cgo), const WEXITED = 32 +pkg syscall (netbsd-386-cgo), const WEXITED ideal-int +pkg syscall (netbsd-amd64), const SYS_WAIT6 = 481 +pkg syscall (netbsd-amd64), const SYS_WAIT6 ideal-int +pkg syscall (netbsd-amd64), const WEXITED = 32 +pkg syscall (netbsd-amd64), const WEXITED ideal-int +pkg syscall (netbsd-amd64-cgo), const SYS_WAIT6 = 481 +pkg syscall (netbsd-amd64-cgo), const SYS_WAIT6 ideal-int +pkg syscall (netbsd-amd64-cgo), const WEXITED = 32 +pkg syscall (netbsd-amd64-cgo), const WEXITED ideal-int +pkg syscall (netbsd-arm), const SYS_WAIT6 = 481 +pkg syscall (netbsd-arm), const SYS_WAIT6 ideal-int +pkg syscall (netbsd-arm), const WEXITED = 32 +pkg syscall (netbsd-arm), const WEXITED ideal-int +pkg syscall (netbsd-arm-cgo), const SYS_WAIT6 = 481 +pkg syscall (netbsd-arm-cgo), const SYS_WAIT6 ideal-int +pkg syscall (netbsd-arm-cgo), const WEXITED = 32 +pkg syscall (netbsd-arm-cgo), const WEXITED ideal-int +pkg syscall (netbsd-arm64), const SYS_WAIT6 = 481 +pkg syscall (netbsd-arm64), const SYS_WAIT6 ideal-int +pkg syscall (netbsd-arm64), const WEXITED = 32 +pkg syscall (netbsd-arm64), const WEXITED ideal-int +pkg syscall (netbsd-arm64-cgo), const SYS_WAIT6 = 481 +pkg syscall (netbsd-arm64-cgo), const SYS_WAIT6 ideal-int +pkg syscall (netbsd-arm64-cgo), const WEXITED = 32 +pkg syscall (netbsd-arm64-cgo), const WEXITED ideal-int +pkg syscall (openbsd-386), const MSG_CMSG_CLOEXEC = 2048 +pkg syscall (openbsd-386), const MSG_CMSG_CLOEXEC ideal-int +pkg syscall (openbsd-386-cgo), const MSG_CMSG_CLOEXEC = 2048 +pkg syscall (openbsd-386-cgo), const MSG_CMSG_CLOEXEC ideal-int +pkg syscall (openbsd-amd64), const MSG_CMSG_CLOEXEC = 2048 +pkg syscall (openbsd-amd64), const MSG_CMSG_CLOEXEC ideal-int +pkg syscall (openbsd-amd64-cgo), const MSG_CMSG_CLOEXEC = 2048 +pkg syscall (openbsd-amd64-cgo), const MSG_CMSG_CLOEXEC ideal-int +pkg syscall (windows-386), type SysProcAttr struct, AdditionalInheritedHandles []Handle +pkg syscall (windows-386), type SysProcAttr struct, ParentProcess Handle +pkg syscall (windows-amd64), type SysProcAttr struct, AdditionalInheritedHandles []Handle +pkg syscall (windows-amd64), type SysProcAttr struct, ParentProcess Handle +pkg testing, method (*B) Setenv(string, string) +pkg testing, method (*T) Setenv(string, string) +pkg text/template/parse, const SkipFuncCheck = 2 +pkg text/template/parse, const SkipFuncCheck Mode +pkg time, const Layout = "01/02 03:04:05PM '06 -0700" +pkg time, const Layout ideal-string +pkg time, func UnixMicro(int64) Time +pkg time, func UnixMilli(int64) Time +pkg time, method (*Time) IsDST() bool +pkg time, method (Time) GoString() string +pkg time, method (Time) UnixMicro() int64 +pkg time, method (Time) UnixMilli() int64 diff --git a/api/next.txt b/api/next.txt index 9e996005c6..e69de29bb2 100644 --- a/api/next.txt +++ b/api/next.txt @@ -1,99 +0,0 @@ -pkg compress/lzw, method (*Reader) Close() error -pkg compress/lzw, method (*Reader) Read([]uint8) (int, error) -pkg compress/lzw, method (*Reader) Reset(io.Reader, Order, int) -pkg compress/lzw, method (*Writer) Close() error -pkg compress/lzw, method (*Writer) Reset(io.Writer, Order, int) -pkg compress/lzw, method (*Writer) Write([]uint8) (int, error) -pkg compress/lzw, type Reader struct -pkg compress/lzw, type Writer struct -pkg crypto/tls, method (*CertificateRequestInfo) Context() context.Context -pkg crypto/tls, method (*ClientHelloInfo) Context() context.Context -pkg crypto/tls, method (*Conn) HandshakeContext(context.Context) error -pkg debug/elf, const SHT_MIPS_ABIFLAGS = 1879048234 -pkg debug/elf, const SHT_MIPS_ABIFLAGS SectionType -pkg encoding/csv, method (*Reader) FieldPos(int) (int, int) -pkg go/ast, method (*FuncDecl) IsMethod() bool -pkg go/build, type Context struct, ToolTags []string -pkg go/parser, const SkipObjectResolution = 64 -pkg go/parser, const SkipObjectResolution Mode -pkg go/types, type Config struct, GoVersion string -pkg io/fs, func FileInfoToDirEntry(FileInfo) DirEntry -pkg net, method (*ParseError) Temporary() bool -pkg net, method (*ParseError) Timeout() bool -pkg net, method (IP) IsPrivate() bool -pkg reflect, func VisibleFields(Type) []StructField -pkg reflect, method (Method) IsExported() bool -pkg reflect, method (StructField) IsExported() bool -pkg runtime/cgo (darwin-amd64-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (darwin-amd64-cgo), method (Handle) Delete() -pkg runtime/cgo (darwin-amd64-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (darwin-amd64-cgo), type Handle uintptr -pkg runtime/cgo (freebsd-386-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (freebsd-386-cgo), method (Handle) Delete() -pkg runtime/cgo (freebsd-386-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (freebsd-386-cgo), type Handle uintptr -pkg runtime/cgo (freebsd-amd64-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (freebsd-amd64-cgo), method (Handle) Delete() -pkg runtime/cgo (freebsd-amd64-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (freebsd-amd64-cgo), type Handle uintptr -pkg runtime/cgo (freebsd-arm-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (freebsd-arm-cgo), method (Handle) Delete() -pkg runtime/cgo (freebsd-arm-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (freebsd-arm-cgo), type Handle uintptr -pkg runtime/cgo (linux-386-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (linux-386-cgo), method (Handle) Delete() -pkg runtime/cgo (linux-386-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (linux-386-cgo), type Handle uintptr -pkg runtime/cgo (linux-amd64-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (linux-amd64-cgo), method (Handle) Delete() -pkg runtime/cgo (linux-amd64-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (linux-amd64-cgo), type Handle uintptr -pkg runtime/cgo (linux-arm-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (linux-arm-cgo), method (Handle) Delete() -pkg runtime/cgo (linux-arm-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (linux-arm-cgo), type Handle uintptr -pkg runtime/cgo (netbsd-386-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (netbsd-386-cgo), method (Handle) Delete() -pkg runtime/cgo (netbsd-386-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (netbsd-386-cgo), type Handle uintptr -pkg runtime/cgo (netbsd-amd64-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (netbsd-amd64-cgo), method (Handle) Delete() -pkg runtime/cgo (netbsd-amd64-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (netbsd-amd64-cgo), type Handle uintptr -pkg runtime/cgo (netbsd-arm-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (netbsd-arm-cgo), method (Handle) Delete() -pkg runtime/cgo (netbsd-arm-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (netbsd-arm-cgo), type Handle uintptr -pkg runtime/cgo (netbsd-arm64-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (netbsd-arm64-cgo), method (Handle) Delete() -pkg runtime/cgo (netbsd-arm64-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (netbsd-arm64-cgo), type Handle uintptr -pkg runtime/cgo (openbsd-386-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (openbsd-386-cgo), method (Handle) Delete() -pkg runtime/cgo (openbsd-386-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (openbsd-386-cgo), type Handle uintptr -pkg runtime/cgo (openbsd-amd64-cgo), func NewHandle(interface{}) Handle -pkg runtime/cgo (openbsd-amd64-cgo), method (Handle) Delete() -pkg runtime/cgo (openbsd-amd64-cgo), method (Handle) Value() interface{} -pkg runtime/cgo (openbsd-amd64-cgo), type Handle uintptr -pkg syscall (openbsd-386), const MSG_CMSG_CLOEXEC = 2048 -pkg syscall (openbsd-386), const MSG_CMSG_CLOEXEC ideal-int -pkg syscall (openbsd-386-cgo), const MSG_CMSG_CLOEXEC = 2048 -pkg syscall (openbsd-386-cgo), const MSG_CMSG_CLOEXEC ideal-int -pkg syscall (openbsd-amd64), const MSG_CMSG_CLOEXEC = 2048 -pkg syscall (openbsd-amd64), const MSG_CMSG_CLOEXEC ideal-int -pkg syscall (openbsd-amd64-cgo), const MSG_CMSG_CLOEXEC = 2048 -pkg syscall (openbsd-amd64-cgo), const MSG_CMSG_CLOEXEC ideal-int -pkg syscall (windows-386), type SysProcAttr struct, AdditionalInheritedHandles []Handle -pkg syscall (windows-386), type SysProcAttr struct, ParentProcess Handle -pkg syscall (windows-amd64), type SysProcAttr struct, AdditionalInheritedHandles []Handle -pkg syscall (windows-amd64), type SysProcAttr struct, ParentProcess Handle -pkg testing, method (*B) Setenv(string, string) -pkg testing, method (*T) Setenv(string, string) -pkg text/template/parse, const SkipFuncCheck = 2 -pkg text/template/parse, const SkipFuncCheck Mode -pkg time, func UnixMicro(int64) Time -pkg time, func UnixMilli(int64) Time -pkg time, method (*Time) IsDST() bool -pkg time, method (Time) UnixMicro() int64 -pkg time, method (Time) UnixMilli() int64 From dc00dc6c6bf3b5554e37f60799aec092276ff807 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Mon, 7 Jun 2021 08:24:22 -0400 Subject: [PATCH 29/45] crypto/tls: let HTTP/1.1 clients connect to servers with NextProtos "h2" Fixes #46310 Change-Id: Idd5e30f05c439f736ae6f3904cbb9cc2ba772315 Reviewed-on: https://go-review.googlesource.com/c/go/+/325432 Trust: Filippo Valsorda Run-TryBot: Filippo Valsorda TryBot-Result: Go Bot Reviewed-by: Roland Shoemaker --- src/crypto/tls/handshake_client.go | 44 ++++---- src/crypto/tls/handshake_client_tls13.go | 14 +-- src/crypto/tls/handshake_server.go | 42 ++++++-- src/crypto/tls/handshake_server_test.go | 21 ++++ src/crypto/tls/handshake_server_tls13.go | 15 ++- .../tls/testdata/Server-TLSv12-ALPN-Fallback | 91 ++++++++++++++++ .../tls/testdata/Server-TLSv13-ALPN-Fallback | 100 ++++++++++++++++++ 7 files changed, 277 insertions(+), 50 deletions(-) create mode 100644 src/crypto/tls/testdata/Server-TLSv12-ALPN-Fallback create mode 100644 src/crypto/tls/testdata/Server-TLSv13-ALPN-Fallback diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go index 13a7f3442c..4af3d998a3 100644 --- a/src/crypto/tls/handshake_client.go +++ b/src/crypto/tls/handshake_client.go @@ -711,17 +711,11 @@ func (hs *clientHandshakeState) processServerHello() (bool, error) { } } - if hs.serverHello.alpnProtocol != "" { - if len(hs.hello.alpnProtocols) == 0 { - c.sendAlert(alertUnsupportedExtension) - return false, errors.New("tls: server advertised unrequested ALPN extension") - } - if mutualProtocol([]string{hs.serverHello.alpnProtocol}, hs.hello.alpnProtocols) == "" { - c.sendAlert(alertUnsupportedExtension) - return false, errors.New("tls: server selected unadvertised ALPN protocol") - } - c.clientProtocol = hs.serverHello.alpnProtocol + if err := checkALPN(hs.hello.alpnProtocols, hs.serverHello.alpnProtocol); err != nil { + c.sendAlert(alertUnsupportedExtension) + return false, err } + c.clientProtocol = hs.serverHello.alpnProtocol c.scts = hs.serverHello.scts @@ -753,6 +747,23 @@ func (hs *clientHandshakeState) processServerHello() (bool, error) { return true, nil } +// checkALPN ensure that the server's choice of ALPN protocol is compatible with +// the protocols that we advertised in the Client Hello. +func checkALPN(clientProtos []string, serverProto string) error { + if serverProto == "" { + return nil + } + if len(clientProtos) == 0 { + return errors.New("tls: server advertised unrequested ALPN extension") + } + for _, proto := range clientProtos { + if proto == serverProto { + return nil + } + } + return errors.New("tls: server selected unadvertised ALPN protocol") +} + func (hs *clientHandshakeState) readFinished(out []byte) error { c := hs.c @@ -979,19 +990,6 @@ func clientSessionCacheKey(serverAddr net.Addr, config *Config) string { return serverAddr.String() } -// mutualProtocol finds the mutual ALPN protocol given list of possible -// protocols and a list of the preference order. -func mutualProtocol(protos, preferenceProtos []string) string { - for _, s := range preferenceProtos { - for _, c := range protos { - if s == c { - return s - } - } - } - return "" -} - // hostnameInSNI converts name into an appropriate hostname for SNI. // Literal IP addresses and absolute FQDNs are not permitted as SNI values. // See RFC 6066, Section 3. diff --git a/src/crypto/tls/handshake_client_tls13.go b/src/crypto/tls/handshake_client_tls13.go index be37c681c6..eb59ac90d1 100644 --- a/src/crypto/tls/handshake_client_tls13.go +++ b/src/crypto/tls/handshake_client_tls13.go @@ -396,17 +396,11 @@ func (hs *clientHandshakeStateTLS13) readServerParameters() error { } hs.transcript.Write(encryptedExtensions.marshal()) - if encryptedExtensions.alpnProtocol != "" { - if len(hs.hello.alpnProtocols) == 0 { - c.sendAlert(alertUnsupportedExtension) - return errors.New("tls: server advertised unrequested ALPN extension") - } - if mutualProtocol([]string{encryptedExtensions.alpnProtocol}, hs.hello.alpnProtocols) == "" { - c.sendAlert(alertUnsupportedExtension) - return errors.New("tls: server selected unadvertised ALPN protocol") - } - c.clientProtocol = encryptedExtensions.alpnProtocol + if err := checkALPN(hs.hello.alpnProtocols, encryptedExtensions.alpnProtocol); err != nil { + c.sendAlert(alertUnsupportedExtension) + return err } + c.clientProtocol = encryptedExtensions.alpnProtocol return nil } diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index b231981e09..43f30e2fef 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -217,15 +217,13 @@ func (hs *serverHandshakeState) processClientHello() error { c.serverName = hs.clientHello.serverName } - if len(c.config.NextProtos) > 0 && len(hs.clientHello.alpnProtocols) > 0 { - selectedProto := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos) - if selectedProto == "" { - c.sendAlert(alertNoApplicationProtocol) - return fmt.Errorf("tls: client requested unsupported application protocols (%s)", hs.clientHello.alpnProtocols) - } - hs.hello.alpnProtocol = selectedProto - c.clientProtocol = selectedProto + selectedProto, err := negotiateALPN(c.config.NextProtos, hs.clientHello.alpnProtocols) + if err != nil { + c.sendAlert(alertNoApplicationProtocol) + return err } + hs.hello.alpnProtocol = selectedProto + c.clientProtocol = selectedProto hs.cert, err = c.config.getCertificate(clientHelloInfo(hs.ctx, c, hs.clientHello)) if err != nil { @@ -277,6 +275,34 @@ func (hs *serverHandshakeState) processClientHello() error { return nil } +// negotiateALPN picks a shared ALPN protocol that both sides support in server +// preference order. If ALPN is not configured or the peer doesn't support it, +// it returns "" and no error. +func negotiateALPN(serverProtos, clientProtos []string) (string, error) { + if len(serverProtos) == 0 || len(clientProtos) == 0 { + return "", nil + } + var http11fallback bool + for _, s := range serverProtos { + for _, c := range clientProtos { + if s == c { + return s, nil + } + if s == "h2" && c == "http/1.1" { + http11fallback = true + } + } + } + // As a special case, let http/1.1 clients connect to h2 servers as if they + // didn't support ALPN. We used not to enforce protocol overlap, so over + // time a number of HTTP servers were configured with only "h2", but + // expected to accept connections from "http/1.1" clients. See Issue 46310. + if http11fallback { + return "", nil + } + return "", fmt.Errorf("tls: client requested unsupported application protocols (%s)", clientProtos) +} + // supportsECDHE returns whether ECDHE key exchanges can be used with this // pre-TLS 1.3 client. func supportsECDHE(c *Config, supportedCurves []CurveID, supportedPoints []uint8) bool { diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go index 4483838045..f61b4c88ef 100644 --- a/src/crypto/tls/handshake_server_test.go +++ b/src/crypto/tls/handshake_server_test.go @@ -949,6 +949,27 @@ func TestHandshakeServerALPNNotConfigured(t *testing.T) { runServerTestTLS13(t, test) } +func TestHandshakeServerALPNFallback(t *testing.T) { + config := testConfig.Clone() + config.NextProtos = []string{"proto1", "h2", "proto2"} + + test := &serverTest{ + name: "ALPN-Fallback", + // Note that this needs OpenSSL 1.0.2 because that is the first + // version that supports the -alpn flag. + command: []string{"openssl", "s_client", "-alpn", "proto3,http/1.1,proto4", "-cipher", "ECDHE-RSA-CHACHA20-POLY1305", "-ciphersuites", "TLS_CHACHA20_POLY1305_SHA256"}, + config: config, + validate: func(state ConnectionState) error { + if state.NegotiatedProtocol != "" { + return fmt.Errorf("Got protocol %q, wanted nothing", state.NegotiatedProtocol) + } + return nil + }, + } + runServerTestTLS12(t, test) + runServerTestTLS13(t, test) +} + // TestHandshakeServerSNI involves a client sending an SNI extension of // "snitest.com", which happens to match the CN of testSNICertificate. The test // verifies that the server correctly selects that certificate. diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go index c375ec4246..08251b84de 100644 --- a/src/crypto/tls/handshake_server_tls13.go +++ b/src/crypto/tls/handshake_server_tls13.go @@ -11,7 +11,6 @@ import ( "crypto/hmac" "crypto/rsa" "errors" - "fmt" "hash" "io" "sync/atomic" @@ -551,15 +550,13 @@ func (hs *serverHandshakeStateTLS13) sendServerParameters() error { encryptedExtensions := new(encryptedExtensionsMsg) - if len(c.config.NextProtos) > 0 && len(hs.clientHello.alpnProtocols) > 0 { - selectedProto := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos) - if selectedProto == "" { - c.sendAlert(alertNoApplicationProtocol) - return fmt.Errorf("tls: client requested unsupported application protocols (%s)", hs.clientHello.alpnProtocols) - } - encryptedExtensions.alpnProtocol = selectedProto - c.clientProtocol = selectedProto + selectedProto, err := negotiateALPN(c.config.NextProtos, hs.clientHello.alpnProtocols) + if err != nil { + c.sendAlert(alertNoApplicationProtocol) + return err } + encryptedExtensions.alpnProtocol = selectedProto + c.clientProtocol = selectedProto hs.transcript.Write(encryptedExtensions.marshal()) if _, err := c.writeRecord(recordTypeHandshake, encryptedExtensions.marshal()); err != nil { diff --git a/src/crypto/tls/testdata/Server-TLSv12-ALPN-Fallback b/src/crypto/tls/testdata/Server-TLSv12-ALPN-Fallback new file mode 100644 index 0000000000..4fadf39062 --- /dev/null +++ b/src/crypto/tls/testdata/Server-TLSv12-ALPN-Fallback @@ -0,0 +1,91 @@ +>>> Flow 1 (client to server) +00000000 16 03 01 00 a6 01 00 00 a2 03 03 b5 c9 ab 32 7f |..............2.| +00000010 e1 af 3f f2 ac 2a 11 dd 33 f9 b5 21 88 0d e4 29 |..?..*..3..!...)| +00000020 e2 47 49 dc c7 31 a8 a5 25 81 0c 00 00 04 cc a8 |.GI..1..%.......| +00000030 00 ff 01 00 00 75 00 0b 00 04 03 00 01 02 00 0a |.....u..........| +00000040 00 0c 00 0a 00 1d 00 17 00 1e 00 19 00 18 00 23 |...............#| +00000050 00 00 00 10 00 19 00 17 06 70 72 6f 74 6f 33 08 |.........proto3.| +00000060 68 74 74 70 2f 31 2e 31 06 70 72 6f 74 6f 34 00 |http/1.1.proto4.| +00000070 16 00 00 00 17 00 00 00 0d 00 30 00 2e 04 03 05 |..........0.....| +00000080 03 06 03 08 07 08 08 08 09 08 0a 08 0b 08 04 08 |................| +00000090 05 08 06 04 01 05 01 06 01 03 03 02 03 03 01 02 |................| +000000a0 01 03 02 02 02 04 02 05 02 06 02 |...........| +>>> Flow 2 (server to client) +00000000 16 03 03 00 3b 02 00 00 37 03 03 00 00 00 00 00 |....;...7.......| +00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| +00000020 00 00 00 44 4f 57 4e 47 52 44 01 00 cc a8 00 00 |...DOWNGRD......| +00000030 0f 00 23 00 00 ff 01 00 01 00 00 0b 00 02 01 00 |..#.............| +00000040 16 03 03 02 59 0b 00 02 55 00 02 52 00 02 4f 30 |....Y...U..R..O0| +00000050 82 02 4b 30 82 01 b4 a0 03 02 01 02 02 09 00 e8 |..K0............| +00000060 f0 9d 3f e2 5b ea a6 30 0d 06 09 2a 86 48 86 f7 |..?.[..0...*.H..| +00000070 0d 01 01 0b 05 00 30 1f 31 0b 30 09 06 03 55 04 |......0.1.0...U.| +00000080 0a 13 02 47 6f 31 10 30 0e 06 03 55 04 03 13 07 |...Go1.0...U....| +00000090 47 6f 20 52 6f 6f 74 30 1e 17 0d 31 36 30 31 30 |Go Root0...16010| +000000a0 31 30 30 30 30 30 30 5a 17 0d 32 35 30 31 30 31 |1000000Z..250101| +000000b0 30 30 30 30 30 30 5a 30 1a 31 0b 30 09 06 03 55 |000000Z0.1.0...U| +000000c0 04 0a 13 02 47 6f 31 0b 30 09 06 03 55 04 03 13 |....Go1.0...U...| +000000d0 02 47 6f 30 81 9f 30 0d 06 09 2a 86 48 86 f7 0d |.Go0..0...*.H...| +000000e0 01 01 01 05 00 03 81 8d 00 30 81 89 02 81 81 00 |.........0......| +000000f0 db 46 7d 93 2e 12 27 06 48 bc 06 28 21 ab 7e c4 |.F}...'.H..(!.~.| +00000100 b6 a2 5d fe 1e 52 45 88 7a 36 47 a5 08 0d 92 42 |..]..RE.z6G....B| +00000110 5b c2 81 c0 be 97 79 98 40 fb 4f 6d 14 fd 2b 13 |[.....y.@.Om..+.| +00000120 8b c2 a5 2e 67 d8 d4 09 9e d6 22 38 b7 4a 0b 74 |....g....."8.J.t| +00000130 73 2b c2 34 f1 d1 93 e5 96 d9 74 7b f3 58 9f 6c |s+.4......t{.X.l| +00000140 61 3c c0 b0 41 d4 d9 2b 2b 24 23 77 5b 1c 3b bd |a<..A..++$#w[.;.| +00000150 75 5d ce 20 54 cf a1 63 87 1d 1e 24 c4 f3 1d 1a |u]. T..c...$....| +00000160 50 8b aa b6 14 43 ed 97 a7 75 62 f4 14 c8 52 d7 |P....C...ub...R.| +00000170 02 03 01 00 01 a3 81 93 30 81 90 30 0e 06 03 55 |........0..0...U| +00000180 1d 0f 01 01 ff 04 04 03 02 05 a0 30 1d 06 03 55 |...........0...U| +00000190 1d 25 04 16 30 14 06 08 2b 06 01 05 05 07 03 01 |.%..0...+.......| +000001a0 06 08 2b 06 01 05 05 07 03 02 30 0c 06 03 55 1d |..+.......0...U.| +000001b0 13 01 01 ff 04 02 30 00 30 19 06 03 55 1d 0e 04 |......0.0...U...| +000001c0 12 04 10 9f 91 16 1f 43 43 3e 49 a6 de 6d b6 80 |.......CC>I..m..| +000001d0 d7 9f 60 30 1b 06 03 55 1d 23 04 14 30 12 80 10 |..`0...U.#..0...| +000001e0 48 13 49 4d 13 7e 16 31 bb a3 01 d5 ac ab 6e 7b |H.IM.~.1......n{| +000001f0 30 19 06 03 55 1d 11 04 12 30 10 82 0e 65 78 61 |0...U....0...exa| +00000200 6d 70 6c 65 2e 67 6f 6c 61 6e 67 30 0d 06 09 2a |mple.golang0...*| +00000210 86 48 86 f7 0d 01 01 0b 05 00 03 81 81 00 9d 30 |.H.............0| +00000220 cc 40 2b 5b 50 a0 61 cb ba e5 53 58 e1 ed 83 28 |.@+[P.a...SX...(| +00000230 a9 58 1a a9 38 a4 95 a1 ac 31 5a 1a 84 66 3d 43 |.X..8....1Z..f=C| +00000240 d3 2d d9 0b f2 97 df d3 20 64 38 92 24 3a 00 bc |.-...... d8.$:..| +00000250 cf 9c 7d b7 40 20 01 5f aa d3 16 61 09 a2 76 fd |..}.@ ._...a..v.| +00000260 13 c3 cc e1 0c 5c ee b1 87 82 f1 6c 04 ed 73 bb |.....\.....l..s.| +00000270 b3 43 77 8d 0c 1c f1 0f a1 d8 40 83 61 c9 4c 72 |.Cw.......@.a.Lr| +00000280 2b 9d ae db 46 06 06 4d f4 c1 b3 3e c0 d1 bd 42 |+...F..M...>...B| +00000290 d4 db fe 3d 13 60 84 5c 21 d3 3b e9 fa e7 16 03 |...=.`.\!.;.....| +000002a0 03 00 ac 0c 00 00 a8 03 00 1d 20 2f e5 7d a3 47 |.......... /.}.G| +000002b0 cd 62 43 15 28 da ac 5f bb 29 07 30 ff f6 84 af |.bC.(.._.).0....| +000002c0 c4 cf c2 ed 90 99 5f 58 cb 3b 74 08 04 00 80 5f |......_X.;t...._| +000002d0 37 27 84 58 1e ea 1e 40 1b de a9 8f 04 d4 94 64 |7'.X...@.......d| +000002e0 4e 27 c7 f1 b3 30 d0 53 f5 3d 57 50 d2 17 97 c8 |N'...0.S.=WP....| +000002f0 3d 61 af a6 21 ab 1c 34 47 70 f8 b1 3b 9c 06 86 |=a..!..4Gp..;...| +00000300 87 00 e2 13 50 83 91 ad bc 84 bd b4 7b f3 4b ed |....P.......{.K.| +00000310 ca 81 0c 94 37 a8 ec 67 ca 9c f3 00 f6 af c2 92 |....7..g........| +00000320 c4 8c 78 07 18 0e 43 24 1b 98 16 50 5c 2b 75 0e |..x...C$...P\+u.| +00000330 40 66 dc 40 cd 10 1a 51 25 f3 96 25 1a 3e 70 af |@f.@...Q%..%.>p.| +00000340 16 24 d0 1c 0e 33 f9 c1 74 cf b7 e2 28 ac 60 16 |.$...3..t...(.`.| +00000350 03 03 00 04 0e 00 00 00 |........| +>>> Flow 3 (client to server) +00000000 16 03 03 00 25 10 00 00 21 20 30 f2 bb f7 a7 ac |....%...! 0.....| +00000010 23 20 22 ee 73 0d 49 9c b3 7b c1 9a db 2c 85 f3 |# ".s.I..{...,..| +00000020 c0 82 31 60 bd 8b 14 4e 73 43 14 03 03 00 01 01 |..1`...NsC......| +00000030 16 03 03 00 20 09 8d c7 86 ee cc f4 c7 36 a3 49 |.... ........6.I| +00000040 d3 f7 a1 4a 68 a2 1e b4 fc cc a2 15 cb 01 92 d8 |...Jh...........| +00000050 72 b0 d1 6f eb |r..o.| +>>> Flow 4 (server to client) +00000000 16 03 03 00 8b 04 00 00 87 00 00 00 00 00 81 50 |...............P| +00000010 46 ad c1 db a8 38 86 7b 2b bb fd d0 c3 42 3e 00 |F....8.{+....B>.| +00000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 94 |................| +00000030 6f e0 18 83 51 ed 14 ef 68 ca 42 c5 4c a2 ac 05 |o...Q...h.B.L...| +00000040 9c 69 69 99 08 9f de a4 d4 e7 37 ab 14 38 4c 47 |.ii.......7..8LG| +00000050 70 f0 97 1d db 2d 0a 14 c2 1e f0 16 9f 6d 37 02 |p....-.......m7.| +00000060 4b f1 16 be 98 3f df 74 83 7c 19 85 61 49 38 16 |K....?.t.|..aI8.| +00000070 ee 35 7a e2 3f 74 fe 8d e3 07 93 a1 5e fa f2 02 |.5z.?t......^...| +00000080 e5 c8 60 3f 11 83 8b 0e 32 52 f1 aa 52 b7 0a 89 |..`?....2R..R...| +00000090 14 03 03 00 01 01 16 03 03 00 20 9e 65 15 cf 45 |.......... .e..E| +000000a0 a5 03 69 c9 b1 d8 9e 92 a3 a2 b0 df 2e 62 b1 3a |..i..........b.:| +000000b0 17 78 cd e5 1d f3 51 42 7e 4e 25 17 03 03 00 1d |.x....QB~N%.....| +000000c0 d9 ae d0 fa b7 90 a9 2f 28 8d 1d 6f 54 1f c0 1e |......./(..oT...| +000000d0 4d ae b6 91 f0 e8 84 cf 86 11 22 25 ea 15 03 03 |M........."%....| +000000e0 00 12 0e 71 f2 11 9e 9f 58 ad c0 d8 fc fa 34 bc |...q....X.....4.| +000000f0 02 5a 60 00 |.Z`.| diff --git a/src/crypto/tls/testdata/Server-TLSv13-ALPN-Fallback b/src/crypto/tls/testdata/Server-TLSv13-ALPN-Fallback new file mode 100644 index 0000000000..6203e6877c --- /dev/null +++ b/src/crypto/tls/testdata/Server-TLSv13-ALPN-Fallback @@ -0,0 +1,100 @@ +>>> Flow 1 (client to server) +00000000 16 03 01 00 eb 01 00 00 e7 03 03 1c d3 8e 3b d9 |..............;.| +00000010 fe 7d e7 f9 9f fa c6 51 c3 8c 1b dd dc 87 95 f4 |.}.....Q........| +00000020 39 23 67 e4 d6 bd 94 93 fc 88 4e 20 c3 c0 e2 c1 |9#g.......N ....| +00000030 3d 12 ec 4c 0a 3f 40 51 13 24 61 11 c0 5d 09 f9 |=..L.?@Q.$a..]..| +00000040 08 d6 3e cd e7 b3 51 c3 06 8f b4 42 00 04 13 03 |..>...Q....B....| +00000050 00 ff 01 00 00 9a 00 0b 00 04 03 00 01 02 00 0a |................| +00000060 00 0c 00 0a 00 1d 00 17 00 1e 00 19 00 18 00 23 |...............#| +00000070 00 00 00 10 00 19 00 17 06 70 72 6f 74 6f 33 08 |.........proto3.| +00000080 68 74 74 70 2f 31 2e 31 06 70 72 6f 74 6f 34 00 |http/1.1.proto4.| +00000090 16 00 00 00 17 00 00 00 0d 00 1e 00 1c 04 03 05 |................| +000000a0 03 06 03 08 07 08 08 08 09 08 0a 08 0b 08 04 08 |................| +000000b0 05 08 06 04 01 05 01 06 01 00 2b 00 03 02 03 04 |..........+.....| +000000c0 00 2d 00 02 01 01 00 33 00 26 00 24 00 1d 00 20 |.-.....3.&.$... | +000000d0 f4 05 eb 4a 7a 73 20 18 74 aa 14 2a 0c 35 63 29 |...Jzs .t..*.5c)| +000000e0 cb f2 ad d1 a2 3d bd 9d 02 b4 62 00 bc eb 10 58 |.....=....b....X| +>>> Flow 2 (server to client) +00000000 16 03 03 00 7a 02 00 00 76 03 03 00 00 00 00 00 |....z...v.......| +00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| +00000020 00 00 00 00 00 00 00 00 00 00 00 20 c3 c0 e2 c1 |........... ....| +00000030 3d 12 ec 4c 0a 3f 40 51 13 24 61 11 c0 5d 09 f9 |=..L.?@Q.$a..]..| +00000040 08 d6 3e cd e7 b3 51 c3 06 8f b4 42 13 03 00 00 |..>...Q....B....| +00000050 2e 00 2b 00 02 03 04 00 33 00 24 00 1d 00 20 2f |..+.....3.$... /| +00000060 e5 7d a3 47 cd 62 43 15 28 da ac 5f bb 29 07 30 |.}.G.bC.(.._.).0| +00000070 ff f6 84 af c4 cf c2 ed 90 99 5f 58 cb 3b 74 14 |.........._X.;t.| +00000080 03 03 00 01 01 17 03 03 00 17 fb 75 d8 5c 50 35 |...........u.\P5| +00000090 55 82 ba 65 1e 63 73 b8 c1 e9 d7 f5 28 68 3c c1 |U..e.cs.....(h<.| +000000a0 5d 17 03 03 02 6d 56 c9 a9 09 73 6a bc fd 1a 3c |]....mV...sj...<| +000000b0 6a f8 3e 32 99 83 e8 f6 01 9e 5e 30 e8 53 7f 72 |j.>2......^0.S.r| +000000c0 fd 86 72 a8 9e 47 25 67 c1 f1 9a 03 c0 9d 6f 9d |..r..G%g......o.| +000000d0 bd ed 29 30 8f 3c 01 ce 49 bb 5f dd 58 9a ae 80 |..)0.<..I._.X...| +000000e0 5c 2d 81 fc ea 7b 03 03 3d 5d bb 92 23 73 67 89 |\-...{..=]..#sg.| +000000f0 2e f0 ec 08 20 8a 36 eb 43 a6 a1 68 d0 39 95 37 |.... .6.C..h.9.7| +00000100 6b 15 a9 0e 46 20 92 51 9c 04 bf 3b 07 97 84 cb |k...F .Q...;....| +00000110 1f 30 38 37 2e ff e7 0f f5 14 93 5a 84 f1 f7 10 |.087.......Z....| +00000120 c2 a5 0d bb 97 96 ef 4a e0 13 c0 63 72 2b 60 f3 |.......J...cr+`.| +00000130 59 b5 57 aa 5f d1 da a9 0e dd 9c dd c2 cb 61 fe |Y.W._.........a.| +00000140 e2 69 8e db 5d 70 6c 3a 33 e0 9e db 9a 31 26 6a |.i..]pl:3....1&j| +00000150 2b 9e 19 8e bb 5d 06 48 ea c0 a1 c6 11 24 fb c4 |+....].H.....$..| +00000160 ce ae 48 54 64 81 d1 84 38 a6 e0 7a 7b 74 2b bc |..HTd...8..z{t+.| +00000170 ce 07 8b b6 04 1f 5b 4c 36 29 68 0c 8c c7 32 15 |......[L6)h...2.| +00000180 93 e0 10 52 c2 27 23 96 c5 0c 9c e9 e2 a9 08 7d |...R.'#........}| +00000190 25 68 65 f5 4e 44 eb a9 85 78 13 e1 0d 86 5e dc |%he.ND...x....^.| +000001a0 fd e5 c6 dd 65 46 8e 2f 32 82 83 0b dd 67 f8 42 |....eF./2....g.B| +000001b0 65 87 3b 08 fe b1 f5 12 e9 74 21 04 12 6d 75 35 |e.;......t!..mu5| +000001c0 b2 eb 93 95 72 10 fa 56 96 77 c3 0c 17 8c 9e f6 |....r..V.w......| +000001d0 77 19 28 37 96 3e 73 98 f4 d2 91 4f 40 db 76 56 |w.(7.>s....O@.vV| +000001e0 ce b5 a8 7a b8 86 d0 9a ba b5 8b 40 c2 63 e1 cf |...z.......@.c..| +000001f0 49 29 2c 5d 1a 9b 8b 56 cb 93 ca 2c c0 d0 15 b7 |I),]...V...,....| +00000200 8a f1 6a d5 0a a8 81 57 b1 6e 10 cd a5 ff b1 4d |..j....W.n.....M| +00000210 47 c6 9b 35 f1 5f 83 91 22 f6 88 68 65 b3 b9 c9 |G..5._.."..he...| +00000220 02 dc 4b f7 13 39 06 e6 3a ec 94 ef 51 15 05 72 |..K..9..:...Q..r| +00000230 1d f4 9d 3b da ca 8d 2c 64 be 9b 45 99 2c 63 cc |...;...,d..E.,c.| +00000240 22 b3 8b 93 ad f6 2c f0 d2 d9 11 3f 5b c0 40 fa |".....,....?[.@.| +00000250 90 6e a0 76 b2 43 b9 4c 72 c4 24 28 a2 bf 56 d6 |.n.v.C.Lr.$(..V.| +00000260 d2 a7 2a d1 8c 5e 1d eb f8 be d0 43 da 7a c7 88 |..*..^.....C.z..| +00000270 61 67 a2 69 85 23 43 3e d4 88 f2 33 c3 5b 38 0a |ag.i.#C>...3.[8.| +00000280 1e de 28 3b 3b 19 de 95 2f 84 c0 37 88 80 59 2f |..(;;.../..7..Y/| +00000290 a6 ee 93 1a 69 08 c3 df 7c cf da c3 9b 96 70 d9 |....i...|.....p.| +000002a0 60 c5 e9 0f 42 f6 1a f2 58 5e f2 32 61 6a b2 a3 |`...B...X^.2aj..| +000002b0 1f 97 fa 08 6c 3f 4b 83 1f 04 66 80 8a 26 3a 7f |....l?K...f..&:.| +000002c0 24 30 ec 10 ae 7d 19 ff 39 91 ca 97 4e ed 0a d7 |$0...}..9...N...| +000002d0 64 3b 6b 50 29 33 0d b2 10 bc 83 63 3c fb 9a 82 |d;kP)3.....c<...| +000002e0 3b 7f bc 04 40 f1 33 64 4a 80 cd 01 f9 f4 c6 89 |;...@.3dJ.......| +000002f0 65 27 25 f9 cf 4f 7e c8 6e d9 0e ec 47 4a 51 29 |e'%..O~.n...GJQ)| +00000300 2f be 34 50 bd 9b d2 d8 b7 ea bb 0b a1 e0 20 1b |/.4P.......... .| +00000310 02 9c f2 17 03 03 00 99 61 dc 0b 3a 30 de 39 f6 |........a..:0.9.| +00000320 f3 db f8 6c 3b fa 4e 1e 7e 62 a5 ae 73 ba e1 41 |...l;.N.~b..s..A| +00000330 58 77 2a c1 7a 0c 50 bb 0c 57 b4 c4 25 bf 2f 9f |Xw*.z.P..W..%./.| +00000340 38 91 e2 65 22 9d ca ac 18 58 7e 81 2d fd 74 24 |8..e"....X~.-.t$| +00000350 28 69 76 11 df 9d 23 b8 be ae 8b e0 93 8e 5d df |(iv...#.......].| +00000360 0a 64 d0 b7 02 68 aa 86 01 0d 55 11 3b 76 70 c6 |.d...h....U.;vp.| +00000370 83 0c 5e 0a e3 37 a5 8b ad 25 50 b9 e8 5c 6b 04 |..^..7...%P..\k.| +00000380 b4 51 ec 9c d3 fa c6 b7 9c f0 46 aa 73 da 3c 0d |.Q........F.s.<.| +00000390 d3 bd 32 81 d4 d2 f1 1a b0 92 f3 73 3e 54 2b 05 |..2........s>T+.| +000003a0 92 24 34 75 df d6 18 a0 6a 82 95 4c 9b fc 7e b6 |.$4u....j..L..~.| +000003b0 8e 17 03 03 00 35 8f 34 0e 3b 91 d8 e7 74 24 71 |.....5.4.;...t$q| +000003c0 0e 7b f3 12 bb 76 2f 31 12 17 b8 9e 24 ce f9 2f |.{...v/1....$../| +000003d0 3f 5d f2 13 4b 2e 9b 1e c4 78 03 a6 c8 07 11 a3 |?]..K....x......| +000003e0 98 79 61 6e 4f 44 6e 18 ee c4 9b 17 03 03 00 93 |.yanODn.........| +000003f0 64 dd 52 a9 d9 51 63 6a a0 a3 c2 75 6b 5d 1d 54 |d.R..Qcj...uk].T| +00000400 ce d4 53 7e 14 8e d9 26 93 28 78 65 16 1b 95 77 |..S~...&.(xe...w| +00000410 68 0a 46 f1 82 36 bb 8a fa 0d df 54 8c 3d 83 e0 |h.F..6.....T.=..| +00000420 d7 de 2d 96 e9 c4 d7 22 d3 97 8e ae 90 f8 fc e6 |..-...."........| +00000430 a6 4b 78 98 4c c5 28 87 91 46 fa f4 1c 8d 0e ec |.Kx.L.(..F......| +00000440 0d 71 40 9a 04 49 b4 e8 5b 62 6f cd 16 c1 d5 fb |.q@..I..[bo.....| +00000450 73 2a 96 8f e5 a2 f4 11 1e df 2d 40 45 6b d5 a9 |s*........-@Ek..| +00000460 e4 e3 f7 93 fc fa d7 20 af d5 f7 b4 0e 09 ad d5 |....... ........| +00000470 26 87 b8 6c e2 20 95 fb c0 70 3e 38 be b7 b1 9f |&..l. ...p>8....| +00000480 70 da c1 |p..| +>>> Flow 3 (client to server) +00000000 14 03 03 00 01 01 17 03 03 00 35 29 d2 b9 bb 9b |..........5)....| +00000010 de 6c 5d 22 23 c1 fe 99 4c c5 33 bf fd 70 36 6b |.l]"#...L.3..p6k| +00000020 f1 a5 92 e8 bf 7c 3d 6e ef 6a 44 73 bc cb 27 1c |.....|=n.jDs..'.| +00000030 09 5d bf 99 4c 19 24 c3 3b 30 91 b5 e3 b6 63 45 |.]..L.$.;0....cE| +>>> Flow 4 (server to client) +00000000 17 03 03 00 1e 52 55 85 7c b8 87 dd c7 b2 d9 5b |.....RU.|......[| +00000010 18 1d bb ac bf b6 ab 76 82 be 64 0e b2 7b 2c 0f |.......v..d..{,.| +00000020 aa 17 92 17 03 03 00 13 79 0a 60 b1 46 20 33 74 |........y.`.F 3t| +00000030 ed 12 a0 23 de 68 88 fc 6f dd 8e |...#.h..o..| From 8d11b1d1172817359d08231deaf29f72d315b762 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 8 Jun 2021 15:53:08 -0400 Subject: [PATCH 30/45] cmd/go: report the imports of CompiledGoFiles in ImportMap Ideally we should encode the load.PackageInternal data in a way that doesn't rely on 1:1 correlations of slices, but this is a minimal fix to unblock Go 1.17. Fixes #46462 Change-Id: I6e029c69f757aadc54d4be02c01d6b294c217542 Reviewed-on: https://go-review.googlesource.com/c/go/+/326610 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills Reviewed-by: Michael Matloob TryBot-Result: Go Bot --- src/cmd/go/internal/list/list.go | 14 ++++++- src/cmd/go/internal/load/pkg.go | 4 +- .../script/list_cgo_compiled_importmap.txt | 38 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 src/cmd/go/testdata/script/list_cgo_compiled_importmap.txt diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go index 53aaf311ec..7cb9ec6d94 100644 --- a/src/cmd/go/internal/list/list.go +++ b/src/cmd/go/internal/list/list.go @@ -724,8 +724,18 @@ func runList(ctx context.Context, cmd *base.Command, args []string) { // Record non-identity import mappings in p.ImportMap. for _, p := range pkgs { - for i, srcPath := range p.Internal.RawImports { - path := p.Imports[i] + nRaw := len(p.Internal.RawImports) + for i, path := range p.Imports { + var srcPath string + if i < nRaw { + srcPath = p.Internal.RawImports[i] + } else { + // This path is not within the raw imports, so it must be an import + // found only within CompiledGoFiles. Those paths are found in + // CompiledImports. + srcPath = p.Internal.CompiledImports[i-nRaw] + } + if path != srcPath { if p.ImportMap == nil { p.ImportMap = make(map[string]string) diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 738904865e..a83cc9a812 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -194,8 +194,8 @@ type PackageInternal struct { // Unexported fields are not part of the public API. Build *build.Package Imports []*Package // this package's direct imports - CompiledImports []string // additional Imports necessary when using CompiledGoFiles (all from standard library) - RawImports []string // this package's original imports as they appear in the text of the program + CompiledImports []string // additional Imports necessary when using CompiledGoFiles (all from standard library); 1:1 with the end of PackagePublic.Imports + RawImports []string // this package's original imports as they appear in the text of the program; 1:1 with the end of PackagePublic.Imports ForceLibrary bool // this package is a library (even if named "main") CmdlineFiles bool // package built from files listed on command line CmdlinePkg bool // package listed on command line diff --git a/src/cmd/go/testdata/script/list_cgo_compiled_importmap.txt b/src/cmd/go/testdata/script/list_cgo_compiled_importmap.txt new file mode 100644 index 0000000000..3d68ef3055 --- /dev/null +++ b/src/cmd/go/testdata/script/list_cgo_compiled_importmap.txt @@ -0,0 +1,38 @@ +# Regression test for https://golang.org/issue/46462. +# +# The "runtime/cgo" import found in synthesized .go files (reported in +# the CompiledGoFiles field) should have a corresponding entry in the +# ImportMap field when a runtime/cgo variant (such as a test variant) +# will be used. + +[short] skip # -compiled can be slow (because it compiles things) +[!cgo] skip + +env CGO_ENABLED=1 +env GOFLAGS=-tags=netcgo # Force net to use cgo even on Windows. + + +# "runtime/cgo [runtime.test]" appears in the the test dependencies of "runtime", +# because "runtime/cgo" itself depends on "runtime" + +go list -deps -test -compiled -f '{{if eq .ImportPath "net [runtime.test]"}}{{printf "%q" .Imports}}{{end}}' runtime + + # Control case: the explicitly-imported package "sync" is a test variant, + # because "sync" depends on "runtime". +stdout '"sync \[runtime\.test\]"' +! stdout '"sync"' + + # Experiment: the implicitly-imported package "runtime/cgo" is also a test variant, + # because "runtime/cgo" also depends on "runtime". +stdout '"runtime/cgo \[runtime\.test\]"' +! stdout '"runtime/cgo"' + + +# Because the import of "runtime/cgo" in the cgo-generated file actually refers +# to "runtime/cgo [runtime.test]", the latter should be listed in the ImportMap. +# BUG(#46462): Today, it is not. + +go list -deps -test -compiled -f '{{if eq .ImportPath "net [runtime.test]"}}{{printf "%q" .ImportMap}}{{end}}' runtime + +stdout '"sync":"sync \[runtime\.test\]"' # control +stdout '"runtime/cgo":"runtime/cgo \[runtime\.test\]"' # experiment From 770f1de8c54256d5b17447028e47b201ba8e62c8 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Thu, 10 Jun 2021 10:50:37 -0700 Subject: [PATCH 31/45] net/http: remove test-only private key from production binaries The net/http/internal package contains a PEM-encoded private key used in tests. This key is initialized at init time, which prevents it from being stripped by the linker in non-test binaries. Move the certificate and key to a new net/http/internal/testcert package to ensure it is only included in binaries that reference it. Fixes #46677. Change-Id: Ie98bda529169314cc791063e7ce4d99ef99113c8 Reviewed-on: https://go-review.googlesource.com/c/go/+/326771 Trust: Damien Neil Run-TryBot: Damien Neil TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills --- src/go/build/deps_test.go | 4 +++- src/net/http/httptest/server.go | 4 ++-- src/net/http/internal/{ => testcert}/testcert.go | 5 +++-- src/net/http/serve_test.go | 7 ++++--- src/net/http/transport_internal_test.go | 4 ++-- src/net/http/transport_test.go | 4 ++-- 6 files changed, 16 insertions(+), 12 deletions(-) rename src/net/http/internal/{ => testcert}/testcert.go (94%) diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 5d1cf7f4c9..45e2f25df7 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -440,7 +440,8 @@ var depsRules = ` # HTTP, King of Dependencies. FMT - < golang.org/x/net/http2/hpack, net/http/internal, net/http/internal/ascii; + < golang.org/x/net/http2/hpack + < net/http/internal, net/http/internal/ascii, net/http/internal/testcert; FMT, NET, container/list, encoding/binary, log < golang.org/x/text/transform @@ -459,6 +460,7 @@ var depsRules = ` golang.org/x/net/http2/hpack, net/http/internal, net/http/internal/ascii, + net/http/internal/testcert, net/http/httptrace, mime/multipart, log diff --git a/src/net/http/httptest/server.go b/src/net/http/httptest/server.go index a02a6d64c3..4f85ff55d8 100644 --- a/src/net/http/httptest/server.go +++ b/src/net/http/httptest/server.go @@ -14,7 +14,7 @@ import ( "log" "net" "net/http" - "net/http/internal" + "net/http/internal/testcert" "os" "strings" "sync" @@ -144,7 +144,7 @@ func (s *Server) StartTLS() { if s.client == nil { s.client = &http.Client{Transport: &http.Transport{}} } - cert, err := tls.X509KeyPair(internal.LocalhostCert, internal.LocalhostKey) + cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey) if err != nil { panic(fmt.Sprintf("httptest: NewTLSServer: %v", err)) } diff --git a/src/net/http/internal/testcert.go b/src/net/http/internal/testcert/testcert.go similarity index 94% rename from src/net/http/internal/testcert.go rename to src/net/http/internal/testcert/testcert.go index 2284a836fb..5f94704ef5 100644 --- a/src/net/http/internal/testcert.go +++ b/src/net/http/internal/testcert/testcert.go @@ -2,7 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package internal +// Package testcert contains a test-only localhost certificate. +package testcert import "strings" @@ -25,7 +26,7 @@ h1fIw3cSS2OolhloGw/XM6RWPWtPAlGykKLciQrBru5NAPvCMsb/I1DAceTiotQM fblo6RBxUQ== -----END CERTIFICATE-----`) -// LocalhostKey is the private key for localhostCert. +// LocalhostKey is the private key for LocalhostCert. var LocalhostKey = []byte(testingKey(`-----BEGIN RSA TESTING KEY----- MIICXgIBAAKBgQDuLnQAI3mDgey3VBzWnB2L39JUU4txjeVE6myuDqkM/uGlfjb9 SjY1bIw4iA5sBBZzHi3z0h1YV8QPuxEbi4nW91IJm2gsvvZhIrCHS3l6afab4pZB diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index c2f8811469..6394da3bb7 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -25,6 +25,7 @@ import ( "net/http/httptest" "net/http/httputil" "net/http/internal" + "net/http/internal/testcert" "net/url" "os" "os/exec" @@ -1475,7 +1476,7 @@ func TestServeTLS(t *testing.T) { defer afterTest(t) defer SetTestHookServerServe(nil) - cert, err := tls.X509KeyPair(internal.LocalhostCert, internal.LocalhostKey) + cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey) if err != nil { t.Fatal(err) } @@ -1599,7 +1600,7 @@ func TestAutomaticHTTP2_Serve_WithTLSConfig(t *testing.T) { } func TestAutomaticHTTP2_ListenAndServe(t *testing.T) { - cert, err := tls.X509KeyPair(internal.LocalhostCert, internal.LocalhostKey) + cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey) if err != nil { t.Fatal(err) } @@ -1609,7 +1610,7 @@ func TestAutomaticHTTP2_ListenAndServe(t *testing.T) { } func TestAutomaticHTTP2_ListenAndServe_GetCertificate(t *testing.T) { - cert, err := tls.X509KeyPair(internal.LocalhostCert, internal.LocalhostKey) + cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey) if err != nil { t.Fatal(err) } diff --git a/src/net/http/transport_internal_test.go b/src/net/http/transport_internal_test.go index 1097ffd173..1cce27235d 100644 --- a/src/net/http/transport_internal_test.go +++ b/src/net/http/transport_internal_test.go @@ -12,7 +12,7 @@ import ( "errors" "io" "net" - "net/http/internal" + "net/http/internal/testcert" "strings" "testing" ) @@ -191,7 +191,7 @@ func (f roundTripFunc) RoundTrip(r *Request) (*Response, error) { // Issue 25009 func TestTransportBodyAltRewind(t *testing.T) { - cert, err := tls.X509KeyPair(internal.LocalhostCert, internal.LocalhostKey) + cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey) if err != nil { t.Fatal(err) } diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index dcaacece61..690e0c299d 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -30,7 +30,7 @@ import ( "net/http/httptest" "net/http/httptrace" "net/http/httputil" - "net/http/internal" + "net/http/internal/testcert" "net/textproto" "net/url" "os" @@ -4299,7 +4299,7 @@ func TestTransportReuseConnEmptyResponseBody(t *testing.T) { // Issue 13839 func TestNoCrashReturningTransportAltConn(t *testing.T) { - cert, err := tls.X509KeyPair(internal.LocalhostCert, internal.LocalhostKey) + cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey) if err != nil { t.Fatal(err) } From 2721da26083f253c46c2fd0c1dadee14ae4202f5 Mon Sep 17 00:00:00 2001 From: Rhys Hiltner Date: Thu, 10 Jun 2021 15:05:07 -0700 Subject: [PATCH 32/45] doc/go1.17: fix formatting near httptest Change-Id: Ic1a0add3a1e137ca5cd0f3e9ce3266191b0b55cd Reviewed-on: https://go-review.googlesource.com/c/go/+/326777 Trust: Alberto Donizetti Reviewed-by: Dmitri Shuralyov --- doc/go1.17.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 49fbabdc3f..101957aabd 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -763,9 +763,9 @@ func Foo() bool {

net/http/httptest

- ResponseRecorder.WriteHeader> + ResponseRecorder.WriteHeader now panics when the provided code is not a valid three-digit HTTP status code. - This matches the behavior of ResponseWriter> + This matches the behavior of ResponseWriter implementations in the net/http package.

From 2f1128461dcc6c9915f21327ce18fa1925269f30 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 10 Jun 2021 16:54:53 -0400 Subject: [PATCH 33/45] cmd/go: match Windows paths in TestScript/mod_invalid_version Fixes #46691 Change-Id: I3bef9a773be640bed96eb2dc395cb11671a0767a Reviewed-on: https://go-review.googlesource.com/c/go/+/326869 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/cmd/go/testdata/script/mod_invalid_version.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cmd/go/testdata/script/mod_invalid_version.txt b/src/cmd/go/testdata/script/mod_invalid_version.txt index 34d9c47674..6846a792a5 100644 --- a/src/cmd/go/testdata/script/mod_invalid_version.txt +++ b/src/cmd/go/testdata/script/mod_invalid_version.txt @@ -19,7 +19,7 @@ cp go.mod.orig go.mod go mod edit -require golang.org/x/text@14c0d48ead0c cd outside ! go list -m golang.org/x/text -stderr 'go list -m: example.com@v0.0.0 \(replaced by \./\..\): parsing ../go.mod: '$WORK'/gopath/src/go.mod:5: require golang.org/x/text: version "14c0d48ead0c" invalid: must be of the form v1.2.3' +stderr 'go list -m: example.com@v0.0.0 \(replaced by \./\.\.\): parsing ..[/\\]go.mod: '$WORK'[/\\]gopath[/\\]src[/\\]go.mod:5: require golang.org/x/text: version "14c0d48ead0c" invalid: must be of the form v1.2.3' cd .. go list -m golang.org/x/text stdout 'golang.org/x/text v0.1.1-0.20170915032832-14c0d48ead0c' @@ -47,10 +47,10 @@ cp go.mod.orig go.mod go mod edit -require golang.org/x/text@v2.1.1-0.20170915032832-14c0d48ead0c cd outside ! go list -m golang.org/x/text -stderr 'go list -m: example.com@v0.0.0 \(replaced by \./\.\.\): parsing ../go.mod: '$WORK'/gopath/src/go.mod:5: require golang.org/x/text: version "v2.1.1-0.20170915032832-14c0d48ead0c" invalid: should be v0 or v1, not v2' +stderr 'go list -m: example.com@v0.0.0 \(replaced by \./\.\.\): parsing ..[/\\]go.mod: '$WORK'[/\\]gopath[/\\]src[/\\]go.mod:5: require golang.org/x/text: version "v2.1.1-0.20170915032832-14c0d48ead0c" invalid: should be v0 or v1, not v2' cd .. ! go list -m golang.org/x/text -stderr $WORK'/gopath/src/go.mod:5: require golang.org/x/text: version "v2.1.1-0.20170915032832-14c0d48ead0c" invalid: should be v0 or v1, not v2' +stderr $WORK'[/\\]gopath[/\\]src[/\\]go.mod:5: require golang.org/x/text: version "v2.1.1-0.20170915032832-14c0d48ead0c" invalid: should be v0 or v1, not v2' # A pseudo-version with fewer than 12 digits of SHA-1 prefix is invalid. cp go.mod.orig go.mod From e2dc6dd5c9b6799c9bb987f3a4600fb0df686d09 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 10 Jun 2021 14:40:58 -0700 Subject: [PATCH 34/45] doc/go1.17: clean up formatting of gofmt section It was the only h3 in , and it lacked

around its content. It looked like it was part of the prior section: https://tip.golang.org/doc/go1.17#gofmt Change-Id: I7e9ef70e9a03474225833f44420aabd70dab3cd5 Reviewed-on: https://go-review.googlesource.com/c/go/+/326774 Reviewed-by: Dmitri Shuralyov Trust: Brad Fitzpatrick --- doc/go1.17.html | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 101957aabd..4fa38921f0 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -293,7 +293,9 @@ Do not send CLs removing the interior tags from such phrases. https://golang.org/design/draft-gobuild.

-

gofmt

+

Gofmt

+ +

gofmt (and go fmt) now synchronizes //go:build lines with // +build lines. If a file only has // +build lines, they will be moved to the appropriate @@ -301,7 +303,7 @@ Do not send CLs removing the interior tags from such phrases. added. Otherwise, // +build lines will be overwritten based on any existing //go:build lines. For more information, see https://golang.org/design/draft-gobuild. - +

Vet

From 77aa209b386a184e7f4b44938f2a05a1b5c5a3cf Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 10 Jun 2021 15:35:05 -0700 Subject: [PATCH 35/45] runtime: loop on EINTR in macOS sigNoteSleep Fixes #46466 Change-Id: I8fb15d0c8ef7ef6e6fc1b9e0e033d213255fe0df Reviewed-on: https://go-review.googlesource.com/c/go/+/326778 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Michael Knyszek Reviewed-by: Cherry Mui --- src/runtime/os_darwin.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go index 00139351ab..079be107d7 100644 --- a/src/runtime/os_darwin.go +++ b/src/runtime/os_darwin.go @@ -118,10 +118,15 @@ func sigNoteWakeup(*note) { // sigNoteSleep waits for a note created by sigNoteSetup to be woken. func sigNoteSleep(*note) { - entersyscallblock() - var b byte - read(sigNoteRead, unsafe.Pointer(&b), 1) - exitsyscall() + for { + var b byte + entersyscallblock() + n := read(sigNoteRead, unsafe.Pointer(&b), 1) + exitsyscall() + if n != -_EINTR { + return + } + } } // BSD interface for threading. From 16b5d766d8af8bc348f93e6cb2b53a4e2d5d72ca Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 11 Jun 2021 17:36:50 +0200 Subject: [PATCH 36/45] syscall: do not load native libraries on non-native powershell on arm The powershell that currently ships on ARM Windows isn't native, so it won't load native DLLs. So just skip the tests for now, and reenable it if this ever changes. Updates #46701. Change-Id: I2559fdf13cb65d3ecdc4c6f6df7dec1b490b9651 Reviewed-on: https://go-review.googlesource.com/c/go/+/327210 TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Trust: Jason A. Donenfeld Run-TryBot: Jason A. Donenfeld --- src/syscall/syscall_windows_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/syscall/syscall_windows_test.go b/src/syscall/syscall_windows_test.go index ea8fa191dc..3243952ded 100644 --- a/src/syscall/syscall_windows_test.go +++ b/src/syscall/syscall_windows_test.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "syscall" "testing" @@ -79,6 +80,9 @@ func TestTOKEN_ALL_ACCESS(t *testing.T) { func TestStdioAreInheritable(t *testing.T) { testenv.MustHaveGoBuild(t) testenv.MustHaveExecPath(t, "gcc") + if runtime.GOARCH == "arm64" || runtime.GOARCH == "arm" { + t.Skip("Powershell is not native on ARM; see golang.org/issues/46701") + } tmpdir := t.TempDir() From e552a6d31270c86064632af1d092e0db5a930250 Mon Sep 17 00:00:00 2001 From: Constantin Konstantinidis Date: Sat, 5 Jun 2021 07:48:30 +0200 Subject: [PATCH 37/45] cmd/go: remove hint when no module is suggested Fixes #46528 Change-Id: I2453d321ece878ea7823865758aa4a16b3ed7fe8 Reviewed-on: https://go-review.googlesource.com/c/go/+/325430 Reviewed-by: Bryan C. Mills Run-TryBot: Bryan C. Mills Trust: Heschi Kreinick Trust: Dmitri Shuralyov TryBot-Result: Go Bot --- src/cmd/go/internal/modload/import.go | 10 ++++++---- src/cmd/go/testdata/script/mod_install_hint.txt | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_install_hint.txt diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index f76befcfe3..60bd26fb22 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -178,11 +178,13 @@ func (e *ImportMissingSumError) Error() string { // Importing package is unknown, or the missing package was named on the // command line. Recommend 'go mod download' for the modules that could // provide the package, since that shouldn't change go.mod. - args := make([]string, len(e.mods)) - for i, mod := range e.mods { - args[i] = mod.Path + if len(e.mods) > 0 { + args := make([]string, len(e.mods)) + for i, mod := range e.mods { + args[i] = mod.Path + } + hint = fmt.Sprintf("; to add:\n\tgo mod download %s", strings.Join(args, " ")) } - hint = fmt.Sprintf("; to add:\n\tgo mod download %s", strings.Join(args, " ")) } else { // Importing package is known (common case). Recommend 'go get' on the // current version of the importing package. diff --git a/src/cmd/go/testdata/script/mod_install_hint.txt b/src/cmd/go/testdata/script/mod_install_hint.txt new file mode 100644 index 0000000000..ab02840eb8 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_install_hint.txt @@ -0,0 +1,5 @@ +# Module is replaced but not required. No hint appears as no module is suggested. +go mod init m +go mod edit -replace=github.com/notrequired@v0.5.0=github.com/doesnotexist@v0.5.0 +! go install github.com/notrequired +! stderr 'to add it:' \ No newline at end of file From 9d46ee5ac4acd6602692f70c5149a3f6db058558 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Fri, 11 Jun 2021 13:58:05 +0000 Subject: [PATCH 38/45] reflect: handle stack-to-register translation in callMethod callMethod previously assumed erroneously that between the "value" and "method" ABIs (that is, the ABI the caller is following to call this method value and the actual ABI of the method), it could never happen that an argument passed on the stack in the former could be passed in registers in the latter. The cited reason was that the latter always uses strictly more registers. However, there are situations where the value ABI could pass a value on the stack, but later is passed in a register. For instance, if the receiver pushes a value passed in registers that uses multiple registers to be passed on the stack, later arguments which were passed on the stack may now be passed in registers. This change fixes callMethod to no longer makes this assumption, and handles the stack-to-register translation explicitly. Fixes #46696. Change-Id: I7100a664d97bbe401302cc893b3a98b28cdcdfc0 Reviewed-on: https://go-review.googlesource.com/c/go/+/327089 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Cherry Mui --- src/reflect/abi_test.go | 43 ++++++++++++++++++++++++++++++++++++++++- src/reflect/value.go | 42 ++++++++++++++++++++++++++++++---------- 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/src/reflect/abi_test.go b/src/reflect/abi_test.go index 1a2a48b5ed..5a0130f7b4 100644 --- a/src/reflect/abi_test.go +++ b/src/reflect/abi_test.go @@ -79,7 +79,34 @@ func TestMethodValueCallABI(t *testing.T) { t.Errorf("bad method value call: got %#v, want %#v", r2, a2) } if s.Value != 3 { - t.Errorf("bad method value call: failed to set s.Value: got %d, want %d", s.Value, 1) + t.Errorf("bad method value call: failed to set s.Value: got %d, want %d", s.Value, 3) + } + + s, i = makeMethodValue("ValueRegMethodSpillInt") + f3 := i.(func(StructFillRegs, int, MagicLastTypeNameForTestingRegisterABI) (StructFillRegs, int)) + r3a, r3b := f3(a2, 42, MagicLastTypeNameForTestingRegisterABI{}) + if r3a != a2 { + t.Errorf("bad method value call: got %#v, want %#v", r3a, a2) + } + if r3b != 42 { + t.Errorf("bad method value call: got %#v, want %#v", r3b, 42) + } + if s.Value != 4 { + t.Errorf("bad method value call: failed to set s.Value: got %d, want %d", s.Value, 4) + } + + s, i = makeMethodValue("ValueRegMethodSpillPtr") + f4 := i.(func(StructFillRegs, *byte, MagicLastTypeNameForTestingRegisterABI) (StructFillRegs, *byte)) + vb := byte(10) + r4a, r4b := f4(a2, &vb, MagicLastTypeNameForTestingRegisterABI{}) + if r4a != a2 { + t.Errorf("bad method value call: got %#v, want %#v", r4a, a2) + } + if r4b != &vb { + t.Errorf("bad method value call: got %#v, want %#v", r4b, &vb) + } + if s.Value != 5 { + t.Errorf("bad method value call: failed to set s.Value: got %d, want %d", s.Value, 5) } } @@ -112,6 +139,20 @@ func (m *StructWithMethods) SpillStructCall(s StructFillRegs, _ MagicLastTypeNam return s } +// When called as a method value, i is passed on the stack. +// When called as a method, i is passed in a register. +func (m *StructWithMethods) ValueRegMethodSpillInt(s StructFillRegs, i int, _ MagicLastTypeNameForTestingRegisterABI) (StructFillRegs, int) { + m.Value = 4 + return s, i +} + +// When called as a method value, i is passed on the stack. +// When called as a method, i is passed in a register. +func (m *StructWithMethods) ValueRegMethodSpillPtr(s StructFillRegs, i *byte, _ MagicLastTypeNameForTestingRegisterABI) (StructFillRegs, *byte) { + m.Value = 5 + return s, i +} + func TestReflectCallABI(t *testing.T) { // Enable register-based reflect.Call and ensure we don't // use potentially incorrect cached versions by clearing diff --git a/src/reflect/value.go b/src/reflect/value.go index 418dff781f..c963a407bc 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -952,25 +952,47 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer, retValid *bool, regs *a continue } - // There are three cases to handle in translating each + // There are four cases to handle in translating each // argument: // 1. Stack -> stack translation. - // 2. Registers -> stack translation. - // 3. Registers -> registers translation. - // The fourth cases can't happen, because a method value - // call uses strictly fewer registers than a method call. + // 2. Stack -> registers translation. + // 3. Registers -> stack translation. + // 4. Registers -> registers translation. + // TODO(mknyszek): Cases 2 and 3 below only work on little endian + // architectures. This is OK for now, but this needs to be fixed + // before supporting the register ABI on big endian architectures. // If the value ABI passes the value on the stack, // then the method ABI does too, because it has strictly // fewer arguments. Simply copy between the two. if vStep := valueSteps[0]; vStep.kind == abiStepStack { mStep := methodSteps[0] - if mStep.kind != abiStepStack || vStep.size != mStep.size { - panic("method ABI and value ABI do not align") + // Handle stack -> stack translation. + if mStep.kind == abiStepStack { + if vStep.size != mStep.size { + panic("method ABI and value ABI do not align") + } + typedmemmove(t, + add(methodFrame, mStep.stkOff, "precomputed stack offset"), + add(valueFrame, vStep.stkOff, "precomputed stack offset")) + continue + } + // Handle stack -> register translation. + for _, mStep := range methodSteps { + from := add(valueFrame, vStep.stkOff+mStep.offset, "precomputed stack offset") + switch mStep.kind { + case abiStepPointer: + // Do the pointer copy directly so we get a write barrier. + methodRegs.Ptrs[mStep.ireg] = *(*unsafe.Pointer)(from) + fallthrough // We need to make sure this ends up in Ints, too. + case abiStepIntReg: + memmove(unsafe.Pointer(&methodRegs.Ints[mStep.ireg]), from, mStep.size) + case abiStepFloatReg: + memmove(unsafe.Pointer(&methodRegs.Floats[mStep.freg]), from, mStep.size) + default: + panic("unexpected method step") + } } - typedmemmove(t, - add(methodFrame, mStep.stkOff, "precomputed stack offset"), - add(valueFrame, vStep.stkOff, "precomputed stack offset")) continue } // Handle register -> stack translation. From 1ed0d129e9ba9b55e9ae36ac1d7f2766ba16b373 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 11 Jun 2021 11:50:42 -0700 Subject: [PATCH 39/45] runtime: testprogcgo: don't call exported Go functions directly from Go Instead route through a C function, to avoid declaration conflicts between the declaration needed in the cgo comment and the declaration generated by cgo in _cgo_export.h. This is not something user code will ever do, so no need to make it work in cgo. Fixes #46502 Change-Id: I1bfffdc76ef8ea63e3829871298d0774157957a5 Reviewed-on: https://go-review.googlesource.com/c/go/+/327309 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Cherry Mui Reviewed-by: Jason A. Donenfeld --- src/runtime/testdata/testprogcgo/aprof.go | 4 ++-- src/runtime/testdata/testprogcgo/aprof_c.c | 9 +++++++++ src/runtime/testdata/testprogcgo/bigstack1_windows.c | 12 ++++++++++++ src/runtime/testdata/testprogcgo/bigstack_windows.go | 4 ++-- 4 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 src/runtime/testdata/testprogcgo/aprof_c.c create mode 100644 src/runtime/testdata/testprogcgo/bigstack1_windows.c diff --git a/src/runtime/testdata/testprogcgo/aprof.go b/src/runtime/testdata/testprogcgo/aprof.go index aabca9e1eb..44a15b0865 100644 --- a/src/runtime/testdata/testprogcgo/aprof.go +++ b/src/runtime/testdata/testprogcgo/aprof.go @@ -10,7 +10,7 @@ package main // The test fails when the function is the first C function. // The exported functions are the first C functions, so we use that. -// extern void GoNop(); +// extern void CallGoNop(); import "C" import ( @@ -38,7 +38,7 @@ func CgoCCodeSIGPROF() { break } } - C.GoNop() + C.CallGoNop() } c <- true }() diff --git a/src/runtime/testdata/testprogcgo/aprof_c.c b/src/runtime/testdata/testprogcgo/aprof_c.c new file mode 100644 index 0000000000..d588e13045 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/aprof_c.c @@ -0,0 +1,9 @@ +// 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. + +#include "_cgo_export.h" + +void CallGoNop() { + GoNop(); +} diff --git a/src/runtime/testdata/testprogcgo/bigstack1_windows.c b/src/runtime/testdata/testprogcgo/bigstack1_windows.c new file mode 100644 index 0000000000..551fb68309 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/bigstack1_windows.c @@ -0,0 +1,12 @@ +// 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. + +// This is not in bigstack_windows.c because it needs to be part of +// testprogcgo but is not part of the DLL built from bigstack_windows.c. + +#include "_cgo_export.h" + +void CallGoBigStack1(char* p) { + goBigStack1(p); +} diff --git a/src/runtime/testdata/testprogcgo/bigstack_windows.go b/src/runtime/testdata/testprogcgo/bigstack_windows.go index f58fcf993f..135b5fcfe0 100644 --- a/src/runtime/testdata/testprogcgo/bigstack_windows.go +++ b/src/runtime/testdata/testprogcgo/bigstack_windows.go @@ -6,7 +6,7 @@ package main /* typedef void callback(char*); -extern void goBigStack1(char*); +extern void CallGoBigStack1(char*); extern void bigStack(callback*); */ import "C" @@ -18,7 +18,7 @@ func init() { func BigStack() { // Create a large thread stack and call back into Go to test // if Go correctly determines the stack bounds. - C.bigStack((*C.callback)(C.goBigStack1)) + C.bigStack((*C.callback)(C.CallGoBigStack1)) } //export goBigStack1 From 67b1b6a2e3a405e3e0b5c6a76f702b2a6071c1f0 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sun, 13 Jun 2021 10:55:19 +0700 Subject: [PATCH 40/45] cmd/compile: allow ir.OSLICE2ARRPTR in mayCall CL 301650 adds conversion from slice to array ptr. The conversion expression may appear as argument to a function call, so it will be tested by mayCall. But ir.OSLICE2ARRPTR op is not handled by mayCall, causes the compiler crashes. Updates #395 Fixes #46720 Change-Id: I39e1b3e38e224a31f3dec46dbbdc855ff3b2c6a5 Reviewed-on: https://go-review.googlesource.com/c/go/+/327649 Trust: Cuong Manh Le Trust: Josh Bleecher Snyder Run-TryBot: Cuong Manh Le Reviewed-by: Matthew Dempsky Reviewed-by: Josh Bleecher Snyder TryBot-Result: Go Bot --- src/cmd/compile/internal/walk/walk.go | 2 +- test/fixedbugs/issue46720.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue46720.go diff --git a/src/cmd/compile/internal/walk/walk.go b/src/cmd/compile/internal/walk/walk.go index fe2c62cd4f..26da6e3145 100644 --- a/src/cmd/compile/internal/walk/walk.go +++ b/src/cmd/compile/internal/walk/walk.go @@ -313,7 +313,7 @@ func mayCall(n ir.Node) bool { return true case ir.OINDEX, ir.OSLICE, ir.OSLICEARR, ir.OSLICE3, ir.OSLICE3ARR, ir.OSLICESTR, - ir.ODEREF, ir.ODOTPTR, ir.ODOTTYPE, ir.ODIV, ir.OMOD: + ir.ODEREF, ir.ODOTPTR, ir.ODOTTYPE, ir.ODIV, ir.OMOD, ir.OSLICE2ARRPTR: // These ops might panic, make sure they are done // before we start marshaling args for a call. See issue 16760. return true diff --git a/test/fixedbugs/issue46720.go b/test/fixedbugs/issue46720.go new file mode 100644 index 0000000000..3b0151ae84 --- /dev/null +++ b/test/fixedbugs/issue46720.go @@ -0,0 +1,15 @@ +// 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() { + nonce := make([]byte, 24) + g((*[24]byte)(nonce)) +} + +//go:noinline +func g(*[24]byte) {} From 24cff0f0444793be81062684c478a3f7ca955499 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sat, 12 Jun 2021 12:34:40 +0200 Subject: [PATCH 41/45] cmd/go, misc/cgo: skip test if no .edata Clang does not produce binaries with an .edata section, even when it exports symbols properly, so just skip this binutils-specific test for that case. Later we can rewrite these tests entirely to do something more robust. Updates #46719. Change-Id: I864b3c2d91e66800c55454ae11d4ab1623693d14 Reviewed-on: https://go-review.googlesource.com/c/go/+/327549 Trust: Jason A. Donenfeld Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- misc/cgo/testcshared/cshared_test.go | 2 +- src/cmd/go/go_test.go | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/misc/cgo/testcshared/cshared_test.go b/misc/cgo/testcshared/cshared_test.go index 90d8c365e6..fdc6df9602 100644 --- a/misc/cgo/testcshared/cshared_test.go +++ b/misc/cgo/testcshared/cshared_test.go @@ -400,7 +400,7 @@ func main() { defer f.Close() section := f.Section(".edata") if section == nil { - t.Fatalf(".edata section is not present") + t.Skip(".edata section is not present") } // TODO: deduplicate this struct from cmd/link/internal/ld/pe.go diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index a059a6dd90..c0c86ab9f5 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -72,7 +72,6 @@ func tooSlow(t *testing.T) { // (temp) directory. var testGOROOT string -var testCC string var testGOCACHE string var testGo string @@ -179,13 +178,6 @@ func TestMain(m *testing.M) { os.Exit(2) } - out, err = exec.Command(gotool, "env", "CC").CombinedOutput() - if err != nil { - fmt.Fprintf(os.Stderr, "could not find testing CC: %v\n%s", err, out) - os.Exit(2) - } - testCC = strings.TrimSpace(string(out)) - cmd := exec.Command(testGo, "env", "CGO_ENABLED") cmd.Stderr = new(strings.Builder) if out, err := cmd.Output(); err != nil { @@ -2185,7 +2177,7 @@ func testBuildmodePIE(t *testing.T, useCgo, setBuildmodeToPIE bool) { // See https://sourceware.org/bugzilla/show_bug.cgi?id=19011 section := f.Section(".edata") if section == nil { - t.Fatalf(".edata section is not present") + t.Skip(".edata section is not present") } // TODO: deduplicate this struct from cmd/link/internal/ld/pe.go type IMAGE_EXPORT_DIRECTORY struct { From 14305bf0b9ab87bcaca11416ab61e7a4ba09690d Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 11 Jun 2021 17:53:29 +0200 Subject: [PATCH 42/45] misc/cgo: generate Windows import libraries for clang LLD won't import a .dll directly and instead requires an import library. So generate these using -out-implib, the same way as was done in CL 312046, where it makes sense, and elsewhere build the import library using a def file. We can't use -out-implib all the time, because the output file gets overwritten each time the linker is called, rather than merged. Updates #46502. Change-Id: Iefe54cb6c576004b83b1039ba673881b8640423d Reviewed-on: https://go-review.googlesource.com/c/go/+/327211 Trust: Jason A. Donenfeld Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- misc/cgo/testcshared/cshared_test.go | 56 +++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/misc/cgo/testcshared/cshared_test.go b/misc/cgo/testcshared/cshared_test.go index fdc6df9602..19ad8c76a8 100644 --- a/misc/cgo/testcshared/cshared_test.go +++ b/misc/cgo/testcshared/cshared_test.go @@ -292,11 +292,60 @@ func createHeaders() error { "-installsuffix", "testcshared", "-o", libgoname, filepath.Join(".", "libgo", "libgo.go")} + if GOOS == "windows" && strings.HasSuffix(args[6], ".a") { + args[6] = strings.TrimSuffix(args[6], ".a") + ".dll" + } cmd = exec.Command(args[0], args[1:]...) out, err = cmd.CombinedOutput() if err != nil { return fmt.Errorf("command failed: %v\n%v\n%s\n", args, err, out) } + if GOOS == "windows" { + // We can't simply pass -Wl,--out-implib, because this relies on having imports from multiple packages, + // which results in the linkers output implib getting overwritten at each step. So instead build the + // import library the traditional way, using a def file. + err = os.WriteFile("libgo.def", + []byte("LIBRARY libgo.dll\nEXPORTS\n\tDidInitRun\n\tDidMainRun\n\tDivu\n\tFromPkg\n\t_cgo_dummy_export\n"), + 0644) + if err != nil { + return fmt.Errorf("unable to write def file: %v", err) + } + out, err = exec.Command(cc[0], append(cc[1:], "-print-prog-name=dlltool")...).CombinedOutput() + if err != nil { + return fmt.Errorf("unable to find dlltool path: %v\n%s\n", err, out) + } + args := []string{strings.TrimSpace(string(out)), "-D", args[6], "-l", libgoname, "-d", "libgo.def"} + + // This is an unfortunate workaround for https://github.com/mstorsjo/llvm-mingw/issues/205 in which + // we basically reimplement the contents of the dlltool.sh wrapper: https://git.io/JZFlU + dlltoolContents, err := os.ReadFile(args[0]) + if err != nil { + return fmt.Errorf("unable to read dlltool: %v\n", err) + } + if bytes.HasPrefix(dlltoolContents, []byte("#!/bin/sh")) && bytes.Contains(dlltoolContents, []byte("llvm-dlltool")) { + base, name := filepath.Split(args[0]) + args[0] = filepath.Join(base, "llvm-dlltool") + var machine string + switch strings.SplitN(name, "-", 2)[0] { + case "i686": + machine = "i386" + case "x86_64": + machine = "i386:x86-64" + case "armv7": + machine = "arm" + case "aarch64": + machine = "arm64" + } + if len(machine) > 0 { + args = append(args, "-m", machine) + } + } + + out, err = exec.Command(args[0], args[1:]...).CombinedOutput() + if err != nil { + return fmt.Errorf("unable to run dlltool to create import library: %v\n%s\n", err, out) + } + } if runtime.GOOS != GOOS && GOOS == "android" { args = append(adbCmd(), "push", libgoname, fmt.Sprintf("%s/%s", androiddir, libgoname)) @@ -749,7 +798,12 @@ func TestGo2C2Go(t *testing.T) { defer os.RemoveAll(tmpdir) lib := filepath.Join(tmpdir, "libtestgo2c2go."+libSuffix) - run(t, nil, "go", "build", "-buildmode=c-shared", "-o", lib, "./go2c2go/go") + var env []string + if GOOS == "windows" && strings.HasSuffix(lib, ".a") { + env = append(env, "CGO_LDFLAGS=-Wl,--out-implib,"+lib, "CGO_LDFLAGS_ALLOW=.*") + lib = strings.TrimSuffix(lib, ".a") + ".dll" + } + run(t, env, "go", "build", "-buildmode=c-shared", "-o", lib, "./go2c2go/go") cgoCflags := os.Getenv("CGO_CFLAGS") if cgoCflags != "" { From 3249b645c986849bbf72c1dc71efc4f90df465ec Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sun, 13 Jun 2021 22:26:21 +0700 Subject: [PATCH 43/45] cmd/compile: factor out rewrite multi-valued f() So next CL can reuse code to rewrite OAS2FUNC. Passes toolstash -cmp. For #46725 Change-Id: I1113ed615b6d6b9494dd87000ce342d7a46d9e7b Reviewed-on: https://go-review.googlesource.com/c/go/+/327650 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky --- .../compile/internal/typecheck/typecheck.go | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/cmd/compile/internal/typecheck/typecheck.go b/src/cmd/compile/internal/typecheck/typecheck.go index 95f7b50259..391e18bd0a 100644 --- a/src/cmd/compile/internal/typecheck/typecheck.go +++ b/src/cmd/compile/internal/typecheck/typecheck.go @@ -945,16 +945,18 @@ func typecheckargs(n ir.InitNode) { return } - // Rewrite f(g()) into t1, t2, ... = g(); f(t1, t2, ...). - // Save n as n.Orig for fmt.go. if ir.Orig(n) == n { n.(ir.OrigNode).SetOrig(ir.SepCopy(n)) } - as := ir.NewAssignListStmt(base.Pos, ir.OAS2, nil, nil) - as.Rhs.Append(list...) + // Rewrite f(g()) into t1, t2, ... = g(); f(t1, t2, ...). + rewriteMultiValueCall(n, list[0]) +} +// rewriteMultiValueCall rewrites multi-valued f() to use temporaries, +// so the backend wouldn't need to worry about tuple-valued expressions. +func rewriteMultiValueCall(n ir.InitNode, call ir.Node) { // If we're outside of function context, then this call will // be executed during the generated init function. However, // init.go hasn't yet created it. Instead, associate the @@ -964,25 +966,30 @@ func typecheckargs(n ir.InitNode) { if static { ir.CurFunc = InitTodoFunc } - list = nil - for _, f := range t.FieldSlice() { - t := Temp(f.Type) - as.PtrInit().Append(ir.NewDecl(base.Pos, ir.ODCL, t)) - as.Lhs.Append(t) - list = append(list, t) + + as := ir.NewAssignListStmt(base.Pos, ir.OAS2, nil, []ir.Node{call}) + results := call.Type().FieldSlice() + list := make([]ir.Node, len(results)) + for i, result := range results { + tmp := Temp(result.Type) + as.PtrInit().Append(ir.NewDecl(base.Pos, ir.ODCL, tmp)) + as.Lhs.Append(tmp) + list[i] = tmp } if static { ir.CurFunc = nil } + n.PtrInit().Append(Stmt(as)) + switch n := n.(type) { + default: + base.Fatalf("rewriteMultiValueCall %+v", n.Op()) case *ir.CallExpr: n.Args = list case *ir.ReturnStmt: n.Results = list } - - n.PtrInit().Append(Stmt(as)) } func checksliceindex(l ir.Node, r ir.Node, tp *types.Type) bool { From 326ea438bb579a2010e38e00f515a04344ff96b0 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sun, 13 Jun 2021 22:28:44 +0700 Subject: [PATCH 44/45] cmd/compile: rewrite a, b = f() to use temporaries when type not identical If any of the LHS expressions of an OAS2FUNC are not identical to the respective function call results, escape analysis mishandles the implicit conversion, causes memory corruption. Instead, we should insert autotmps like we already do for f(g()) calls and return g() statements. Fixes #46725 Change-Id: I71a08da0bf1a03d09a023da5b6f78fb37a4a4690 Reviewed-on: https://go-review.googlesource.com/c/go/+/327651 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/typecheck/stmt.go | 14 +++++- .../compile/internal/typecheck/typecheck.go | 10 ++++ test/declbad.go | 4 +- test/fixedbugs/issue46725.go | 48 +++++++++++++++++++ 4 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 test/fixedbugs/issue46725.go diff --git a/src/cmd/compile/internal/typecheck/stmt.go b/src/cmd/compile/internal/typecheck/stmt.go index 175216f279..922a01bfbe 100644 --- a/src/cmd/compile/internal/typecheck/stmt.go +++ b/src/cmd/compile/internal/typecheck/stmt.go @@ -204,8 +204,20 @@ assignOK: r.Use = ir.CallUseList rtyp := r.Type() + mismatched := false + failed := false for i := range lhs { - assignType(i, rtyp.Field(i).Type) + result := rtyp.Field(i).Type + assignType(i, result) + + if lhs[i].Type() == nil || result == nil { + failed = true + } else if lhs[i] != ir.BlankNode && !types.Identical(lhs[i].Type(), result) { + mismatched = true + } + } + if mismatched && !failed { + rewriteMultiValueCall(stmt, r) } return } diff --git a/src/cmd/compile/internal/typecheck/typecheck.go b/src/cmd/compile/internal/typecheck/typecheck.go index 391e18bd0a..bf52941b2c 100644 --- a/src/cmd/compile/internal/typecheck/typecheck.go +++ b/src/cmd/compile/internal/typecheck/typecheck.go @@ -989,6 +989,16 @@ func rewriteMultiValueCall(n ir.InitNode, call ir.Node) { n.Args = list case *ir.ReturnStmt: n.Results = list + case *ir.AssignListStmt: + if n.Op() != ir.OAS2FUNC { + base.Fatalf("rewriteMultiValueCall: invalid op %v", n.Op()) + } + as.SetOp(ir.OAS2FUNC) + n.SetOp(ir.OAS2) + n.Rhs = make([]ir.Node, len(list)) + for i, tmp := range list { + n.Rhs[i] = AssignConv(tmp, n.Lhs[i].Type(), "assignment") + } } } diff --git a/test/declbad.go b/test/declbad.go index 728eceb7f1..b978652a2b 100644 --- a/test/declbad.go +++ b/test/declbad.go @@ -23,13 +23,13 @@ func main() { { // change of type for f i, f, s := f3() - f, g, t := f3() // ERROR "redeclared|cannot assign|incompatible" + f, g, t := f3() // ERROR "redeclared|cannot assign|incompatible|cannot use" _, _, _, _, _ = i, f, s, g, t } { // change of type for i i, f, s := f3() - j, i, t := f3() // ERROR "redeclared|cannot assign|incompatible" + j, i, t := f3() // ERROR "redeclared|cannot assign|incompatible|cannot use" _, _, _, _, _ = i, f, s, j, t } { diff --git a/test/fixedbugs/issue46725.go b/test/fixedbugs/issue46725.go new file mode 100644 index 0000000000..29799c7d7e --- /dev/null +++ b/test/fixedbugs/issue46725.go @@ -0,0 +1,48 @@ +// run + +// 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 main + +import "runtime" + +type T [4]int + +//go:noinline +func g(x []*T) ([]*T, []*T) { return x, x } + +func main() { + const Jenny = 8675309 + s := [10]*T{{Jenny}} + + done := make(chan struct{}) + runtime.SetFinalizer(s[0], func(p *T) { close(done) }) + + var h, _ interface{} = g(s[:]) + + if wait(done) { + panic("GC'd early") + } + + if h.([]*T)[0][0] != Jenny { + panic("lost Jenny's number") + } + + if !wait(done) { + panic("never GC'd") + } +} + +func wait(done <-chan struct{}) bool { + for i := 0; i < 10; i++ { + runtime.GC() + select { + case <-done: + return true + default: + } + } + return false +} From fdab5be159c508c4c1cf5be84119fd2b38403cdf Mon Sep 17 00:00:00 2001 From: Joel Sing Date: Mon, 14 Jun 2021 23:47:10 +1000 Subject: [PATCH 45/45] doc/go1.17: further revise OpenBSD release notes Simplify and remove forward-compatibility reference, as OpenBSD 6.9 has already been released (1st of May 2021). Updates #44513 Change-Id: I0a1abbb397f31d15c80a970edaa9723f894cafa9 Reviewed-on: https://go-review.googlesource.com/c/go/+/327652 Trust: Joel Sing Reviewed-by: Cherry Mui --- doc/go1.17.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 4fa38921f0..35d0f97450 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -106,8 +106,7 @@ Do not send CLs removing the interior tags from such phrases. of directly using machine instructions. In Go 1.17, this is also done on the 32-bit x86 and 32-bit ARM architectures on OpenBSD (the openbsd/386 and openbsd/arm ports). - This ensures forward-compatibility with future versions of - OpenBSD, in particular, with OpenBSD 6.9 onwards, which requires + This ensures compatibility with OpenBSD 6.9 onwards, which require system calls to be made through libc for non-static Go binaries.