src/cmd/compile/internal/ssa/prove.go | 5 ++++- src/cmd/go.mod | 2 +- src/cmd/go.sum | 4 ++-- src/cmd/go/internal/test/test.go | 4 ++-- src/cmd/go/internal/vet/vet.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++--- src/cmd/go/internal/work/action.go | 15 +++++++++------ src/cmd/go/internal/work/exec.go | 83 ++++++++++++++++++++++++++++++++++++++++------------- src/cmd/go/testdata/script/test_fuzz.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz.txt | 0 src/cmd/go/testdata/script/test_fuzz_cache.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_cache.txt | 0 src/cmd/go/testdata/script/test_fuzz_cgo.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_cgo.txt | 0 src/cmd/go/testdata/script/test_fuzz_chatty.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_chatty.txt | 0 src/cmd/go/testdata/script/test_fuzz_cleanup.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_cleanup.txt | 0 src/cmd/go/testdata/script/test_fuzz_context.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_context.txt | 0 src/cmd/go/testdata/script/test_fuzz_cov.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_cov.txt | 0 src/cmd/go/testdata/script/test_fuzz_deadline.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_deadline.txt | 0 src/cmd/go/testdata/script/test_fuzz_dup_cache.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_dup_cache.txt | 0 src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_err_deadlock.txt | 0 src/cmd/go/testdata/script/test_fuzz_fuzztime.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_fuzztime.txt | 0 src/cmd/go/testdata/script/test_fuzz_io_error.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_io_error.txt | 0 src/cmd/go/testdata/script/test_fuzz_limit_dup_entry.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_limit_dup_entry.txt | 0 src/cmd/go/testdata/script/test_fuzz_match.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_match.txt | 0 src/cmd/go/testdata/script/test_fuzz_minimize.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize.txt | 0 src/cmd/go/testdata/script/test_fuzz_minimize_dirty_cov.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize_dirty_cov.txt | 0 src/cmd/go/testdata/script/test_fuzz_minimize_interesting.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize_interesting.txt | 0 src/cmd/go/testdata/script/test_fuzz_multiple.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_multiple.txt | 0 src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutate_crash.txt | 0 src/cmd/go/testdata/script/test_fuzz_mutate_fail.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutate_fail.txt | 0 src/cmd/go/testdata/script/test_fuzz_mutator.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutator.txt | 0 src/cmd/go/testdata/script/test_fuzz_mutator_repeat.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutator_repeat.txt | 0 src/cmd/go/testdata/script/test_fuzz_non_crash_signal.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_non_crash_signal.txt | 0 src/cmd/go/testdata/script/test_fuzz_parallel.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_parallel.txt | 0 src/cmd/go/testdata/script/test_fuzz_profile_flags.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_profile_flags.txt | 0 src/cmd/go/testdata/script/test_fuzz_return.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_return.txt | 0 src/cmd/go/testdata/script/test_fuzz_run.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_run.txt | 0 src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_seed_corpus.txt | 0 src/cmd/go/testdata/script/test_fuzz_setenv.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_setenv.txt | 0 src/cmd/go/testdata/script/test_fuzz_test_race.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_test_race.txt | 0 src/cmd/go/testdata/script/test_fuzz_unsupported.txt => src/cmd/internal/fuzztest/testdata/script/test_fuzz_unsupported.txt | 0 src/cmd/go/testdata/script/vet_basic.txt | 1 + src/cmd/go/testdata/script/vet_cache.txt | 2 ++ src/cmd/internal/fuzztest/script_test.go | 22 ++++++++++++++++++++++ src/cmd/internal/fuzztest/testdata/script/README | 286 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/cmd/internal/script/scripttest/run.go | 3 +++ src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/doc.go | 13 ++++++++++--- src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/omitzero.go | 152 +++++++++++++++++++++++++++++++---------------------- src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/stringscut.go | 128 +++++++++++++++++++++++++++++------------------------ src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go | 41 ++++++++++++++++++++++++++++++++++++++--- src/cmd/vendor/golang.org/x/tools/internal/analysis/driverutil/fix.go | 13 +++++++------ src/cmd/vendor/golang.org/x/tools/internal/refactor/inline/callee.go | 44 ++++++++++++++++++++++++++++++++++++++++++-- src/cmd/vendor/modules.txt | 2 +- src/crypto/md5/gen.go | 2 +- src/crypto/tls/ticket.go | 2 +- src/encoding/gob/decgen.go | 4 ++-- src/go/ast/walk.go | 2 +- src/internal/runtime/maps/table.go | 4 +++- src/internal/runtime/sys/dit_arm64.s | 5 +++++ src/net/http/cookie.go | 3 ++- src/runtime/crash_test.go | 9 ++++++++- src/runtime/export_test.go | 7 +++++-- src/runtime/goroutineleakprofile_test.go | 32 ++++++++++++++++---------------- src/runtime/malloc_test.go | 18 ++++++++++++++++-- src/runtime/metrics_test.go | 15 +++++++++++++++ src/runtime/mgc.go | 2 +- src/runtime/pinner.go | 22 +++++++++++++++++++++- src/runtime/proc.go | 54 ++++++++++++++++++++++++++++++++++++++++++++--------- src/runtime/race_ppc64le.s | 4 ++++ src/runtime/runtime2.go | 2 +- src/runtime/testdata/testprog/schedmetrics.go | 6 ++++-- src/runtime/testdata/testprogcgo/notingo.go | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++ test/fixedbugs/issue76709.go | 42 ++++++++++++++++++++++++++++++++++++++++++ diff --git a/src/cmd/compile/internal/ssa/prove.go b/src/cmd/compile/internal/ssa/prove.go index 39080a015e938415193c29c40729444d5730a417..de16dfb3406eecc179a2f54f831b0aec974d9abf 100644 --- a/src/cmd/compile/internal/ssa/prove.go +++ b/src/cmd/compile/internal/ssa/prove.go @@ -2119,7 +2119,10 @@ var lenOffset *Value if bound := ow.Args[0]; (bound.Op == OpSliceLen || bound.Op == OpStringLen) && bound.Args[0] == slice { lenOffset = ow.Args[1] } else if bound := ow.Args[1]; (bound.Op == OpSliceLen || bound.Op == OpStringLen) && bound.Args[0] == slice { - lenOffset = ow.Args[0] + // Do not infer K - slicelen, see issue #76709. + if ow.Op == OpAdd64 { + lenOffset = ow.Args[0] + } } if lenOffset == nil || lenOffset.Op != OpConst64 { continue diff --git a/src/cmd/go.mod b/src/cmd/go.mod index 9a1a38bccca6b5a1abf818ba08f21ec39b1147b4..90c3717334cd850b2d61af8536f0da1d2bd7b672 100644 --- a/src/cmd/go.mod +++ b/src/cmd/go.mod @@ -11,7 +11,7 @@ golang.org/x/sync v0.18.0 golang.org/x/sys v0.38.1-0.20251125153526-08e54827f670 golang.org/x/telemetry v0.0.0-20251128220624-abf20d0e57ec golang.org/x/term v0.37.0 - golang.org/x/tools v0.39.1-0.20251130212600-1ad6f3d02713 + golang.org/x/tools v0.39.1-0.20251205000126-062ef7b6ced2 ) require ( diff --git a/src/cmd/go.sum b/src/cmd/go.sum index 52d55d81732f7f2671f4f7aa3033b1c31cf69bf9..774820d54c19bde75d46c95cbe7c18564f3b9037 100644 --- a/src/cmd/go.sum +++ b/src/cmd/go.sum @@ -22,7 +22,7 @@ golang.org/x/term v0.37.0 h1:8EGAD0qCmHYZg6J17DvsMy9/wJ7/D/4pV/wfnld5lTU= golang.org/x/term v0.37.0/go.mod h1:5pB4lxRNYYVZuTLmy8oR2BH8dflOR+IbTYFD8fi3254= golang.org/x/text v0.31.1-0.20251128220601-087616b6cde9 h1:IjQf87/qLz2y0SiCc0uY3DwajALXkAgP1Pxal0mmdrM= golang.org/x/text v0.31.1-0.20251128220601-087616b6cde9/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM= -golang.org/x/tools v0.39.1-0.20251130212600-1ad6f3d02713 h1:i4GzAuZW4RuKXltwKyLYAfk7E1TSKQBxRAI7XKfLjSk= -golang.org/x/tools v0.39.1-0.20251130212600-1ad6f3d02713/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ= +golang.org/x/tools v0.39.1-0.20251205000126-062ef7b6ced2 h1:2Qqv605Nus9iUp3ErvEU/q92Q3HAzeROztzl9pzAno8= +golang.org/x/tools v0.39.1-0.20251205000126-062ef7b6ced2/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ= rsc.io/markdown v0.0.0-20240306144322-0bf8f97ee8ef h1:mqLYrXCXYEZOop9/Dbo6RPX11539nwiCNBb1icVPmw8= rsc.io/markdown v0.0.0-20240306144322-0bf8f97ee8ef/go.mod h1:8xcPgWmwlZONN1D9bjxtHEjrUtSEa3fakVF8iaewYKQ= diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index b30b2abc0e9ef7a4e67e47c3e5da96c192089d0a..916943904d0cfab2c6f24ef8d5f9c40068019b31 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -736,7 +736,7 @@ // Reject the '-fuzz' flag if the package is outside the main module. // Otherwise, if fuzzing identifies a failure it could corrupt checksums in // the module cache (or permanently alter the behavior of std tests for all // users) by writing the failing input to the package's testdata directory. - // (See https://golang.org/issue/48495 and test_fuzz_modcache.txt.) + // (See https://golang.org/issue/48495 and cmd/internal/fuzztest/test_fuzz_modcache.txt.) mainMods := moduleLoaderState.MainModules if m := pkgs[0].Module; m != nil && m.Path != "" { if !mainMods.Contains(m.Path) { @@ -1371,7 +1371,7 @@ if testVet.off { return } - vet := b.VetAction(loaderstate, work.ModeBuild, work.ModeBuild, p) + vet := b.VetAction(loaderstate, work.ModeBuild, work.ModeBuild, false, p) runAction.Deps = append(runAction.Deps, vet) // Install will clean the build directory. // Make sure vet runs first. diff --git a/src/cmd/go/internal/vet/vet.go b/src/cmd/go/internal/vet/vet.go index 9055446325af6ede854d9c9e33b1691c408aa603..34d904cffc61f85bc495d441e2b0a1a329383540 100644 --- a/src/cmd/go/internal/vet/vet.go +++ b/src/cmd/go/internal/vet/vet.go @@ -6,6 +6,8 @@ // Package vet implements the “go vet” and “go fix” commands. package vet import ( + "archive/zip" + "bytes" "context" "encoding/json" "errors" @@ -176,6 +178,7 @@ // here (see printDiagnostics), and do not pass the flag through. work.VetExplicit = len(toolFlags) > 0 + applyFixes := false if cmd.Name() == "fix" || *vetFixFlag { // fix mode: 'go fix' or 'go vet -fix' if jsonFlag { @@ -186,6 +189,8 @@ } else { toolFlags = append(toolFlags, "-fix") if diffFlag { toolFlags = append(toolFlags, "-diff") + } else { + applyFixes = true } } if contextFlag != -1 { @@ -226,6 +231,7 @@ if len(pkgs) == 0 { base.Fatalf("no packages to %s", cmd.Name()) } + // Build action graph. b := work.NewBuilder("", moduleLoaderState.VendorDirOrEmpty) defer func() { if err := b.Close(); err != nil { @@ -233,6 +239,13 @@ base.Fatal(err) } }() + root := &work.Action{Mode: "go " + cmd.Name()} + + addVetAction := func(p *load.Package) { + act := b.VetAction(moduleLoaderState, work.ModeBuild, work.ModeBuild, applyFixes, p) + root.Deps = append(root.Deps, act) + } + // To avoid file corruption from duplicate application of // fixes (in fix mode), and duplicate reporting of diagnostics // (in vet mode), we must run the tool only once for each @@ -248,8 +261,6 @@ // // We needn't worry about intermediate test variants, as they // will only be executed in VetxOnly mode, for facts but not // diagnostics. - - root := &work.Action{Mode: "go " + cmd.Name()} for _, p := range pkgs { _, ptest, pxtest, perr := load.TestPackagesFor(moduleLoaderState, ctx, pkgOpts, p, nil) if perr != nil { @@ -262,13 +273,65 @@ continue } if len(ptest.GoFiles) > 0 || len(ptest.CgoFiles) > 0 { // The test package includes all the files of primary package. - root.Deps = append(root.Deps, b.VetAction(moduleLoaderState, work.ModeBuild, work.ModeBuild, ptest)) + addVetAction(ptest) } if pxtest != nil { - root.Deps = append(root.Deps, b.VetAction(moduleLoaderState, work.ModeBuild, work.ModeBuild, pxtest)) + addVetAction(pxtest) } } b.Do(ctx, root) + + // Apply fixes. + // + // We do this as a separate phase after the build to avoid + // races between source file updates and reads of those same + // files by concurrent actions of the ongoing build. + // + // If a file is fixed by multiple actions, they must be consistent. + if applyFixes { + contents := make(map[string][]byte) + // Gather the fixes. + for _, act := range root.Deps { + if act.FixArchive != "" { + if err := readZip(act.FixArchive, contents); err != nil { + base.Errorf("reading archive of fixes: %v", err) + return + } + } + } + // Apply them. + for filename, content := range contents { + if err := os.WriteFile(filename, content, 0644); err != nil { + base.Errorf("applying fix: %v", err) + } + } + } +} + +// readZip reads the zipfile entries into the provided map. +// It reports an error if updating the map would change an existing entry. +func readZip(zipfile string, out map[string][]byte) error { + r, err := zip.OpenReader(zipfile) + if err != nil { + return err + } + defer r.Close() // ignore error + for _, f := range r.File { + rc, err := f.Open() + if err != nil { + return err + } + content, err := io.ReadAll(rc) + rc.Close() // ignore error + if err != nil { + return err + } + if prev, ok := out[f.Name]; ok && !bytes.Equal(prev, content) { + return fmt.Errorf("inconsistent fixes to file %v", f.Name) + } + out[f.Name] = content + } + return nil } // printJSONDiagnostics parses JSON (from the tool's stdout) and diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index 698a523c25145fb10193a544ac77e1c63252c11f..f0e34c33689b6a16881fc2cdc623d3939fe6730e 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -109,11 +109,13 @@ cachedExecutable string // the cached executable, if CacheExecutable was set actionID cache.ActionID // cache ID of action input buildID string // build ID of action output - VetxOnly bool // Mode=="vet": only being called to supply info about dependencies - needVet bool // Mode=="build": need to fill in vet config - needBuild bool // Mode=="build": need to do actual build (can be false if needVet is true) - vetCfg *vetConfig // vet config - output []byte // output redirect buffer (nil means use b.Print) + VetxOnly bool // Mode=="vet": only being called to supply info about dependencies + needVet bool // Mode=="build": need to fill in vet config + needBuild bool // Mode=="build": need to do actual build (can be false if needVet is true) + needFix bool // Mode=="vet": need secondary target, a .zip file containing fixes + vetCfg *vetConfig // vet config + FixArchive string // the created .zip file containing fixes (if needFix) + output []byte // output redirect buffer (nil means use b.Print) sh *Shell // lazily created per-Action shell; see Builder.Shell @@ -869,9 +871,10 @@ // VetAction returns the action for running go vet on package p. // It depends on the action for compiling p. // If the caller may be causing p to be installed, it is up to the caller // to make sure that the install depends on (runs after) vet. -func (b *Builder) VetAction(s *modload.State, mode, depMode BuildMode, p *load.Package) *Action { +func (b *Builder) VetAction(s *modload.State, mode, depMode BuildMode, needFix bool, p *load.Package) *Action { a := b.vetAction(s, mode, depMode, p) a.VetxOnly = false + a.needFix = needFix return a } diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 0c9e96aebbf080ddedc80a11f0b296e5406dbfa9..654e9e9374b2585f0b570f256113e97db21ea5cb 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -1187,6 +1187,7 @@ VetxOnly bool // only compute vetx data; don't report detected problems VetxOutput string // write vetx data to this output file Stdout string // write stdout (JSON, unified diff) to this output file GoVersion string // Go version for package + FixArchive string // write fixed files to this zip archive, if non-empty SucceedOnTypecheckFailure bool // awful hack; see #18395 and below } @@ -1308,6 +1309,9 @@ // We use "vet" terminology even when building action graphs for go fix. vcfg.VetxOnly = a.VetxOnly vcfg.VetxOutput = a.Objdir + "vet.out" vcfg.Stdout = a.Objdir + "vet.stdout" + if a.needFix { + vcfg.FixArchive = a.Objdir + "vet.fix.zip" + } vcfg.PackageVetx = make(map[string]string) h := cache.NewHash("vet " + a.Package.ImportPath) @@ -1368,31 +1372,60 @@ fmt.Fprintf(h, "vetout %q %s\n", a1.Package.ImportPath, b.fileHash(a1.built)) vcfg.PackageVetx[a1.Package.ImportPath] = a1.built } } - vetxKey := cache.ActionID(h.Sum()) // for .vetx file - - fmt.Fprintf(h, "stdout\n") - stdoutKey := cache.ActionID(h.Sum()) // for .stdout file + var ( + id = cache.ActionID(h.Sum()) // for .vetx file + stdoutKey = cache.Subkey(id, "stdout") // for .stdout file + fixArchiveKey = cache.Subkey(id, "fix.zip") // for .fix.zip file + ) // Check the cache; -a forces a rebuild. if !cfg.BuildA { c := cache.Default() - if vcfg.VetxOnly { - if file, _, err := cache.GetFile(c, vetxKey); err == nil { - a.built = file - return nil + + // There may be multiple artifacts in the cache. + // We need to retrieve them all, or none: + // the effect must be transactional. + var ( + vetxFile string // name of cached .vetx file + fixArchive string // name of cached .fix.zip file + stdout io.Reader = bytes.NewReader(nil) // cached stdout stream + ) + + // Obtain location of cached .vetx file. + vetxFile, _, err := cache.GetFile(c, id) + if err != nil { + goto cachemiss + } + + // Obtain location of cached .fix.zip file (if needed). + if a.needFix { + file, _, err := cache.GetFile(c, fixArchiveKey) + if err != nil { + goto cachemiss } - } else { - // Copy cached vet.std files to stdout. - if file, _, err := cache.GetFile(c, stdoutKey); err == nil { - f, err := os.Open(file) - if err != nil { - return err - } - defer f.Close() // ignore error (can't fail) - return VetHandleStdout(f) + fixArchive = file + } + + // Copy cached .stdout file to stdout. + if file, _, err := cache.GetFile(c, stdoutKey); err == nil { + f, err := os.Open(file) + if err != nil { + goto cachemiss } + defer f.Close() // ignore error (can't fail) + stdout = f } + + // Cache hit: commit transaction. + a.built = vetxFile + a.FixArchive = fixArchive + if err := VetHandleStdout(stdout); err != nil { + return err // internal error (don't fall through to cachemiss) + } + + return nil } +cachemiss: js, err := json.MarshalIndent(vcfg, "", "\t") if err != nil { @@ -1419,13 +1452,23 @@ if err := sh.run(p.Dir, p.ImportPath, env, cfg.BuildToolexec, tool, vetFlags, a.Objdir+"vet.cfg"); err != nil { return err } - // Vet tool succeeded, possibly with facts and JSON stdout. Save both in cache. + // Vet tool succeeded, possibly with facts, fixes, or JSON stdout. + // Save all in cache. - // Save facts + // Save facts. if f, err := os.Open(vcfg.VetxOutput); err == nil { defer f.Close() // ignore error a.built = vcfg.VetxOutput - cache.Default().Put(vetxKey, f) // ignore error + cache.Default().Put(id, f) // ignore error + } + + // Save fix archive (if any). + if a.needFix { + if f, err := os.Open(vcfg.FixArchive); err == nil { + defer f.Close() // ignore error + a.FixArchive = vcfg.FixArchive + cache.Default().Put(fixArchiveKey, f) // ignore error + } } // Save stdout. diff --git a/src/cmd/go/testdata/script/test_fuzz.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz.txt rename from src/cmd/go/testdata/script/test_fuzz.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_cache.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_cache.txt rename from src/cmd/go/testdata/script/test_fuzz_cache.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_cache.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_cgo.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_cgo.txt rename from src/cmd/go/testdata/script/test_fuzz_cgo.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_cgo.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_chatty.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_chatty.txt rename from src/cmd/go/testdata/script/test_fuzz_chatty.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_chatty.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_cleanup.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_cleanup.txt rename from src/cmd/go/testdata/script/test_fuzz_cleanup.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_cleanup.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_context.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_context.txt rename from src/cmd/go/testdata/script/test_fuzz_context.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_context.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_cov.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_cov.txt rename from src/cmd/go/testdata/script/test_fuzz_cov.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_cov.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_deadline.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_deadline.txt rename from src/cmd/go/testdata/script/test_fuzz_deadline.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_deadline.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_dup_cache.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_dup_cache.txt rename from src/cmd/go/testdata/script/test_fuzz_dup_cache.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_dup_cache.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_err_deadlock.txt rename from src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_err_deadlock.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_fuzztime.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_fuzztime.txt rename from src/cmd/go/testdata/script/test_fuzz_fuzztime.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_fuzztime.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_io_error.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_io_error.txt rename from src/cmd/go/testdata/script/test_fuzz_io_error.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_io_error.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_limit_dup_entry.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_limit_dup_entry.txt rename from src/cmd/go/testdata/script/test_fuzz_limit_dup_entry.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_limit_dup_entry.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_match.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_match.txt rename from src/cmd/go/testdata/script/test_fuzz_match.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_match.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_minimize.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize.txt rename from src/cmd/go/testdata/script/test_fuzz_minimize.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_minimize_dirty_cov.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize_dirty_cov.txt rename from src/cmd/go/testdata/script/test_fuzz_minimize_dirty_cov.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize_dirty_cov.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_minimize_interesting.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize_interesting.txt rename from src/cmd/go/testdata/script/test_fuzz_minimize_interesting.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_minimize_interesting.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_multiple.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_multiple.txt rename from src/cmd/go/testdata/script/test_fuzz_multiple.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_multiple.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutate_crash.txt rename from src/cmd/go/testdata/script/test_fuzz_mutate_crash.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutate_crash.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_mutate_fail.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutate_fail.txt rename from src/cmd/go/testdata/script/test_fuzz_mutate_fail.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutate_fail.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_mutator.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutator.txt rename from src/cmd/go/testdata/script/test_fuzz_mutator.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutator.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_mutator_repeat.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutator_repeat.txt rename from src/cmd/go/testdata/script/test_fuzz_mutator_repeat.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_mutator_repeat.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_non_crash_signal.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_non_crash_signal.txt rename from src/cmd/go/testdata/script/test_fuzz_non_crash_signal.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_non_crash_signal.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_parallel.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_parallel.txt rename from src/cmd/go/testdata/script/test_fuzz_parallel.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_parallel.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_profile_flags.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_profile_flags.txt rename from src/cmd/go/testdata/script/test_fuzz_profile_flags.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_profile_flags.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_return.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_return.txt rename from src/cmd/go/testdata/script/test_fuzz_return.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_return.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_run.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_run.txt rename from src/cmd/go/testdata/script/test_fuzz_run.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_run.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_seed_corpus.txt rename from src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_seed_corpus.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_setenv.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_setenv.txt rename from src/cmd/go/testdata/script/test_fuzz_setenv.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_setenv.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_test_race.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_test_race.txt rename from src/cmd/go/testdata/script/test_fuzz_test_race.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_test_race.txt diff --git a/src/cmd/go/testdata/script/test_fuzz_unsupported.txt b/src/cmd/internal/fuzztest/testdata/script/test_fuzz_unsupported.txt rename from src/cmd/go/testdata/script/test_fuzz_unsupported.txt rename to src/cmd/internal/fuzztest/testdata/script/test_fuzz_unsupported.txt diff --git a/src/cmd/go/testdata/script/vet_basic.txt b/src/cmd/go/testdata/script/vet_basic.txt index 5ae66438ea3d81b683ecbc63480990868be02cf1..a0dd3ae2d84d7f25c2dd235474988bda96106871 100644 --- a/src/cmd/go/testdata/script/vet_basic.txt +++ b/src/cmd/go/testdata/script/vet_basic.txt @@ -91,6 +91,7 @@ # Legacy way of selecting fixers is a no-op. go fix -fix=old1,old2 example.com/x stderr 'go fix: the -fix=old1,old2 flag is obsolete and has no effect' +cp x.go.bak x.go # -c=n flag shows n lines of context. ! go vet -c=2 -printf example.com/x diff --git a/src/cmd/go/testdata/script/vet_cache.txt b/src/cmd/go/testdata/script/vet_cache.txt index c84844000a43d0ffe41dc9e81925248022566d70..624df5573240c12556d7196d9721c17ffa9ea774 100644 --- a/src/cmd/go/testdata/script/vet_cache.txt +++ b/src/cmd/go/testdata/script/vet_cache.txt @@ -1,6 +1,8 @@ # Test that go vet's caching of vet tool actions replays # the recorded stderr output even after a cache hit. +[short] skip 'uses a fresh build cache' + # Set up fresh GOCACHE. env GOCACHE=$WORK/gocache diff --git a/src/cmd/internal/fuzztest/script_test.go b/src/cmd/internal/fuzztest/script_test.go new file mode 100644 index 0000000000000000000000000000000000000000..e8fe701e1763941a75d5eb8f219df831e1d223a1 --- /dev/null +++ b/src/cmd/internal/fuzztest/script_test.go @@ -0,0 +1,22 @@ +// Copyright 2025 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 fuzztest + +import ( + "cmd/internal/script/scripttest" + "flag" + "internal/testenv" + "testing" +) + +//go:generate go test cmd/internal/fuzztest -v -run=TestScript/README --fixreadme + +var fixReadme = flag.Bool("fixreadme", false, "if true, update README for script tests") + +func TestScript(t *testing.T) { + testenv.MustHaveGoBuild(t) + testenv.SkipIfShortAndSlow(t) + scripttest.RunToolScriptTest(t, nil, "testdata/script", *fixReadme) +} diff --git a/src/cmd/internal/fuzztest/testdata/script/README b/src/cmd/internal/fuzztest/testdata/script/README new file mode 100644 index 0000000000000000000000000000000000000000..9ec997a138f0f27be42882b819a1531f63600394 --- /dev/null +++ b/src/cmd/internal/fuzztest/testdata/script/README @@ -0,0 +1,286 @@ +This file is generated by 'go generate'. DO NOT EDIT. + +This directory holds test scripts *.txt run during 'go test cmd/'. +To run a specific script foo.txt + + go test cmd/ -run=Script/^foo$ + +In general script files should have short names: a few words, + not whole sentences. +The first word should be the general category of behavior being tested, +often the name of a go subcommand (build, link, compile, ...) or concept (vendor, pattern). + +Each script is a text archive (go doc internal/txtar). +The script begins with an actual command script to run +followed by the content of zero or more supporting files to +create in the script's temporary file system before it starts executing. + +As an example, run_hello.txt says: + + # hello world + go run hello.go + stderr 'hello world' + ! stdout . + + -- hello.go -- + package main + func main() { println("hello world") } + +Each script runs in a fresh temporary work directory tree, available to scripts as $WORK. +Scripts also have access to other environment variables, including: + + GOARCH= + GOOS= + TMPDIR=$WORK/tmp + devnull= + goversion= + +On Plan 9, the variables $path and $home are set instead of $PATH and $HOME. +On Windows, the variables $USERPROFILE and $TMP are set instead of +$HOME and $TMPDIR. + +The lines at the top of the script are a sequence of commands to be executed by +a small script engine configured in .../cmd/internal/script/scripttest/run.go (not the system shell). + +Each line of a script is parsed into a sequence of space-separated command +words, with environment variable expansion within each word and # marking +an end-of-line comment. Additional variables named ':' and '/' are expanded +within script arguments (expanding to the value of os.PathListSeparator and +os.PathSeparator respectively) but are not inherited in subprocess environments. + +Adding single quotes around text keeps spaces in that text from being treated +as word separators and also disables environment variable expansion. Inside a +single-quoted block of text, a repeated single quote indicates a literal single +quote, as in: + + 'Don''t communicate by sharing memory.' + +A line beginning with # is a comment and conventionally explains what is being +done or tested at the start of a new section of the script. + +Commands are executed one at a time, and errors are checked for each command; +if any command fails unexpectedly, no subsequent commands in the script are +executed. The command prefix ! indicates that the command on the rest of the +line (typically go or a matching predicate) must fail instead of succeeding. +The command prefix ? indicates that the command may or may not succeed, but the +script should continue regardless. + +The command prefix [cond] indicates that the command on the rest of the line +should only run when the condition is satisfied. + +A condition can be negated: [!root] means to run the rest of the line only if +the user is not root. Multiple conditions may be given for a single command, +for example, '[linux] [amd64] skip'. The command will run if all conditions are +satisfied. + +When TestScript runs a script and the script fails, by default TestScript shows +the execution of the most recent phase of the script (since the last # comment) +and only shows the # comments for earlier phases. + +Note also that in reported output, the actual name of the per-script temporary directory +has been consistently replaced with the literal string $WORK. + +The available commands are: +cat files... + concatenate files and print to the script's stdout buffer + + +cc args... + run the platform C compiler + + +cd dir + change the working directory + + +chmod perm paths... + change file mode bits + + Changes the permissions of the named files or directories to + be equal to perm. + Only numerical permissions are supported. + +cmp [-q] file1 file2 + compare files for differences + + By convention, file1 is the actual data and file2 is the + expected data. + The command succeeds if the file contents are identical. + File1 can be 'stdout' or 'stderr' to compare the stdout or + stderr buffer from the most recent command. + +cmpenv [-q] file1 file2 + compare files for differences, with environment expansion + + By convention, file1 is the actual data and file2 is the + expected data. + The command succeeds if the file contents are identical + after substituting variables from the script environment. + File1 can be 'stdout' or 'stderr' to compare the script's + stdout or stderr buffer. + +cp src... dst + copy files to a target file or directory + + src can include 'stdout' or 'stderr' to copy from the + script's stdout or stderr buffer. + +echo string... + display a line of text + + +env [key[=value]...] + set or log the values of environment variables + + With no arguments, print the script environment to the log. + Otherwise, add the listed key=value pairs to the environment + or print the listed keys. + +exec program [args...] [&] + run an executable program with arguments + + Note that 'exec' does not terminate the script (unlike Unix + shells). + +exists [-readonly] [-exec] file... + check that files exist + + +go [args...] [&] + run the 'go' program provided by the script host + + +grep [-count=N] [-q] 'pattern' file + find lines in a file that match a pattern + + The command succeeds if at least one match (or the exact + count, if given) is found. + The -q flag suppresses printing of matches. + +help [-v] name... + log help text for commands and conditions + + To display help for a specific condition, enclose it in + brackets: 'help [amd64]'. + To display complete documentation when listing all commands, + pass the -v flag. + +mkdir path... + create directories, if they do not already exist + + Unlike Unix mkdir, parent directories are always created if + needed. + +mv old new + rename a file or directory to a new path + + OS-specific restrictions may apply when old and new are in + different directories. + +replace [old new]... file + replace strings in a file + + The 'old' and 'new' arguments are unquoted as if in quoted + Go strings. + +rm path... + remove a file or directory + + If the path is a directory, its contents are removed + recursively. + +skip [msg] + skip the current test + + +sleep duration [&] + sleep for a specified duration + + The duration must be given as a Go time.Duration string. + +stderr [-count=N] [-q] 'pattern' file + find lines in the stderr buffer that match a pattern + + The command succeeds if at least one match (or the exact + count, if given) is found. + The -q flag suppresses printing of matches. + +stdout [-count=N] [-q] 'pattern' file + find lines in the stdout buffer that match a pattern + + The command succeeds if at least one match (or the exact + count, if given) is found. + The -q flag suppresses printing of matches. + +stop [msg] + stop execution of the script + + The message is written to the script log, but no error is + reported from the script engine. + +symlink path -> target + create a symlink + + Creates path as a symlink to target. + The '->' token (like in 'ls -l' output on Unix) is required. + +wait + wait for completion of background commands + + Waits for all background commands to complete. + The output (and any error) from each command is printed to + the log in the order in which the commands were started. + After the call to 'wait', the script's stdout and stderr + buffers contain the concatenation of the background + commands' outputs. + + + +The available conditions are: +[GOARCH:*] + runtime.GOARCH == +[GODEBUG:*] + GODEBUG contains +[GOEXPERIMENT:*] + GOEXPERIMENT is enabled +[GOOS:*] + runtime.GOOS == +[asan] + GOOS/GOARCH supports -asan +[buildmode:*] + go supports -buildmode= +[cgo] + host CGO_ENABLED +[cgolinkext] + platform requires external linking for cgo +[compiler:*] + runtime.Compiler == +[cross] + cmd/go GOOS/GOARCH != GOHOSTOS/GOHOSTARCH +[exec:*] + names an executable in the test binary's PATH +[fuzz] + GOOS/GOARCH supports -fuzz +[fuzz-instrumented] + GOOS/GOARCH supports -fuzz with instrumentation +[go-builder] + GO_BUILDER_NAME is non-empty +[link] + testenv.HasLink() +[msan] + GOOS/GOARCH supports -msan +[mustlinkext] + platform always requires external linking +[pielinkext] + platform requires external linking for PIE +[race] + GOOS/GOARCH supports -race +[root] + os.Geteuid() == 0 +[short] + testing.Short() +[symlink] + testenv.HasSymlink() +[verbose] + testing.Verbose() + diff --git a/src/cmd/internal/script/scripttest/run.go b/src/cmd/internal/script/scripttest/run.go index 8dff13e22edfc47be061bd6f401ae63ea0132345..2a909f656d5e896bfba2fe73f0363a28bb0a366f 100644 --- a/src/cmd/internal/script/scripttest/run.go +++ b/src/cmd/internal/script/scripttest/run.go @@ -137,6 +137,9 @@ // Environment setup. env := os.Environ() prependToPath(env, filepath.Join(tgr, "bin")) env = setenv(env, "GOROOT", tgr) + // GOOS and GOARCH are expected to be set by the toolchain script conditions. + env = setenv(env, "GOOS", runtime.GOOS) + env = setenv(env, "GOARCH", runtime.GOARCH) for _, repl := range repls { // consistency check chunks := strings.Split(repl.EnvVar, "=") diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/doc.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/doc.go index 7469002f56e43fbb0c4a04825febeb8a29097089..45aed7909c36348da0cdf378680dc8aef1271fac 100644 --- a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/doc.go +++ b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/doc.go @@ -199,11 +199,18 @@ # Analyzer omitzero omitzero: suggest replacing omitempty with omitzero for struct fields -The omitzero analyzer identifies uses of the `omitempty` JSON struct tag on -fields that are themselves structs. The `omitempty` tag has no effect on -struct-typed fields. The analyzer offers two suggestions: either remove the +The omitzero analyzer identifies uses of the `omitempty` JSON struct +tag on fields that are themselves structs. For struct-typed fields, +the `omitempty` tag has no effect on the behavior of json.Marshal and +json.Unmarshal. The analyzer offers two suggestions: either remove the tag, or replace it with `omitzero` (added in Go 1.24), which correctly omits the field if the struct value is zero. + +However, some other serialization packages (notably kubebuilder, see +https://book.kubebuilder.io/reference/markers.html) may have their own +interpretation of the `json:",omitzero"` tag, so removing it may affect +program behavior. For this reason, the omitzero modernizer will not +make changes in any package that contains +kubebuilder annotations. Replacing `omitempty` with `omitzero` is a change in behavior. The original code would always encode the struct field, whereas the diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/omitzero.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/omitzero.go index 4a05d64f42a002f0e958369d4117a7bebf993098..59ba9506511de5c53eb7b950c89a770b474faf14 100644 --- a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/omitzero.go +++ b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/omitzero.go @@ -9,6 +9,8 @@ "go/ast" "go/types" "reflect" "strconv" + "strings" + "sync" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -25,82 +27,106 @@ Run: omitzero, URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#omitzero", } -func checkOmitEmptyField(pass *analysis.Pass, info *types.Info, curField *ast.Field) { - typ := info.TypeOf(curField.Type) - _, ok := typ.Underlying().(*types.Struct) - if !ok { - // Not a struct - return - } - tag := curField.Tag - if tag == nil { - // No tag to check - return - } - // The omitempty tag may be used by other packages besides json, but we should only modify its use with json - tagconv, _ := strconv.Unquote(tag.Value) - match := omitemptyRegex.FindStringSubmatchIndex(tagconv) - if match == nil { - // No omitempty in json tag - return - } - omitEmpty, err := astutil.RangeInStringLiteral(curField.Tag, match[2], match[3]) - if err != nil { - return - } - var remove analysis.Range = omitEmpty +// The omitzero pass searches for instances of "omitempty" in a json field tag on a +// struct. Since "omitfilesUsingGoVersions not have any effect when applied to a struct field, +// it suggests either deleting "omitempty" or replacing it with "omitzero", which +// correctly excludes structs from a json encoding. +func omitzero(pass *analysis.Pass) (any, error) { + // usesKubebuilder reports whether "+kubebuilder:" appears in + // any comment in the package, since it has its own + // interpretation of what omitzero means; see go.dev/issue/76649. + // It is computed once, on demand. + usesKubebuilder := sync.OnceValue[bool](func() bool { + for _, file := range pass.Files { + for _, comment := range file.Comments { + if strings.Contains(comment.Text(), "+kubebuilder:") { + return true + } + } + } + return false + }) - jsonTag := reflect.StructTag(tagconv).Get("json") - if jsonTag == ",omitempty" { - // Remove the entire struct tag if json is the only package used - if match[1]-match[0] == len(tagconv) { - remove = curField.Tag - } else { - // Remove the json tag if omitempty is the only field - remove, err = astutil.RangeInStringLiteral(curField.Tag, match[0], match[1]) - if err != nil { - return + checkField := func(field *ast.Field) { + typ := pass.TypesInfo.TypeOf(field.Type) + _, ok := typ.Underlying().(*types.Struct) + if !ok { + // Not a struct + return + } + tag := field.Tag + if tag == nil { + // No tag to check + return + } + // The omitempty tag may be used by other packages besides json, but we should only modify its use with json + tagconv, _ := strconv.Unquote(tag.Value) + match := omitemptyRegex.FindStringSubmatchIndex(tagconv) + if match == nil { + // No omitempty in json tag + return + } + omitEmpty, err := astutil.RangeInStringLiteral(field.Tag, match[2], match[3]) + if err != nil { + return + } + var remove analysis.Range = omitEmpty + + jsonTag := reflect.StructTag(tagconv).Get("json") + if jsonTag == ",omitempty" { + // Remove the entire struct tag if json is the only package used + if match[1]-match[0] == len(tagconv) { + remove = field.Tag + } else { + // Remove the json tag if omitempty is the only field + remove, err = astutil.RangeInStringLiteral(field.Tag, match[0], match[1]) + if err != nil { + return + } } } - } - pass.Report(analysis.Diagnostic{ - Pos: curField.Tag.Pos(), - End: curField.Tag.End(), - Message: "Omitempty has no effect on nested struct fields", - SuggestedFixes: []analysis.SuggestedFix{ - { - Message: "Remove redundant omitempty tag", - TextEdits: []analysis.TextEdit{ - { - Pos: remove.Pos(), - End: remove.End(), + + // Don't offer a fix if the package seems to use kubebuilder, + // as it has its own intepretation of "omitzero" tags. + // https://book.kubebuilder.io/reference/markers.html + if usesKubebuilder() { + return + } + + pass.Report(analysis.Diagnostic{ + Pos: field.Tag.Pos(), + End: field.Tag.End(), + Message: "Omitempty has no effect on nested struct fields", + SuggestedFixes: []analysis.SuggestedFix{ + { + Message: "Remove redundant omitempty tag", + TextEdits: []analysis.TextEdit{ + { + Pos: remove.Pos(), + End: remove.End(), + }, }, }, - }, - { - Message: "Replace omitempty with omitzero (behavior change)", - TextEdits: []analysis.TextEdit{ - { - Pos: omitEmpty.Pos(), - End: omitEmpty.End(), - NewText: []byte(",omitzero"), + { + Message: "Replace omitempty with omitzero (behavior change)", + TextEdits: []analysis.TextEdit{ + { + Pos: omitEmpty.Pos(), + End: omitEmpty.End(), + NewText: []byte(",omitzero"), + }, }, }, - }, - }}) -} + }}) + } -// The omitzero pass searches for instances of "omitempty" in a json field tag on a -// struct. Since "omitfilesUsingGoVersions not have any effect when applied to a struct field, -// it suggests either deleting "omitempty" or replacing it with "omitzero", which -// correctly excludes structs from a json encoding. -func omitzero(pass *analysis.Pass) (any, error) { for curFile := range filesUsingGoVersion(pass, versions.Go1_24) { for curStruct := range curFile.Preorder((*ast.StructType)(nil)) { for _, curField := range curStruct.Node().(*ast.StructType).Fields.List { - checkOmitEmptyField(pass, pass.TypesInfo, curField) + checkField(curField) } } } + return nil, nil } diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/stringscut.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/stringscut.go index bc4ad677cd093c5792e748372c7fe489e0525dda..954997ad6f1c89ad1b7db5a5953e8054c99a90fc 100644 --- a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/stringscut.go +++ b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/modernize/stringscut.go @@ -50,9 +50,14 @@ // Consider some candidate for replacement i := strings.Index(s, substr). // The following must hold for a replacement to occur: // // 1. All instances of i and s must be in one of these forms. -// Binary expressions: -// (a): establishing that i < 0: e.g.: i < 0, 0 > i, i == -1, -1 == i -// (b): establishing that i > -1: e.g.: i >= 0, 0 <= i, i == 0, 0 == i +// +// Binary expressions must be inequalities equivalent to +// "Index failed" (e.g. i < 0) or "Index succeeded" (i >= 0), +// or identities such as these (and their negations): +// +// 0 > i (flips left and right) +// i <= -1, -1 >= i (replace strict inequality by non-strict) +// i == -1, -1 == i (Index() guarantees i < 0 => i == -1) // // Slice expressions: // a: s[:i], s[0:i] @@ -86,9 +91,9 @@ // if ok { // use(before, after) // } // -// If the condition involving `i` establishes that i > -1, then we replace it with -// `if ok“. Variants listed above include i >= 0, i > 0, and i == 0. -// If the condition is negated (e.g. establishes `i < 0`), we use `if !ok` instead. +// If the condition involving `i` is equivalent to i >= 0, then we replace it with +// `if ok“. +// If the condition is negated (e.g. equivalent to `i < 0`), we use `if !ok` instead. // If the slices of `s` match `s[:i]` or `s[i+len(substr):]` or their variants listed above, // then we replace them with before and after. // @@ -178,16 +183,16 @@ // forms mentioned above (e.g. i < 0, i >= 0, s[:i] and s[i + // len(substr)]), then we can replace the call to Index() // with a call to Cut() and use the returned ok, before, // and after variables accordingly. - lessZero, greaterNegOne, beforeSlice, afterSlice := checkIdxUses(pass.TypesInfo, index.Uses(iObj), s, substr) + negative, nonnegative, beforeSlice, afterSlice := checkIdxUses(pass.TypesInfo, index.Uses(iObj), s, substr) // Either there are no uses of before, after, or ok, or some use // of i does not match our criteria - don't suggest a fix. - if lessZero == nil && greaterNegOne == nil && beforeSlice == nil && afterSlice == nil { + if negative == nil && nonnegative == nil && beforeSlice == nil && afterSlice == nil { continue } // If the only uses are ok and !ok, don't suggest a Cut() fix - these should be using Contains() - isContains := (len(lessZero) > 0 || len(greaterNegOne) > 0) && len(beforeSlice) == 0 && len(afterSlice) == 0 + isContains := (len(negative) > 0 || len(nonnegative) > 0) && len(beforeSlice) == 0 && len(afterSlice) == 0 scope := iObj.Parent() var ( @@ -200,7 +205,7 @@ ) // If there will be no uses of ok, before, or after, use the // blank identifier instead. - if len(lessZero) == 0 && len(greaterNegOne) == 0 { + if len(negative) == 0 && len(nonnegative) == 0 { okVarName = "_" } if len(beforeSlice) == 0 { @@ -226,8 +231,8 @@ indexCallId := typesinternal.UsedIdent(info, indexCall.Fun) replacedFunc := "Cut" if isContains { replacedFunc = "Contains" - replace(lessZero, "!"+foundVarName) // idx < 0 -> !found - replace(greaterNegOne, foundVarName) // idx > -1 -> found + replace(negative, "!"+foundVarName) // idx < 0 -> !found + replace(nonnegative, foundVarName) // idx > -1 -> found // Replace the assignment with found, and replace the call to // Index or IndexByte with a call to Contains. @@ -244,8 +249,8 @@ End: indexCallId.End(), NewText: []byte("Contains"), }) } else { - replace(lessZero, "!"+okVarName) // idx < 0 -> !ok - replace(greaterNegOne, okVarName) // idx > -1 -> ok + replace(negative, "!"+okVarName) // idx < 0 -> !ok + replace(nonnegative, okVarName) // idx > -1 -> ok replace(beforeSlice, beforeVarName) // s[:idx] -> before replace(afterSlice, afterVarName) // s[idx+k:] -> after @@ -364,11 +369,11 @@ // allows us to suggest a modernization. If all uses of i, s and substr match // one of the following four valid formats, it returns a list of occurrences for // each format. If any of the uses do not match one of the formats, return nil // for all values, since we should not offer a replacement. -// 1. lessZero - a condition involving i establishing that i is negative (e.g. i < 0, 0 > i, i == -1, -1 == i) -// 2. greaterNegOne - a condition involving i establishing that i is non-negative (e.g. i >= 0, 0 <= i, i == 0, 0 == i) +// 1. negative - a condition equivalent to i < 0 +// 2. nonnegative - a condition equivalent to i >= 0 // 3. beforeSlice - a slice of `s` that matches either s[:i], s[0:i] // 4. afterSlice - a slice of `s` that matches one of: s[i+len(substr):], s[len(substr) + i:], s[i + const], s[k + i] (where k = len(substr)) -func checkIdxUses(info *types.Info, uses iter.Seq[inspector.Cursor], s, substr ast.Expr) (lessZero, greaterNegOne, beforeSlice, afterSlice []ast.Expr) { +func checkIdxUses(info *types.Info, uses iter.Seq[inspector.Cursor], s, substr ast.Expr) (negative, nonnegative, beforeSlice, afterSlice []ast.Expr) { use := func(cur inspector.Cursor) bool { ek, _ := cur.ParentEdge() n := cur.Parent().Node() @@ -377,13 +382,13 @@ case edge.BinaryExpr_X, edge.BinaryExpr_Y: check := n.(*ast.BinaryExpr) switch checkIdxComparison(info, check) { case -1: - lessZero = append(lessZero, check) + negative = append(negative, check) return true case 1: - greaterNegOne = append(greaterNegOne, check) + nonnegative = append(nonnegative, check) return true } - // Check does not establish that i < 0 or i > -1. + // Check is not equivalent to that i < 0 or i >= 0. // Might be part of an outer slice expression like s[i + k] // which requires a different check. // Check that the thing being sliced is s and that the slice @@ -421,7 +426,7 @@ if !use(curIdent) { return nil, nil, nil, nil } } - return lessZero, greaterNegOne, beforeSlice, afterSlice + return negative, nonnegative, beforeSlice, afterSlice } // hasModifyingUses reports whether any of the uses involve potential @@ -451,52 +456,57 @@ } return false } -// checkIdxComparison reports whether the check establishes that i is negative -// or non-negative. It returns -1 in the first case, 1 in the second, and 0 if -// we can confirm neither condition. We assume that a check passed to -// checkIdxComparison has i as one of its operands. +// checkIdxComparison reports whether the check is equivalent to i < 0 or its negation, or neither. +// For equivalent to i >= 0, we only accept this exact BinaryExpr since +// expressions like i > 0 or i >= 1 make a stronger statement about the value of i. +// We avoid suggesting a fix in this case since it may result in an invalid +// transformation (See golang/go#76687). +// Since strings.Index returns exactly -1 if the substring is not found, we +// don't need to handle expressions like i <= -3. +// We return 0 if the expression does not match any of these options. +// We assume that a check passed to checkIdxComparison has i as one of its operands. func checkIdxComparison(info *types.Info, check *ast.BinaryExpr) int { - // Check establishes that i is negative. - // e.g.: i < 0, 0 > i, i == -1, -1 == i - if check.Op == token.LSS && (isNegativeConst(info, check.Y) || isZeroIntConst(info, check.Y)) || //i < (0 or neg) - check.Op == token.GTR && (isNegativeConst(info, check.X) || isZeroIntConst(info, check.X)) || // (0 or neg) > i - check.Op == token.LEQ && (isNegativeConst(info, check.Y)) || //i <= (neg) - check.Op == token.GEQ && (isNegativeConst(info, check.X)) || // (neg) >= i - check.Op == token.EQL && - (isNegativeConst(info, check.X) || isNegativeConst(info, check.Y)) { // i == neg; neg == i - return -1 + // Ensure that the constant (if any) is on the right. + x, op, y := check.X, check.Op, check.Y + if info.Types[x].Value != nil { + x, op, y = y, flip(op), x } - // Check establishes that i is non-negative. - // e.g.: i >= 0, 0 <= i, i == 0, 0 == i - if check.Op == token.GTR && (isNonNegativeConst(info, check.Y) || isIntLiteral(info, check.Y, -1)) || // i > (non-neg or -1) - check.Op == token.LSS && (isNonNegativeConst(info, check.X) || isIntLiteral(info, check.X, -1)) || // (non-neg or -1) < i - check.Op == token.GEQ && isNonNegativeConst(info, check.Y) || // i >= (non-neg) - check.Op == token.LEQ && isNonNegativeConst(info, check.X) || // (non-neg) <= i - check.Op == token.EQL && - (isNonNegativeConst(info, check.X) || isNonNegativeConst(info, check.Y)) { // i == non-neg; non-neg == i - return 1 + + yIsInt := func(k int64) bool { + return isIntLiteral(info, y, k) } - return 0 -} -// isNegativeConst returns true if the expr is a const int with value < zero. -func isNegativeConst(info *types.Info, expr ast.Expr) bool { - if tv, ok := info.Types[expr]; ok && tv.Value != nil && tv.Value.Kind() == constant.Int { - if v, ok := constant.Int64Val(tv.Value); ok { - return v < 0 - } + if op == token.LSS && yIsInt(0) || // i < 0 + op == token.EQL && yIsInt(-1) || // i == -1 + op == token.LEQ && yIsInt(-1) { // i <= -1 + return -1 // check <=> i is negative } - return false + + if op == token.GEQ && yIsInt(0) || // i >= 0 + op == token.NEQ && yIsInt(-1) || // i != -1 + op == token.GTR && yIsInt(-1) { // i > -1 + return +1 // check <=> i is non-negative + } + + return 0 // unknown } -// isNonNegativeConst returns true if the expr is a const int with value >= zero. -func isNonNegativeConst(info *types.Info, expr ast.Expr) bool { - if tv, ok := info.Types[expr]; ok && tv.Value != nil && tv.Value.Kind() == constant.Int { - if v, ok := constant.Int64Val(tv.Value); ok { - return v >= 0 - } +// flip changes the comparison token as if the operands were flipped. +// It is defined only for == and the four inequalities. +func flip(op token.Token) token.Token { + switch op { + case token.EQL: + return token.EQL // (same) + case token.GEQ: + return token.LEQ + case token.GTR: + return token.LSS + case token.LEQ: + return token.GEQ + case token.LSS: + return token.GTR } - return false + return op } // isBeforeSlice reports whether the SliceExpr is of the form s[:i] or s[0:i]. diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go index 0180a341e5606b193323cae11bead085201068be..bc15ef8b968ebce2d33603c32a5b301c5936d3a3 100644 --- a/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go +++ b/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go @@ -27,6 +27,7 @@ // to create base facts for, say, the fmt package for the // printf checker. import ( + "archive/zip" "encoding/gob" "encoding/json" "flag" @@ -74,6 +75,7 @@ PackageVetx map[string]string // maps package path to file of fact information VetxOnly bool // run analysis only for facts, not diagnostics VetxOutput string // where to write file of fact information Stdout string // write stdout (e.g. JSON, unified diff) to this file + FixArchive string // write fixed files to this zip archive, if non-empty SucceedOnTypecheckFailure bool // obsolete awful hack; see #18395 and below } @@ -153,7 +155,7 @@ code := 0 // In VetxOnly mode, the analysis is run only for facts. if !cfg.VetxOnly { - code = processResults(fset, cfg.ID, results) + code = processResults(fset, cfg.ID, cfg.FixArchive, results) } os.Exit(code) @@ -177,7 +179,7 @@ } return cfg, nil } -func processResults(fset *token.FileSet, id string, results []result) (exit int) { +func processResults(fset *token.FileSet, id, fixArchive string, results []result) (exit int) { if analysisflags.Fix { // Don't print the diagnostics, // but apply all fixes from the root actions. @@ -194,7 +196,40 @@ ReadFileFunc: os.ReadFile, // TODO(adonovan): respect overlays Diagnostics: res.diagnostics, } } - if err := driverutil.ApplyFixes(fixActions, analysisflags.Diff, false); err != nil { + + // By default, fixes overwrite the original file. + // With the -diff flag, print the diffs to stdout. + // If "go fix" provides a fix archive, we write files + // into it so that mutations happen after the build. + write := func(filename string, content []byte) error { + return os.WriteFile(filename, content, 0644) + } + if fixArchive != "" { + f, err := os.Create(fixArchive) + if err != nil { + log.Fatalf("can't create -fix archive: %v", err) + } + zw := zip.NewWriter(f) + zw.SetComment(id) // ignore error + defer func() { + if err := zw.Close(); err != nil { + log.Fatalf("closing -fix archive zip writer: %v", err) + } + if err := f.Close(); err != nil { + log.Fatalf("closing -fix archive file: %v", err) + } + }() + write = func(filename string, content []byte) error { + f, err := zw.Create(filename) + if err != nil { + return err + } + _, err = f.Write(content) + return err + } + } + + if err := driverutil.ApplyFixes(fixActions, write, analysisflags.Diff, false); err != nil { // Fail when applying fixes failed. log.Print(err) exit = 1 diff --git a/src/cmd/vendor/golang.org/x/tools/internal/analysis/driverutil/fix.go b/src/cmd/vendor/golang.org/x/tools/internal/analysis/driverutil/fix.go index 37b09588a7a1b8d0aef66320d66ad86f65f6a009..7769b39beb8fbb94f997ae5269f4df4f89a9ef93 100644 --- a/src/cmd/vendor/golang.org/x/tools/internal/analysis/driverutil/fix.go +++ b/src/cmd/vendor/golang.org/x/tools/internal/analysis/driverutil/fix.go @@ -93,11 +93,13 @@ // merged, and the corresponding files were successfully updated. // // If printDiff (from the -diff flag) is set, instead of updating the // files it display the final patch composed of all the cleanly merged -// fixes. +// fixes. (It is tempting to factor printDiff as just a variant of +// writeFile that is provided the old and new content, but it's hard +// to generate a good summary that way.) // // TODO(adonovan): handle file-system level aliases such as symbolic // links using robustio.FileID. -func ApplyFixes(actions []FixAction, printDiff, verbose bool) error { +func ApplyFixes(actions []FixAction, writeFile func(filename string, content []byte) error, printDiff, verbose bool) error { generated := make(map[*token.File]bool) // Select fixes to apply. @@ -264,12 +266,11 @@ // TODO(adonovan): abstract the I/O. os.Stdout.WriteString(unified) } else { - // write + // write file totalFiles++ - // TODO(adonovan): abstract the I/O. - if err := os.WriteFile(file, final, 0644); err != nil { + if err := writeFile(file, final); err != nil { log.Println(err) - continue + continue // (causes ApplyFix to return an error) } filesUpdated++ } diff --git a/src/cmd/vendor/golang.org/x/tools/internal/refactor/inline/callee.go b/src/cmd/vendor/golang.org/x/tools/internal/refactor/inline/callee.go index ce5beb27244bb3940203d83fe584aa1f221f7a21..9a960bd293cb55990451f64d86c6f5dbc60de519 100644 --- a/src/cmd/vendor/golang.org/x/tools/internal/refactor/inline/callee.go +++ b/src/cmd/vendor/golang.org/x/tools/internal/refactor/inline/callee.go @@ -725,8 +725,48 @@ // Find the relevant parameter type, accounting for variadics. paramType := paramTypeAtIndex(sig, call, i) ifaceAssign := paramType == nil || types.IsInterface(paramType) affectsInference := false - if fn := typeutil.StaticCallee(info, call); fn != nil { - if sig2 := fn.Type().(*types.Signature); sig2.Recv() == nil { + switch callee := typeutil.Callee(info, call).(type) { + case *types.Builtin: + // Consider this litmus test: + // + // func f(x int64) any { return max(x) } + // func main() { fmt.Printf("%T", f(42)) } + // + // If we lose the implicit conversion from untyped int + // to int64, the type inferred for the max(x) call changes, + // resulting in a different dynamic behavior: it prints + // int, not int64. + // + // Inferred result type affected: + // new + // complex, real, imag + // min, max + // + // Dynamic behavior change: + // append -- dynamic type of append([]any(nil), x)[0] + // delete(m, x) -- dynamic key type where m is map[any]unit + // panic -- dynamic type of panic value + // + // Unaffected: + // recover + // make + // len, cap + // clear + // close + // copy + // print, println -- only uses underlying types (?) + // + // The dynamic type cases are all covered by + // the ifaceAssign logic. + switch callee.Name() { + case "new", "complex", "real", "imag", "min", "max": + affectsInference = true + } + + case *types.Func: + // Only standalone (non-method) functions have type + // parameters affected by the call arguments. + if sig2 := callee.Signature(); sig2.Recv() == nil { originParamType := paramTypeAtIndex(sig2, call, i) affectsInference = originParamType == nil || new(typeparams.Free).Has(originParamType) } diff --git a/src/cmd/vendor/modules.txt b/src/cmd/vendor/modules.txt index 8641f2dca3dde88aeb8d43f768b491f60d83f11d..7e48798071ff8fd55f77108c1e9148c793a23a99 100644 --- a/src/cmd/vendor/modules.txt +++ b/src/cmd/vendor/modules.txt @@ -73,7 +73,7 @@ golang.org/x/text/internal/tag golang.org/x/text/language golang.org/x/text/transform golang.org/x/text/unicode/norm -# golang.org/x/tools v0.39.1-0.20251130212600-1ad6f3d02713 +# golang.org/x/tools v0.39.1-0.20251205000126-062ef7b6ced2 ## explicit; go 1.24.0 golang.org/x/tools/cmd/bisect golang.org/x/tools/cover diff --git a/src/crypto/md5/gen.go b/src/crypto/md5/gen.go index bcbc70367de9b16efa6818be8f08e311a43fa5d5..cad9e9140557fb238fd82a7574292eaf956deb02 100644 --- a/src/crypto/md5/gen.go +++ b/src/crypto/md5/gen.go @@ -153,7 +153,7 @@ 0x676f02d9, 0x8d2a4c8a, }, Table3: []uint32{ - // round3 + // round 3 0xfffa3942, 0x8771f681, 0x6d9d6122, diff --git a/src/crypto/tls/ticket.go b/src/crypto/tls/ticket.go index c56898c6f79813c957fb8497ea416f0f6400a34c..11f7efde858539ff85d93db2addc4d3330d22413 100644 --- a/src/crypto/tls/ticket.go +++ b/src/crypto/tls/ticket.go @@ -81,7 +81,7 @@ version uint16 isClient bool cipherSuite uint16 - // createdAt is the generation time of the secret on the sever (which for + // createdAt is the generation time of the secret on the server (which for // TLS 1.0–1.2 might be earlier than the current session) and the time at // which the ticket was received on the client. createdAt uint64 // seconds since UNIX epoch diff --git a/src/encoding/gob/decgen.go b/src/encoding/gob/decgen.go index 420c86207c200ed58f710650a0181634c7fea008..95d84b67623e013abcb5a3aa1eff483d71cdbc94 100644 --- a/src/encoding/gob/decgen.go +++ b/src/encoding/gob/decgen.go @@ -4,8 +4,8 @@ // license that can be found in the LICENSE file. //go:build ignore -// encgen writes the helper functions for encoding. Intended to be -// used with go generate; see the invocation in encode.go. +// decgen writes the helper functions for decoding. Intended to be +// used with go generate; see the invocation in decode.go. // TODO: We could do more by being unsafe. Add a -unsafe flag? diff --git a/src/go/ast/walk.go b/src/go/ast/walk.go index 24cdc60d73d5f2d626699b5f54c2ac4ecfebd59f..9328bccae41054c28bc596db56a85c6625d62688 100644 --- a/src/go/ast/walk.go +++ b/src/go/ast/walk.go @@ -370,7 +370,7 @@ // recursively for each of the non-nil children of node, followed by a // call of f(nil). // // In many cases it may be more convenient to use [Preorder], which -// returns an iterator over the sqeuence of nodes, or [PreorderStack], +// returns an iterator over the sequence of nodes, or [PreorderStack], // which (like [Inspect]) provides control over descent into subtrees, // but additionally reports the stack of enclosing nodes. func Inspect(node Node, f func(Node) bool) { diff --git a/src/internal/runtime/maps/table.go b/src/internal/runtime/maps/table.go index 8a1932e453f3c55ffdff130226fe338450cc939c..977f3091ade20383446ca3735369734217b43a2e 100644 --- a/src/internal/runtime/maps/table.go +++ b/src/internal/runtime/maps/table.go @@ -1260,8 +1260,10 @@ } // probeSeq maintains the state for a probe sequence that iterates through the // groups in a table. The sequence is a triangular progression of the form +// hash, hash + 1, hash + 1 + 2, hash + 1 + 2 + 3, ..., modulo mask + 1. +// The i-th term of the sequence is // -// p(i) := (i^2 + i)/2 + hash (mod mask+1) +// p(i) := hash + (i^2 + i)/2 (mod mask+1) // // The sequence effectively outputs the indexes of *groups*. The group // machinery allows us to check an entire group with minimal branching. diff --git a/src/internal/runtime/sys/dit_arm64.s b/src/internal/runtime/sys/dit_arm64.s index c27dfc9af3c1ec8b3813c0fc160e0956eec9e877..408b60c8c0a12c6f1a379556bfcc8dec0522212f 100644 --- a/src/internal/runtime/sys/dit_arm64.s +++ b/src/internal/runtime/sys/dit_arm64.s @@ -9,6 +9,11 @@ MRS DIT, R0 UBFX $24, R0, $1, R1 MOVB R1, ret+0(FP) MSR $1, DIT + // TODO(roland): the SB instruction is significantly more + // performant when available. We should detect its availability + // and use it when we can. + DSB $7 // nsh + ISB $15 // sy RET TEXT ·DITEnabled(SB),$0-1 diff --git a/src/net/http/cookie.go b/src/net/http/cookie.go index f74bc1043c509e2dc67073fe4b785e75a321173e..4cc9ae31522aeb98dfbfda5f702ca92c4aa4a07a 100644 --- a/src/net/http/cookie.go +++ b/src/net/http/cookie.go @@ -443,7 +443,8 @@ return false } if s[0] == '.' { - // A cookie a domain attribute may start with a leading dot. + // A cookie domain attribute may start with a leading dot. + // Per RFC 6265 section 5.2.3, a leading dot is ignored. s = s[1:] } last := byte('.') diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go index 00e67aeca0c46e3a7d3befda6dc73c2739190ca0..91f9740616f04ab178f42edf94e8e4e1cb144e5b 100644 --- a/src/runtime/crash_test.go +++ b/src/runtime/crash_test.go @@ -97,6 +97,13 @@ func runBuiltTestProg(t *testing.T, exe, name string, env ...string) string { t.Helper() + out, _ := runBuiltTestProgErr(t, exe, name, env...) + return out +} + +func runBuiltTestProgErr(t *testing.T, exe, name string, env ...string) (string, error) { + t.Helper() + if *flagQuick { t.Skip("-quick") } @@ -120,7 +127,7 @@ } else { t.Fatalf("%v failed to start: %v", cmd, err) } } - return string(out) + return string(out), err } var serializeBuild = make(chan bool, 2) diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index 26341c43001689ad25c15323261bf0acf04b908a..4f6ef9a3f262c4289b1f6dd9b62f94a50727381a 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -551,8 +551,11 @@ } return } -func GetNextArenaHint() uintptr { - return mheap_.arenaHints.addr +func NextArenaHint() (uintptr, bool) { + if mheap_.arenaHints == nil { + return 0, false + } + return mheap_.arenaHints.addr, true } type G = g diff --git a/src/runtime/goroutineleakprofile_test.go b/src/runtime/goroutineleakprofile_test.go index f5d2dd6372e54e304e358e80151add902de3e14a..9ab92d17c4eb255188396770591d90faada8cf3d 100644 --- a/src/runtime/goroutineleakprofile_test.go +++ b/src/runtime/goroutineleakprofile_test.go @@ -14,10 +14,13 @@ "testing" ) func TestGoroutineLeakProfile(t *testing.T) { - if strings.Contains(os.Getenv("GOFLAGS"), "mayMoreStackPreempt") { - // Some tests have false negatives under mayMoreStackPreempt. This may be a test-only issue, - // but needs more investigation. - testenv.SkipFlaky(t, 75729) + // Some tests have false negatives under mayMoreStackPreempt and mayMoreStackMove. + // This may be a test-only issue in that they're just sensitive to scheduling, but it + // needs more investigation. + for _, cfg := range []string{"mayMoreStackPreempt", "mayMoreStackMove"} { + if strings.Contains(os.Getenv("GOFLAGS"), cfg) { + testenv.SkipFlaky(t, 75729) + } } // Goroutine leak test case. @@ -486,10 +489,7 @@ // Combine all test cases into a single list. testCases := append(microTests, stressTestCases...) testCases = append(testCases, patternTestCases...) - // Test cases must not panic or cause fatal exceptions. - failStates := regexp.MustCompile(`fatal|panic|DATA RACE`) - - testApp := func(exepath string, testCases []testCase) { + runTests := func(exepath string, testCases []testCase) { // Build the test program once. exe, err := buildTestProg(t, exepath) @@ -503,7 +503,7 @@ t.Parallel() cmdEnv := []string{ "GODEBUG=asyncpreemptoff=1", - "GOEXPERIMENT=greenteagc,goroutineleakprofile", + "GOEXPERIMENT=goroutineleakprofile", } if tcase.simple { @@ -515,14 +515,14 @@ var output string for i := 0; i < tcase.repetitions; i++ { // Run program for one repetition and get runOutput trace. - runOutput := runBuiltTestProg(t, exe, tcase.name, cmdEnv...) + runOutput, err := runBuiltTestProgErr(t, exe, tcase.name, cmdEnv...) if len(runOutput) == 0 { t.Errorf("Test %s produced no output. Is the goroutine leak profile collected?", tcase.name) } - - // Zero tolerance policy for fatal exceptions, panics, or data races. - if failStates.MatchString(runOutput) { - t.Errorf("unexpected fatal exception or panic\noutput:\n%s\n\n", runOutput) + // Test cases must not end in a non-zero exit code, or otherwise experience a failure to + // actually execute. + if err != nil { + t.Errorf("unexpected failure\noutput:\n%s\n\n", runOutput) } output += runOutput + "\n\n" @@ -598,6 +598,6 @@ }) } } - testApp("testgoroutineleakprofile", testCases) - testApp("testgoroutineleakprofile/goker", gokerTestCases) + runTests("testgoroutineleakprofile", testCases) + runTests("testgoroutineleakprofile/goker", gokerTestCases) } diff --git a/src/runtime/malloc_test.go b/src/runtime/malloc_test.go index 97cf0eed54c77ceac805b00ee942f13389e69a1f..b76b0a02acf2c04b11daff19fca0dbfc3bdb09d3 100644 --- a/src/runtime/malloc_test.go +++ b/src/runtime/malloc_test.go @@ -664,10 +664,24 @@ t.Skipf("failed to reserve memory at next arena hint [%#x, %#x)", start, end) } t.Logf("reserved [%#x, %#x)", start, end) disallowed = append(disallowed, [2]uintptr{start, end}) + + hint, ok := NextArenaHint() + if !ok { + // We're out of arena hints. There's not much we can do now except give up. + // This might happen for a number of reasons, like if there's just something + // else already mapped in the address space where we put our hints. This is + // a bit more common than it used to be thanks to heap base randomization. + t.Skip("ran out of arena hints") + } + // Allocate until the runtime tries to use the hint we // just mapped over. - hint := GetNextArenaHint() - for GetNextArenaHint() == hint { + for { + if next, ok := NextArenaHint(); !ok { + t.Skip("ran out of arena hints") + } else if next != hint { + break + } ac := new(acLink) arenaCollisionSink = append(arenaCollisionSink, ac) // The allocation must not have fallen into diff --git a/src/runtime/metrics_test.go b/src/runtime/metrics_test.go index 92cec75465ce4cc2c93ccfaf699c092e05982026..efd2c2adb9bd851407e148c8fc2d0f5c794b6ef7 100644 --- a/src/runtime/metrics_test.go +++ b/src/runtime/metrics_test.go @@ -1584,3 +1584,18 @@ if output != want { t.Fatalf("output:\n%s\n\nwanted:\n%s", output, want) } } + +func TestNotInGoMetricCallback(t *testing.T) { + switch runtime.GOOS { + case "windows", "plan9": + t.Skip("unsupported on Windows and Plan9") + } + + // This test is run in a subprocess to prevent other tests from polluting the metrics + // and because we need to make some cgo callbacks. + output := runTestProg(t, "testprogcgo", "NotInGoMetricCallback") + want := "OK\n" + if output != want { + t.Fatalf("output:\n%s\n\nwanted:\n%s", output, want) + } +} diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 32cd8cb0e892647954bef0e6806c7453eb41e2a4..a3bed4b3eb62f990c4d5581d484a944a8b0b5263 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -5,7 +5,7 @@ // Garbage collector (GC). // // The GC runs concurrently with mutator threads, is type accurate (aka precise), allows multiple -// GC thread to run in parallel. It is a concurrent mark and sweep that uses a write barrier. It is +// GC threads to run in parallel. It is a concurrent mark and sweep that uses a write barrier. It is // non-generational and non-compacting. Allocation is done using size segregated per P allocation // areas to minimize fragmentation while eliminating locks in the common case. // diff --git a/src/runtime/pinner.go b/src/runtime/pinner.go index dad14a4d09c5e394c3953ba955bc64218cc813b9..fa0a8f5c53d7a3b6946cefd80357b0558c86b882 100644 --- a/src/runtime/pinner.go +++ b/src/runtime/pinner.go @@ -12,7 +12,25 @@ ) // A Pinner is a set of Go objects each pinned to a fixed location in memory. The // [Pinner.Pin] method pins one object, while [Pinner.Unpin] unpins all pinned -// objects. See their comments for more information. +// objects. +// +// The purpose of a Pinner is two-fold. +// First, it allows C code to safely use Go pointers that have not been passed +// explicitly to the C code via a cgo call. +// For example, for safely interacting with a pointer stored inside of a struct +// whose pointer is passed to a C function. +// Second, it allows C memory to safely retain that Go pointer even after the +// cgo call returns, provided the object remains pinned. +// +// A Pinner arranges for its objects to be automatically unpinned some time after +// it becomes unreachable, so its referents will not leak. However, this means the +// Pinner itself must be kept alive across a cgo call, or as long as C retains a +// reference to the pinned Go pointers. +// +// Reusing a Pinner is safe, and in fact encouraged, to avoid the cost of +// initializing new Pinners on first use. +// +// The zero value of Pinner is ready to use. type Pinner struct { *pinner } @@ -26,6 +44,7 @@ // contains pointers to Go objects, these objects must be pinned separately if they // are going to be accessed from C code. // // The argument must be a pointer of any type or an [unsafe.Pointer]. +// // It's safe to call Pin on non-Go pointers, in which case Pin will do nothing. func (p *Pinner) Pin(pointer any) { if p.pinner == nil { @@ -63,6 +82,7 @@ } } // Unpin unpins all pinned objects of the [Pinner]. +// It's safe and encouraged to reuse a Pinner after calling Unpin. func (p *Pinner) Unpin() { p.pinner.unpin() diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 16538098cf6c152627d8936cdf15a635f9de24fd..52def488ffca42f8a9cf3547020a709c551050f3 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -2433,7 +2433,7 @@ setg(mp.g0) sp := sys.GetCallerSP() callbackUpdateSystemStack(mp, sp, signal) - // Should mark we are already in Go now. + // We must mark that we are already in Go now. // Otherwise, we may call needm again when we get a signal, before cgocallbackg1, // which means the extram list may be empty, that will cause a deadlock. mp.isExtraInC = false @@ -2455,7 +2455,8 @@ // mp.curg is now a real goroutine. casgstatus(mp.curg, _Gdeadextra, _Gsyscall) sched.ngsys.Add(-1) - sched.nGsyscallNoP.Add(1) + // N.B. We do not update nGsyscallNoP, because isExtraInC threads are not + // counted as real goroutines while they're in C. if !signal { if trace.ok() { @@ -2590,7 +2591,7 @@ // Return mp.curg to _Gdeadextra state. casgstatus(mp.curg, _Gsyscall, _Gdeadextra) mp.curg.preemptStop = false sched.ngsys.Add(1) - sched.nGsyscallNoP.Add(-1) + decGSyscallNoP(mp) if !mp.isExtraInSig { if trace.ok() { @@ -4732,7 +4733,7 @@ if trace.ok() { trace.ProcStop(pp) } - sched.nGsyscallNoP.Add(1) + addGSyscallNoP(gp.m) // We gave up our P voluntarily. pp.gcStopTime = nanotime() pp.syscalltick++ if sched.stopwait--; sched.stopwait == 0 { @@ -4763,7 +4764,7 @@ gp.stackguard0 = stackPreempt // see comment in entersyscall gp.m.syscalltick = gp.m.p.ptr().syscalltick gp.m.p.ptr().syscalltick++ - sched.nGsyscallNoP.Add(1) + addGSyscallNoP(gp.m) // We're going to give up our P. // Leave SP around for GC and traceback. pc := sys.GetCallerPC() @@ -5001,8 +5002,8 @@ // Try to steal our old P back. if oldp != nil { if thread, ok := setBlockOnExitSyscall(oldp); ok { thread.takeP() + addGSyscallNoP(thread.mp) // takeP does the opposite, but this is a net zero change. thread.resume() - sched.nGsyscallNoP.Add(-1) // takeP adds 1. return oldp } } @@ -5017,7 +5018,7 @@ notewakeup(&sched.sysmonnote) } unlock(&sched.lock) if pp != nil { - sched.nGsyscallNoP.Add(-1) + decGSyscallNoP(getg().m) // We got a P for ourselves. return pp } } @@ -5043,7 +5044,7 @@ // lost our P or not (determined by exitsyscallfast). trace.GoSysExit(true) traceRelease(trace) } - sched.nGsyscallNoP.Add(-1) + decGSyscallNoP(getg().m) dropg() lock(&sched.lock) var pp *p @@ -5081,6 +5082,41 @@ stopm() schedule() // Never returns. } +// addGSyscallNoP must be called when a goroutine in a syscall loses its P. +// This function updates all relevant accounting. +// +// nosplit because it's called on the syscall paths. +// +//go:nosplit +func addGSyscallNoP(mp *m) { + // It's safe to read isExtraInC here because it's only mutated + // outside of _Gsyscall, and we know this thread is attached + // to a goroutine in _Gsyscall and blocked from exiting. + if !mp.isExtraInC { + // Increment nGsyscallNoP since we're taking away a P + // from a _Gsyscall goroutine, but only if isExtraInC + // is not set on the M. If it is, then this thread is + // back to being a full C thread, and will just inflate + // the count of not-in-go goroutines. See go.dev/issue/76435. + sched.nGsyscallNoP.Add(1) + } +} + +// decGSsyscallNoP must be called whenever a goroutine in a syscall without +// a P exits the system call. This function updates all relevant accounting. +// +// nosplit because it's called from dropm. +// +//go:nosplit +func decGSyscallNoP(mp *m) { + // Update nGsyscallNoP, but only if this is not a thread coming + // out of C. See the comment in addGSyscallNoP. This logic must match, + // to avoid unmatched increments and decrements. + if !mp.isExtraInC { + sched.nGsyscallNoP.Add(-1) + } +} + // Called from syscall package before fork. // // syscall_runtime_BeforeFork is for package syscall, @@ -6758,7 +6794,7 @@ if trace.ok() { trace.ProcSteal(s.pp) traceRelease(trace) } - sched.nGsyscallNoP.Add(1) + addGSyscallNoP(s.mp) s.pp.syscalltick++ } diff --git a/src/runtime/race_ppc64le.s b/src/runtime/race_ppc64le.s index b327e49a2f181ea576ebf1b96af1f6fafe8fda3b..41cd232392e5641b7429b9690daa79f195eb0e7f 100644 --- a/src/runtime/race_ppc64le.s +++ b/src/runtime/race_ppc64le.s @@ -329,11 +329,13 @@ // And TEXT sync∕atomic·AndInt32(SB), NOSPLIT, $0-20 GO_ARGS MOVD $__tsan_go_atomic32_fetch_and(SB), R8 + ADD $32, R1, R6 BR racecallatomic<>(SB) TEXT sync∕atomic·AndInt64(SB), NOSPLIT, $0-24 GO_ARGS MOVD $__tsan_go_atomic64_fetch_and(SB), R8 + ADD $32, R1, R6 BR racecallatomic<>(SB) TEXT sync∕atomic·AndUint32(SB), NOSPLIT, $0-20 @@ -352,11 +354,13 @@ // Or TEXT sync∕atomic·OrInt32(SB), NOSPLIT, $0-20 GO_ARGS MOVD $__tsan_go_atomic32_fetch_or(SB), R8 + ADD $32, R1, R6 BR racecallatomic<>(SB) TEXT sync∕atomic·OrInt64(SB), NOSPLIT, $0-24 GO_ARGS MOVD $__tsan_go_atomic64_fetch_or(SB), R8 + ADD $32, R1, R6 BR racecallatomic<>(SB) TEXT sync∕atomic·OrUint32(SB), NOSPLIT, $0-20 diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index cd75e2dd7c5a5e3ff428c6ccadcd1c9fcadb245d..fde378ff25ce540dfecd06510a225c93e3adb919 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -945,7 +945,7 @@ nmsys int32 // number of system m's not counted for deadlock nmfreed int64 // cumulative number of freed m's ngsys atomic.Int32 // number of system goroutines - nGsyscallNoP atomic.Int32 // number of goroutines in syscalls without a P + nGsyscallNoP atomic.Int32 // number of goroutines in syscalls without a P but whose M is not isExtraInC pidle puintptr // idle p's npidle atomic.Int32 diff --git a/src/runtime/testdata/testprog/schedmetrics.go b/src/runtime/testdata/testprog/schedmetrics.go index 8e8abc4484501681b7a62d286dbeffbee12da726..7fad95a976a1a87bf22dfccc35444e5de519dca9 100644 --- a/src/runtime/testdata/testprog/schedmetrics.go +++ b/src/runtime/testdata/testprog/schedmetrics.go @@ -91,8 +91,10 @@ // Certain build modes may also cause the creation of additional // threads through frequent scheduling, like mayMoreStackPreempt. // A slack of 5 is arbitrary but appears to be enough to cover // the leftovers plus any inflation from scheduling-heavy build - // modes. - const threadsSlack = 5 + // modes. We then also add initialGMP to this slack, since we're + // about to call runtime.GC, and in the worst case this will + // spin up GOMAXPROCS new threads to run those workers. + threadsSlack := 5 + uint64(initialGMP) // Make sure GC isn't running, since GC workers interfere with // expected counts. diff --git a/src/runtime/testdata/testprogcgo/notingo.go b/src/runtime/testdata/testprogcgo/notingo.go new file mode 100644 index 0000000000000000000000000000000000000000..e5b1062e9e3eab339efb28e8c0e2f487a10a8d14 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/notingo.go @@ -0,0 +1,108 @@ +// Copyright 2025 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. + +//go:build !plan9 && !windows +// +build !plan9,!windows + +package main + +/* +#include +#include +#include + +extern void Ready(); + +static int spinning; +static int released; + +static void* enterGoThenSpinTwice(void* arg __attribute__ ((unused))) { + Ready(); + atomic_fetch_add(&spinning, 1); + while(atomic_load(&released) == 0) {}; + + Ready(); + atomic_fetch_add(&spinning, 1); + while(1) {}; + return NULL; +} + +static void SpinTwiceInNewCThread() { + pthread_t tid; + pthread_create(&tid, NULL, enterGoThenSpinTwice, NULL); +} + +static int Spinning() { + return atomic_load(&spinning); +} + +static void Release() { + atomic_store(&spinning, 0); + atomic_store(&released, 1); +} +*/ +import "C" + +import ( + "os" + "runtime" + "runtime/metrics" +) + +func init() { + register("NotInGoMetricCallback", NotInGoMetricCallback) +} + +func NotInGoMetricCallback() { + const N = 10 + s := []metrics.Sample{{Name: "/sched/goroutines/not-in-go:goroutines"}} + + // Create N new C threads that have called into Go at least once. + for range N { + C.SpinTwiceInNewCThread() + } + + // Synchronize with spinning threads twice. + // + // This helps catch bad accounting by taking at least a couple other + // codepaths which would cause the accounting to change. + for i := range 2 { + // Make sure they pass through Go. + // N.B. Ready is called twice by the new threads. + for j := range N { + <-readyCh + if j == 2 { + // Try to trigger an update in the immediate STW handoff case. + runtime.ReadMemStats(&m) + } + } + + // Make sure they're back in C. + for C.Spinning() < N { + } + + // Do something that stops the world to take all the Ps back. + runtime.ReadMemStats(&m) + + if i == 0 { + C.Release() + } + } + + // Read not-in-go. + metrics.Read(s) + if n := s[0].Value.Uint64(); n != 0 { + println("expected 0 not-in-go goroutines, found", n) + os.Exit(2) + } + println("OK") +} + +var m runtime.MemStats +var readyCh = make(chan bool) + +//export Ready +func Ready() { + readyCh <- true +} diff --git a/test/fixedbugs/issue76709.go b/test/fixedbugs/issue76709.go new file mode 100644 index 0000000000000000000000000000000000000000..e5a3a99635adbec9a715d253b5d9ecf79e5b9048 --- /dev/null +++ b/test/fixedbugs/issue76709.go @@ -0,0 +1,42 @@ +// run + +// Copyright 2025 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 "fmt" + +//go:noinline +func bug1(a []int, i int) int { + if i < 0 || i > 20-len(a) { + return 0 + } + diff := len(a) - i + if diff < 10 { + return 1 + } + return 2 +} + +//go:noinline +func bug2(s []int, i int) int { + if i < 0 { + return 0 + } + if i <= 10-len(s) { + x := len(s) - i + return x / 2 + } + return 0 +} + +func main() { + if got := bug1(make([]int, 5), 15); got != 1 { + panic(fmt.Sprintf("bug1: got %d, want 1", got)) + } + if got := bug2(make([]int, 3), 7); got != -2 { + panic(fmt.Sprintf("bug2: got %d, want -2", got)) + } +}