From 24e9707cbfa6b1ed6abdd4b11f9ddaf3aac5ad88 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 25 May 2021 16:31:41 -0700 Subject: [PATCH 01/30] cmd/link, cmd/cgo: support -flto in CFLAGS The linker now accepts unrecognized object files in external linking mode. These objects will simply be passed to the external linker. This permits using -flto which can generate pure byte code objects, whose symbol table the linker does not know how to read. The cgo tool now passes -fno-lto when generating objects whose symbols it needs to read. The cgo tool now emits matching types in different objects, so that the lto linker does not report a mismatch. This is based on https://golang.org/cl/293290 by Derek Parker. For #43505 Fixes #43830 Fixes #46295 Change-Id: I6787de213417466784ddef5af8899e453b4ae1ad Reviewed-on: https://go-review.googlesource.com/c/go/+/322614 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Michael Hudson-Doyle --- src/cmd/cgo/gcc.go | 2 + src/cmd/cgo/out.go | 16 ++++++-- src/cmd/dist/test.go | 29 ++++++++++---- .../testdata/script/cgo_lto2_issue43830.txt | 33 ++++++++++++++++ .../go/testdata/script/cgo_lto_issue43830.txt | 39 +++++++++++++++++++ src/cmd/link/internal/ld/ar.go | 4 ++ src/cmd/link/internal/ld/config.go | 6 ++- src/cmd/link/internal/ld/lib.go | 26 +++++++++++-- 8 files changed, 140 insertions(+), 15 deletions(-) create mode 100644 src/cmd/go/testdata/script/cgo_lto2_issue43830.txt create mode 100644 src/cmd/go/testdata/script/cgo_lto_issue43830.txt diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index ae61725bc7..a73e998877 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -1638,6 +1638,8 @@ func (p *Package) gccCmd() []string { c = append(c, "-maix64") c = append(c, "-mcmodel=large") } + // disable LTO so we get an object whose symbols we can read + c = append(c, "-fno-lto") c = append(c, "-") //read input from standard input return c } diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index 8c31d5b794..94152f4278 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -168,8 +168,18 @@ func (p *Package) writeDefs() { if *gccgo { fmt.Fprintf(fc, "extern byte *%s;\n", n.C) } else { - fmt.Fprintf(fm, "extern char %s[];\n", n.C) - fmt.Fprintf(fm, "void *_cgohack_%s = %s;\n\n", n.C, n.C) + // Force a reference to all symbols so that + // the external linker will add DT_NEEDED + // entries as needed on ELF systems. + // Treat function variables differently + // to avoid type confict errors from LTO + // (Link Time Optimization). + if n.Kind == "fpvar" { + fmt.Fprintf(fm, "extern void %s();\n", n.C) + } else { + fmt.Fprintf(fm, "extern char %s[];\n", n.C) + fmt.Fprintf(fm, "void *_cgohack_%s = %s;\n\n", n.C, n.C) + } fmt.Fprintf(fgo2, "//go:linkname __cgo_%s %s\n", n.C, n.C) fmt.Fprintf(fgo2, "//go:cgo_import_static %s\n", n.C) fmt.Fprintf(fgo2, "var __cgo_%s byte\n", n.C) @@ -1042,7 +1052,7 @@ func (p *Package) writeExports(fgo2, fm, fgcc, fgcch io.Writer) { // This unpacks the argument struct above and calls the Go function. fmt.Fprintf(fgo2, "func _cgoexp%s_%s(a *%s) {\n", cPrefix, exp.ExpName, gotype) - fmt.Fprintf(fm, "int _cgoexp%s_%s;\n", cPrefix, exp.ExpName) + fmt.Fprintf(fm, "void _cgoexp%s_%s(void* p){}\n", cPrefix, exp.ExpName) if gccResult != "void" { // Write results back to frame. diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index 50bf80ba59..bc49c6d804 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -722,14 +722,29 @@ func (t *tester) registerTests() { }, }) if t.hasCxx() { - t.tests = append(t.tests, distTest{ - name: "swig_callback", - heading: "../misc/swig/callback", - fn: func(dt *distTest) error { - t.addCmd(dt, "misc/swig/callback", t.goTest()) - return nil + t.tests = append(t.tests, + distTest{ + name: "swig_callback", + heading: "../misc/swig/callback", + fn: func(dt *distTest) error { + t.addCmd(dt, "misc/swig/callback", t.goTest()) + return nil + }, }, - }) + distTest{ + name: "swig_callback_lto", + heading: "../misc/swig/callback", + 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", + ) + return nil + }, + }, + ) } } } diff --git a/src/cmd/go/testdata/script/cgo_lto2_issue43830.txt b/src/cmd/go/testdata/script/cgo_lto2_issue43830.txt new file mode 100644 index 0000000000..e2483ba784 --- /dev/null +++ b/src/cmd/go/testdata/script/cgo_lto2_issue43830.txt @@ -0,0 +1,33 @@ +# tests golang.org/issue/43830 + +[!cgo] skip 'skipping test without cgo' +[openbsd] env CC='clang' +[openbsd] [!exec:clang] skip 'skipping test without clang present' +[!openbsd] env CC='gcc' +[!openbsd] [!exec:gcc] skip 'skipping test without gcc present' + +env CGO_CFLAGS='-Wno-ignored-optimization-argument -flto -ffat-lto-objects' + +go build main.go + +-- main.go -- + +package main + +import "fmt" + +// #include "hello.h" +import "C" + +func main() { + hello := C.hello + fmt.Printf("%v\n", hello) +} + +-- hello.h -- + +#include + +void hello(void) { + printf("hello\n"); +} diff --git a/src/cmd/go/testdata/script/cgo_lto_issue43830.txt b/src/cmd/go/testdata/script/cgo_lto_issue43830.txt new file mode 100644 index 0000000000..06ab2f34c9 --- /dev/null +++ b/src/cmd/go/testdata/script/cgo_lto_issue43830.txt @@ -0,0 +1,39 @@ +# tests golang.org/issue/43830 + +[!cgo] skip 'skipping test without cgo' +[openbsd] env CC='clang' +[openbsd] [!exec:clang] skip 'skipping test without clang present' +[!openbsd] env CC='gcc' +[!openbsd] [!exec:gcc] skip 'skipping test without gcc present' + +env CGO_CFLAGS='-Wno-ignored-optimization-argument -flto -ffat-lto-objects' + +go build main.go add.go + +-- main.go -- + +package main + +/* +int c_add(int a, int b) { + return myadd(a, b); +} +*/ +import "C" + +func main() { + println(C.c_add(1, 2)) +} + +-- add.go -- + +package main + +import "C" + +/* test */ + +//export myadd +func myadd(a C.int, b C.int) C.int { + return a + b +} diff --git a/src/cmd/link/internal/ld/ar.go b/src/cmd/link/internal/ld/ar.go index 22f53a4df2..23915f9032 100644 --- a/src/cmd/link/internal/ld/ar.go +++ b/src/cmd/link/internal/ld/ar.go @@ -124,6 +124,10 @@ func hostArchive(ctxt *Link, name string) { libgcc := sym.Library{Pkg: "libgcc"} h := ldobj(ctxt, f, &libgcc, l, pname, name) + if h.ld == nil { + Errorf(nil, "%s unrecognized object file at offset %d", name, off) + continue + } f.MustSeek(h.off, 0) h.ld(ctxt, f, h.pkg, h.length, h.pn) } diff --git a/src/cmd/link/internal/ld/config.go b/src/cmd/link/internal/ld/config.go index ae0d7520eb..20f1d0b8c1 100644 --- a/src/cmd/link/internal/ld/config.go +++ b/src/cmd/link/internal/ld/config.go @@ -241,6 +241,10 @@ func mustLinkExternal(ctxt *Link) (res bool, reason string) { return true, "dynamically linking with a shared library" } + if unknownObjFormat { + return true, "some input objects have an unrecognized file format" + } + return false, "" } @@ -248,7 +252,7 @@ func mustLinkExternal(ctxt *Link) (res bool, reason string) { // // It is called after flags are processed and inputs are processed, // so the ctxt.LinkMode variable has an initial value from the -linkmode -// flag and the iscgo externalobj variables are set. +// flag and the iscgo, externalobj, and unknownObjFormat variables are set. func determineLinkMode(ctxt *Link) { extNeeded, extReason := mustLinkExternal(ctxt) via := "" diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go index e8f001ba8e..644faeb2fb 100644 --- a/src/cmd/link/internal/ld/lib.go +++ b/src/cmd/link/internal/ld/lib.go @@ -343,10 +343,16 @@ var ( const pkgdef = "__.PKGDEF" var ( - // Set if we see an object compiled by the host compiler that is not - // from a package that is known to support internal linking mode. + // externalobj is set to true if we see an object compiled by + // the host compiler that is not from a package that is known + // to support internal linking mode. externalobj = false - theline string + + // unknownObjFormat is set to true if we see an object whose + // format we don't recognize. + unknownObjFormat = false + + theline string ) func Lflag(ctxt *Link, arg string) { @@ -1065,6 +1071,10 @@ func hostobjs(ctxt *Link) { } f.MustSeek(h.off, 0) + if h.ld == nil { + Errorf(nil, "%s: unrecognized object file format", h.pn) + continue + } h.ld(ctxt, f, h.pkg, h.length, h.pn) f.Close() } @@ -1855,6 +1865,14 @@ func ldobj(ctxt *Link, f *bio.Reader, lib *sym.Library, length int64, pn string, return ldhostobj(ldxcoff, ctxt.HeadType, f, pkg, length, pn, file) } + if c1 != 'g' || c2 != 'o' || c3 != ' ' || c4 != 'o' { + // An unrecognized object is just passed to the external linker. + // If we try to read symbols from this object, we will + // report an error at that time. + unknownObjFormat = true + return ldhostobj(nil, ctxt.HeadType, f, pkg, length, pn, file) + } + /* check the header */ line, err := f.ReadString('\n') if err != nil { @@ -1874,7 +1892,7 @@ func ldobj(ctxt *Link, f *bio.Reader, lib *sym.Library, length int64, pn string, return nil } - Errorf(nil, "%s: not an object file: @%d %02x%02x%02x%02x", pn, start, c1, c2, c3, c4) + Errorf(nil, "%s: not an object file: @%d %q", pn, start, line) return nil } From 567ee865f690cde59d5aeadc04bcc926d2316db8 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 1 Jun 2021 13:47:42 -0700 Subject: [PATCH 02/30] cmd/go: add declaration to cgo_lto_issue43830 test This permits the test to work in C99 mode. For #43830 Change-Id: Ide54bd62239cfe602e2664300f04e472df5daf43 Reviewed-on: https://go-review.googlesource.com/c/go/+/324009 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor Reviewed-by: Cherry Mui TryBot-Result: Go Bot --- src/cmd/go/testdata/script/cgo_lto_issue43830.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cmd/go/testdata/script/cgo_lto_issue43830.txt b/src/cmd/go/testdata/script/cgo_lto_issue43830.txt index 06ab2f34c9..8bc7d8a540 100644 --- a/src/cmd/go/testdata/script/cgo_lto_issue43830.txt +++ b/src/cmd/go/testdata/script/cgo_lto_issue43830.txt @@ -15,6 +15,7 @@ go build main.go add.go package main /* +extern int myadd(int, int); int c_add(int a, int b) { return myadd(a, b); } From cae68700cc76d3118e470180a1cbeac616f3dfad Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 1 Jun 2021 17:00:22 -0700 Subject: [PATCH 03/30] runtime: fix formatting Fix up a gofmt complaint from CL 310591. Change-Id: I73534ef064a4cfc53539e5e65a8653e2cd684c64 Reviewed-on: https://go-review.googlesource.com/c/go/+/324090 Trust: Keith Randall Run-TryBot: Keith Randall Reviewed-by: Cherry Mui TryBot-Result: Go Bot --- src/runtime/internal/atomic/atomic_arm64.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/internal/atomic/atomic_arm64.go b/src/runtime/internal/atomic/atomic_arm64.go index 3c8736997f..dbb1796ec0 100644 --- a/src/runtime/internal/atomic/atomic_arm64.go +++ b/src/runtime/internal/atomic/atomic_arm64.go @@ -8,8 +8,8 @@ package atomic import ( - "unsafe" "internal/cpu" + "unsafe" ) const ( From 84c0e5d47f46f2e1a7ce92341477d9801f0ef777 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 1 Jun 2021 14:58:36 -0700 Subject: [PATCH 04/30] cmd/link: move issue 43830 tests out of TestScript These tests pass or fail depending on the exact compiler version, which the TestScript tests don't support. Rewrite into Go. For #43830 For #46295 Change-Id: I91b61dfe329d518e461ee56f186f0e9b42858e77 Reviewed-on: https://go-review.googlesource.com/c/go/+/324049 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Cherry Mui --- .../testdata/script/cgo_lto2_issue43830.txt | 33 ----- .../go/testdata/script/cgo_lto_issue43830.txt | 40 ----- src/cmd/link/cgo_test.go | 138 ++++++++++++++++++ 3 files changed, 138 insertions(+), 73 deletions(-) delete mode 100644 src/cmd/go/testdata/script/cgo_lto2_issue43830.txt delete mode 100644 src/cmd/go/testdata/script/cgo_lto_issue43830.txt create mode 100644 src/cmd/link/cgo_test.go diff --git a/src/cmd/go/testdata/script/cgo_lto2_issue43830.txt b/src/cmd/go/testdata/script/cgo_lto2_issue43830.txt deleted file mode 100644 index e2483ba784..0000000000 --- a/src/cmd/go/testdata/script/cgo_lto2_issue43830.txt +++ /dev/null @@ -1,33 +0,0 @@ -# tests golang.org/issue/43830 - -[!cgo] skip 'skipping test without cgo' -[openbsd] env CC='clang' -[openbsd] [!exec:clang] skip 'skipping test without clang present' -[!openbsd] env CC='gcc' -[!openbsd] [!exec:gcc] skip 'skipping test without gcc present' - -env CGO_CFLAGS='-Wno-ignored-optimization-argument -flto -ffat-lto-objects' - -go build main.go - --- main.go -- - -package main - -import "fmt" - -// #include "hello.h" -import "C" - -func main() { - hello := C.hello - fmt.Printf("%v\n", hello) -} - --- hello.h -- - -#include - -void hello(void) { - printf("hello\n"); -} diff --git a/src/cmd/go/testdata/script/cgo_lto_issue43830.txt b/src/cmd/go/testdata/script/cgo_lto_issue43830.txt deleted file mode 100644 index 8bc7d8a540..0000000000 --- a/src/cmd/go/testdata/script/cgo_lto_issue43830.txt +++ /dev/null @@ -1,40 +0,0 @@ -# tests golang.org/issue/43830 - -[!cgo] skip 'skipping test without cgo' -[openbsd] env CC='clang' -[openbsd] [!exec:clang] skip 'skipping test without clang present' -[!openbsd] env CC='gcc' -[!openbsd] [!exec:gcc] skip 'skipping test without gcc present' - -env CGO_CFLAGS='-Wno-ignored-optimization-argument -flto -ffat-lto-objects' - -go build main.go add.go - --- main.go -- - -package main - -/* -extern int myadd(int, int); -int c_add(int a, int b) { - return myadd(a, b); -} -*/ -import "C" - -func main() { - println(C.c_add(1, 2)) -} - --- add.go -- - -package main - -import "C" - -/* test */ - -//export myadd -func myadd(a C.int, b C.int) C.int { - return a + b -} diff --git a/src/cmd/link/cgo_test.go b/src/cmd/link/cgo_test.go new file mode 100644 index 0000000000..09390daeb7 --- /dev/null +++ b/src/cmd/link/cgo_test.go @@ -0,0 +1,138 @@ +// 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 ( + "bytes" + "fmt" + "internal/testenv" + "os" + "os/exec" + "path/filepath" + "testing" +) + +// Issues 43830, 46295 +func TestCGOLTO(t *testing.T) { + testenv.MustHaveCGO(t) + testenv.MustHaveGoBuild(t) + + t.Parallel() + + for _, cc := range []string{"gcc", "clang"} { + for test := 0; test < 2; test++ { + t.Run(fmt.Sprintf("%s-%d", cc, test), func(t *testing.T) { + testCGOLTO(t, cc, test) + }) + } + } +} + +const test1_main = ` +package main + +/* +extern int myadd(int, int); +int c_add(int a, int b) { + return myadd(a, b); +} +*/ +import "C" + +func main() { + println(C.c_add(1, 2)) +} +` + +const test1_add = ` +package main + +import "C" + +/* test */ + +//export myadd +func myadd(a C.int, b C.int) C.int { + return a + b +} +` + +const test2_main = ` +package main + +import "fmt" + +/* +#include + +void hello(void) { + printf("hello\n"); +} +*/ +import "C" + +func main() { + hello := C.hello + fmt.Printf("%v\n", hello) +} +` + +func testCGOLTO(t *testing.T, cc string, test int) { + t.Parallel() + + if _, err := exec.LookPath(cc); err != nil { + t.Skipf("no %s compiler", cc) + } + + dir := t.TempDir() + + writeTempFile := func(name, contents string) { + if err := os.WriteFile(filepath.Join(dir, name), []byte(contents), 0644); err != nil { + t.Fatal(err) + } + } + + writeTempFile("go.mod", "module cgolto\n") + + switch test { + case 0: + writeTempFile("main.go", test1_main) + writeTempFile("add.go", test1_add) + case 1: + writeTempFile("main.go", test2_main) + default: + t.Fatalf("bad case %d", test) + } + + cmd := exec.Command(testenv.GoToolPath(t), "build") + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "CC="+cc, + "CGO_CFLAGS=-flto", + ) + + t.Log("go build") + out, err := cmd.CombinedOutput() + t.Logf("%s", out) + + if err != nil { + t.Logf("go build failed: %v", err) + + // Error messages we've seen indicating that LTO is not supported. + var noLTO = []string{ + `unrecognized command line option "-flto"`, + "unable to pass LLVM bit-code files to linker", + "file not recognized: File format not recognized", + "LTO support has not been enabled", + } + for _, msg := range noLTO { + if bytes.Contains(out, []byte(msg)) { + t.Skipf("C compiler %v does not support LTO", cc) + } + } + + t.Error("failed") + } +} From dc8f87b7493e173d65d3587389cc41468ba16dc0 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Tue, 1 Jun 2021 12:22:12 +0200 Subject: [PATCH 05/30] runtime/internal/sys: generate //go:build lines in gengoos.go For #41184 Change-Id: If7a1c3980f47bc28d0a13fe497eaba6178c65c91 Reviewed-on: https://go-review.googlesource.com/c/go/+/323750 Trust: Tobias Klauser Run-TryBot: Tobias Klauser TryBot-Result: Go Bot Reviewed-by: Russ Cox --- src/runtime/internal/sys/gengoos.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/runtime/internal/sys/gengoos.go b/src/runtime/internal/sys/gengoos.go index 51f64a6e5c..ffe962f71d 100644 --- a/src/runtime/internal/sys/gengoos.go +++ b/src/runtime/internal/sys/gengoos.go @@ -48,18 +48,21 @@ func main() { if target == "nacl" { continue } - var buf bytes.Buffer - fmt.Fprintf(&buf, "// Code generated by gengoos.go using 'go generate'. DO NOT EDIT.\n\n") + var tags []string if target == "linux" { - fmt.Fprintf(&buf, "// +build !android\n") // must explicitly exclude android for linux + tags = append(tags, "!android") // must explicitly exclude android for linux } if target == "solaris" { - fmt.Fprintf(&buf, "// +build !illumos\n") // must explicitly exclude illumos for solaris + tags = append(tags, "!illumos") // must explicitly exclude illumos for solaris } if target == "darwin" { - fmt.Fprintf(&buf, "// +build !ios\n") // must explicitly exclude ios for darwin + tags = append(tags, "!ios") // must explicitly exclude ios for darwin } - fmt.Fprintf(&buf, "// +build %s\n\n", target) // must explicitly include target for bootstrapping purposes + tags = append(tags, target) // must explicitly include target for bootstrapping purposes + var buf bytes.Buffer + fmt.Fprintf(&buf, "// Code generated by gengoos.go using 'go generate'. DO NOT EDIT.\n\n") + fmt.Fprintf(&buf, "//go:build %s\n", strings.Join(tags, " && ")) + fmt.Fprintf(&buf, "// +build %s\n\n", strings.Join(tags, ",")) fmt.Fprintf(&buf, "package sys\n\n") fmt.Fprintf(&buf, "const GOOS = `%s`\n\n", target) for _, goos := range gooses { @@ -81,6 +84,7 @@ func main() { } var buf bytes.Buffer fmt.Fprintf(&buf, "// Code generated by gengoos.go using 'go generate'. DO NOT EDIT.\n\n") + fmt.Fprintf(&buf, "//go:build %s\n", target) fmt.Fprintf(&buf, "// +build %s\n\n", target) // must explicitly include target for bootstrapping purposes fmt.Fprintf(&buf, "package sys\n\n") fmt.Fprintf(&buf, "const GOARCH = `%s`\n\n", target) From d743e67e0695a8082f03fd90bb07e71cf9f34cf1 Mon Sep 17 00:00:00 2001 From: KimMachineGun Date: Sun, 23 May 2021 14:05:15 +0000 Subject: [PATCH 06/30] doc/go1.17: document flag changes for Go 1.17 For #44513 Fixes #46010 Change-Id: I1fe638e5db0b4f3b64dbfbd948154a7c7a80afc9 GitHub-Last-Rev: d5bd53b1df202329661ffb1818803f2ec1d3f57a GitHub-Pull-Request: golang/go#46150 Reviewed-on: https://go-review.googlesource.com/c/go/+/319273 Reviewed-by: Heschi Kreinick Trust: Robert Findley --- doc/go1.17.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index ee498f7603..6ddef3d47e 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -434,7 +434,7 @@ Do not send CLs removing the interior tags from such phrases.
flag

- TODO: https://golang.org/cl/271788: panic if flag name begins with - or contains = + Flag declarations now panic if an invalid name is specified.

From 1c6a2ea2ea4b04416f7344ee5effe81816c7200b Mon Sep 17 00:00:00 2001 From: Uddeshya Singh Date: Fri, 7 May 2021 13:18:05 +0530 Subject: [PATCH 07/30] doc/go1.17: document time changes for Go1.17 Documents the newly implemented changes of - Time.IsDST() method - Addition of Time.UnixMilli, Time.UnixMicro and to-Time helpers UnixMicro, UnixMilli methods - Addition of comma "," support as separator for fraction seconds For #44513 Fixes #46026 Change-Id: Ib8d3449d3b061f013112d33362b50e68ad6ddffa Reviewed-on: https://go-review.googlesource.com/c/go/+/317913 Reviewed-by: Dmitri Shuralyov Reviewed-by: Ian Lance Taylor --- doc/go1.17.html | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index 6ddef3d47e..b287d41309 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -689,15 +689,26 @@ Do not send CLs removing the interior tags from such phrases.

- TODO: https://golang.org/cl/264077: add Time.IsDST() to check if its Location is in Daylight Savings Time + The new Time.IsDST method can be used to check whether the time + is in Daylight Savings Time in its configured location.

- TODO: https://golang.org/cl/293349: add Time.Unix{Milli,Micro} and to-Time helpers UnixMicro, UnixMilli + The new Time.UnixMilli and + Time.UnixMicro methods return the number of milliseconds and + microseconds elapsed since January 1, 1970 UTC respectively.
+ The new UnixMilli and UnixMicro functions return local Time corresponding to given + Unix time.

- TODO: https://golang.org/cl/300996: support "," as separator for fractional seconds + The package now accepts comma "," as a separator for fractional seconds when parsing and formatting time. + The following time formats are now accepted: +

    +
  • 2006-01-02 14:06:03,999999999 -0700 MST
    • +
  • Mon Jan _2 14:06:03,120007 2006
    • +
  • Mon Jan 2 14:06:03,120007 2006
    • +

From ff9f5fb8591c6d3e4cd4881e75f49440a3a875c2 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 2 Jun 2021 07:43:57 -0700 Subject: [PATCH 08/30] cmd/link: recognize clang linker error in testCGOLTO Also recognize a case in which GCC does not run (from https://build.golang.org/log/7f6d8b35c905b9829f05906beccca44f208aa569). Fixes #46517 Change-Id: I4fe4164a5df92b2dec08fd767f65a4d5479f3f36 Reviewed-on: https://go-review.googlesource.com/c/go/+/324169 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills Reviewed-by: Tobias Klauser --- src/cmd/link/cgo_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cmd/link/cgo_test.go b/src/cmd/link/cgo_test.go index 09390daeb7..26ab802454 100644 --- a/src/cmd/link/cgo_test.go +++ b/src/cmd/link/cgo_test.go @@ -121,11 +121,14 @@ func testCGOLTO(t *testing.T, cc string, test int) { t.Logf("go build failed: %v", err) // Error messages we've seen indicating that LTO is not supported. + // These errors come from GCC or clang, not Go. var noLTO = []string{ `unrecognized command line option "-flto"`, "unable to pass LLVM bit-code files to linker", "file not recognized: File format not recognized", "LTO support has not been enabled", + "linker command failed with exit code", + "gcc: can't load library", } for _, msg := range noLTO { if bytes.Contains(out, []byte(msg)) { From 6e189afd3e7a3722c72b320ef604bf2910aee9e7 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 28 May 2021 17:28:12 -0700 Subject: [PATCH 09/30] doc/go1.17: mention SYS_WAIT6/WEXITED on NetBSD For #13987 For #16028 For #44513 Change-Id: I7a73446fcc80a01fa6de24eec1e5b993e543be37 Reviewed-on: https://go-review.googlesource.com/c/go/+/323489 Trust: Ian Lance Taylor Reviewed-by: Emmanuel Odeke --- doc/go1.17.html | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/go1.17.html b/doc/go1.17.html index b287d41309..d80e68d434 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -651,6 +651,14 @@ Do not send CLs removing the interior tags from such phrases. DragonFly and all OpenBSD systems (it was already defined on some OpenBSD systems and all FreeBSD, NetBSD, and Linux systems).

+ +

+ The constants SYS_WAIT6 and WEXITED + are now defined on NetBSD systems (SYS_WAIT6 was + already defined on DragonFly and FreeBSD systems; + WEXITED was already defined on Darwin, DragonFly, + FreeBSD, Linux, and Solaris systems). +

From e11d14225c032a7c2722798db3c309762fa99757 Mon Sep 17 00:00:00 2001 From: Jeremy Faller Date: Wed, 2 Jun 2021 11:20:40 -0400 Subject: [PATCH 10/30] doc/go1.17: remove runtime section Updates #44513 Change-Id: I359d56fc3eeece3005f092cca2cb485664affc23 Reviewed-on: https://go-review.googlesource.com/c/go/+/324209 Trust: Jeremy Faller Run-TryBot: Jeremy Faller TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov Reviewed-by: Michael Knyszek --- doc/go1.17.html | 6 ------ 1 file changed, 6 deletions(-) diff --git a/doc/go1.17.html b/doc/go1.17.html index d80e68d434..27ef524286 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -224,12 +224,6 @@ Do not send CLs removing the interior tags from such phrases. TODO: complete the Vet section

-

Runtime

- -

- TODO: complete the Runtime section -

-

Compiler

From 4f572d707661e3e84ff262d6c605eb6fa1f77abd Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 25 Feb 2021 08:21:52 -0800 Subject: [PATCH 11/30] io/fs: minor corrections to Sub docs Fixes #44376 Change-Id: I9cd21adb9d4d434c3d8b8eb8af3042b70c763ea1 Reviewed-on: https://go-review.googlesource.com/c/go/+/296389 Trust: Ian Lance Taylor Reviewed-by: Russ Cox --- src/io/fs/sub.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/io/fs/sub.go b/src/io/fs/sub.go index 7822e555ea..ae20e030a9 100644 --- a/src/io/fs/sub.go +++ b/src/io/fs/sub.go @@ -19,10 +19,10 @@ type SubFS interface { // Sub returns an FS corresponding to the subtree rooted at fsys's dir. // -// If fs implements SubFS, Sub calls returns fsys.Sub(dir). -// Otherwise, if dir is ".", Sub returns fsys unchanged. +// If dir is ".", Sub returns fsys unchanged. +// Otherwise, if fs implements SubFS, Sub returns fsys.Sub(dir). // Otherwise, Sub returns a new FS implementation sub that, -// in effect, implements sub.Open(dir) as fsys.Open(path.Join(dir, name)). +// in effect, implements sub.Open(name) as fsys.Open(path.Join(dir, name)). // The implementation also translates calls to ReadDir, ReadFile, and Glob appropriately. // // Note that Sub(os.DirFS("/"), "prefix") is equivalent to os.DirFS("/prefix") From dd7ba3ba2c860c40be6d70b63d4a678449cae80f Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 2 Jun 2021 09:20:22 -0700 Subject: [PATCH 12/30] net: don't rely on system hosts in TestCVE202133195 Also don't unnecessarily deref the error return. Fixes #46504 Change-Id: I22d14ac76776f8988fa0774bdcb5fcd801ce0185 Reviewed-on: https://go-review.googlesource.com/c/go/+/324190 Trust: David Chase Trust: Damien Neil Run-TryBot: David Chase TryBot-Result: Go Bot Reviewed-by: Damien Neil --- src/net/dnsclient_unix_test.go | 39 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go index a718e75a72..a59be7fea0 100644 --- a/src/net/dnsclient_unix_test.go +++ b/src/net/dnsclient_unix_test.go @@ -1898,61 +1898,62 @@ func TestCVE202133195(t *testing.T) { // Change the default resolver to match our manipulated resolver originalDefault := DefaultResolver DefaultResolver = &r - defer func() { - DefaultResolver = originalDefault - }() + defer func() { DefaultResolver = originalDefault }() + // Redirect host file lookups. + defer func(orig string) { testHookHostsPath = orig }(testHookHostsPath) + testHookHostsPath = "testdata/hosts" _, err := r.LookupCNAME(context.Background(), "golang.org") if expected := "lookup golang.org: CNAME target is invalid"; err == nil || err.Error() != expected { - t.Errorf("Resolver.LookupCNAME returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("Resolver.LookupCNAME returned unexpected error, got %q, want %q", err, expected) } _, err = LookupCNAME("golang.org") if expected := "lookup golang.org: CNAME target is invalid"; err == nil || err.Error() != expected { - t.Errorf("LookupCNAME returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("LookupCNAME returned unexpected error, got %q, want %q", err, expected) } _, _, err = r.LookupSRV(context.Background(), "target", "tcp", "golang.org") if expected := "lookup golang.org: SRV target is invalid"; err == nil || err.Error() != expected { - t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err, expected) } _, _, err = LookupSRV("target", "tcp", "golang.org") if expected := "lookup golang.org: SRV target is invalid"; err == nil || err.Error() != expected { - t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err, expected) } _, _, err = r.LookupSRV(context.Background(), "hdr", "tcp", "golang.org") if expected := "lookup golang.org: SRV header name is invalid"; err == nil || err.Error() != expected { - t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err, expected) } _, _, err = LookupSRV("hdr", "tcp", "golang.org") if expected := "lookup golang.org: SRV header name is invalid"; err == nil || err.Error() != expected { - t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err, expected) } _, err = r.LookupMX(context.Background(), "golang.org") if expected := "lookup golang.org: MX target is invalid"; err == nil || err.Error() != expected { - t.Errorf("Resolver.LookupMX returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("Resolver.LookupMX returned unexpected error, got %q, want %q", err, expected) } _, err = LookupMX("golang.org") if expected := "lookup golang.org: MX target is invalid"; err == nil || err.Error() != expected { - t.Errorf("LookupMX returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("LookupMX returned unexpected error, got %q, want %q", err, expected) } _, err = r.LookupNS(context.Background(), "golang.org") if expected := "lookup golang.org: NS target is invalid"; err == nil || err.Error() != expected { - t.Errorf("Resolver.LookupNS returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("Resolver.LookupNS returned unexpected error, got %q, want %q", err, expected) } _, err = LookupNS("golang.org") if expected := "lookup golang.org: NS target is invalid"; err == nil || err.Error() != expected { - t.Errorf("LookupNS returned unexpected error, got %q, want %q", err.Error(), expected) + t.Errorf("LookupNS returned unexpected error, got %q, want %q", err, expected) } - _, err = r.LookupAddr(context.Background(), "1.2.3.4") - if expected := "lookup 1.2.3.4: PTR target is invalid"; err == nil || err.Error() != expected { - t.Errorf("Resolver.LookupAddr returned unexpected error, got %q, want %q", err.Error(), expected) + _, err = r.LookupAddr(context.Background(), "192.0.2.42") + if expected := "lookup 192.0.2.42: PTR target is invalid"; err == nil || err.Error() != expected { + t.Errorf("Resolver.LookupAddr returned unexpected error, got %q, want %q", err, expected) } - _, err = LookupAddr("1.2.3.4") - if expected := "lookup 1.2.3.4: PTR target is invalid"; err == nil || err.Error() != expected { - t.Errorf("LookupAddr returned unexpected error, got %q, want %q", err.Error(), expected) + _, err = LookupAddr("192.0.2.42") + if expected := "lookup 192.0.2.42: PTR target is invalid"; err == nil || err.Error() != expected { + t.Errorf("LookupAddr returned unexpected error, got %q, want %q", err, expected) } } From e0d029f75846f84f79e63f6100c57047f4a3fa98 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Wed, 2 Jun 2021 17:44:43 -0400 Subject: [PATCH 13/30] runtime: avoid gp.lockedm race in exitsyscall0 Following https://golang.org/cl/291329, exitsyscall0 accesses gp.lockedm after releasing gp to the global runq. This creates a race window where another M may schedule the (unlocked) G, which subsequently calls LockOSThread, setting gp.lockedm and thus causing exitsyscall0 to think it should call stoplockedm. Avoid this race by checking if gp is locked before releasing it to the global runq. Fixes #46524 Change-Id: I3acdaf09e7a2178725adbe61e985130e9ebd0680 Reviewed-on: https://go-review.googlesource.com/c/go/+/324350 Trust: Michael Pratt Run-TryBot: Michael Pratt TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Reviewed-by: Michael Knyszek --- src/runtime/proc.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index ded406cc28..59160c6525 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -4083,8 +4083,16 @@ func exitsyscall0(gp *g) { if schedEnabled(gp) { _p_ = pidleget() } + var locked bool if _p_ == nil { globrunqput(gp) + + // Below, we stoplockedm if gp is locked. globrunqput releases + // ownership of gp, so we must check if gp is locked prior to + // committing the release by unlocking sched.lock, otherwise we + // could race with another M transitioning gp from unlocked to + // locked. + locked = gp.lockedm != 0 } else if atomic.Load(&sched.sysmonwait) != 0 { atomic.Store(&sched.sysmonwait, 0) notewakeup(&sched.sysmonnote) @@ -4094,7 +4102,7 @@ func exitsyscall0(gp *g) { acquirep(_p_) execute(gp, false) // Never returns. } - if gp.lockedm != 0 { + if locked { // Wait until another thread schedules gp and so m again. // // N.B. lockedm must be this M, as this g was running on this M From 6d9830111402d9bd69893a8ad6074ac92a5ddd0d Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 3 Jun 2021 14:50:10 -0400 Subject: [PATCH 14/30] cmd/link: use correct alignment in PE DWARF sections Set the correct section flags to insure that .debug_* sections are using 1-byte alignment instead of the default. This seems to be important for later versions of LLVM-mingw on windows (shows up on the windows/arm64 builder). Updates #46406. Change-Id: I023d5208374f867552ba68b45011f7990159868f Reviewed-on: https://go-review.googlesource.com/c/go/+/324763 Trust: Than McIntosh Reviewed-by: Cherry Mui Run-TryBot: Than McIntosh TryBot-Result: Go Bot --- src/cmd/link/internal/ld/pe.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go index 3540c07da1..8eb4231c3a 100644 --- a/src/cmd/link/internal/ld/pe.go +++ b/src/cmd/link/internal/ld/pe.go @@ -475,7 +475,7 @@ func (f *peFile) addDWARFSection(name string, size int) *peSection { off := f.stringTable.add(name) h := f.addSection(name, size, size) h.shortName = fmt.Sprintf("/%d", off) - h.characteristics = IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_DISCARDABLE | IMAGE_SCN_CNT_INITIALIZED_DATA + h.characteristics = IMAGE_SCN_ALIGN_1BYTES | IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_DISCARDABLE | IMAGE_SCN_CNT_INITIALIZED_DATA return h } From b29b123e079183a05abc1066007a51d4f565cd88 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Thu, 3 Jun 2021 18:00:53 -0700 Subject: [PATCH 15/30] cmd/compile: remove spurious ir.Dump This ir.Dump call is a debugging artifact introduced in golang.org/cl/274103, which should never be printed for valid, non-generic code, but evidently can now sometimes appear due to how the parser handles invalid syntax. The parser should probably not recognize "x[2]" as a type expression in non-generics mode, but also probably we shouldn't try noding after reporting syntax errors. Either way, this diagnostic has outlived its usefulness, and noder's days are numbered anyway, so we might as well just remove it to save end users any confusion. Updates #46558. Change-Id: Ib68502ef834d610b883c2f2bb11d9b385bc66e37 Reviewed-on: https://go-review.googlesource.com/c/go/+/324991 Trust: Matthew Dempsky Run-TryBot: Matthew Dempsky Reviewed-by: Robert Griesemer TryBot-Result: Go Bot --- src/cmd/compile/internal/noder/noder.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go index 4c7c9fc322..5fcad096c2 100644 --- a/src/cmd/compile/internal/noder/noder.go +++ b/src/cmd/compile/internal/noder/noder.go @@ -882,9 +882,6 @@ func (p *noder) typeExpr(typ syntax.Expr) ir.Ntype { if n == nil { return nil } - if _, ok := n.(ir.Ntype); !ok { - ir.Dump("NOT NTYPE", n) - } return n.(ir.Ntype) } From 962d5c997af450af1de9a38eb6510cdfc86ea689 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 2 Jun 2021 12:22:50 -0700 Subject: [PATCH 16/30] cmd/compile,go/types: restrict use of unsafe.{Add,Slice} to go1.17 or newer This CL updates cmd/compile (including types2) and go/types to report errors about using unsafe.Add and unsafe.Slice when language compatibility is set to Go 1.16 or older. Fixes #46525. Change-Id: I1bfe025a672d9f4b929f443064ad1effd38d0363 Reviewed-on: https://go-review.googlesource.com/c/go/+/324369 Run-TryBot: Matthew Dempsky Reviewed-by: Robert Findley Reviewed-by: Robert Griesemer TryBot-Result: Go Bot Trust: Matthew Dempsky --- src/cmd/compile/internal/typecheck/func.go | 12 ++++++++++++ src/cmd/compile/internal/types2/builtins.go | 10 ++++++++++ src/go/types/builtins.go | 10 ++++++++++ test/fixedbugs/issue46525.go | 14 ++++++++++++++ 4 files changed, 46 insertions(+) create mode 100644 test/fixedbugs/issue46525.go diff --git a/src/cmd/compile/internal/typecheck/func.go b/src/cmd/compile/internal/typecheck/func.go index f381e1dbdc..a6dfbbf569 100644 --- a/src/cmd/compile/internal/typecheck/func.go +++ b/src/cmd/compile/internal/typecheck/func.go @@ -981,6 +981,12 @@ func tcRecover(n *ir.CallExpr) ir.Node { // tcUnsafeAdd typechecks an OUNSAFEADD node. func tcUnsafeAdd(n *ir.BinaryExpr) *ir.BinaryExpr { + if !types.AllowsGoVersion(curpkg(), 1, 17) { + base.ErrorfVers("go1.17", "unsafe.Add") + n.SetType(nil) + return n + } + n.X = AssignConv(Expr(n.X), types.Types[types.TUNSAFEPTR], "argument to unsafe.Add") n.Y = DefaultLit(Expr(n.Y), types.Types[types.TINT]) if n.X.Type() == nil || n.Y.Type() == nil { @@ -997,6 +1003,12 @@ func tcUnsafeAdd(n *ir.BinaryExpr) *ir.BinaryExpr { // tcUnsafeSlice typechecks an OUNSAFESLICE node. func tcUnsafeSlice(n *ir.BinaryExpr) *ir.BinaryExpr { + if !types.AllowsGoVersion(curpkg(), 1, 17) { + base.ErrorfVers("go1.17", "unsafe.Slice") + n.SetType(nil) + return n + } + n.X = Expr(n.X) n.Y = Expr(n.Y) if n.X.Type() == nil || n.Y.Type() == nil { diff --git a/src/cmd/compile/internal/types2/builtins.go b/src/cmd/compile/internal/types2/builtins.go index b9e178dd57..f90e06f226 100644 --- a/src/cmd/compile/internal/types2/builtins.go +++ b/src/cmd/compile/internal/types2/builtins.go @@ -579,6 +579,11 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) ( case _Add: // unsafe.Add(ptr unsafe.Pointer, len IntegerType) unsafe.Pointer + if !check.allowVersion(check.pkg, 1, 17) { + check.error(call.Fun, "unsafe.Add requires go1.17 or later") + return + } + check.assignment(x, Typ[UnsafePointer], "argument to unsafe.Add") if x.mode == invalid { return @@ -675,6 +680,11 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) ( case _Slice: // unsafe.Slice(ptr *T, len IntegerType) []T + if !check.allowVersion(check.pkg, 1, 17) { + check.error(call.Fun, "unsafe.Slice requires go1.17 or later") + return + } + typ := asPointer(x.typ) if typ == nil { check.errorf(x, invalidArg+"%s is not a pointer", x) diff --git a/src/go/types/builtins.go b/src/go/types/builtins.go index 739051cc61..2a2d54da88 100644 --- a/src/go/types/builtins.go +++ b/src/go/types/builtins.go @@ -588,6 +588,11 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b case _Add: // unsafe.Add(ptr unsafe.Pointer, len IntegerType) unsafe.Pointer + if !check.allowVersion(check.pkg, 1, 17) { + check.errorf(call.Fun, _InvalidUnsafeAdd, "unsafe.Add requires go1.17 or later") + return + } + check.assignment(x, Typ[UnsafePointer], "argument to unsafe.Add") if x.mode == invalid { return @@ -684,6 +689,11 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b case _Slice: // unsafe.Slice(ptr *T, len IntegerType) []T + if !check.allowVersion(check.pkg, 1, 17) { + check.errorf(call.Fun, _InvalidUnsafeSlice, "unsafe.Slice requires go1.17 or later") + return + } + typ := asPointer(x.typ) if typ == nil { check.invalidArg(x, _InvalidUnsafeSlice, "%s is not a pointer", x) diff --git a/test/fixedbugs/issue46525.go b/test/fixedbugs/issue46525.go new file mode 100644 index 0000000000..164e1473ce --- /dev/null +++ b/test/fixedbugs/issue46525.go @@ -0,0 +1,14 @@ +// errorcheck -lang=go1.16 + +// 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 + +import "unsafe" + +func main() { + _ = unsafe.Add(unsafe.Pointer(nil), 0) // ERROR "unsafe.Add requires go1.17 or later" + _ = unsafe.Slice(new(byte), 1) // ERROR "unsafe.Slice requires go1.17 or later" +} From 021444007590da4c1f6e504904e2871a1012c0bf Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Thu, 13 May 2021 13:19:14 +0200 Subject: [PATCH 17/30] syscall: do not pass console handles to PROC_THREAD_ATTRIBUTE_HANDLE_LIST on Windows 7 On Windows 7 (and below), console handles are not real kernel handles but are rather userspace objects, with information passed via special bits in the handle itself. That means they can't be passed in PROC_THREAD_ATTRIBUTE_HANDLE_LIST, even though they can be inherited. So, we filter the list passed to PROC_THREAD_ATTRIBUTE_HANDLE_LIST to not have any console handles on Windows 7. At the same time, it turns out that the presence of a NULL handle in the list is enough to render PROC_THREAD_ATTRIBUTE_HANDLE_LIST completely useless, so filter these out too. Console handles also can't be duplicated into parent processes, as inhertance always happens from the present process, so duplicate always into the present process even when a parent process is specified. Fixes #45914. Change-Id: I70b4ff4874dbf0507d9ec9278f63b9b4dd4f1999 Reviewed-on: https://go-review.googlesource.com/c/go/+/319310 Trust: Jason A. Donenfeld Trust: Alex Brainman Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Alex Brainman --- src/syscall/exec_windows.go | 54 ++++++++++++++++++++++++++++----- src/syscall/syscall_windows.go | 1 + src/syscall/zsyscall_windows.go | 7 +++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/syscall/exec_windows.go b/src/syscall/exec_windows.go index 253e9e8c1f..18d15028c3 100644 --- a/src/syscall/exec_windows.go +++ b/src/syscall/exec_windows.go @@ -313,6 +313,17 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle } } + var maj, min, build uint32 + rtlGetNtVersionNumbers(&maj, &min, &build) + isWin7 := maj < 6 || (maj == 6 && min <= 1) + // NT kernel handles are divisible by 4, with the bottom 3 bits left as + // a tag. The fully set tag correlates with the types of handles we're + // concerned about here. Except, the kernel will interpret some + // special handle values, like -1, -2, and so forth, so kernelbase.dll + // checks to see that those bottom three bits are checked, but that top + // bit is not checked. + isLegacyWin7ConsoleHandle := func(handle Handle) bool { return isWin7 && handle&0x10000003 == 3 } + p, _ := GetCurrentProcess() parentProcess := p if sys.ParentProcess != 0 { @@ -321,7 +332,15 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle fd := make([]Handle, len(attr.Files)) for i := range attr.Files { if attr.Files[i] > 0 { - err := DuplicateHandle(p, Handle(attr.Files[i]), parentProcess, &fd[i], 0, true, DUPLICATE_SAME_ACCESS) + destinationProcessHandle := parentProcess + + // On Windows 7, console handles aren't real handles, and can only be duplicated + // into the current process, not a parent one, which amounts to the same thing. + if parentProcess != p && isLegacyWin7ConsoleHandle(Handle(attr.Files[i])) { + destinationProcessHandle = p + } + + err := DuplicateHandle(p, Handle(attr.Files[i]), destinationProcessHandle, &fd[i], 0, true, DUPLICATE_SAME_ACCESS) if err != nil { return 0, 0, err } @@ -351,19 +370,40 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle si.StdErr = fd[2] fd = append(fd, sys.AdditionalInheritedHandles...) + + // On Windows 7, console handles aren't real handles, so don't pass them + // through to PROC_THREAD_ATTRIBUTE_HANDLE_LIST. + for i := range fd { + if isLegacyWin7ConsoleHandle(fd[i]) { + fd[i] = 0 + } + } + + // The presence of a NULL handle in the list is enough to cause PROC_THREAD_ATTRIBUTE_HANDLE_LIST + // to treat the entire list as empty, so remove NULL handles. + j := 0 + for i := range fd { + if fd[i] != 0 { + fd[j] = fd[i] + j++ + } + } + fd = fd[:j] + // Do not accidentally inherit more than these handles. - err = updateProcThreadAttribute(si.ProcThreadAttributeList, 0, _PROC_THREAD_ATTRIBUTE_HANDLE_LIST, unsafe.Pointer(&fd[0]), uintptr(len(fd))*unsafe.Sizeof(fd[0]), nil, nil) - if err != nil { - return 0, 0, err + if len(fd) > 0 { + err = updateProcThreadAttribute(si.ProcThreadAttributeList, 0, _PROC_THREAD_ATTRIBUTE_HANDLE_LIST, unsafe.Pointer(&fd[0]), uintptr(len(fd))*unsafe.Sizeof(fd[0]), nil, nil) + if err != nil { + return 0, 0, err + } } pi := new(ProcessInformation) - flags := sys.CreationFlags | CREATE_UNICODE_ENVIRONMENT | _EXTENDED_STARTUPINFO_PRESENT if sys.Token != 0 { - err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) + err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, len(fd) > 0 && !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) } else { - err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) + err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, len(fd) > 0 && !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) } if err != nil { return 0, 0, err diff --git a/src/syscall/syscall_windows.go b/src/syscall/syscall_windows.go index fc734effbb..660179ae9e 100644 --- a/src/syscall/syscall_windows.go +++ b/src/syscall/syscall_windows.go @@ -198,6 +198,7 @@ func NewCallbackCDecl(fn interface{}) uintptr { //sys FreeLibrary(handle Handle) (err error) //sys GetProcAddress(module Handle, procname string) (proc uintptr, err error) //sys GetVersion() (ver uint32, err error) +//sys rtlGetNtVersionNumbers(majorVersion *uint32, minorVersion *uint32, buildNumber *uint32) = ntdll.RtlGetNtVersionNumbers //sys formatMessage(flags uint32, msgsrc uintptr, msgid uint32, langid uint32, buf []uint16, args *byte) (n uint32, err error) = FormatMessageW //sys ExitProcess(exitcode uint32) //sys CreateFile(name *uint16, access uint32, mode uint32, sa *SecurityAttributes, createmode uint32, attrs uint32, templatefile int32) (handle Handle, err error) [failretval==InvalidHandle] = CreateFileW diff --git a/src/syscall/zsyscall_windows.go b/src/syscall/zsyscall_windows.go index 10d0f54e8c..b9e429693d 100644 --- a/src/syscall/zsyscall_windows.go +++ b/src/syscall/zsyscall_windows.go @@ -41,6 +41,7 @@ var ( moddnsapi = NewLazyDLL(sysdll.Add("dnsapi.dll")) modiphlpapi = NewLazyDLL(sysdll.Add("iphlpapi.dll")) modkernel32 = NewLazyDLL(sysdll.Add("kernel32.dll")) + modntdll = NewLazyDLL(sysdll.Add("ntdll.dll")) modmswsock = NewLazyDLL(sysdll.Add("mswsock.dll")) modnetapi32 = NewLazyDLL(sysdll.Add("netapi32.dll")) modsecur32 = NewLazyDLL(sysdll.Add("secur32.dll")) @@ -167,6 +168,7 @@ var ( procNetApiBufferFree = modnetapi32.NewProc("NetApiBufferFree") procNetGetJoinInformation = modnetapi32.NewProc("NetGetJoinInformation") procNetUserGetInfo = modnetapi32.NewProc("NetUserGetInfo") + procRtlGetNtVersionNumbers = modntdll.NewProc("RtlGetNtVersionNumbers") procGetUserNameExW = modsecur32.NewProc("GetUserNameExW") procTranslateNameW = modsecur32.NewProc("TranslateNameW") procCommandLineToArgvW = modshell32.NewProc("CommandLineToArgvW") @@ -1213,6 +1215,11 @@ func NetUserGetInfo(serverName *uint16, userName *uint16, level uint32, buf **by return } +func rtlGetNtVersionNumbers(majorVersion *uint32, minorVersion *uint32, buildNumber *uint32) { + Syscall(procRtlGetNtVersionNumbers.Addr(), 3, uintptr(unsafe.Pointer(majorVersion)), uintptr(unsafe.Pointer(minorVersion)), uintptr(unsafe.Pointer(buildNumber))) + return +} + func GetUserNameEx(nameFormat uint32, nameBuffre *uint16, nSize *uint32) (err error) { r1, _, e1 := Syscall(procGetUserNameExW.Addr(), 3, uintptr(nameFormat), uintptr(unsafe.Pointer(nameBuffre)), uintptr(unsafe.Pointer(nSize))) if r1&0xff == 0 { From c6b62112292fa741d5708dfd63bd89eed3b6f8ee Mon Sep 17 00:00:00 2001 From: Aaron Sheah Date: Thu, 3 Jun 2021 17:43:36 +0000 Subject: [PATCH 18/30] doc/go1.17: document testing changes for Go 1.17 For #44513. Fixes #46024 Change-Id: Icf3877d1fcd67448fbc79a0ce3db3f319ad4a0e9 GitHub-Last-Rev: 8c015935c2e376134d81aa577bffdca7fc03170d GitHub-Pull-Request: golang/go#46324 Reviewed-on: https://go-review.googlesource.com/c/go/+/322109 Reviewed-by: Heschi Kreinick Trust: Jeremy Faller Trust: Dmitri Shuralyov --- 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 27ef524286..7438d894fe 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -659,9 +659,8 @@ Do not send CLs removing the interior tags from such phrases.

testing

- TODO: https://golang.org/cl/310033: add -shuffle=off|on|N to alter the execution order of tests and benchmarks + Added a new testing flag -shuffle which controls the execution order of tests and benchmarks.

-

The new T.Setenv From 79cd407f88f640b889df7645bf3e0491ed25eac7 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 4 Jun 2021 10:56:06 -0400 Subject: [PATCH 19/30] syscall: regenerate zsyscall_windows.go The declaration order in CL 319310 does not match what the generator produces from scratch. That currently causes cmd/internal/moddeps.TestAllDependencies to fail, since it is explicitly checking for that kind of skew. Updates #45914 Change-Id: If2a9cabc3d54e21deba7cb438fa364df205f38ac Reviewed-on: https://go-review.googlesource.com/c/go/+/325112 Trust: Bryan C. Mills Trust: Jason A. Donenfeld Reviewed-by: Jason A. Donenfeld Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot --- src/syscall/zsyscall_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/syscall/zsyscall_windows.go b/src/syscall/zsyscall_windows.go index b9e429693d..7bfff16be6 100644 --- a/src/syscall/zsyscall_windows.go +++ b/src/syscall/zsyscall_windows.go @@ -41,9 +41,9 @@ var ( moddnsapi = NewLazyDLL(sysdll.Add("dnsapi.dll")) modiphlpapi = NewLazyDLL(sysdll.Add("iphlpapi.dll")) modkernel32 = NewLazyDLL(sysdll.Add("kernel32.dll")) - modntdll = NewLazyDLL(sysdll.Add("ntdll.dll")) modmswsock = NewLazyDLL(sysdll.Add("mswsock.dll")) modnetapi32 = NewLazyDLL(sysdll.Add("netapi32.dll")) + modntdll = NewLazyDLL(sysdll.Add("ntdll.dll")) modsecur32 = NewLazyDLL(sysdll.Add("secur32.dll")) modshell32 = NewLazyDLL(sysdll.Add("shell32.dll")) moduserenv = NewLazyDLL(sysdll.Add("userenv.dll")) From 105c5b50e0098720b9e24aea5efa8e161c31db6d Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Tue, 25 May 2021 16:23:16 +0200 Subject: [PATCH 20/30] os: terminate windows processes via handle directly We already have a handle to the process, so use that for termination, rather than doing a new lookup based on the PID. Change-Id: I2958c1817f12f3dd783412baacbf629049f6956a Reviewed-on: https://go-review.googlesource.com/c/go/+/322509 Trust: Jason A. Donenfeld Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/os/exec_windows.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/os/exec_windows.go b/src/os/exec_windows.go index 5710401acd..b59a01a75e 100644 --- a/src/os/exec_windows.go +++ b/src/os/exec_windows.go @@ -45,16 +45,6 @@ func (p *Process) wait() (ps *ProcessState, err error) { return &ProcessState{p.Pid, syscall.WaitStatus{ExitCode: ec}, &u}, nil } -func terminateProcess(pid, exitcode int) error { - h, e := syscall.OpenProcess(syscall.PROCESS_TERMINATE, false, uint32(pid)) - if e != nil { - return NewSyscallError("OpenProcess", e) - } - defer syscall.CloseHandle(h) - e = syscall.TerminateProcess(h, uint32(exitcode)) - return NewSyscallError("TerminateProcess", e) -} - func (p *Process) signal(sig Signal) error { handle := atomic.LoadUintptr(&p.handle) if handle == uintptr(syscall.InvalidHandle) { @@ -64,9 +54,15 @@ func (p *Process) signal(sig Signal) error { return ErrProcessDone } if sig == Kill { - err := terminateProcess(p.Pid, 1) + var terminationHandle syscall.Handle + e := syscall.DuplicateHandle(^syscall.Handle(0), syscall.Handle(handle), ^syscall.Handle(0), &terminationHandle, syscall.PROCESS_TERMINATE, false, 0) + if e != nil { + return NewSyscallError("DuplicateHandle", e) + } runtime.KeepAlive(p) - return err + defer syscall.CloseHandle(terminationHandle) + e = syscall.TerminateProcess(syscall.Handle(terminationHandle), 1) + return NewSyscallError("TerminateProcess", e) } // TODO(rsc): Handle Interrupt too? return syscall.Errno(syscall.EWINDOWS) From 3a9d906edcfd0fa574ecd5498f8999b56f1e5fa1 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Tue, 25 May 2021 16:24:41 +0200 Subject: [PATCH 21/30] os: avoid finalizer race in windows process object If proc.Release is called concurrently, a handle will be double-freed. Change-Id: I0c0c32e312e07bc8615e0bf9e9b691214444d8d5 Reviewed-on: https://go-review.googlesource.com/c/go/+/322510 Trust: Jason A. Donenfeld Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/os/exec_windows.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/os/exec_windows.go b/src/os/exec_windows.go index b59a01a75e..239bed198f 100644 --- a/src/os/exec_windows.go +++ b/src/os/exec_windows.go @@ -69,7 +69,7 @@ func (p *Process) signal(sig Signal) error { } func (p *Process) release() error { - handle := atomic.LoadUintptr(&p.handle) + handle := atomic.SwapUintptr(&p.handle, uintptr(syscall.InvalidHandle)) if handle == uintptr(syscall.InvalidHandle) { return syscall.EINVAL } @@ -77,7 +77,6 @@ func (p *Process) release() error { if e != nil { return NewSyscallError("CloseHandle", e) } - atomic.StoreUintptr(&p.handle, uintptr(syscall.InvalidHandle)) // no need for a finalizer anymore runtime.SetFinalizer(p, nil) return nil From 831f9376d8d730b16fb33dfd775618dffe13ce7a Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Fri, 12 Mar 2021 13:53:11 -0800 Subject: [PATCH 22/30] net/http: fix ResponseWriter.ReadFrom with short reads CL 249238 changes ResponseWriter.ReadFrom to probe the source with a single read of sniffLen bytes before writing the response header. If the source returns less than sniffLen bytes without reaching EOF, this can cause Content-Type and Content-Length detection to fail. Fix ResponseWrite.ReadFrom to copy a full sniffLen bytes from the source as a probe. Drop the explicit call to w.WriteHeader; writing the probe will trigger a WriteHeader call. Consistently use io.CopyBuffer; ReadFrom has already acquired a copy buffer, so it may as well use it. Fixes #44953. Change-Id: Ic49305fb827a2bd7da4764b68d64b797b5157dc0 Reviewed-on: https://go-review.googlesource.com/c/go/+/301449 Trust: Damien Neil Run-TryBot: Damien Neil TryBot-Result: Go Bot Reviewed-by: Brad Fitzpatrick --- src/net/http/server.go | 40 +++--------- src/net/http/sniff_test.go | 122 +++++++++++++++++++++++++++++-------- 2 files changed, 107 insertions(+), 55 deletions(-) diff --git a/src/net/http/server.go b/src/net/http/server.go index 4e73508973..430019de50 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -577,37 +577,17 @@ func (w *response) ReadFrom(src io.Reader) (n int64, err error) { return io.CopyBuffer(writerOnly{w}, src, buf) } - // sendfile path: - - // Do not start actually writing response until src is readable. - // If body length is <= sniffLen, sendfile/splice path will do - // little anyway. This small read also satisfies sniffing the - // body in case Content-Type is missing. - nr, er := src.Read(buf[:sniffLen]) - atEOF := errors.Is(er, io.EOF) - n += int64(nr) - - if nr > 0 { - // Write the small amount read normally. - nw, ew := w.Write(buf[:nr]) - if ew != nil { - err = ew - } else if nr != nw { - err = io.ErrShortWrite + // Copy the first sniffLen bytes before switching to ReadFrom. + // This ensures we don't start writing the response before the + // source is available (see golang.org/issue/5660) and provides + // enough bytes to perform Content-Type sniffing when required. + if !w.cw.wroteHeader { + n0, err := io.CopyBuffer(writerOnly{w}, io.LimitReader(src, sniffLen), buf) + n += n0 + if err != nil || n0 < sniffLen { + return n, err } } - if err == nil && er != nil && !atEOF { - err = er - } - - // Do not send StatusOK in the error case where nothing has been written. - if err == nil && !w.wroteHeader { - w.WriteHeader(StatusOK) // nr == 0, no error (or EOF) - } - - if err != nil || atEOF { - return n, err - } w.w.Flush() // get rid of any previous writes w.cw.flush() // make sure Header is written; flush data to rwc @@ -620,7 +600,7 @@ func (w *response) ReadFrom(src io.Reader) (n int64, err error) { return n, err } - n0, err := io.Copy(writerOnly{w}, src) + n0, err := io.CopyBuffer(writerOnly{w}, src, buf) n += n0 return n, err } diff --git a/src/net/http/sniff_test.go b/src/net/http/sniff_test.go index 8d5350374d..e91335729a 100644 --- a/src/net/http/sniff_test.go +++ b/src/net/http/sniff_test.go @@ -157,9 +157,25 @@ func testServerIssue5953(t *testing.T, h2 bool) { resp.Body.Close() } -func TestContentTypeWithCopy_h1(t *testing.T) { testContentTypeWithCopy(t, h1Mode) } -func TestContentTypeWithCopy_h2(t *testing.T) { testContentTypeWithCopy(t, h2Mode) } -func testContentTypeWithCopy(t *testing.T, h2 bool) { +type byteAtATimeReader struct { + buf []byte +} + +func (b *byteAtATimeReader) Read(p []byte) (n int, err error) { + if len(p) < 1 { + return 0, nil + } + if len(b.buf) == 0 { + return 0, io.EOF + } + p[0] = b.buf[0] + b.buf = b.buf[1:] + return 1, nil +} + +func TestContentTypeWithVariousSources_h1(t *testing.T) { testContentTypeWithVariousSources(t, h1Mode) } +func TestContentTypeWithVariousSources_h2(t *testing.T) { testContentTypeWithVariousSources(t, h2Mode) } +func testContentTypeWithVariousSources(t *testing.T, h2 bool) { defer afterTest(t) const ( @@ -167,30 +183,86 @@ func testContentTypeWithCopy(t *testing.T, h2 bool) { expected = "text/html; charset=utf-8" ) - cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) { - // Use io.Copy from a bytes.Buffer to trigger ReadFrom. - buf := bytes.NewBuffer([]byte(input)) - n, err := io.Copy(w, buf) - if int(n) != len(input) || err != nil { - t.Errorf("io.Copy(w, %q) = %v, %v want %d, nil", input, n, err, len(input)) - } - })) - defer cst.close() + for _, test := range []struct { + name string + handler func(ResponseWriter, *Request) + }{{ + name: "write", + handler: func(w ResponseWriter, r *Request) { + // Write the whole input at once. + n, err := w.Write([]byte(input)) + if int(n) != len(input) || err != nil { + t.Errorf("w.Write(%q) = %v, %v want %d, nil", input, n, err, len(input)) + } + }, + }, { + name: "write one byte at a time", + handler: func(w ResponseWriter, r *Request) { + // Write the input one byte at a time. + buf := []byte(input) + for i := range buf { + n, err := w.Write(buf[i : i+1]) + if n != 1 || err != nil { + t.Errorf("w.Write(%q) = %v, %v want 1, nil", input, n, err) + } + } + }, + }, { + name: "copy from Reader", + handler: func(w ResponseWriter, r *Request) { + // Use io.Copy from a plain Reader. + type readerOnly struct{ io.Reader } + buf := bytes.NewBuffer([]byte(input)) + n, err := io.Copy(w, readerOnly{buf}) + if int(n) != len(input) || err != nil { + t.Errorf("io.Copy(w, %q) = %v, %v want %d, nil", input, n, err, len(input)) + } + }, + }, { + name: "copy from bytes.Buffer", + handler: func(w ResponseWriter, r *Request) { + // Use io.Copy from a bytes.Buffer to trigger ReadFrom. + buf := bytes.NewBuffer([]byte(input)) + n, err := io.Copy(w, buf) + if int(n) != len(input) || err != nil { + t.Errorf("io.Copy(w, %q) = %v, %v want %d, nil", input, n, err, len(input)) + } + }, + }, { + name: "copy one byte at a time", + handler: func(w ResponseWriter, r *Request) { + // Use io.Copy from a Reader that returns one byte at a time. + n, err := io.Copy(w, &byteAtATimeReader{[]byte(input)}) + if int(n) != len(input) || err != nil { + t.Errorf("io.Copy(w, %q) = %v, %v want %d, nil", input, n, err, len(input)) + } + }, + }} { + t.Run(test.name, func(t *testing.T) { + cst := newClientServerTest(t, h2, HandlerFunc(test.handler)) + defer cst.close() + + resp, err := cst.c.Get(cst.ts.URL) + if err != nil { + t.Fatalf("Get: %v", err) + } + if ct := resp.Header.Get("Content-Type"); ct != expected { + t.Errorf("Content-Type = %q, want %q", ct, expected) + } + if want, got := resp.Header.Get("Content-Length"), fmt.Sprint(len(input)); want != got { + t.Errorf("Content-Length = %q, want %q", want, got) + } + data, err := io.ReadAll(resp.Body) + if err != nil { + t.Errorf("reading body: %v", err) + } else if !bytes.Equal(data, []byte(input)) { + t.Errorf("data is %q, want %q", data, input) + } + resp.Body.Close() + + }) - resp, err := cst.c.Get(cst.ts.URL) - if err != nil { - t.Fatalf("Get: %v", err) } - if ct := resp.Header.Get("Content-Type"); ct != expected { - t.Errorf("Content-Type = %q, want %q", ct, expected) - } - data, err := io.ReadAll(resp.Body) - if err != nil { - t.Errorf("reading body: %v", err) - } else if !bytes.Equal(data, []byte(input)) { - t.Errorf("data is %q, want %q", data, input) - } - resp.Body.Close() } func TestSniffWriteSize_h1(t *testing.T) { testSniffWriteSize(t, h1Mode) } From 95939e8de71d9e8d8deea3d1605bd34130588292 Mon Sep 17 00:00:00 2001 From: sryoya Date: Sat, 5 Jun 2021 03:12:03 +0900 Subject: [PATCH 23/30] cmd/compile/internal/abi: fix typo in comment Change-Id: I196045314b2b0e908d7b31ac0cea5b25404f3ee0 Reviewed-on: https://go-review.googlesource.com/c/go/+/325249 Reviewed-by: Matthew Dempsky Trust: Keith Randall --- src/cmd/compile/internal/abi/abiutils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/compile/internal/abi/abiutils.go b/src/cmd/compile/internal/abi/abiutils.go index cb8e9d7b0f..b8ea1955d1 100644 --- a/src/cmd/compile/internal/abi/abiutils.go +++ b/src/cmd/compile/internal/abi/abiutils.go @@ -449,7 +449,7 @@ func (config *ABIConfig) ABIAnalyze(t *types.Type, setNname bool) *ABIParamResul // parameterUpdateMu protects the Offset field of function/method parameters (a subset of structure Fields) var parameterUpdateMu sync.Mutex -// FieldOffsetOf returns a concurency-safe version of f.Offset +// FieldOffsetOf returns a concurrency-safe version of f.Offset func FieldOffsetOf(f *types.Field) int64 { parameterUpdateMu.Lock() defer parameterUpdateMu.Unlock() From 9d669ed47a502ca540c7f3329f84d89fc0c53971 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 4 Jun 2021 08:16:58 -0700 Subject: [PATCH 24/30] misc/cgo/errors: use expected column numbers The test was using the wrong column numbers, and was erroneously passing because there happened to be line numbers that matched those column numbers. Change the test harness to require the expected line number for the ERROR HERE regexp case, so that this doesn't happen again. Also rename a couple of variables in the test to avoid useless redeclaration errors. Fixes #46534 Change-Id: I2fcbf5e379c346de5346035c73d174a3980c0927 Reviewed-on: https://go-review.googlesource.com/c/go/+/324970 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky --- misc/cgo/errors/errors_test.go | 3 ++- misc/cgo/errors/testdata/err2.go | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/misc/cgo/errors/errors_test.go b/misc/cgo/errors/errors_test.go index a077b59478..68a30a44fe 100644 --- a/misc/cgo/errors/errors_test.go +++ b/misc/cgo/errors/errors_test.go @@ -40,7 +40,8 @@ func check(t *testing.T, file string) { if len(frags) == 1 { continue } - re, err := regexp.Compile(string(frags[1])) + frag := fmt.Sprintf(":%d:.*%s", i+1, frags[1]) + re, err := regexp.Compile(frag) if err != nil { t.Errorf("Invalid regexp after `ERROR HERE: `: %#q", frags[1]) continue diff --git a/misc/cgo/errors/testdata/err2.go b/misc/cgo/errors/testdata/err2.go index 1d22401aee..a90598fe35 100644 --- a/misc/cgo/errors/testdata/err2.go +++ b/misc/cgo/errors/testdata/err2.go @@ -40,15 +40,15 @@ func main() { C.foop = x // ERROR HERE // issue 13129: used to output error about C.unsignedshort with CC=clang - var x C.ushort - x = int(0) // ERROR HERE: C\.ushort + var x1 C.ushort + x1 = int(0) // ERROR HERE: C\.ushort // issue 13423 _ = C.fopen() // ERROR HERE // issue 13467 - var x rune = '✈' - var _ rune = C.transform(x) // ERROR HERE: C\.int + var x2 rune = '✈' + var _ rune = C.transform(x2) // ERROR HERE: C\.int // issue 13635: used to output error about C.unsignedchar. // This test tests all such types. @@ -91,10 +91,10 @@ func main() { // issue 26745 _ = func(i int) int { - return C.i + 1 // ERROR HERE: :13 + return C.i + 1 // ERROR HERE: 14 } _ = func(i int) { - C.fi(i) // ERROR HERE: :6 + C.fi(i) // ERROR HERE: 7 } C.fi = C.fi // ERROR HERE From e3cb3817049ca5e9d96543500b72117f6ca659b8 Mon Sep 17 00:00:00 2001 From: Sergey Zagursky Date: Fri, 4 Jun 2021 12:25:51 +0300 Subject: [PATCH 25/30] go/internal/gcimporter: don't waste CPU copying bytes in `io.ReadAll` `io.ReadAll` dynamically reallocates byte slice because it doesn't know its size in advance. We don't need to read an entire file into memory and therefore may use `bufio.Reader` to read its contents. Fixes #46564 Change-Id: Id504b1512662b6dea4775d523455896fa4162ab3 Reviewed-on: https://go-review.googlesource.com/c/go/+/325429 Reviewed-by: Dominik Honnef Reviewed-by: Matthew Dempsky Trust: Matthew Dempsky Trust: Dominik Honnef Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot --- src/go/internal/gcimporter/gcimporter.go | 11 ++++------- src/go/internal/gcimporter/iimport.go | 21 +++++++++++---------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/go/internal/gcimporter/gcimporter.go b/src/go/internal/gcimporter/gcimporter.go index b74daca246..73cf6334fd 100644 --- a/src/go/internal/gcimporter/gcimporter.go +++ b/src/go/internal/gcimporter/gcimporter.go @@ -145,17 +145,14 @@ func Import(fset *token.FileSet, packages map[string]*types.Package, path, srcDi err = fmt.Errorf("import %q: old textual export format no longer supported (recompile library)", path) case "$$B\n": - var data []byte - data, err = io.ReadAll(buf) - if err != nil { - break - } + var exportFormat byte + exportFormat, err = buf.ReadByte() // The indexed export format starts with an 'i'; the older // binary export format starts with a 'c', 'd', or 'v' // (from "version"). Select appropriate importer. - if len(data) > 0 && data[0] == 'i' { - _, pkg, err = iImportData(fset, packages, data[1:], id) + if err == nil && exportFormat == 'i' { + pkg, err = iImportData(fset, packages, buf, id) } else { err = fmt.Errorf("import %q: old binary export format no longer supported (recompile library)", path) } diff --git a/src/go/internal/gcimporter/iimport.go b/src/go/internal/gcimporter/iimport.go index a3184e7641..76d47d08f1 100644 --- a/src/go/internal/gcimporter/iimport.go +++ b/src/go/internal/gcimporter/iimport.go @@ -8,6 +8,7 @@ package gcimporter import ( + "bufio" "bytes" "encoding/binary" "fmt" @@ -20,7 +21,7 @@ import ( ) type intReader struct { - *bytes.Reader + *bufio.Reader path string } @@ -61,7 +62,7 @@ const ( // and returns the number of bytes consumed and a reference to the package. // If the export data version is not recognized or the format is otherwise // compromised, an error is returned. -func iImportData(fset *token.FileSet, imports map[string]*types.Package, data []byte, path string) (_ int, pkg *types.Package, err error) { +func iImportData(fset *token.FileSet, imports map[string]*types.Package, dataReader *bufio.Reader, path string) (pkg *types.Package, err error) { const currentVersion = 1 version := int64(-1) defer func() { @@ -74,7 +75,7 @@ func iImportData(fset *token.FileSet, imports map[string]*types.Package, data [] } }() - r := &intReader{bytes.NewReader(data), path} + r := &intReader{dataReader, path} version = int64(r.uint64()) switch version { @@ -86,10 +87,12 @@ func iImportData(fset *token.FileSet, imports map[string]*types.Package, data [] sLen := int64(r.uint64()) dLen := int64(r.uint64()) - whence, _ := r.Seek(0, io.SeekCurrent) - stringData := data[whence : whence+sLen] - declData := data[whence+sLen : whence+sLen+dLen] - r.Seek(sLen+dLen, io.SeekCurrent) + data := make([]byte, sLen+dLen) + if _, err := io.ReadFull(r, data); err != nil { + errorf("cannot read %d bytes of stringData and declData: %s", len(data), err) + } + stringData := data[:sLen] + declData := data[sLen:] p := iimporter{ ipath: path, @@ -165,9 +168,7 @@ func iImportData(fset *token.FileSet, imports map[string]*types.Package, data [] // package was imported completely and without errors localpkg.MarkComplete() - - consumed, _ := r.Seek(0, io.SeekCurrent) - return int(consumed), localpkg, nil + return localpkg, nil } type iimporter struct { From f4901341263adf7fc177e8e5e2e79576b490bb8f Mon Sep 17 00:00:00 2001 From: DQNEO Date: Mon, 31 May 2021 12:15:12 +0900 Subject: [PATCH 26/30] spec: improve wording by choosing an official term "keyword" Replace "reserved word" by "keyword" as the latter is the official term. Change-Id: I9f269759b872026034a9f47e4a761cff2d348ca0 Reviewed-on: https://go-review.googlesource.com/c/go/+/323729 Reviewed-by: Robert Griesemer Trust: Ian Lance Taylor Trust: Robert Griesemer --- doc/go_spec.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/go_spec.html b/doc/go_spec.html index e59b3554f2..7a2b3a80f0 100644 --- a/doc/go_spec.html +++ b/doc/go_spec.html @@ -1,6 +1,6 @@ @@ -5020,7 +5020,7 @@ floating point, or string constants in case expressions. A type switch compares types rather than values. It is otherwise similar to an expression switch. It is marked by a special switch expression that has the form of a type assertion -using the reserved word type rather than an actual type: +using the keyword type rather than an actual type: 


From e1fa26026db313463a09289c2105591de33cf7b8 Mon Sep 17 00:00:00 2001
From: DQNEO 
Date: Mon, 31 May 2021 12:33:28 +0900
Subject: [PATCH 27/30] spec: improve wording consistency by eliminating
 "specifier"

The word "specifier" is used once only here and technically not defined.

Change-Id: Ifc9f0582f4eb3c3011ba60d8008234de511d4be6
Reviewed-on: https://go-review.googlesource.com/c/go/+/323730
Reviewed-by: Robert Griesemer 
Trust: Ian Lance Taylor 
Trust: Robert Griesemer 
---
 doc/go_spec.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/go_spec.html b/doc/go_spec.html
index 7a2b3a80f0..561d44271a 100644
--- a/doc/go_spec.html
+++ b/doc/go_spec.html
@@ -4909,7 +4909,7 @@ if x := f(); x < y {
 
 

 "Switch" statements provide multi-way execution.
-An expression or type specifier is compared to the "cases"
+An expression or type is compared to the "cases"
 inside the "switch" to determine which branch
 to execute.
 


From e3176bbc3ec7ab3889f02432f6fd088c90fc12dd Mon Sep 17 00:00:00 2001
From: Filippo Valsorda 
Date: Mon, 7 Jun 2021 10:24:11 -0400
Subject: [PATCH 28/30] crypto/tls: fix typo in Config.NextProtos docs

Change-Id: I916df584859595067e5e86c35607869397dbbd8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/325651
Trust: Filippo Valsorda 
Run-TryBot: Filippo Valsorda 
TryBot-Result: Go Bot 
Reviewed-by: Dmitri Shuralyov 
---
 src/crypto/tls/common.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index 77957ef82b..d561e61707 100644
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -619,7 +619,7 @@ type Config struct {
 	// protocol will be one from this list, and the connection will fail
 	// if there is no mutually supported protocol. If NextProtos is empty
 	// or the peer doesn't support ALPN, the connection will succeed and
-	// ConnectionState.NegotiatedProtocol will be empty."
+	// ConnectionState.NegotiatedProtocol will be empty.
 	NextProtos []string
 
 	// ServerName is used to verify the hostname on the returned

From 7406180012d828f536112c9bffb7d3edd9ea5c7e Mon Sep 17 00:00:00 2001
From: Branden J Brown 
Date: Wed, 2 Jun 2021 14:55:34 -0400
Subject: [PATCH 29/30] fmt: split package documentation into more sections

The package-level documentation on fmt previously had only two formal
sections, for printing and scanning. Because of this, the section on
printing was very long, including some pseudo-sections describing
particular features. This feature makes those pseudo-sections into
proper sections, both to improve readability and so that those sections
have hyperlinks on documentation sites.

Fixes #46522

Change-Id: I38b7bc3447610faca446051da235edcbbd063f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/324349
Reviewed-by: Rob Pike 
Reviewed-by: Dmitri Shuralyov 
Run-TryBot: Dmitri Shuralyov 
TryBot-Result: Go Bot 
Trust: Tobias Klauser 
---
 src/fmt/doc.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/fmt/doc.go b/src/fmt/doc.go
index d05ee519c3..c584cc9465 100644
--- a/src/fmt/doc.go
+++ b/src/fmt/doc.go
@@ -189,7 +189,7 @@
 	When printing a struct, fmt cannot and therefore does not invoke
 	formatting methods such as Error or String on unexported fields.
 
-	Explicit argument indexes:
+	Explicit argument indexes
 
 	In Printf, Sprintf, and Fprintf, the default behavior is for each
 	formatting verb to format successive arguments passed in the call.
@@ -211,7 +211,7 @@
 		fmt.Sprintf("%d %d %#[1]x %#x", 16, 17)
 	will yield "16 17 0x10 0x11".
 
-	Format errors:
+	Format errors
 
 	If an invalid argument is given for a verb, such as providing
 	a string to %d, the generated string will contain a

From 821270787109408ae7c86a01ccc93162be9c020c Mon Sep 17 00:00:00 2001
From: Filippo Valsorda 
Date: Mon, 7 Jun 2021 10:22:05 -0400
Subject: [PATCH 30/30] crypto/elliptic: update P-521 docs to say it's
 constant-time

This is true since CL 315274.

Also adjust the P-256 note, since Add, Double, and IsOnCurve use the
generic, non-constant-time implementation.

Change-Id: I4b3b340f65bce91dcca30bcf86456cc8ce4dd4bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/325650
Trust: Filippo Valsorda 
Trust: Katie Hockman 
Run-TryBot: Filippo Valsorda 
Reviewed-by: Katie Hockman 
TryBot-Result: Go Bot 
---
 src/crypto/elliptic/elliptic.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/crypto/elliptic/elliptic.go b/src/crypto/elliptic/elliptic.go
index b8e5a3097d..f072960bfe 100644
--- a/src/crypto/elliptic/elliptic.go
+++ b/src/crypto/elliptic/elliptic.go
@@ -455,7 +455,7 @@ func initP384() {
 // Multiple invocations of this function will return the same value, so it can
 // be used for equality checks and switch statements.
 //
-// The cryptographic operations are implemented using constant-time algorithms.
+// ScalarMult and ScalarBaseMult are implemented using constant-time algorithms.
 func P256() Curve {
 	initonce.Do(initAll)
 	return p256
@@ -479,7 +479,7 @@ func P384() Curve {
 // Multiple invocations of this function will return the same value, so it can
 // be used for equality checks and switch statements.
 //
-// The cryptographic operations do not use constant-time algorithms.
+// The cryptographic operations are implemented using constant-time algorithms.
 func P521() Curve {
 	initonce.Do(initAll)
 	return p521