From d01bc571f7e55c7376f34e86be4e5660887bd30c Mon Sep 17 00:00:00 2001 From: Tao Qingyun Date: Tue, 22 Jun 2021 00:24:05 +0000 Subject: [PATCH 01/24] runtime: make ncgocall a global counter ncgocall was stored per M, runtime.NumCgoCall lost the counter when a M die. Fixes #46789 Change-Id: I85831fbb2713f4c30d1800d07e1f47aa0031970e GitHub-Last-Rev: cbc15fa870de776d3fbf3b62fc9a5e01792e6a26 GitHub-Pull-Request: golang/go#46842 Reviewed-on: https://go-review.googlesource.com/c/go/+/329729 Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Trust: Alexander Rakoczy --- src/runtime/cgocall.go | 2 ++ src/runtime/debug.go | 2 +- src/runtime/proc.go | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go index 0e287d0b8e..8ffb48a888 100644 --- a/src/runtime/cgocall.go +++ b/src/runtime/cgocall.go @@ -110,6 +110,8 @@ func syscall_cgocaller(fn unsafe.Pointer, args ...uintptr) uintptr { return as.retval } +var ncgocall uint64 // number of cgo calls in total for dead m + // Call from Go to C. // // This must be nosplit because it's used for syscalls on some diff --git a/src/runtime/debug.go b/src/runtime/debug.go index f411b22676..82deefa200 100644 --- a/src/runtime/debug.go +++ b/src/runtime/debug.go @@ -45,7 +45,7 @@ func NumCPU() int { // NumCgoCall returns the number of cgo calls made by the current process. func NumCgoCall() int64 { - var n int64 + var n = int64(atomic.Load64(&ncgocall)) for mp := (*m)(atomic.Loadp(unsafe.Pointer(&allm))); mp != nil; mp = mp.alllink { n += int64(mp.ncgocall) } diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 8f1a443945..4c92588a66 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1522,6 +1522,8 @@ found: } unlock(&sched.lock) + atomic.Xadd64(&ncgocall, int64(m.ncgocall)) + // Release the P. handoffp(releasep()) // After this point we must not have write barriers. From 5160896c69a83f14bc54beb04be4c089333a0387 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 23 Jun 2021 00:50:43 -0400 Subject: [PATCH 02/24] go/types: in TestStdlib, import from source instead of export data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestStdlib was failing after running rm -r $(go env GOROOT)/pkg/*/cmd as the builders do when building binary releases.¹ For users who write programs that depend on go/types, it should be reasonable to run the tests for go/types as part of 'go test all', and those tests should pass even if they installed Go from a binary release. I had originally drafted this as a fallback to import from source only if the affected packages can't be imported by the default export-data importer. Unfortunately, I realized that we don't currently have a builder that tests the actual release (#46900), so it is quite likely that the fallback path would bit-rot and produce unexpected test regressions. So instead, we now unconditionally import from source in TestStdlib. That makes the test substantially slower (~15s instead of ~5s on my workstation), but with less risk of regression, and TestStdlib is skipped in short mode already so short-mode test time is unaffected. If we change the builders to test the actual release configuration, we can consider restoring the faster path when export data is available. ¹https://github.com/golang/build/blob/df58bbac082bc87c4a3cdfe336d1ffe60bbaa916/cmd/release/release.go#L533-L545 For #43232 Change-Id: I764ec56926c104053bb2ef23cf258c8f0f773290 Reviewed-on: https://go-review.googlesource.com/c/go/+/330252 Trust: Bryan C. Mills Trust: Robert Griesemer Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/go/types/stdlib_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/go/types/stdlib_test.go b/src/go/types/stdlib_test.go index 503d0a6f44..d86a77a110 100644 --- a/src/go/types/stdlib_test.go +++ b/src/go/types/stdlib_test.go @@ -26,9 +26,15 @@ import ( . "go/types" ) +// The cmd/*/internal packages may have been deleted as part of a binary +// release. Import from source instead. +// +// (See https://golang.org/issue/43232 and +// https://github.com/golang/build/blob/df58bbac082bc87c4a3cdfe336d1ffe60bbaa916/cmd/release/release.go#L533-L545.) +// // Use the same importer for all std lib tests to // avoid repeated importing of the same packages. -var stdLibImporter = importer.Default() +var stdLibImporter = importer.ForCompiler(token.NewFileSet(), "source", nil) func TestStdlib(t *testing.T) { testenv.MustHaveGoBuild(t) From d1916e5e843d0341c2d82edf08335ac181c41bd8 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 23 Jun 2021 22:06:50 -0400 Subject: [PATCH 03/24] go/types: in TestCheck/issues.src, import regexp/syntax instead of cmd/compile/internal/syntax TestCheck/issues.src was failing after running rm -r $(go env GOROOT)/pkg/*/cmd as the builders do when building binary releases. For users who write programs that depend on go/types, it should be reasonable for end users to run the tests for go/types as part of 'go test all', and those tests should pass even if they installed Go from a binary release. The test case in issues.src was importing cmd/compile/internal/syntax in order to check the reported package name. I tried to fix the problem by having the test import from source instead of from export data. Unfortunately, that changed the behavior under test: the go/types.Package.Imports reports (and is documented to report) a different set of imported packages when loading from source as compared to when loading from export data. For this particular test, after CL 313035 that difference resulted in go/types treating the "syntax" name as ambiguous when importing from source, because a transitive dependency on "regexp/syntax" is found when loading from source but omitted when loading from export data. The simple fix to make the package unambiguous again is to adapt the test to import regexp/syntax directly. That not only makes the package unambiguous with all importers, but also avoids depending on a cmd-internal package that cannot be loaded from export data in binary distributions of the Go toolchain. For #43232 Change-Id: Iba45a680ea20d26daa86ac538fd8f1938e8b73ab Reviewed-on: https://go-review.googlesource.com/c/go/+/330431 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Robert Findley --- src/go/types/testdata/check/issues.src | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/go/types/testdata/check/issues.src b/src/go/types/testdata/check/issues.src index 74d185cbc3..55fe220337 100644 --- a/src/go/types/testdata/check/issues.src +++ b/src/go/types/testdata/check/issues.src @@ -6,7 +6,7 @@ package issues import ( "fmt" - syn "cmd/compile/internal/syntax" + syn "regexp/syntax" t1 "text/template" t2 "html/template" ) @@ -329,10 +329,10 @@ func (... /* ERROR can only use ... with final parameter */ TT) f() func issue28281g() (... /* ERROR can only use ... with final parameter */ TT) // Issue #26234: Make various field/method lookup errors easier to read by matching cmd/compile's output -func issue26234a(f *syn.File) { +func issue26234a(f *syn.Prog) { // The error message below should refer to the actual package name (syntax) // not the local package name (syn). - f.foo /* ERROR f\.foo undefined \(type \*syntax\.File has no field or method foo\) */ + f.foo /* ERROR f\.foo undefined \(type \*syntax\.Prog has no field or method foo\) */ } type T struct { @@ -357,7 +357,7 @@ func issue35895() { var _ T = 0 // ERROR cannot use 0 \(untyped int constant\) as T // There is only one package with name syntax imported, only use the (global) package name in error messages. - var _ *syn.File = 0 // ERROR cannot use 0 \(untyped int constant\) as \*syntax.File + var _ *syn.Prog = 0 // ERROR cannot use 0 \(untyped int constant\) as \*syntax.Prog // Because both t1 and t2 have the same global package name (template), // qualify packages with full path name in this case. From ed01ceaf4838cd67fd802df481769fa9ae9d0440 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Fri, 25 Jun 2021 15:58:38 -0400 Subject: [PATCH 04/24] runtime/race: use race build tag on syso_test.go All other test files in the runtime/race package have race build tag, except syso_test.go. The test is only relevant if the race detector is supported. So apply the build tag. Fixes #46931. Change-Id: Icdb94214d3821b4ccf61133412ef39b4d7cc7691 Reviewed-on: https://go-review.googlesource.com/c/go/+/331050 Trust: Cherry Mui Reviewed-by: Elias Naur Run-TryBot: Cherry Mui TryBot-Result: Go Bot --- src/runtime/race/syso_test.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/runtime/race/syso_test.go b/src/runtime/race/syso_test.go index cbce5a8f18..f5095737a4 100644 --- a/src/runtime/race/syso_test.go +++ b/src/runtime/race/syso_test.go @@ -2,14 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build !android && !js && !ppc64le -// +build !android,!js,!ppc64le - -// Note: we don't run on Android or ppc64 because if there is any non-race test -// file in this package, the OS tries to link the .syso file into the -// test (even when we're not in race mode), which fails. I'm not sure -// why, but easiest to just punt - as long as a single builder runs -// this test, we're good. +//go:build race +// +build race package race From c95464f0ea3f87232b1f3937d1b37da6f335f336 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 25 Jun 2021 13:24:10 -0700 Subject: [PATCH 05/24] internal/buildcfg: refactor GOEXPERIMENT parsing code somewhat This CL extracts out a ParseGOEXPERIMENT helper function that parses GOOS/GOARCH/GOEXPERIMENT values and returns active and baseline experiment flag sets and an error value, without affecting any global state. This will be used in the subsequent CL for 'go env' support for GOEXPERIMENT to validate configuration changes. The existing package initialization for Experiment and experimentBaseline and also UpdateExperiments are updated to use it as well. Change-Id: Ic2ed3fd36d2a6f7f3d8172fccb865e02505c0052 Reviewed-on: https://go-review.googlesource.com/c/go/+/331109 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Trust: Matthew Dempsky --- src/cmd/go/internal/cfg/cfg.go | 2 +- src/internal/buildcfg/exp.go | 65 +++++++++++++++++++++------------- 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go index fc6989097e..57a3c1ff6f 100644 --- a/src/cmd/go/internal/cfg/cfg.go +++ b/src/cmd/go/internal/cfg/cfg.go @@ -79,7 +79,7 @@ func defaultContext() build.Context { // The experiments flags are based on GOARCH, so they may // need to change. TODO: This should be cleaned up. - buildcfg.UpdateExperiments(ctxt.GOARCH) + buildcfg.UpdateExperiments(ctxt.GOOS, ctxt.GOARCH, envOr("GOEXPERIMENT", buildcfg.DefaultGOEXPERIMENT)) ctxt.ToolTags = nil for _, exp := range buildcfg.EnabledExperiments() { ctxt.ToolTags = append(ctxt.ToolTags, "goexperiment."+exp) diff --git a/src/internal/buildcfg/exp.go b/src/internal/buildcfg/exp.go index 640aa1934d..9a60253aab 100644 --- a/src/internal/buildcfg/exp.go +++ b/src/internal/buildcfg/exp.go @@ -6,7 +6,6 @@ package buildcfg import ( "fmt" - "os" "reflect" "strings" @@ -18,20 +17,19 @@ import ( // // (This is not necessarily the set of experiments the compiler itself // was built with.) -var Experiment goexperiment.Flags = parseExperiments(GOARCH) - -var regabiSupported = GOARCH == "amd64" && (GOOS == "android" || GOOS == "linux" || GOOS == "darwin" || GOOS == "windows") - +// // experimentBaseline specifies the experiment flags that are enabled by // default in the current toolchain. This is, in effect, the "control" // configuration and any variation from this is an experiment. -var experimentBaseline = goexperiment.Flags{ - RegabiWrappers: regabiSupported, - RegabiG: regabiSupported, - RegabiReflect: regabiSupported, - RegabiDefer: regabiSupported, - RegabiArgs: regabiSupported, -} +var Experiment, experimentBaseline = func() (goexperiment.Flags, goexperiment.Flags) { + flags, baseline, err := ParseGOEXPERIMENT(GOOS, GOARCH, envOr("GOEXPERIMENT", defaultGOEXPERIMENT)) + if err != nil { + Error = err + } + return flags, baseline +}() + +const DefaultGOEXPERIMENT = defaultGOEXPERIMENT // FramePointerEnabled enables the use of platform conventions for // saving frame pointers. @@ -42,16 +40,29 @@ var experimentBaseline = goexperiment.Flags{ // Note: must agree with runtime.framepointer_enabled. var FramePointerEnabled = GOARCH == "amd64" || GOARCH == "arm64" -func parseExperiments(goarch string) goexperiment.Flags { +// ParseGOEXPERIMENT parses a (GOOS, GOARCH, GOEXPERIMENT) +// configuration tuple and returns the enabled and baseline experiment +// flag sets. +// +// TODO(mdempsky): Move to internal/goexperiment. +func ParseGOEXPERIMENT(goos, goarch, goexp string) (flags, baseline goexperiment.Flags, err error) { + regabiSupported := goarch == "amd64" && (goos == "android" || goos == "linux" || goos == "darwin" || goos == "windows") + + baseline = goexperiment.Flags{ + RegabiWrappers: regabiSupported, + RegabiG: regabiSupported, + RegabiReflect: regabiSupported, + RegabiDefer: regabiSupported, + RegabiArgs: regabiSupported, + } + // Start with the statically enabled set of experiments. - flags := experimentBaseline + flags = baseline // Pick up any changes to the baseline configuration from the // GOEXPERIMENT environment. This can be set at make.bash time // and overridden at build time. - env := envOr("GOEXPERIMENT", defaultGOEXPERIMENT) - - if env != "" { + if goexp != "" { // Create a map of known experiment names. names := make(map[string]func(bool)) rv := reflect.ValueOf(&flags).Elem() @@ -74,7 +85,7 @@ func parseExperiments(goarch string) goexperiment.Flags { } // Parse names. - for _, f := range strings.Split(env, ",") { + for _, f := range strings.Split(goexp, ",") { if f == "" { continue } @@ -91,8 +102,8 @@ func parseExperiments(goarch string) goexperiment.Flags { } set, ok := names[f] if !ok { - fmt.Printf("unknown experiment %s\n", f) - os.Exit(2) + err = fmt.Errorf("unknown GOEXPERIMENT %s", f) + return } set(val) } @@ -108,12 +119,12 @@ func parseExperiments(goarch string) goexperiment.Flags { } // Check regabi dependencies. if flags.RegabiG && !flags.RegabiWrappers { - Error = fmt.Errorf("GOEXPERIMENT regabig requires regabiwrappers") + err = fmt.Errorf("GOEXPERIMENT regabig requires regabiwrappers") } if flags.RegabiArgs && !(flags.RegabiWrappers && flags.RegabiG && flags.RegabiReflect && flags.RegabiDefer) { - Error = fmt.Errorf("GOEXPERIMENT regabiargs requires regabiwrappers,regabig,regabireflect,regabidefer") + err = fmt.Errorf("GOEXPERIMENT regabiargs requires regabiwrappers,regabig,regabireflect,regabidefer") } - return flags + return } // expList returns the list of lower-cased experiment names for @@ -169,6 +180,10 @@ func AllExperiments() []string { // UpdateExperiments updates the Experiment global based on a new GOARCH value. // This is only required for cmd/go, which can change GOARCH after // program startup due to use of "go env -w". -func UpdateExperiments(goarch string) { - Experiment = parseExperiments(goarch) +func UpdateExperiments(goos, goarch, goexperiment string) { + var err error + Experiment, experimentBaseline, err = ParseGOEXPERIMENT(goos, goarch, goexperiment) + if err != nil { + Error = err + } } From 361159c05507b7f6c28e29575c02a6b7b6656f84 Mon Sep 17 00:00:00 2001 From: Koichi Shiraishi Date: Sun, 27 Jun 2021 11:50:17 +0900 Subject: [PATCH 06/24] cmd/cgo: fix 'see gmp.go' to 'see doc.go' Change-Id: I303edc9dfbf4185b5b461b121ab504f6ed9f8630 Reviewed-on: https://go-review.googlesource.com/c/go/+/330839 Reviewed-by: Ian Lance Taylor Trust: Dmitri Shuralyov --- src/cmd/cgo/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go index 03a662e689..c6a0c525e6 100644 --- a/src/cmd/cgo/main.go +++ b/src/cmd/cgo/main.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Cgo; see gmp.go for an overview. +// Cgo; see doc.go for an overview. // TODO(rsc): // Emit correct line number annotations. From 901510ed4ef1a979321f33159b534e374290ef65 Mon Sep 17 00:00:00 2001 From: hao Date: Fri, 28 May 2021 03:40:12 +0000 Subject: [PATCH 07/24] cmd/link/internal/ld: skip the windows ASLR test when CGO_ENABLED=0 the test case is still using gcc when CGO is disabled. Change-Id: I2d255bfaeb92816c8343ab72fd7984b6632d421d GitHub-Last-Rev: de14748bd54c7db8687263a7c37080ec884d982a GitHub-Pull-Request: golang/go#46120 Reviewed-on: https://go-review.googlesource.com/c/go/+/319169 Reviewed-by: Cherry Mui Run-TryBot: Cherry Mui TryBot-Result: Go Bot Trust: Dmitri Shuralyov --- src/cmd/link/internal/ld/ld_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cmd/link/internal/ld/ld_test.go b/src/cmd/link/internal/ld/ld_test.go index ca764632c3..3702a4d08f 100644 --- a/src/cmd/link/internal/ld/ld_test.go +++ b/src/cmd/link/internal/ld/ld_test.go @@ -174,6 +174,8 @@ func TestWindowsBuildmodeCSharedASLR(t *testing.T) { t.Skip("skipping windows amd64/386 only test") } + testenv.MustHaveCGO(t) + t.Run("aslr", func(t *testing.T) { testWindowsBuildmodeCSharedASLR(t, true) }) From a1d27269d698d684497d0dc61c968a1c2dbe00b3 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Sat, 19 Jun 2021 03:35:15 -0700 Subject: [PATCH 08/24] cmd/go: prep for 'go env' refactoring This CL refactors code a little to make it easier to add GOEXPERIMENT support in the future. Change-Id: I87903056f7863049e58be72047b2b8a60a213baf Reviewed-on: https://go-review.googlesource.com/c/go/+/329654 Run-TryBot: Matthew Dempsky Trust: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills --- src/cmd/go/internal/envcmd/env.go | 210 ++++++++++++----------- src/cmd/go/main.go | 64 +++---- src/cmd/go/testdata/script/env_unset.txt | 23 +++ 3 files changed, 170 insertions(+), 127 deletions(-) create mode 100644 src/cmd/go/testdata/script/env_unset.txt diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go index b30c37ab27..d88dcce5c0 100644 --- a/src/cmd/go/internal/envcmd/env.go +++ b/src/cmd/go/internal/envcmd/env.go @@ -10,6 +10,7 @@ import ( "encoding/json" "fmt" "go/build" + "internal/buildcfg" "io" "os" "path/filepath" @@ -197,6 +198,21 @@ func runEnv(ctx context.Context, cmd *base.Command, args []string) { if *envU && *envW { base.Fatalf("go env: cannot use -u with -w") } + + // Handle 'go env -w' and 'go env -u' before calling buildcfg.Check, + // so they can be used to recover from an invalid configuration. + if *envW { + runEnvW(args) + return + } + + if *envU { + runEnvU(args) + return + } + + buildcfg.Check() + env := cfg.CmdEnv env = append(env, ExtraEnvVars()...) @@ -206,14 +222,7 @@ func runEnv(ctx context.Context, cmd *base.Command, args []string) { // Do we need to call ExtraEnvVarsCostly, which is a bit expensive? needCostly := false - if *envU || *envW { - // We're overwriting or removing default settings, - // so it doesn't really matter what the existing settings are. - // - // Moreover, we haven't validated the new settings yet, so it is - // important that we NOT perform any actions based on them, - // such as initializing the builder to compute other variables. - } else if len(args) == 0 { + if len(args) == 0 { // We're listing all environment variables ("go env"), // including the expensive ones. needCostly = true @@ -238,95 +247,6 @@ func runEnv(ctx context.Context, cmd *base.Command, args []string) { env = append(env, ExtraEnvVarsCostly()...) } - if *envW { - // Process and sanity-check command line. - if len(args) == 0 { - base.Fatalf("go env -w: no KEY=VALUE arguments given") - } - osEnv := make(map[string]string) - for _, e := range cfg.OrigEnv { - if i := strings.Index(e, "="); i >= 0 { - osEnv[e[:i]] = e[i+1:] - } - } - add := make(map[string]string) - for _, arg := range args { - i := strings.Index(arg, "=") - if i < 0 { - base.Fatalf("go env -w: arguments must be KEY=VALUE: invalid argument: %s", arg) - } - key, val := arg[:i], arg[i+1:] - if err := checkEnvWrite(key, val); err != nil { - base.Fatalf("go env -w: %v", err) - } - if _, ok := add[key]; ok { - base.Fatalf("go env -w: multiple values for key: %s", key) - } - add[key] = val - if osVal := osEnv[key]; osVal != "" && osVal != val { - fmt.Fprintf(os.Stderr, "warning: go env -w %s=... does not override conflicting OS environment variable\n", key) - } - } - - goos, okGOOS := add["GOOS"] - goarch, okGOARCH := add["GOARCH"] - if okGOOS || okGOARCH { - if !okGOOS { - goos = cfg.Goos - } - if !okGOARCH { - goarch = cfg.Goarch - } - if err := work.CheckGOOSARCHPair(goos, goarch); err != nil { - base.Fatalf("go env -w: %v", err) - } - } - - gotmp, okGOTMP := add["GOTMPDIR"] - if okGOTMP { - if !filepath.IsAbs(gotmp) && gotmp != "" { - base.Fatalf("go env -w: GOTMPDIR must be an absolute path") - } - } - - updateEnvFile(add, nil) - return - } - - if *envU { - // Process and sanity-check command line. - if len(args) == 0 { - base.Fatalf("go env -u: no arguments given") - } - del := make(map[string]bool) - for _, arg := range args { - if err := checkEnvWrite(arg, ""); err != nil { - base.Fatalf("go env -u: %v", err) - } - del[arg] = true - } - if del["GOOS"] || del["GOARCH"] { - goos, goarch := cfg.Goos, cfg.Goarch - if del["GOOS"] { - goos = getOrigEnv("GOOS") - if goos == "" { - goos = build.Default.GOOS - } - } - if del["GOARCH"] { - goarch = getOrigEnv("GOARCH") - if goarch == "" { - goarch = build.Default.GOARCH - } - } - if err := work.CheckGOOSARCHPair(goos, goarch); err != nil { - base.Fatalf("go env -u: %v", err) - } - } - updateEnvFile(nil, del) - return - } - if len(args) > 0 { if *envJson { var es []cfg.EnvVar @@ -351,6 +271,102 @@ func runEnv(ctx context.Context, cmd *base.Command, args []string) { PrintEnv(os.Stdout, env) } +func runEnvW(args []string) { + // Process and sanity-check command line. + if len(args) == 0 { + base.Fatalf("go env -w: no KEY=VALUE arguments given") + } + osEnv := make(map[string]string) + for _, e := range cfg.OrigEnv { + if i := strings.Index(e, "="); i >= 0 { + osEnv[e[:i]] = e[i+1:] + } + } + add := make(map[string]string) + for _, arg := range args { + i := strings.Index(arg, "=") + if i < 0 { + base.Fatalf("go env -w: arguments must be KEY=VALUE: invalid argument: %s", arg) + } + key, val := arg[:i], arg[i+1:] + if err := checkEnvWrite(key, val); err != nil { + base.Fatalf("go env -w: %v", err) + } + if _, ok := add[key]; ok { + base.Fatalf("go env -w: multiple values for key: %s", key) + } + add[key] = val + if osVal := osEnv[key]; osVal != "" && osVal != val { + fmt.Fprintf(os.Stderr, "warning: go env -w %s=... does not override conflicting OS environment variable\n", key) + } + } + + if err := checkBuildConfig(add, nil); err != nil { + base.Fatalf("go env -w: %v", err) + } + + gotmp, okGOTMP := add["GOTMPDIR"] + if okGOTMP { + if !filepath.IsAbs(gotmp) && gotmp != "" { + base.Fatalf("go env -w: GOTMPDIR must be an absolute path") + } + } + + updateEnvFile(add, nil) +} + +func runEnvU(args []string) { + // Process and sanity-check command line. + if len(args) == 0 { + base.Fatalf("go env -u: no arguments given") + } + del := make(map[string]bool) + for _, arg := range args { + if err := checkEnvWrite(arg, ""); err != nil { + base.Fatalf("go env -u: %v", err) + } + del[arg] = true + } + + if err := checkBuildConfig(nil, del); err != nil { + base.Fatalf("go env -u: %v", err) + } + + updateEnvFile(nil, del) +} + +// checkBuildConfig checks whether the build configuration is valid +// after the specified configuration environment changes are applied. +func checkBuildConfig(add map[string]string, del map[string]bool) error { + // get returns the value for key after applying add and del and + // reports whether it changed. cur should be the current value + // (i.e., before applying changes) and def should be the default + // value (i.e., when no environment variables are provided at all). + get := func(key, cur, def string) (string, bool) { + if val, ok := add[key]; ok { + return val, true + } + if del[key] { + val := getOrigEnv(key) + if val == "" { + val = def + } + return val, true + } + return cur, false + } + + goos, okGOOS := get("GOOS", cfg.Goos, build.Default.GOOS) + goarch, okGOARCH := get("GOARCH", cfg.Goarch, build.Default.GOARCH) + if okGOOS || okGOARCH { + if err := work.CheckGOOSARCHPair(goos, goarch); err != nil { + return err + } + } + + return nil +} + // PrintEnv prints the environment variables to w. func PrintEnv(w io.Writer, env []cfg.EnvVar) { for _, e := range env { diff --git a/src/cmd/go/main.go b/src/cmd/go/main.go index 02174a56ff..16361e02ca 100644 --- a/src/cmd/go/main.go +++ b/src/cmd/go/main.go @@ -145,24 +145,6 @@ func main() { os.Exit(2) } - if err := buildcfg.Error; err != nil { - fmt.Fprintf(os.Stderr, "go: %v\n", buildcfg.Error) - os.Exit(2) - } - - // Set environment (GOOS, GOARCH, etc) explicitly. - // In theory all the commands we invoke should have - // the same default computation of these as we do, - // but in practice there might be skew - // This makes sure we all agree. - cfg.OrigEnv = os.Environ() - cfg.CmdEnv = envcmd.MkEnv() - for _, env := range cfg.CmdEnv { - if os.Getenv(env.Name) != env.Value { - os.Setenv(env.Name, env.Value) - } - } - BigCmdLoop: for bigCmd := base.Go; ; { for _, cmd := range bigCmd.Commands { @@ -188,18 +170,7 @@ BigCmdLoop: if !cmd.Runnable() { continue } - cmd.Flag.Usage = func() { cmd.Usage() } - if cmd.CustomFlags { - args = args[1:] - } else { - base.SetFromGOFLAGS(&cmd.Flag) - cmd.Flag.Parse(args[1:]) - args = cmd.Flag.Args() - } - ctx := maybeStartTrace(context.Background()) - ctx, span := trace.StartSpan(ctx, fmt.Sprint("Running ", cmd.Name(), " command")) - cmd.Run(ctx, cmd, args) - span.Done() + invoke(cmd, args) base.Exit() return } @@ -213,6 +184,39 @@ BigCmdLoop: } } +func invoke(cmd *base.Command, args []string) { + // 'go env' handles checking the build config + if cmd != envcmd.CmdEnv { + buildcfg.Check() + } + + // Set environment (GOOS, GOARCH, etc) explicitly. + // In theory all the commands we invoke should have + // the same default computation of these as we do, + // but in practice there might be skew + // This makes sure we all agree. + cfg.OrigEnv = os.Environ() + cfg.CmdEnv = envcmd.MkEnv() + for _, env := range cfg.CmdEnv { + if os.Getenv(env.Name) != env.Value { + os.Setenv(env.Name, env.Value) + } + } + + cmd.Flag.Usage = func() { cmd.Usage() } + if cmd.CustomFlags { + args = args[1:] + } else { + base.SetFromGOFLAGS(&cmd.Flag) + cmd.Flag.Parse(args[1:]) + args = cmd.Flag.Args() + } + ctx := maybeStartTrace(context.Background()) + ctx, span := trace.StartSpan(ctx, fmt.Sprint("Running ", cmd.Name(), " command")) + cmd.Run(ctx, cmd, args) + span.Done() +} + func init() { base.Usage = mainUsage } diff --git a/src/cmd/go/testdata/script/env_unset.txt b/src/cmd/go/testdata/script/env_unset.txt new file mode 100644 index 0000000000..35fbb0a8a2 --- /dev/null +++ b/src/cmd/go/testdata/script/env_unset.txt @@ -0,0 +1,23 @@ +# Test that we can unset variables, even if initially invalid, +# as long as resulting config is valid. + +env GOENV=badenv +env GOOS= +env GOARCH= + +! go env +stderr '^cmd/go: unsupported GOOS/GOARCH pair bados/badarch$' + +! go env -u GOOS +stderr '^go env -u: unsupported GOOS/GOARCH pair \w+/badarch$' + +! go env -u GOARCH +stderr '^go env -u: unsupported GOOS/GOARCH pair bados/\w+$' + +go env -u GOOS GOARCH + +go env + +-- badenv -- +GOOS=bados +GOARCH=badarch From 956c81bfe60306e49e26f7edd4f48cbc24c8fe28 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 16 Jun 2021 15:10:57 -0700 Subject: [PATCH 09/24] cmd/go: add GOEXPERIMENT to `go env` output This CL adds GOEXPERIMENT to `go env` output, and also makes it configurable via `GOENV`. Thanks to Baokun Lee's CL 304350 for the test and initial work on this. Fixes #45226. Change-Id: Ie7f92a8a503b6a2a4df3f6598f0b2bf2915e2e7d Reviewed-on: https://go-review.googlesource.com/c/go/+/328751 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills Trust: Bryan C. Mills Trust: Matthew Dempsky --- src/cmd/go/alldocs.go | 6 ++++++ src/cmd/go/internal/envcmd/env.go | 8 ++++++++ src/cmd/go/internal/help/helpdoc.go | 6 ++++++ src/cmd/go/testdata/script/env_exp.txt | 17 +++++++++++++++++ src/cmd/go/testdata/script/env_unset.txt | 7 +++++++ src/cmd/go/testdata/script/env_write.txt | 6 ++++++ 6 files changed, 50 insertions(+) create mode 100644 src/cmd/go/testdata/script/env_exp.txt diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index fd95da23eb..90eb3e2a00 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -1903,6 +1903,12 @@ // GCCGOTOOLDIR // If set, where to find gccgo tools, such as cgo. // The default is based on how gccgo was configured. +// GOEXPERIMENT +// Comma-separated list of toolchain experiments to enable or disable. +// The list of available experiments may change arbitrarily over time. +// See src/internal/goexperiment/flags.go for currently valid values. +// Warning: This variable is provided for the development and testing +// of the Go toolchain itself. Use beyond that purpose is unsupported. // GOROOT_FINAL // The root of the installed Go tree, when it is // installed in a location other than where it is built. diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go index d88dcce5c0..1553d26391 100644 --- a/src/cmd/go/internal/envcmd/env.go +++ b/src/cmd/go/internal/envcmd/env.go @@ -73,6 +73,7 @@ func MkEnv() []cfg.EnvVar { {Name: "GOCACHE", Value: cache.DefaultDir()}, {Name: "GOENV", Value: envFile}, {Name: "GOEXE", Value: cfg.ExeSuffix}, + {Name: "GOEXPERIMENT", Value: buildcfg.GOEXPERIMENT()}, {Name: "GOFLAGS", Value: cfg.Getenv("GOFLAGS")}, {Name: "GOHOSTARCH", Value: runtime.GOARCH}, {Name: "GOHOSTOS", Value: runtime.GOOS}, @@ -364,6 +365,13 @@ func checkBuildConfig(add map[string]string, del map[string]bool) error { } } + goexperiment, okGOEXPERIMENT := get("GOEXPERIMENT", buildcfg.GOEXPERIMENT(), "") + if okGOEXPERIMENT { + if _, _, err := buildcfg.ParseGOEXPERIMENT(goos, goarch, goexperiment); err != nil { + return err + } + } + return nil } diff --git a/src/cmd/go/internal/help/helpdoc.go b/src/cmd/go/internal/help/helpdoc.go index b552777e3e..490ff1fb7c 100644 --- a/src/cmd/go/internal/help/helpdoc.go +++ b/src/cmd/go/internal/help/helpdoc.go @@ -610,6 +610,12 @@ Special-purpose environment variables: GCCGOTOOLDIR If set, where to find gccgo tools, such as cgo. The default is based on how gccgo was configured. + GOEXPERIMENT + Comma-separated list of toolchain experiments to enable or disable. + The list of available experiments may change arbitrarily over time. + See src/internal/goexperiment/flags.go for currently valid values. + Warning: This variable is provided for the development and testing + of the Go toolchain itself. Use beyond that purpose is unsupported. GOROOT_FINAL The root of the installed Go tree, when it is installed in a location other than where it is built. diff --git a/src/cmd/go/testdata/script/env_exp.txt b/src/cmd/go/testdata/script/env_exp.txt new file mode 100644 index 0000000000..681512d87c --- /dev/null +++ b/src/cmd/go/testdata/script/env_exp.txt @@ -0,0 +1,17 @@ +# Test GOEXPERIMENT variable + +# go env shows default empty GOEXPERIMENT +go env +stdout GOEXPERIMENT= + +# go env shows valid experiments +env GOEXPERIMENT=fieldtrack,staticlockranking +go env GOEXPERIMENT +stdout '.*fieldtrack.*staticlockranking.*' +go env +stdout 'GOEXPERIMENT=.*fieldtrack.*staticlockranking.*' + +# go env rejects unknown experiments +env GOEXPERIMENT=bad +! go env GOEXPERIMENT +stderr 'unknown GOEXPERIMENT bad' diff --git a/src/cmd/go/testdata/script/env_unset.txt b/src/cmd/go/testdata/script/env_unset.txt index 35fbb0a8a2..4e0f249509 100644 --- a/src/cmd/go/testdata/script/env_unset.txt +++ b/src/cmd/go/testdata/script/env_unset.txt @@ -4,6 +4,12 @@ env GOENV=badenv env GOOS= env GOARCH= +env GOEXPERIMENT= + +! go env +stderr '^go(\.exe)?: unknown GOEXPERIMENT badexp$' + +go env -u GOEXPERIMENT ! go env stderr '^cmd/go: unsupported GOOS/GOARCH pair bados/badarch$' @@ -21,3 +27,4 @@ go env -- badenv -- GOOS=bados GOARCH=badarch +GOEXPERIMENT=badexp diff --git a/src/cmd/go/testdata/script/env_write.txt b/src/cmd/go/testdata/script/env_write.txt index 4fa39df104..b5e9739167 100644 --- a/src/cmd/go/testdata/script/env_write.txt +++ b/src/cmd/go/testdata/script/env_write.txt @@ -179,3 +179,9 @@ stderr 'unsupported GOOS/GOARCH.*windows/mips$' stderr 'go env -w: GOMODCACHE entry is relative; must be absolute path: "~/test"' ! go env -w GOMODCACHE=./test stderr 'go env -w: GOMODCACHE entry is relative; must be absolute path: "./test"' + +# go env -w checks validity of GOEXPERIMENT +env GOEXPERIMENT= +! go env -w GOEXPERIMENT=badexp +stderr 'unknown GOEXPERIMENT badexp' +go env -w GOEXPERIMENT=fieldtrack From 5385e2386b64b10960c1b40113ee7dae271c8369 Mon Sep 17 00:00:00 2001 From: Mia Zhu Date: Mon, 28 Jun 2021 17:10:56 +0000 Subject: [PATCH 10/24] runtime/internal/atomic: drop Cas64 pointer indirection in comments Change-Id: Ieff0065cbd81e045594ce12e10338b0666816d70 GitHub-Last-Rev: d842f5cb3e5d75f87957c068f6accc9d4a4ac224 GitHub-Pull-Request: golang/go#46949 Reviewed-on: https://go-review.googlesource.com/c/go/+/331309 Trust: Keith Randall Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/runtime/internal/atomic/atomic_amd64.s | 2 +- src/runtime/internal/atomic/atomic_arm64.s | 2 +- src/runtime/internal/atomic/atomic_mips64x.s | 2 +- src/runtime/internal/atomic/atomic_ppc64x.s | 2 +- src/runtime/internal/atomic/atomic_riscv64.s | 3 ++- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/runtime/internal/atomic/atomic_amd64.s b/src/runtime/internal/atomic/atomic_amd64.s index 57cd59dd8c..d21514b36b 100644 --- a/src/runtime/internal/atomic/atomic_amd64.s +++ b/src/runtime/internal/atomic/atomic_amd64.s @@ -37,7 +37,7 @@ TEXT ·Cas(SB),NOSPLIT,$0-17 // bool ·Cas64(uint64 *val, uint64 old, uint64 new) // Atomically: -// if(*val == *old){ +// if(*val == old){ // *val = new; // return 1; // } else { diff --git a/src/runtime/internal/atomic/atomic_arm64.s b/src/runtime/internal/atomic/atomic_arm64.s index e9467afecd..5f77d92deb 100644 --- a/src/runtime/internal/atomic/atomic_arm64.s +++ b/src/runtime/internal/atomic/atomic_arm64.s @@ -192,7 +192,7 @@ ok: // bool ·Cas64(uint64 *ptr, uint64 old, uint64 new) // Atomically: -// if(*val == *old){ +// if(*val == old){ // *val = new; // return 1; // } else { diff --git a/src/runtime/internal/atomic/atomic_mips64x.s b/src/runtime/internal/atomic/atomic_mips64x.s index fba668f94a..fedfc4a175 100644 --- a/src/runtime/internal/atomic/atomic_mips64x.s +++ b/src/runtime/internal/atomic/atomic_mips64x.s @@ -37,7 +37,7 @@ cas_fail: // bool cas64(uint64 *ptr, uint64 old, uint64 new) // Atomically: -// if(*val == *old){ +// if(*val == old){ // *val = new; // return 1; // } else { diff --git a/src/runtime/internal/atomic/atomic_ppc64x.s b/src/runtime/internal/atomic/atomic_ppc64x.s index dca26cb334..226b3b6216 100644 --- a/src/runtime/internal/atomic/atomic_ppc64x.s +++ b/src/runtime/internal/atomic/atomic_ppc64x.s @@ -107,7 +107,7 @@ cas_fail: // bool ·Cas64(uint64 *ptr, uint64 old, uint64 new) // Atomically: -// if(*val == *old){ +// if(*val == old){ // *val = new; // return 1; // } else { diff --git a/src/runtime/internal/atomic/atomic_riscv64.s b/src/runtime/internal/atomic/atomic_riscv64.s index ec05302a78..21d5adcdbc 100644 --- a/src/runtime/internal/atomic/atomic_riscv64.s +++ b/src/runtime/internal/atomic/atomic_riscv64.s @@ -30,8 +30,9 @@ #include "textflag.h" +// func Cas(ptr *uint64, old, new uint64) bool // Atomically: -// if(*val == *old){ +// if(*val == old){ // *val = new; // return 1; // } else { From 1519271a939ad27da133318dc4bde7e6a41a35b5 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 25 Jun 2021 11:17:43 -0700 Subject: [PATCH 11/24] spec: change unsafe.Slice((*T)(nil), 0) to return []T(nil) Updates #46742. Change-Id: I044933a657cd1a5cdb29863e49751df5fe9c258a Reviewed-on: https://go-review.googlesource.com/c/go/+/331069 Run-TryBot: Matthew Dempsky Trust: Matthew Dempsky Trust: Robert Griesemer Reviewed-by: Robert Griesemer --- doc/go_spec.html | 10 ++++++++-- src/unsafe/unsafe.go | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/doc/go_spec.html b/doc/go_spec.html index b59b37fd55..e0602418e8 100644 --- a/doc/go_spec.html +++ b/doc/go_spec.html @@ -1,6 +1,6 @@ @@ -6789,11 +6789,17 @@ and whose length and capacity are len: (*[len]ArbitraryType)(unsafe.Pointer(ptr))[:] +

+As a special case, if ptr is nil and len is zero, +Slice returns nil. +

+

The len argument must be of integer type or an untyped constant. A constant len argument must be non-negative and representable by a value of type int; if it is an untyped constant it is given type int. -If ptr is nil or len is negative at run time, +At run time, if len is negative, +or if ptr is nil and len is not zero, a run-time panic occurs.

diff --git a/src/unsafe/unsafe.go b/src/unsafe/unsafe.go index ecbd28c523..eaf72c9618 100644 --- a/src/unsafe/unsafe.go +++ b/src/unsafe/unsafe.go @@ -221,8 +221,11 @@ func Add(ptr Pointer, len IntegerType) Pointer // // (*[len]ArbitraryType)(unsafe.Pointer(ptr))[:] // +// As a special case, if ptr is nil and len is zero, Slice returns nil. +// // The len argument must be of integer type or an untyped constant. // A constant len argument must be non-negative and representable by a value of type int; // if it is an untyped constant it is given type int. -// If ptr is nil or len is negative at run time, a run-time panic occurs. +// At run time, if len is negative, or if ptr is nil and len is not zero, +// a run-time panic occurs. func Slice(ptr *ArbitraryType, len IntegerType) []ArbitraryType From 4bb0847b088eb3eb6122a18a87e1ca7756281dcc Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 25 Jun 2021 11:07:28 -0700 Subject: [PATCH 12/24] cmd/compile,runtime: change unsafe.Slice((*T)(nil), 0) to return []T(nil) This CL removes the unconditional OCHECKNIL check added in walkUnsafeSlice by instead passing it as a pointer to runtime.unsafeslice, and hiding the check behind a `len == 0` check. While here, this CL also implements checkptr functionality for unsafe.Slice and disallows use of unsafe.Slice with //go:notinheap types. Updates #46742. Change-Id: I743a445ac124304a4d7322a7fe089c4a21b9a655 Reviewed-on: https://go-review.googlesource.com/c/go/+/331070 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Trust: Matthew Dempsky Reviewed-by: Keith Randall --- src/cmd/compile/internal/typecheck/builtin.go | 5 ++-- .../internal/typecheck/builtin/runtime.go | 5 ++-- src/cmd/compile/internal/typecheck/func.go | 7 +++++ src/cmd/compile/internal/walk/builtin.go | 26 +++++++------------ src/runtime/checkptr.go | 21 ++++++++++++++- src/runtime/checkptr_test.go | 2 ++ src/runtime/slice.go | 24 ++++++++++++++--- src/runtime/testdata/testprog/checkptr.go | 13 ++++++++++ test/unsafebuiltins.go | 7 +++-- 9 files changed, 83 insertions(+), 27 deletions(-) diff --git a/src/cmd/compile/internal/typecheck/builtin.go b/src/cmd/compile/internal/typecheck/builtin.go index 67a894c7ed..833b17b414 100644 --- a/src/cmd/compile/internal/typecheck/builtin.go +++ b/src/cmd/compile/internal/typecheck/builtin.go @@ -138,6 +138,7 @@ var runtimeDecls = [...]struct { {"growslice", funcTag, 116}, {"unsafeslice", funcTag, 117}, {"unsafeslice64", funcTag, 118}, + {"unsafeslicecheckptr", funcTag, 118}, {"memmove", funcTag, 119}, {"memclrNoHeapPointers", funcTag, 120}, {"memclrHasPointers", funcTag, 120}, @@ -341,8 +342,8 @@ func runtimeTypes() []*types.Type { typs[114] = newSig(params(typs[1], typs[15], typs[15], typs[7]), params(typs[7])) typs[115] = types.NewSlice(typs[2]) typs[116] = newSig(params(typs[1], typs[115], typs[15]), params(typs[115])) - typs[117] = newSig(params(typs[1], typs[15]), nil) - typs[118] = newSig(params(typs[1], typs[22]), nil) + typs[117] = newSig(params(typs[1], typs[7], typs[15]), nil) + typs[118] = newSig(params(typs[1], typs[7], typs[22]), nil) typs[119] = newSig(params(typs[3], typs[3], typs[5]), nil) typs[120] = newSig(params(typs[7], typs[5]), nil) typs[121] = newSig(params(typs[3], typs[3], typs[5]), params(typs[6])) diff --git a/src/cmd/compile/internal/typecheck/builtin/runtime.go b/src/cmd/compile/internal/typecheck/builtin/runtime.go index ebeaeae79e..2b29ea3c08 100644 --- a/src/cmd/compile/internal/typecheck/builtin/runtime.go +++ b/src/cmd/compile/internal/typecheck/builtin/runtime.go @@ -183,8 +183,9 @@ func makeslice(typ *byte, len int, cap int) unsafe.Pointer func makeslice64(typ *byte, len int64, cap int64) unsafe.Pointer func makeslicecopy(typ *byte, tolen int, fromlen int, from unsafe.Pointer) unsafe.Pointer func growslice(typ *byte, old []any, cap int) (ary []any) -func unsafeslice(typ *byte, len int) -func unsafeslice64(typ *byte, len int64) +func unsafeslice(typ *byte, ptr unsafe.Pointer, len int) +func unsafeslice64(typ *byte, ptr unsafe.Pointer, len int64) +func unsafeslicecheckptr(typ *byte, ptr unsafe.Pointer, len int64) func memmove(to *any, frm *any, length uintptr) func memclrNoHeapPointers(ptr unsafe.Pointer, n uintptr) diff --git a/src/cmd/compile/internal/typecheck/func.go b/src/cmd/compile/internal/typecheck/func.go index a6dfbbf569..fbcc784627 100644 --- a/src/cmd/compile/internal/typecheck/func.go +++ b/src/cmd/compile/internal/typecheck/func.go @@ -1018,7 +1018,14 @@ func tcUnsafeSlice(n *ir.BinaryExpr) *ir.BinaryExpr { t := n.X.Type() if !t.IsPtr() { base.Errorf("first argument to unsafe.Slice must be pointer; have %L", t) + } else if t.Elem().NotInHeap() { + // TODO(mdempsky): This can be relaxed, but should only affect the + // Go runtime itself. End users should only see //go:notinheap + // types due to incomplete C structs in cgo, and those types don't + // have a meaningful size anyway. + base.Errorf("unsafe.Slice of incomplete (or unallocatable) type not allowed") } + if !checkunsafeslice(&n.Y) { n.SetType(nil) return n diff --git a/src/cmd/compile/internal/walk/builtin.go b/src/cmd/compile/internal/walk/builtin.go index 62eb4298f4..1f08e4d312 100644 --- a/src/cmd/compile/internal/walk/builtin.go +++ b/src/cmd/compile/internal/walk/builtin.go @@ -654,36 +654,28 @@ func walkRecover(nn *ir.CallExpr, init *ir.Nodes) ir.Node { } func walkUnsafeSlice(n *ir.BinaryExpr, init *ir.Nodes) ir.Node { + ptr := safeExpr(n.X, init) len := safeExpr(n.Y, init) fnname := "unsafeslice64" - argtype := types.Types[types.TINT64] + lenType := types.Types[types.TINT64] // Type checking guarantees that TIDEAL len/cap are positive and fit in an int. // The case of len or cap overflow when converting TUINT or TUINTPTR to TINT // will be handled by the negative range checks in unsafeslice during runtime. - if len.Type().IsKind(types.TIDEAL) || len.Type().Size() <= types.Types[types.TUINT].Size() { + if ir.ShouldCheckPtr(ir.CurFunc, 1) { + fnname = "unsafeslicecheckptr" + // for simplicity, unsafeslicecheckptr always uses int64 + } else if len.Type().IsKind(types.TIDEAL) || len.Type().Size() <= types.Types[types.TUINT].Size() { fnname = "unsafeslice" - argtype = types.Types[types.TINT] + lenType = types.Types[types.TINT] } t := n.Type() - // Call runtime.unsafeslice[64] to check that the length argument is - // non-negative and smaller than the max length allowed for the - // element type. + // Call runtime.unsafeslice{,64,checkptr} to check ptr and len. fn := typecheck.LookupRuntime(fnname) - init.Append(mkcall1(fn, nil, init, reflectdata.TypePtr(t.Elem()), typecheck.Conv(len, argtype))) - - ptr := walkExpr(n.X, init) - - c := ir.NewUnaryExpr(n.Pos(), ir.OCHECKNIL, ptr) - c.SetTypecheck(1) - init.Append(c) - - // TODO(mdempsky): checkptr instrumentation. Maybe merge into length - // check above, along with nil check? Need to be careful about - // notinheap pointers though: can't pass them as unsafe.Pointer. + init.Append(mkcall1(fn, nil, init, reflectdata.TypePtr(t.Elem()), typecheck.Conv(ptr, types.Types[types.TUNSAFEPTR]), typecheck.Conv(len, lenType))) h := ir.NewSliceHeaderExpr(n.Pos(), t, typecheck.Conv(ptr, types.Types[types.TUNSAFEPTR]), diff --git a/src/runtime/checkptr.go b/src/runtime/checkptr.go index 59891a06a5..d42950844b 100644 --- a/src/runtime/checkptr.go +++ b/src/runtime/checkptr.go @@ -16,11 +16,30 @@ func checkptrAlignment(p unsafe.Pointer, elem *_type, n uintptr) { } // Check that (*[n]elem)(p) doesn't straddle multiple heap objects. - if size := n * elem.size; size > 1 && checkptrBase(p) != checkptrBase(add(p, size-1)) { + // TODO(mdempsky): Fix #46938 so we don't need to worry about overflow here. + if checkptrStraddles(p, n*elem.size) { throw("checkptr: converted pointer straddles multiple allocations") } } +// checkptrStraddles reports whether the first size-bytes of memory +// addressed by ptr is known to straddle more than one Go allocation. +func checkptrStraddles(ptr unsafe.Pointer, size uintptr) bool { + if size <= 1 { + return false + } + + end := add(ptr, size-1) + if uintptr(end) < uintptr(ptr) { + return true + } + + // TODO(mdempsky): Detect when [ptr, end] contains Go allocations, + // but neither ptr nor end point into one themselves. + + return checkptrBase(ptr) != checkptrBase(end) +} + func checkptrArithmetic(p unsafe.Pointer, originals []unsafe.Pointer) { if 0 < uintptr(p) && uintptr(p) < minLegalPointer { throw("checkptr: pointer arithmetic computed bad pointer value") diff --git a/src/runtime/checkptr_test.go b/src/runtime/checkptr_test.go index 194cc1243a..2a5c364e97 100644 --- a/src/runtime/checkptr_test.go +++ b/src/runtime/checkptr_test.go @@ -30,6 +30,8 @@ func TestCheckPtr(t *testing.T) { {"CheckPtrArithmetic2", "fatal error: checkptr: pointer arithmetic result points to invalid allocation\n"}, {"CheckPtrSize", "fatal error: checkptr: converted pointer straddles multiple allocations\n"}, {"CheckPtrSmall", "fatal error: checkptr: pointer arithmetic computed bad pointer value\n"}, + {"CheckPtrSliceOK", ""}, + {"CheckPtrSliceFail", "fatal error: checkptr: unsafe.Slice result straddles multiple allocations\n"}, } for _, tc := range testCases { diff --git a/src/runtime/slice.go b/src/runtime/slice.go index f9d4154acf..01cdcaeee3 100644 --- a/src/runtime/slice.go +++ b/src/runtime/slice.go @@ -112,19 +112,37 @@ func makeslice64(et *_type, len64, cap64 int64) unsafe.Pointer { return makeslice(et, len, cap) } -func unsafeslice(et *_type, len int) { +func unsafeslice(et *_type, ptr unsafe.Pointer, len int) { + if len == 0 { + return + } + + if ptr == nil { + panic(errorString("unsafe.Slice: ptr is nil and len is not zero")) + } + mem, overflow := math.MulUintptr(et.size, uintptr(len)) if overflow || mem > maxAlloc || len < 0 { panicunsafeslicelen() } } -func unsafeslice64(et *_type, len64 int64) { +func unsafeslice64(et *_type, ptr unsafe.Pointer, len64 int64) { len := int(len64) if int64(len) != len64 { panicunsafeslicelen() } - unsafeslice(et, len) + unsafeslice(et, ptr, len) +} + +func unsafeslicecheckptr(et *_type, ptr unsafe.Pointer, len64 int64) { + unsafeslice64(et, ptr, len64) + + // Check that underlying array doesn't straddle multiple heap objects. + // unsafeslice64 has already checked for overflow. + if checkptrStraddles(ptr, uintptr(len64)*et.size) { + throw("checkptr: unsafe.Slice result straddles multiple allocations") + } } func panicunsafeslicelen() { diff --git a/src/runtime/testdata/testprog/checkptr.go b/src/runtime/testdata/testprog/checkptr.go index e0a2794f4c..f76b64ad96 100644 --- a/src/runtime/testdata/testprog/checkptr.go +++ b/src/runtime/testdata/testprog/checkptr.go @@ -13,6 +13,8 @@ func init() { register("CheckPtrArithmetic2", CheckPtrArithmetic2) register("CheckPtrSize", CheckPtrSize) register("CheckPtrSmall", CheckPtrSmall) + register("CheckPtrSliceOK", CheckPtrSliceOK) + register("CheckPtrSliceFail", CheckPtrSliceFail) } func CheckPtrAlignmentNoPtr() { @@ -49,3 +51,14 @@ func CheckPtrSize() { func CheckPtrSmall() { sink2 = unsafe.Pointer(uintptr(1)) } + +func CheckPtrSliceOK() { + p := new([4]int64) + sink2 = unsafe.Slice(&p[1], 3) +} + +func CheckPtrSliceFail() { + p := new(int64) + sink2 = p + sink2 = unsafe.Slice(p, 100) +} diff --git a/test/unsafebuiltins.go b/test/unsafebuiltins.go index c10f8084a7..4c940aa855 100644 --- a/test/unsafebuiltins.go +++ b/test/unsafebuiltins.go @@ -30,8 +30,11 @@ func main() { assert(len(s) == len(p)) assert(cap(s) == len(p)) - // nil pointer - mustPanic(func() { _ = unsafe.Slice((*int)(nil), 0) }) + // nil pointer with zero length returns nil + assert(unsafe.Slice((*int)(nil), 0) == nil) + + // nil pointer with positive length panics + mustPanic(func() { _ = unsafe.Slice((*int)(nil), 1) }) // negative length var neg int = -1 From e2e05af6e189131162b533184eb04de5d597d544 Mon Sep 17 00:00:00 2001 From: eric fang Date: Mon, 21 Jun 2021 02:11:25 +0000 Subject: [PATCH 13/24] cmd/internal/obj/arm64: fix an encoding error of CMPW instruction For arm64 CMP, ADD and other similar extended register instructions, if there is no extension, the default extion is LSL<<0, but the default encoding value (the value of 'option' field) of 32-bit instruction and 64-bit instruction is different, 32-bit is 2 and 64-bit is 3. But the current assembler incorrectly encodes the value of 32-bit instruction to 3. This CL fixes this error. Change-Id: I0e09af2c9c5047a4ed2db7d1183290283db9c31c Reviewed-on: https://go-review.googlesource.com/c/go/+/329749 Reviewed-by: eric fang Reviewed-by: Cherry Mui Run-TryBot: eric fang Run-TryBot: Cherry Mui Trust: Dmitri Shuralyov --- src/cmd/asm/internal/asm/testdata/arm64.s | 3 ++- src/cmd/internal/obj/arm64/asm7.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cmd/asm/internal/asm/testdata/arm64.s b/src/cmd/asm/internal/asm/testdata/arm64.s index 5f1e68545b..d8a20edfc1 100644 --- a/src/cmd/asm/internal/asm/testdata/arm64.s +++ b/src/cmd/asm/internal/asm/testdata/arm64.s @@ -89,7 +89,7 @@ TEXT foo(SB), DUPOK|NOSPLIT, $-8 CMP R1<<33, R2 CMP R22.SXTX, RSP // ffe336eb CMP $0x22220000, RSP // CMP $572653568, RSP // 5b44a4d2ff633beb - CMPW $0x22220000, RSP // CMPW $572653568, RSP // 5b44a452ff633b6b + CMPW $0x22220000, RSP // CMPW $572653568, RSP // 5b44a452ff433b6b CCMN MI, ZR, R1, $4 // e44341ba // MADD Rn,Rm,Ra,Rd MADD R1, R2, R3, R4 // 6408019b @@ -377,6 +377,7 @@ TEXT foo(SB), DUPOK|NOSPLIT, $-8 MOVD $0x1000100010001000, RSP // MOVD $1152939097061330944, RSP // ff8304b2 MOVW $0x10001000, RSP // MOVW $268439552, RSP // ff830432 ADDW $0x10001000, R1 // ADDW $268439552, R1 // fb83043221001b0b + ADDW $0x22220000, RSP, R3 // ADDW $572653568, RSP, R3 // 5b44a452e3433b0b // move a large constant to a Vd. VMOVS $0x80402010, V11 // VMOVS $2151686160, V11 diff --git a/src/cmd/internal/obj/arm64/asm7.go b/src/cmd/internal/obj/arm64/asm7.go index b8c3cd97c7..d99afa3d27 100644 --- a/src/cmd/internal/obj/arm64/asm7.go +++ b/src/cmd/internal/obj/arm64/asm7.go @@ -4333,8 +4333,10 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { if p.To.Reg == REG_RSP && isADDSop(p.As) { c.ctxt.Diag("illegal destination register: %v\n", p) } + lsl0 := LSL0_64 if isADDWop(p.As) || isANDWop(p.As) { o1 = c.omovconst(AMOVW, p, &p.From, REGTMP) + lsl0 = LSL0_32 } else { o1 = c.omovconst(AMOVD, p, &p.From, REGTMP) } @@ -4350,7 +4352,7 @@ func (c *ctxt7) asmout(p *obj.Prog, o *Optab, out []uint32) { if p.To.Reg == REGSP || r == REGSP { o2 = c.opxrrr(p, p.As, false) o2 |= REGTMP & 31 << 16 - o2 |= LSL0_64 + o2 |= uint32(lsl0) } else { o2 = c.oprrr(p, p.As) o2 |= REGTMP & 31 << 16 /* shift is 0 */ From fd4b587da3f9a2bde193a5b9fd2ba96667e08f2d Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Tue, 29 Jun 2021 10:11:31 +0700 Subject: [PATCH 14/24] cmd/compile: suppress details error for invalid variadic argument type CL 255241 made error message involving variadic calls clearer. To do it, we added a check that the type of variadic argument must be a slice. That's why the compiler crashes for invalid variadic argument type. Instead, we can just omit the details error message, and report not enough arguments error, which matches the behavior of go/types and types2. Fixes #46957 Change-Id: I638d7e8f031f0ee344d5d802104fd93a60aae00a Reviewed-on: https://go-review.googlesource.com/c/go/+/331569 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/typecheck/typecheck.go | 17 ++++++++++++----- test/fixedbugs/issue46957.go | 13 +++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 test/fixedbugs/issue46957.go diff --git a/src/cmd/compile/internal/typecheck/typecheck.go b/src/cmd/compile/internal/typecheck/typecheck.go index bf52941b2c..359f662369 100644 --- a/src/cmd/compile/internal/typecheck/typecheck.go +++ b/src/cmd/compile/internal/typecheck/typecheck.go @@ -1460,15 +1460,22 @@ toomany: } func errorDetails(nl ir.Nodes, tstruct *types.Type, isddd bool) string { - // If we don't know any type at a call site, let's suppress any return - // message signatures. See Issue https://golang.org/issues/19012. + // Suppress any return message signatures if: + // + // (1) We don't know any type at a call site (see #19012). + // (2) Any node has an unknown type. + // (3) Invalid type for variadic parameter (see #46957). if tstruct == nil { - return "" + return "" // case 1 } - // If any node has an unknown type, suppress it as well + + if isddd && !nl[len(nl)-1].Type().IsSlice() { + return "" // case 3 + } + for _, n := range nl { if n.Type() == nil { - return "" + return "" // case 2 } } return fmt.Sprintf("\n\thave %s\n\twant %v", fmtSignature(nl, isddd), tstruct) diff --git a/test/fixedbugs/issue46957.go b/test/fixedbugs/issue46957.go new file mode 100644 index 0000000000..f3ed3c3def --- /dev/null +++ b/test/fixedbugs/issue46957.go @@ -0,0 +1,13 @@ +// errorcheck + +// 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 + +func f(a int, b ...int) {} + +func main() { + f(nil...) // ERROR "not enough arguments in call to f$" +} From 3463852b7631adbdd65646539fc87d967dcd13e6 Mon Sep 17 00:00:00 2001 From: tkawakita Date: Wed, 30 Jun 2021 00:46:05 +0900 Subject: [PATCH 15/24] math/big: fix typo of comment (`BytesScanner` to `ByteScanner`) Change-Id: I0c2d26d6ede1452008992efbea7392162da65014 Reviewed-on: https://go-review.googlesource.com/c/go/+/331651 Reviewed-by: Robert Griesemer Reviewed-by: Ian Lance Taylor --- src/math/big/int.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/math/big/int.go b/src/math/big/int.go index 65f32487b5..7647346486 100644 --- a/src/math/big/int.go +++ b/src/math/big/int.go @@ -425,7 +425,7 @@ func (z *Int) SetString(s string, base int) (*Int, bool) { return z.setFromScanner(strings.NewReader(s), base) } -// setFromScanner implements SetString given an io.BytesScanner. +// setFromScanner implements SetString given an io.ByteScanner. // For documentation see comments of SetString. func (z *Int) setFromScanner(r io.ByteScanner, base int) (*Int, bool) { if _, _, err := z.scan(r, base); err != nil { From e294b8a49e5ff157041b4ac6c3c3413dafd13673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Levi=EF=BC=88=E3=83=AA=E3=83=BC=E3=83=90=E3=82=A4=EF=BC=89?= <19405788+lescoggi@users.noreply.github.com> Date: Tue, 29 Jun 2021 12:45:52 +0000 Subject: [PATCH 16/24] doc/go1.17: fix typo "MacOS" -> "macOS" Change-Id: Ie2ada2bf875a93b1cc9e86a81c8a25de39ce4752 GitHub-Last-Rev: 462753db015949eb88c6c4e64b6aae1a49ac89b4 GitHub-Pull-Request: golang/go#46962 Reviewed-on: https://go-review.googlesource.com/c/go/+/331589 Reviewed-by: Ian Lance Taylor Reviewed-by: Dmitri Shuralyov --- 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 22896c8c27..3551ba46c8 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -401,7 +401,7 @@ func Foo() bool {

Go 1.17 implements a new way of passing function arguments and results using - registers instead of the stack. This work is enabled for Linux, MacOS, and + registers instead of the stack. This work is enabled for Linux, macOS, and Windows on the 64-bit x86 architecture (the linux/amd64, darwin/amd64, windows/amd64 ports). For a representative set of Go packages and programs, benchmarking has shown From f9d50953b94c15936a72a39a205b3d72ea6dee41 Mon Sep 17 00:00:00 2001 From: Xiangdong Ji Date: Mon, 28 Jun 2021 20:27:30 +0800 Subject: [PATCH 17/24] net: fix failure of TestCVE202133195 TestCVE202133195 fails in testing LookupSRV if /etc/resolv.conf sets the option 'ndots' larger than the number of dots in the domain name under query. Fix the issue by making the input domain name in test codes 'rooted' to skip search list qualifying. Fixes #46955 Change-Id: I1909fa7e54e9c9af57623e57cafc905729ff99fa Reviewed-on: https://go-review.googlesource.com/c/go/+/330842 Reviewed-by: Damien Neil Run-TryBot: Damien Neil TryBot-Result: Go Bot Trust: Dmitri Shuralyov --- src/net/dnsclient_unix_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go index a59be7fea0..d69107a2f2 100644 --- a/src/net/dnsclient_unix_test.go +++ b/src/net/dnsclient_unix_test.go @@ -1921,12 +1921,12 @@ func TestCVE202133195(t *testing.T) { 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 { + _, _, 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, expected) } - _, _, err = LookupSRV("hdr", "tcp", "golang.org") - if expected := "lookup golang.org: SRV header name is invalid"; err == nil || err.Error() != 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, expected) } From c45e800e0cb237fcedc9a3e4fd243e3a7f47334c Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 29 Jun 2021 09:20:04 -0700 Subject: [PATCH 18/24] crypto/x509: don't fail on optional auth key id fields If a certificate contains an AuthorityKeyIdentifier extension that lacks the keyIdentifier field, but contains the authorityCertIssuer and/or the authorityCertSerialNumber fields, don't return an error and continue parsing. Fixes #46854 Change-Id: I82739b415441f639a722755cc1f449d73078adfc Reviewed-on: https://go-review.googlesource.com/c/go/+/331689 Trust: Roland Shoemaker Run-TryBot: Roland Shoemaker TryBot-Result: Go Bot Reviewed-by: Filippo Valsorda --- src/crypto/x509/parser.go | 8 ++++--- src/crypto/x509/x509_test.go | 42 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go index 3d51ddd7f5..f085162a4e 100644 --- a/src/crypto/x509/parser.go +++ b/src/crypto/x509/parser.go @@ -734,10 +734,12 @@ func processExtensions(out *Certificate) error { if !val.ReadASN1(&akid, cryptobyte_asn1.SEQUENCE) { return errors.New("x509: invalid authority key identifier") } - if !akid.ReadASN1(&akid, cryptobyte_asn1.Tag(0).ContextSpecific()) { - return errors.New("x509: invalid authority key identifier") + if akid.PeekASN1Tag(cryptobyte_asn1.Tag(0).ContextSpecific()) { + if !akid.ReadASN1(&akid, cryptobyte_asn1.Tag(0).ContextSpecific()) { + return errors.New("x509: invalid authority key identifier") + } + out.AuthorityKeyId = akid } - out.AuthorityKeyId = akid case 37: out.ExtKeyUsage, out.UnknownExtKeyUsage, err = parseExtKeyUsageExtension(e.Value) if err != nil { diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index 802bce0f9a..a4053abf41 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -3174,3 +3174,45 @@ func TestSigAlgMismatch(t *testing.T) { } } } + +const optionalAuthKeyIDPEM = `-----BEGIN CERTIFICATE----- +MIIFEjCCBHugAwIBAgICAQwwDQYJKoZIhvcNAQEFBQAwgbsxJDAiBgNVBAcTG1Zh +bGlDZXJ0IFZhbGlkYXRpb24gTmV0d29yazEXMBUGA1UEChMOVmFsaUNlcnQsIElu +Yy4xNTAzBgNVBAsTLFZhbGlDZXJ0IENsYXNzIDIgUG9saWN5IFZhbGlkYXRpb24g +QXV0aG9yaXR5MSEwHwYDVQQDExhodHRwOi8vd3d3LnZhbGljZXJ0LmNvbS8xIDAe +BgkqhkiG9w0BCQEWEWluZm9AdmFsaWNlcnQuY29tMB4XDTA0MDYyOTE3MzkxNloX +DTI0MDYyOTE3MzkxNlowaDELMAkGA1UEBhMCVVMxJTAjBgNVBAoTHFN0YXJmaWVs +ZCBUZWNobm9sb2dpZXMsIEluYy4xMjAwBgNVBAsTKVN0YXJmaWVsZCBDbGFzcyAy +IENlcnRpZmljYXRpb24gQXV0aG9yaXR5MIIBIDANBgkqhkiG9w0BAQEFAAOCAQ0A +MIIBCAKCAQEAtzLI/ulxpgSFrQwRZN/OTe/IAxiHP6Gr+zymn/DDodrU2G4rU5D7 +JKQ+hPCe6F/s5SdE9SimP3ve4CrwyK9TL57KBQGTHo9mHDmnTfpatnMEJWbrd3/n +WcZKmSUUVOsmx/N/GdUwcI+vsEYq/63rKe3Xn6oEh6PU+YmlNF/bQ5GCNtlmPLG4 +uYL9nDo+EMg77wZlZnqbGRg9/3FRPDAuX749d3OyXQZswyNWmiuFJpIcpwKz5D8N +rwh5grg2Peqc0zWzvGnK9cyd6P1kjReAM25eSl2ZyR6HtJ0awNVuEzUjXt+bXz3v +1vd2wuo+u3gNHEJnawTY+Nbab4vyRKABqwIBA6OCAfMwggHvMB0GA1UdDgQWBBS/ +X7fRzt0fhvRbVazc1xDCDqmI5zCB0gYDVR0jBIHKMIHHoYHBpIG+MIG7MSQwIgYD +VQQHExtWYWxpQ2VydCBWYWxpZGF0aW9uIE5ldHdvcmsxFzAVBgNVBAoTDlZhbGlD +ZXJ0LCBJbmMuMTUwMwYDVQQLEyxWYWxpQ2VydCBDbGFzcyAyIFBvbGljeSBWYWxp +ZGF0aW9uIEF1dGhvcml0eTEhMB8GA1UEAxMYaHR0cDovL3d3dy52YWxpY2VydC5j +b20vMSAwHgYJKoZIhvcNAQkBFhFpbmZvQHZhbGljZXJ0LmNvbYIBATAPBgNVHRMB +Af8EBTADAQH/MDkGCCsGAQUFBwEBBC0wKzApBggrBgEFBQcwAYYdaHR0cDovL29j +c3Auc3RhcmZpZWxkdGVjaC5jb20wSgYDVR0fBEMwQTA/oD2gO4Y5aHR0cDovL2Nl +cnRpZmljYXRlcy5zdGFyZmllbGR0ZWNoLmNvbS9yZXBvc2l0b3J5L3Jvb3QuY3Js +MFEGA1UdIARKMEgwRgYEVR0gADA+MDwGCCsGAQUFBwIBFjBodHRwOi8vY2VydGlm +aWNhdGVzLnN0YXJmaWVsZHRlY2guY29tL3JlcG9zaXRvcnkwDgYDVR0PAQH/BAQD +AgEGMA0GCSqGSIb3DQEBBQUAA4GBAKVi8afCXSWlcD284ipxs33kDTcdVWptobCr +mADkhWBKIMuh8D1195TaQ39oXCUIuNJ9MxB73HZn8bjhU3zhxoNbKXuNSm8uf0So +GkVrMgfHeMpkksK0hAzc3S1fTbvdiuo43NlmouxBulVtWmQ9twPMHOKRUJ7jCUSV +FxdzPcwl +-----END CERTIFICATE-----` + +func TestAuthKeyIdOptional(t *testing.T) { + b, _ := pem.Decode([]byte(optionalAuthKeyIDPEM)) + if b == nil { + t.Fatalf("couldn't decode test certificate") + } + _, err := ParseCertificate(b.Bytes) + if err != nil { + t.Fatalf("ParseCertificate to failed to parse certificate with optional authority key identifier fields: %s", err) + } +} From d19a53338fa6272b4fe9c39d66812a79e1464cd2 Mon Sep 17 00:00:00 2001 From: Nigel Tao Date: Tue, 29 Jun 2021 16:53:44 +1000 Subject: [PATCH 19/24] image: add Uniform.RGBA64At and Rectangle.RGBA64At These types already implemented the Image interface. They should also implement the RGBA64Image interface (new in Go 1.17) Updates #44808 Change-Id: I9a2b13e305997088ae874efb95ad9e1648f94812 Reviewed-on: https://go-review.googlesource.com/c/go/+/331570 Trust: Nigel Tao Run-TryBot: Nigel Tao TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- api/go1.17.txt | 2 ++ doc/go1.17.html | 4 ++-- src/image/geom.go | 8 ++++++++ src/image/image_test.go | 12 ++++++++++++ src/image/names.go | 5 +++++ 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/api/go1.17.txt b/api/go1.17.txt index c5eb381708..8e4c0f5624 100644 --- a/api/go1.17.txt +++ b/api/go1.17.txt @@ -47,7 +47,9 @@ pkg image, method (*Paletted) RGBA64At(int, int) color.RGBA64 pkg image, method (*Paletted) SetRGBA64(int, int, color.RGBA64) pkg image, method (*RGBA) RGBA64At(int, int) color.RGBA64 pkg image, method (*RGBA) SetRGBA64(int, int, color.RGBA64) +pkg image, method (*Uniform) RGBA64At(int, int) color.RGBA64 pkg image, method (*YCbCr) RGBA64At(int, int) color.RGBA64 +pkg image, method (Rectangle) RGBA64At(int, int) color.RGBA64 pkg image, type RGBA64Image interface { At, Bounds, ColorModel, RGBA64At } pkg image, type RGBA64Image interface, At(int, int) color.Color pkg image, type RGBA64Image interface, Bounds() Rectangle diff --git a/doc/go1.17.html b/doc/go1.17.html index 3551ba46c8..b72752d77d 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -811,8 +811,8 @@ func Foo() bool {

The concrete image types (RGBA, Gray16 and so on) now implement a new RGBA64Image - interface. Those concrete types, other than the chroma-subsampling - related YCbCr and NYCbCrA, also now implement + interface. The concrete types that previously implemented + draw.Image now also implement draw.RGBA64Image, a new interface in the image/draw package.

diff --git a/src/image/geom.go b/src/image/geom.go index 78e9e49d4f..e71aa61187 100644 --- a/src/image/geom.go +++ b/src/image/geom.go @@ -246,6 +246,14 @@ func (r Rectangle) At(x, y int) color.Color { return color.Transparent } +// RGBA64At implements the RGBA64Image interface. +func (r Rectangle) RGBA64At(x, y int) color.RGBA64 { + if (Point{x, y}).In(r) { + return color.RGBA64{0xffff, 0xffff, 0xffff, 0xffff} + } + return color.RGBA64{} +} + // Bounds implements the Image interface. func (r Rectangle) Bounds() Rectangle { return r diff --git a/src/image/image_test.go b/src/image/image_test.go index c64b6107b7..7f41bcb6c7 100644 --- a/src/image/image_test.go +++ b/src/image/image_test.go @@ -213,7 +213,9 @@ func TestRGBA64Image(t *testing.T) { NewPaletted(r, palette.Plan9), NewRGBA(r), NewRGBA64(r), + NewUniform(color.RGBA64{}), NewYCbCr(r, YCbCrSubsampleRatio444), + r, } for _, tc := range testCases { switch tc := tc.(type) { @@ -226,6 +228,9 @@ func TestRGBA64Image(t *testing.T) { // means that setting one pixel can modify neighboring pixels. They // don't have Set or SetRGBA64 methods because that side effect could // be surprising. Here, we just memset the channel buffers instead. + // + // The Uniform and Rectangle types are also special-cased, as they + // don't have a Set or SetRGBA64 method. case interface { SetRGBA64(x, y int, c color.RGBA64) }: @@ -237,11 +242,18 @@ func TestRGBA64Image(t *testing.T) { memset(tc.YCbCr.Cr, 0x99) memset(tc.A, 0xAA) + case *Uniform: + tc.C = color.RGBA64{0x7FFF, 0x3FFF, 0x0000, 0x7FFF} + case *YCbCr: memset(tc.Y, 0x77) memset(tc.Cb, 0x88) memset(tc.Cr, 0x99) + case Rectangle: + // No-op. Rectangle pixels' colors are immutable. They're always + // color.Opaque. + default: t.Errorf("could not initialize pixels for %T", tc) continue diff --git a/src/image/names.go b/src/image/names.go index 8595a35014..17b06588ac 100644 --- a/src/image/names.go +++ b/src/image/names.go @@ -41,6 +41,11 @@ func (c *Uniform) Bounds() Rectangle { return Rectangle{Point{-1e9, -1e9}, Point func (c *Uniform) At(x, y int) color.Color { return c.C } +func (c *Uniform) RGBA64At(x, y int) color.RGBA64 { + r, g, b, a := c.C.RGBA() + return color.RGBA64{uint16(r), uint16(g), uint16(b), uint16(a)} +} + // Opaque scans the entire image and reports whether it is fully opaque. func (c *Uniform) Opaque() bool { _, _, _, a := c.C.RGBA() From 0fa3265fe14c775668fc8272f47adf4fbaa60bac Mon Sep 17 00:00:00 2001 From: Eli Bendersky Date: Tue, 29 Jun 2021 16:31:18 -0700 Subject: [PATCH 20/24] os: change example to avoid deprecated function The IsNotExist function is deprecated; change package example to avoid it and use the recommended way instead. Fixes #46976 Change-Id: I3c301d0a89b6bda42184df314ba8418062ca39ee Reviewed-on: https://go-review.googlesource.com/c/go/+/331692 Trust: Dmitri Shuralyov Reviewed-by: Ian Lance Taylor Reviewed-by: Robert Griesemer --- src/os/example_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/os/example_test.go b/src/os/example_test.go index 3adce51784..e8554b0b12 100644 --- a/src/os/example_test.go +++ b/src/os/example_test.go @@ -5,6 +5,7 @@ package os_test import ( + "errors" "fmt" "io/fs" "log" @@ -71,9 +72,9 @@ func ExampleFileMode() { } } -func ExampleIsNotExist() { +func ExampleErrNotExist() { filename := "a-nonexistent-file" - if _, err := os.Stat(filename); os.IsNotExist(err) { + if _, err := os.Stat(filename); errors.Is(err, fs.ErrNotExist) { fmt.Println("file does not exist") } // Output: From 7d0e9e6e74b45cf658257363151a79baf030033f Mon Sep 17 00:00:00 2001 From: uji Date: Wed, 30 Jun 2021 00:06:23 +0900 Subject: [PATCH 21/24] image/gif: fix typo in the comment (io.ReadByte -> io.ByteReader) Fixes #46967 Change-Id: I66e69c70b74e904623e8ca854562d255692b2143 Reviewed-on: https://go-review.googlesource.com/c/go/+/331649 Reviewed-by: Ian Lance Taylor Trust: Carlos Amedee --- src/image/gif/reader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/image/gif/reader.go b/src/image/gif/reader.go index e580ab049e..9e8268c86f 100644 --- a/src/image/gif/reader.go +++ b/src/image/gif/reader.go @@ -116,7 +116,7 @@ type decoder struct { // consumed when checking that the blockReader is exhausted. // // To avoid the allocation of a bufio.Reader for the lzw Reader, blockReader -// implements io.ReadByte and buffers blocks into the decoder's "tmp" buffer. +// implements io.ByteReader and buffers blocks into the decoder's "tmp" buffer. type blockReader struct { d *decoder i, j uint8 // d.tmp[i:j] contains the buffered bytes From c080d0323bce56e25622a51dffecf756758c95a1 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 30 Jun 2021 10:19:02 -0700 Subject: [PATCH 22/24] cmd/dist: pass -Wno-unknown-warning-option in swig_callback_lto For #46557 Fixes #46991 Change-Id: Ic88ebaa13d3edf904657dc19ada4fd4ff7f44a8f Reviewed-on: https://go-review.googlesource.com/c/go/+/332010 Trust: Ian Lance Taylor Trust: Josh Bleecher Snyder Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Josh Bleecher Snyder --- 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 f2c4cf0b43..4acd357974 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 -Wno-lto-type-mismatch", - "CGO_CXXFLAGS=-flto -Wno-lto-type-mismatch", - "CGO_LDFLAGS=-flto -Wno-lto-type-mismatch", + "CGO_CFLAGS=-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option", + "CGO_CXXFLAGS=-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option", + "CGO_LDFLAGS=-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option", ) return nil }, From ed56ea73e8aa60269bbb3d33af9e7614e6b3babf Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Wed, 30 Jun 2021 09:44:30 -0700 Subject: [PATCH 23/24] path/filepath: deflake TestEvalSymlinksAboveRoot on darwin On darwin, under load, it appears that the system occasionally deletes the temp dir mid-test. Don't fail the test when that happens. It would be nice to fix this in a deeper way. See golang.org/cl/332009 for some discussion. In the meantime, this will at least stop the flakiness. Updates #37910 Change-Id: I6669e466fed9abda4a87ca88345c04cd7986b41e Reviewed-on: https://go-review.googlesource.com/c/go/+/332009 Trust: Josh Bleecher Snyder Run-TryBot: Josh Bleecher Snyder TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills --- src/path/filepath/path_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index cd107b6c85..bc5509b49c 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -1469,11 +1469,16 @@ func TestEvalSymlinksAboveRoot(t *testing.T) { // Try different numbers of "..". for _, i := range []int{c, c + 1, c + 2} { check := strings.Join([]string{evalTmpDir, strings.Join(dd[:i], string(os.PathSeparator)), evalTmpDir[len(vol)+1:], "b", "file"}, string(os.PathSeparator)) - if resolved, err := filepath.EvalSymlinks(check); err != nil { + resolved, err := filepath.EvalSymlinks(check) + switch { + case runtime.GOOS == "darwin" && errors.Is(err, fs.ErrNotExist): + // On darwin, the temp dir is sometimes cleaned up mid-test (issue 37910). + testenv.SkipFlaky(t, 37910) + case err != nil: t.Errorf("EvalSymlinks(%q) failed: %v", check, err) - } else if !strings.HasSuffix(resolved, wantSuffix) { + case !strings.HasSuffix(resolved, wantSuffix): t.Errorf("EvalSymlinks(%q) = %q does not end with %q", check, resolved, wantSuffix) - } else { + default: t.Logf("EvalSymlinks(%q) = %q", check, resolved) } } From 4711bf30e5fec4cf290964785871deba5955c29a Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 30 Jun 2021 11:54:46 -0400 Subject: [PATCH 24/24] doc/go1.17: linkify "language changes" in the runtime section Change-Id: I82bd3954bfc5da59c7952eba2a28ff0e3b41427f Reviewed-on: https://go-review.googlesource.com/c/go/+/331969 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov --- 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 b72752d77d..66b4f48b61 100644 --- a/doc/go1.17.html +++ b/doc/go1.17.html @@ -993,7 +993,7 @@ func Foo() bool { is no longer sufficient to guarantee that a call to Value.Convert will not panic. It may panic when converting `[]T` to `*[N]T` if the slice's length is less than N. - See the language changes section above. + See the language changes section above.