diff --git a/doc/godebug.md b/doc/godebug.md index 15be9da5df0..d107b1baf15 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -187,7 +187,7 @@ Go 1.25 switched to SHA-256 to fill in missing SubjectKeyId in crypto/x509.CreateCertificate. The setting `x509sha256skid=0` reverts to SHA-1. Go 1.25 corrected the semantics of contention reports for runtime-internal locks, -and so removed the [`runtimecontentionstacks` setting](/pkg/runtime#hdr-Environment_Variable). +and so removed the [`runtimecontentionstacks` setting](/pkg/runtime#hdr-Environment_Variables). ### Go 1.24 @@ -369,7 +369,7 @@ In particular, a common default Linux kernel configuration can result in significant memory overheads, and Go 1.22 no longer works around this default. To work around this issue without adjusting kernel settings, transparent huge pages can be disabled for Go memory with the -[`disablethp` setting](/pkg/runtime#hdr-Environment_Variable). +[`disablethp` setting](/pkg/runtime#hdr-Environment_Variables). This behavior was backported to Go 1.21.1, but the setting is only available starting with Go 1.21.6. This setting may be removed in a future release, and users impacted by this issue @@ -381,7 +381,7 @@ Go 1.22 added contention on runtime-internal locks to the [`mutex` profile](/pkg/runtime/pprof#Profile). Contention on these locks is always reported at `runtime._LostContendedRuntimeLock`. Complete stack traces of runtime locks can be enabled with the [`runtimecontentionstacks` -setting](/pkg/runtime#hdr-Environment_Variable). These stack traces have +setting](/pkg/runtime#hdr-Environment_Variables). These stack traces have non-standard semantics, see setting documentation for details. Go 1.22 added a new [`crypto/x509.Certificate`](/pkg/crypto/x509/#Certificate) diff --git a/lib/wasm/go_wasip1_wasm_exec b/lib/wasm/go_wasip1_wasm_exec index 3b2d12ec458..2de1758793f 100755 --- a/lib/wasm/go_wasip1_wasm_exec +++ b/lib/wasm/go_wasip1_wasm_exec @@ -14,7 +14,7 @@ case "$GOWASIRUNTIME" in exec wazero run -mount /:/ -env-inherit -cachedir "${TMPDIR:-/tmp}"/wazero ${GOWASIRUNTIMEARGS:-} "$1" "${@:2}" ;; "wasmtime" | "") - exec wasmtime run --dir=/ --env PWD="$PWD" --env PATH="$PATH" -W max-wasm-stack=1048576 ${GOWASIRUNTIMEARGS:-} "$1" "${@:2}" + exec wasmtime run --dir=/ --env PWD="$PWD" --env PATH="$PATH" -W max-wasm-stack=8388608 ${GOWASIRUNTIMEARGS:-} "$1" "${@:2}" ;; *) echo "Unknown Go WASI runtime specified: $GOWASIRUNTIME" diff --git a/src/cmd/compile/internal/types2/api_test.go b/src/cmd/compile/internal/types2/api_test.go index 44fb6afe985..0d3c8b8e3e5 100644 --- a/src/cmd/compile/internal/types2/api_test.go +++ b/src/cmd/compile/internal/types2/api_test.go @@ -358,6 +358,11 @@ func TestTypesInfo(t *testing.T) { // go.dev/issue/47895 {`package p; import "unsafe"; type S struct { f int }; var s S; var _ = unsafe.Offsetof(s.f)`, `s.f`, `int`}, + // go.dev/issue/74303. Note that interface field types are synthetic, so + // even though `func()` doesn't appear in the source, it appears in the + // syntax tree. + {`package p; type T interface { M(int) }`, `func(int)`, `func(int)`}, + // go.dev/issue/50093 {`package u0a; func _[_ interface{int}]() {}`, `int`, `int`}, {`package u1a; func _[_ interface{~int}]() {}`, `~int`, `~int`}, diff --git a/src/cmd/compile/internal/types2/interface.go b/src/cmd/compile/internal/types2/interface.go index b32e5c21fe2..522f1dd3fe3 100644 --- a/src/cmd/compile/internal/types2/interface.go +++ b/src/cmd/compile/internal/types2/interface.go @@ -137,19 +137,17 @@ func (check *Checker) interfaceType(ityp *Interface, iface *syntax.InterfaceType name := f.Name.Value if name == "_" { check.error(f.Name, BlankIfaceMethod, "methods must have a unique non-blank name") - continue // ignore method + continue // ignore } - // Type-check method declaration. - // Note: Don't call check.typ(f.Type) as that would record - // the method incorrectly as a type expression in Info.Types. - ftyp, _ := f.Type.(*syntax.FuncType) - if ftyp == nil { - check.errorf(f.Type, InvalidSyntaxTree, "%s is not a method signature", f.Type) - continue // ignore method + typ := check.typ(f.Type) + sig, _ := typ.(*Signature) + if sig == nil { + if isValid(typ) { + check.errorf(f.Type, InvalidSyntaxTree, "%s is not a method signature", typ) + } + continue // ignore } - sig := new(Signature) - check.funcType(sig, nil, nil, ftyp) // use named receiver type if available (for better error messages) var recvTyp Type = ityp diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index e7f7d98afdc..b731ea51e5a 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -336,6 +336,10 @@ type goTest struct { // omitVariant indicates that variant is used solely for the dist test name and // that the set of test names run by each variant (including empty) of a package // is non-overlapping. + // + // TODO(mknyszek): Consider removing omitVariant as it is no longer set to true + // by any test. It's too valuable to have timing information in ResultDB that + // corresponds directly with dist names for tests. omitVariant bool // We have both pkg and pkgs as a convenience. Both may be set, in which @@ -595,8 +599,11 @@ func (t *tester) registerRaceBenchTest(pkg string) { defer timelog("end", dt.name) ranGoBench = true return (&goTest{ - variant: "racebench", - omitVariant: true, // The only execution of benchmarks in dist; benchmark names are guaranteed not to overlap with test names. + variant: "racebench", + // Include the variant even though there's no overlap in test names. + // This makes the test targets distinct, allowing our build system to record + // elapsed time for each one, which is useful for load-balancing test shards. + omitVariant: false, timeout: 1200 * time.Second, // longer timeout for race with benchmarks race: true, bench: true, @@ -736,6 +743,15 @@ func (t *tester) registerTests() { } } + // Test GOEXPERIMENT=jsonv2. + if !strings.Contains(goexperiment, "jsonv2") { + t.registerTest("GOEXPERIMENT=jsonv2 go test encoding/json/...", &goTest{ + variant: "jsonv2", + env: []string{"GOEXPERIMENT=jsonv2"}, + pkg: "encoding/json/...", + }) + } + // Test ios/amd64 for the iOS simulator. if goos == "darwin" && goarch == "amd64" && t.cgoEnabled { t.registerTest("GOOS=ios on darwin/amd64", @@ -985,8 +1001,11 @@ func (t *tester) registerTests() { id := fmt.Sprintf("%d_%d", shard, nShards) t.registerTest("../test", &goTest{ - variant: id, - omitVariant: true, // Shards of the same Go package; tests are guaranteed not to overlap. + variant: id, + // Include the variant even though there's no overlap in test names. + // This makes the test target more clearly distinct in our build + // results and is important for load-balancing test shards. + omitVariant: false, pkg: "cmd/internal/testdir", testFlags: []string{fmt.Sprintf("-shard=%d", shard), fmt.Sprintf("-shards=%d", nShards)}, runOnHost: true, diff --git a/src/cmd/go/internal/bug/bug.go b/src/cmd/go/internal/bug/bug.go index d3f9065d3da..4ff45d2d888 100644 --- a/src/cmd/go/internal/bug/bug.go +++ b/src/cmd/go/internal/bug/bug.go @@ -69,7 +69,7 @@ const bugFooter = `### What did you do? diff --git a/src/cmd/go/internal/cache/default.go b/src/cmd/go/internal/cache/default.go index b2dd69edc53..cc4e0517b4a 100644 --- a/src/cmd/go/internal/cache/default.go +++ b/src/cmd/go/internal/cache/default.go @@ -28,7 +28,7 @@ var initDefaultCacheOnce = sync.OnceValue(initDefaultCache) const cacheREADME = `This directory holds cached build artifacts from the Go build system. Run "go clean -cache" if the directory is getting too large. Run "go clean -fuzzcache" to delete the fuzz cache. -See golang.org to learn more about Go. +See go.dev to learn more about Go. ` // initDefaultCache does the work of finding the default cache diff --git a/src/cmd/go/internal/fips140/fips140.go b/src/cmd/go/internal/fips140/fips140.go index 328e06088e3..7ca0cde5880 100644 --- a/src/cmd/go/internal/fips140/fips140.go +++ b/src/cmd/go/internal/fips140/fips140.go @@ -114,7 +114,11 @@ func Init() { fsys.Bind(Dir(), filepath.Join(cfg.GOROOT, "src/crypto/internal/fips140")) } - if cfg.Experiment.BoringCrypto && Enabled() { + // ExperimentErr != nil if GOEXPERIMENT failed to parse. Typically + // cmd/go main will exit in this case, but it is allowed during + // toolchain selection, as the GOEXPERIMENT may be valid for the + // selected toolchain version. + if cfg.ExperimentErr == nil && cfg.Experiment.BoringCrypto && Enabled() { base.Fatalf("go: cannot use GOFIPS140 with GOEXPERIMENT=boringcrypto") } } diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 6561362210d..d439092737a 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -2368,7 +2368,6 @@ var blockedLinknames = map[string][]string{ "crypto/internal/sysrand.fatal": {"crypto/internal/sysrand"}, "crypto/rand.fatal": {"crypto/rand"}, "internal/runtime/maps.errNilAssign": {"internal/runtime/maps"}, - "internal/runtime/maps.typeString": {"internal/runtime/maps"}, "internal/runtime/maps.fatal": {"internal/runtime/maps"}, "internal/runtime/maps.newarray": {"internal/runtime/maps"}, "internal/runtime/maps.newobject": {"internal/runtime/maps"}, @@ -2399,6 +2398,23 @@ var blockedLinknames = map[string][]string{ "runtime.mapdelete_fast32": {"runtime"}, "runtime.mapdelete_fast64": {"runtime"}, "runtime.mapdelete_faststr": {"runtime"}, + // New internal linknames in Go 1.25 + // Pushed from runtime + "internal/cpu.riscvHWProbe": {"internal/cpu"}, + "internal/runtime/cgroup.throw": {"internal/runtime/cgroup"}, + "internal/runtime/maps.typeString": {"internal/runtime/maps"}, + "internal/synctest.IsInBubble": {"internal/synctest"}, + "internal/synctest.associate": {"internal/synctest"}, + "internal/synctest.disassociate": {"internal/synctest"}, + "internal/synctest.isAssociated": {"internal/synctest"}, + "runtime/trace.runtime_readTrace": {"runtime/trace"}, + "runtime/trace.runtime_traceClockUnitsPerSecond": {"runtime/trace"}, + "sync_test.runtime_blockUntilEmptyCleanupQueue": {"sync_test"}, + "time.runtimeIsBubbled": {"time"}, + "unique.runtime_blockUntilEmptyCleanupQueue": {"unique"}, + // Others + "net.newWindowsFile": {"net"}, // pushed from os + "testing/synctest.testingSynctestTest": {"testing/synctest"}, // pushed from testing } // check if a linkname reference to symbol s from pkg is allowed diff --git a/src/cmd/trace/gen.go b/src/cmd/trace/gen.go index 4455f830461..9cc22df1f68 100644 --- a/src/cmd/trace/gen.go +++ b/src/cmd/trace/gen.go @@ -283,11 +283,11 @@ func (g *globalMetricGenerator) GlobalMetric(ctx *traceContext, ev *trace.Event) m := ev.Metric() switch m.Name { case "/memory/classes/heap/objects:bytes": - ctx.HeapAlloc(ctx.elapsed(ev.Time()), m.Value.ToUint64()) + ctx.HeapAlloc(ctx.elapsed(ev.Time()), m.Value.Uint64()) case "/gc/heap/goal:bytes": - ctx.HeapGoal(ctx.elapsed(ev.Time()), m.Value.ToUint64()) + ctx.HeapGoal(ctx.elapsed(ev.Time()), m.Value.Uint64()) case "/sched/gomaxprocs:threads": - ctx.Gomaxprocs(m.Value.ToUint64()) + ctx.Gomaxprocs(m.Value.Uint64()) } } diff --git a/src/context/context_test.go b/src/context/context_test.go index 57066c96859..ad47f853dd4 100644 --- a/src/context/context_test.go +++ b/src/context/context_test.go @@ -5,7 +5,7 @@ package context // Tests in package context cannot depend directly on package testing due to an import cycle. -// If your test does requires access to unexported members of the context package, +// If your test requires access to unexported members of the context package, // add your test below as `func XTestFoo(t testingT)` and add a `TestFoo` to x_test.go // that calls it. Otherwise, write a regular test in a test.go file in package context_test. diff --git a/src/crypto/cipher/gcm.go b/src/crypto/cipher/gcm.go index 5580f96d55a..73493f6cd23 100644 --- a/src/crypto/cipher/gcm.go +++ b/src/crypto/cipher/gcm.go @@ -82,7 +82,7 @@ func newGCM(cipher Block, nonceSize, tagSize int) (AEAD, error) { // NewGCMWithRandomNonce returns the given cipher wrapped in Galois Counter // Mode, with randomly-generated nonces. The cipher must have been created by -// [aes.NewCipher]. +// [crypto/aes.NewCipher]. // // It generates a random 96-bit nonce, which is prepended to the ciphertext by Seal, // and is extracted from the ciphertext by Open. The NonceSize of the AEAD is zero, diff --git a/src/encoding/json/bench_test.go b/src/encoding/json/bench_test.go index cd55ceed901..047188131ce 100644 --- a/src/encoding/json/bench_test.go +++ b/src/encoding/json/bench_test.go @@ -14,9 +14,9 @@ package json import ( "bytes" - "compress/gzip" "fmt" "internal/testenv" + "internal/zstd" "io" "os" "reflect" @@ -46,15 +46,12 @@ var codeJSON []byte var codeStruct codeResponse func codeInit() { - f, err := os.Open("testdata/code.json.gz") + f, err := os.Open("internal/jsontest/testdata/golang_source.json.zst") if err != nil { panic(err) } defer f.Close() - gz, err := gzip.NewReader(f) - if err != nil { - panic(err) - } + gz := zstd.NewReader(f) data, err := io.ReadAll(gz) if err != nil { panic(err) diff --git a/src/encoding/json/decode_test.go b/src/encoding/json/decode_test.go index 5bc3d3c8564..473fd028330 100644 --- a/src/encoding/json/decode_test.go +++ b/src/encoding/json/decode_test.go @@ -1189,6 +1189,27 @@ var unmarshalTests = []struct { out: []int{1, 2, 0, 4, 5}, err: &UnmarshalTypeError{Value: "bool", Type: reflect.TypeFor[int](), Offset: 9}, }, + + { + CaseName: Name("DashComma"), + in: `{"-":"hello"}`, + ptr: new(struct { + F string `json:"-,"` + }), + out: struct { + F string `json:"-,"` + }{"hello"}, + }, + { + CaseName: Name("DashCommaOmitEmpty"), + in: `{"-":"hello"}`, + ptr: new(struct { + F string `json:"-,omitempty"` + }), + out: struct { + F string `json:"-,omitempty"` + }{"hello"}, + }, } func TestMarshal(t *testing.T) { diff --git a/src/encoding/json/jsontext/doc.go b/src/encoding/json/jsontext/doc.go index 755305151fb..8e4bced015d 100644 --- a/src/encoding/json/jsontext/doc.go +++ b/src/encoding/json/jsontext/doc.go @@ -10,6 +10,11 @@ // primitive data types such as booleans, strings, and numbers, // in addition to structured data types such as objects and arrays. // +// This package (encoding/json/jsontext) is experimental, +// and not subject to the Go 1 compatibility promise. +// It only exists when building with the GOEXPERIMENT=jsonv2 environment variable set. +// Most users should use [encoding/json]. +// // The [Encoder] and [Decoder] types are used to encode or decode // a stream of JSON tokens or values. // @@ -20,8 +25,8 @@ // - a JSON literal (i.e., null, true, or false) // - a JSON string (e.g., "hello, world!") // - a JSON number (e.g., 123.456) -// - a start or end delimiter for a JSON object (i.e., '{' or '}') -// - a start or end delimiter for a JSON array (i.e., '[' or ']') +// - a begin or end delimiter for a JSON object (i.e., '{' or '}') +// - a begin or end delimiter for a JSON array (i.e., '[' or ']') // // A JSON token is represented by the [Token] type in Go. Technically, // there are two additional structural characters (i.e., ':' and ','), diff --git a/src/encoding/json/jsontext/encode.go b/src/encoding/json/jsontext/encode.go index a1e6307adc8..4853a11059e 100644 --- a/src/encoding/json/jsontext/encode.go +++ b/src/encoding/json/jsontext/encode.go @@ -713,7 +713,7 @@ func (e *encoderState) reformatValue(dst []byte, src Value, depth int) ([]byte, // appends it to the end of src, reformatting whitespace and strings as needed. // It returns the extended dst buffer and the number of consumed input bytes. func (e *encoderState) reformatObject(dst []byte, src Value, depth int) ([]byte, int, error) { - // Append object start. + // Append object begin. if len(src) == 0 || src[0] != '{' { panic("BUG: reformatObject must be called with a buffer that starts with '{'") } else if depth == maxNestingDepth+1 { @@ -824,7 +824,7 @@ func (e *encoderState) reformatObject(dst []byte, src Value, depth int) ([]byte, // appends it to the end of dst, reformatting whitespace and strings as needed. // It returns the extended dst buffer and the number of consumed input bytes. func (e *encoderState) reformatArray(dst []byte, src Value, depth int) ([]byte, int, error) { - // Append array start. + // Append array begin. if len(src) == 0 || src[0] != '[' { panic("BUG: reformatArray must be called with a buffer that starts with '['") } else if depth == maxNestingDepth+1 { diff --git a/src/encoding/json/jsontext/state.go b/src/encoding/json/jsontext/state.go index 1e8b4f22dbf..d214fd51903 100644 --- a/src/encoding/json/jsontext/state.go +++ b/src/encoding/json/jsontext/state.go @@ -297,7 +297,7 @@ func (m *stateMachine) appendNumber() error { return m.appendLiteral() } -// pushObject appends a JSON start object token as next in the sequence. +// pushObject appends a JSON begin object token as next in the sequence. // If an error is returned, the state is not mutated. func (m *stateMachine) pushObject() error { switch { @@ -332,7 +332,7 @@ func (m *stateMachine) popObject() error { } } -// pushArray appends a JSON start array token as next in the sequence. +// pushArray appends a JSON begin array token as next in the sequence. // If an error is returned, the state is not mutated. func (m *stateMachine) pushArray() error { switch { diff --git a/src/encoding/json/jsontext/token.go b/src/encoding/json/jsontext/token.go index 22717b154ac..e78c3f84d86 100644 --- a/src/encoding/json/jsontext/token.go +++ b/src/encoding/json/jsontext/token.go @@ -33,8 +33,8 @@ var errInvalidToken = errors.New("invalid jsontext.Token") // - a JSON literal (i.e., null, true, or false) // - a JSON string (e.g., "hello, world!") // - a JSON number (e.g., 123.456) -// - a start or end delimiter for a JSON object (i.e., { or } ) -// - a start or end delimiter for a JSON array (i.e., [ or ] ) +// - a begin or end delimiter for a JSON object (i.e., { or } ) +// - a begin or end delimiter for a JSON array (i.e., [ or ] ) // // A Token cannot represent entire array or object values, while a [Value] can. // There is no Token to represent commas and colons since @@ -481,9 +481,9 @@ func (t Token) Kind() Kind { // - 't': true // - '"': string // - '0': number -// - '{': object start +// - '{': object begin // - '}': object end -// - '[': array start +// - '[': array begin // - ']': array end // // An invalid kind is usually represented using 0, diff --git a/src/encoding/json/testdata/code.json.gz b/src/encoding/json/testdata/code.json.gz deleted file mode 100644 index 1572a92bfbd..00000000000 Binary files a/src/encoding/json/testdata/code.json.gz and /dev/null differ diff --git a/src/encoding/json/v2/arshal.go b/src/encoding/json/v2/arshal.go index 99fcc5bd467..10b16efe4a6 100644 --- a/src/encoding/json/v2/arshal.go +++ b/src/encoding/json/v2/arshal.go @@ -147,17 +147,23 @@ var export = jsontext.Internal.Export(&internal.AllowInternalUse) // If the format matches one of the format constants declared // in the time package (e.g., RFC1123), then that format is used. // If the format is "unix", "unixmilli", "unixmicro", or "unixnano", -// then the timestamp is encoded as a JSON number of the number of seconds -// (or milliseconds, microseconds, or nanoseconds) since the Unix epoch, -// which is January 1st, 1970 at 00:00:00 UTC. +// then the timestamp is encoded as a possibly fractional JSON number +// of the number of seconds (or milliseconds, microseconds, or nanoseconds) +// since the Unix epoch, which is January 1st, 1970 at 00:00:00 UTC. +// To avoid a fractional component, round the timestamp to the relevant unit. // Otherwise, the format is used as-is with [time.Time.Format] if non-empty. // -// - A Go [time.Duration] is encoded as a JSON string containing the duration -// formatted according to [time.Duration.String]. +// - A Go [time.Duration] currently has no default representation and +// requires an explicit format to be specified. // If the format is "sec", "milli", "micro", or "nano", -// then the duration is encoded as a JSON number of the number of seconds -// (or milliseconds, microseconds, or nanoseconds) in the duration. -// If the format is "units", it uses [time.Duration.String]. +// then the duration is encoded as a possibly fractional JSON number +// of the number of seconds (or milliseconds, microseconds, or nanoseconds). +// To avoid a fractional component, round the duration to the relevant unit. +// If the format is "units", it is encoded as a JSON string formatted using +// [time.Duration.String] (e.g., "1h30m" for 1 hour 30 minutes). +// If the format is "iso8601", it is encoded as a JSON string using the +// ISO 8601 standard for durations (e.g., "PT1H30M" for 1 hour 30 minutes) +// using only accurate units of hours, minutes, and seconds. // // - All other Go types (e.g., complex numbers, channels, and functions) // have no default representation and result in a [SemanticError]. @@ -375,17 +381,21 @@ func marshalEncode(out *jsontext.Encoder, in any, mo *jsonopts.Struct) (err erro // If the format matches one of the format constants declared in // the time package (e.g., RFC1123), then that format is used for parsing. // If the format is "unix", "unixmilli", "unixmicro", or "unixnano", -// then the timestamp is decoded from a JSON number of the number of seconds -// (or milliseconds, microseconds, or nanoseconds) since the Unix epoch, -// which is January 1st, 1970 at 00:00:00 UTC. +// then the timestamp is decoded from an optionally fractional JSON number +// of the number of seconds (or milliseconds, microseconds, or nanoseconds) +// since the Unix epoch, which is January 1st, 1970 at 00:00:00 UTC. // Otherwise, the format is used as-is with [time.Time.Parse] if non-empty. // -// - A Go [time.Duration] is decoded from a JSON string by -// passing the decoded string to [time.ParseDuration]. +// - A Go [time.Duration] currently has no default representation and +// requires an explicit format to be specified. // If the format is "sec", "milli", "micro", or "nano", -// then the duration is decoded from a JSON number of the number of seconds -// (or milliseconds, microseconds, or nanoseconds) in the duration. -// If the format is "units", it uses [time.ParseDuration]. +// then the duration is decoded from an optionally fractional JSON number +// of the number of seconds (or milliseconds, microseconds, or nanoseconds). +// If the format is "units", it is decoded from a JSON string parsed using +// [time.ParseDuration] (e.g., "1h30m" for 1 hour 30 minutes). +// If the format is "iso8601", it is decoded from a JSON string using the +// ISO 8601 standard for durations (e.g., "PT1H30M" for 1 hour 30 minutes) +// accepting only accurate units of hours, minutes, or seconds. // // - All other Go types (e.g., complex numbers, channels, and functions) // have no default representation and result in a [SemanticError]. diff --git a/src/encoding/json/v2/arshal_test.go b/src/encoding/json/v2/arshal_test.go index f1060cccb53..8494deed03b 100644 --- a/src/encoding/json/v2/arshal_test.go +++ b/src/encoding/json/v2/arshal_test.go @@ -365,7 +365,7 @@ type ( Interface any `json:",omitzero,format:invalid"` } structDurationFormat struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:units"` D3 time.Duration `json:",format:sec"` D4 time.Duration `json:",string,format:sec"` @@ -375,6 +375,7 @@ type ( D8 time.Duration `json:",string,format:micro"` D9 time.Duration `json:",format:nano"` D10 time.Duration `json:",string,format:nano"` + D11 time.Duration `json:",format:iso8601"` } structTimeFormat struct { T1 time.Time @@ -4312,14 +4313,14 @@ func TestMarshal(t *testing.T) { }, { name: jsontest.Name("Duration/Zero"), in: struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{0, 0}, want: `{"D1":"0s","D2":0}`, }, { name: jsontest.Name("Duration/Positive"), in: struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{ 123456789123456789, @@ -4329,7 +4330,7 @@ func TestMarshal(t *testing.T) { }, { name: jsontest.Name("Duration/Negative"), in: struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{ -123456789123456789, @@ -4356,11 +4357,12 @@ func TestMarshal(t *testing.T) { want: `{"D"`, wantErr: EM(errInvalidFormatFlag).withPos(`{"D":`, "/D").withType(0, T[time.Duration]()), }, { + /* TODO(https://go.dev/issue/71631): Re-enable this test case. name: jsontest.Name("Duration/IgnoreInvalidFormat"), opts: []Options{invalidFormatOption}, in: time.Duration(0), want: `"0s"`, - }, { + }, { */ name: jsontest.Name("Duration/Format"), opts: []Options{jsontext.Multiline(true)}, in: structDurationFormat{ @@ -4374,6 +4376,7 @@ func TestMarshal(t *testing.T) { 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, }, want: `{ "D1": "12h34m56.078090012s", @@ -4385,21 +4388,24 @@ func TestMarshal(t *testing.T) { "D7": 45296078090.012, "D8": "45296078090.012", "D9": 45296078090012, - "D10": "45296078090012" + "D10": "45296078090012", + "D11": "PT12H34M56.078090012S" }`, }, { + /* TODO(https://go.dev/issue/71631): Re-enable this test case. name: jsontest.Name("Duration/Format/Legacy"), opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1}, in: structDurationFormat{ D1: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, D2: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, }, - want: `{"D1":45296078090012,"D2":"12h34m56.078090012s","D3":0,"D4":"0","D5":0,"D6":"0","D7":0,"D8":"0","D9":0,"D10":"0"}`, - }, { + want: `{"D1":45296078090012,"D2":"12h34m56.078090012s","D3":0,"D4":"0","D5":0,"D6":"0","D7":0,"D8":"0","D9":0,"D10":"0","D11":"PT0S"}`, + }, { */ + /* TODO(https://go.dev/issue/71631): Re-enable this test case. name: jsontest.Name("Duration/MapKey"), in: map[time.Duration]string{time.Second: ""}, want: `{"1s":""}`, - }, { + }, { */ name: jsontest.Name("Duration/MapKey/Legacy"), opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1}, in: map[time.Duration]string{time.Second: ""}, @@ -8713,33 +8719,33 @@ func TestUnmarshal(t *testing.T) { name: jsontest.Name("Duration/Null"), inBuf: `{"D1":null,"D2":null}`, inVal: addr(struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{1, 1}), want: addr(struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{0, 0}), }, { name: jsontest.Name("Duration/Zero"), inBuf: `{"D1":"0s","D2":0}`, inVal: addr(struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{1, 1}), want: addr(struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{0, 0}), }, { name: jsontest.Name("Duration/Positive"), inBuf: `{"D1":"34293h33m9.123456789s","D2":123456789123456789}`, inVal: new(struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }), want: addr(struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{ 123456789123456789, @@ -8749,11 +8755,11 @@ func TestUnmarshal(t *testing.T) { name: jsontest.Name("Duration/Negative"), inBuf: `{"D1":"-34293h33m9.123456789s","D2":-123456789123456789}`, inVal: new(struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }), want: addr(struct { - D1 time.Duration + D1 time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. D2 time.Duration `json:",format:nano"` }{ -123456789123456789, @@ -8801,20 +8807,20 @@ func TestUnmarshal(t *testing.T) { name: jsontest.Name("Duration/String/Mismatch"), inBuf: `{"D":-123456789123456789}`, inVal: addr(struct { - D time.Duration + D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. }{1}), want: addr(struct { - D time.Duration + D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. }{1}), wantErr: EU(nil).withPos(`{"D":`, "/D").withType('0', timeDurationType), }, { name: jsontest.Name("Duration/String/Invalid"), inBuf: `{"D":"5minkutes"}`, inVal: addr(struct { - D time.Duration + D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. }{1}), want: addr(struct { - D time.Duration + D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. }{1}), wantErr: EU(func() error { _, err := time.ParseDuration("5minkutes") @@ -8824,12 +8830,41 @@ func TestUnmarshal(t *testing.T) { name: jsontest.Name("Duration/Syntax/Invalid"), inBuf: `{"D":x}`, inVal: addr(struct { - D time.Duration + D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. }{1}), want: addr(struct { - D time.Duration + D time.Duration `json:",format:units"` // TODO(https://go.dev/issue/71631): Remove the format flag. }{1}), wantErr: newInvalidCharacterError("x", "at start of value", len64(`{"D":`), "/D"), + }, { + name: jsontest.Name("Duration/Format"), + inBuf: `{ + "D1": "12h34m56.078090012s", + "D2": "12h34m56.078090012s", + "D3": 45296.078090012, + "D4": "45296.078090012", + "D5": 45296078.090012, + "D6": "45296078.090012", + "D7": 45296078090.012, + "D8": "45296078090.012", + "D9": 45296078090012, + "D10": "45296078090012", + "D11": "PT12H34M56.078090012S" + }`, + inVal: new(structDurationFormat), + want: addr(structDurationFormat{ + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, + }), }, { name: jsontest.Name("Duration/Format/Invalid"), inBuf: `{"D":"0s"}`, @@ -8841,6 +8876,7 @@ func TestUnmarshal(t *testing.T) { }{1}), wantErr: EU(errInvalidFormatFlag).withPos(`{"D":`, "/D").withType(0, timeDurationType), }, { + /* TODO(https://go.dev/issue/71631): Re-enable this test case. name: jsontest.Name("Duration/Format/Legacy"), inBuf: `{"D1":45296078090012,"D2":"12h34m56.078090012s"}`, opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1}, @@ -8849,24 +8885,26 @@ func TestUnmarshal(t *testing.T) { D1: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, D2: 12*time.Hour + 34*time.Minute + 56*time.Second + 78*time.Millisecond + 90*time.Microsecond + 12*time.Nanosecond, }), - }, { + }, { */ + /* TODO(https://go.dev/issue/71631): Re-enable this test case. name: jsontest.Name("Duration/MapKey"), inBuf: `{"1s":""}`, inVal: new(map[time.Duration]string), want: addr(map[time.Duration]string{time.Second: ""}), - }, { + }, { */ name: jsontest.Name("Duration/MapKey/Legacy"), opts: []Options{jsonflags.FormatTimeWithLegacySemantics | 1}, inBuf: `{"1000000000":""}`, inVal: new(map[time.Duration]string), want: addr(map[time.Duration]string{time.Second: ""}), }, { + /* TODO(https://go.dev/issue/71631): Re-enable this test case. name: jsontest.Name("Duration/IgnoreInvalidFormat"), opts: []Options{invalidFormatOption}, inBuf: `"1s"`, inVal: addr(time.Duration(0)), want: addr(time.Second), - }, { + }, { */ name: jsontest.Name("Time/Zero"), inBuf: `{"T1":"0001-01-01T00:00:00Z","T2":"01 Jan 01 00:00 UTC","T3":"0001-01-01","T4":"0001-01-01T00:00:00Z","T5":"0001-01-01T00:00:00Z"}`, inVal: new(struct { diff --git a/src/encoding/json/v2/arshal_time.go b/src/encoding/json/v2/arshal_time.go index e40a04f12a0..fefa50ff5f0 100644 --- a/src/encoding/json/v2/arshal_time.go +++ b/src/encoding/json/v2/arshal_time.go @@ -52,6 +52,9 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler { } } else if mo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) { return marshalNano(enc, va, mo) + } else { + // TODO(https://go.dev/issue/71631): Decide on default duration representation. + return newMarshalErrorBefore(enc, t, errors.New("no default representation (see https://go.dev/issue/71631); specify an explicit format")) } // TODO(https://go.dev/issue/62121): Use reflect.Value.AssertTo. @@ -75,6 +78,9 @@ func makeTimeArshaler(fncs *arshaler, t reflect.Type) *arshaler { } } else if uo.Flags.Get(jsonflags.FormatTimeWithLegacySemantics) { return unmarshalNano(dec, va, uo) + } else { + // TODO(https://go.dev/issue/71631): Decide on default duration representation. + return newUnmarshalErrorBeforeWithSkipping(dec, uo, t, errors.New("no default representation (see https://go.dev/issue/71631); specify an explicit format")) } stringify := !u.isNumeric() || xd.Tokens.Last.NeedObjectName() || uo.Flags.Get(jsonflags.StringifyNumbers) @@ -200,6 +206,7 @@ type durationArshaler struct { // - 0 uses time.Duration.String // - 1e0, 1e3, 1e6, or 1e9 use a decimal encoding of the duration as // nanoseconds, microseconds, milliseconds, or seconds. + // - 8601 uses ISO 8601 base uint64 } @@ -215,6 +222,8 @@ func (a *durationArshaler) initFormat(format string) (ok bool) { a.base = 1e3 case "nano": a.base = 1e0 + case "iso8601": + a.base = 8601 default: return false } @@ -222,13 +231,15 @@ func (a *durationArshaler) initFormat(format string) (ok bool) { } func (a *durationArshaler) isNumeric() bool { - return a.base != 0 && a.base != 60 + return a.base != 0 && a.base != 8601 } func (a *durationArshaler) appendMarshal(b []byte) ([]byte, error) { switch a.base { case 0: return append(b, a.td.String()...), nil + case 8601: + return appendDurationISO8601(b, a.td), nil default: return appendDurationBase10(b, a.td, a.base), nil } @@ -238,6 +249,8 @@ func (a *durationArshaler) unmarshal(b []byte) (err error) { switch a.base { case 0: a.td, err = time.ParseDuration(string(b)) + case 8601: + a.td, err = parseDurationISO8601(b) default: a.td, err = parseDurationBase10(b, a.base) } @@ -412,7 +425,7 @@ func appendDurationBase10(b []byte, d time.Duration, pow10 uint64) []byte { // parseDurationBase10 parses d from a decimal fractional number, // where pow10 is a power-of-10 used to scale up the number. func parseDurationBase10(b []byte, pow10 uint64) (time.Duration, error) { - suffix, neg := consumeSign(b) // consume sign + suffix, neg := consumeSign(b, false) // consume sign wholeBytes, fracBytes := bytesCutByte(suffix, '.', true) // consume whole and frac fields whole, okWhole := jsonwire.ParseUint(wholeBytes) // parse whole field; may overflow frac, okFrac := parseFracBase10(fracBytes, pow10) // parse frac field @@ -428,6 +441,166 @@ func parseDurationBase10(b []byte, pow10 uint64) (time.Duration, error) { } } +// appendDurationISO8601 appends an ISO 8601 duration with a restricted grammar, +// where leading and trailing zeroes and zero-value designators are omitted. +// It only uses hour, minute, and second designators since ISO 8601 defines +// those as being "accurate", while year, month, week, and day are "nominal". +func appendDurationISO8601(b []byte, d time.Duration) []byte { + if d == 0 { + return append(b, "PT0S"...) + } + b, n := mayAppendDurationSign(b, d) + b = append(b, "PT"...) + n, nsec := bits.Div64(0, n, 1e9) // compute nsec field + n, sec := bits.Div64(0, n, 60) // compute sec field + hour, min := bits.Div64(0, n, 60) // compute hour and min fields + if hour > 0 { + b = append(strconv.AppendUint(b, hour, 10), 'H') + } + if min > 0 { + b = append(strconv.AppendUint(b, min, 10), 'M') + } + if sec > 0 || nsec > 0 { + b = append(appendFracBase10(strconv.AppendUint(b, sec, 10), nsec, 1e9), 'S') + } + return b +} + +// daysPerYear is the exact average number of days in a year according to +// the Gregorian calender, which has an extra day each year that is +// a multiple of 4, unless it is evenly divisible by 100 but not by 400. +// This does not take into account leap seconds, which are not deterministic. +const daysPerYear = 365.2425 + +var errInaccurateDateUnits = errors.New("inaccurate year, month, week, or day units") + +// parseDurationISO8601 parses a duration according to ISO 8601-1:2019, +// section 5.5.2.2 and 5.5.2.3 with the following restrictions or extensions: +// +// - A leading minus sign is permitted for negative duration according +// to ISO 8601-2:2019, section 4.4.1.9. We do not permit negative values +// for each "time scale component", which is permitted by section 4.4.1.1, +// but rarely supported by parsers. +// +// - A leading plus sign is permitted (and ignored). +// This is not required by ISO 8601, but not forbidden either. +// There is some precedent for this as it is supported by the principle of +// duration arithmetic as specified in ISO 8601-2-2019, section 14.1. +// Of note, the JavaScript grammar for ISO 8601 permits a leading plus sign. +// +// - A fractional value is only permitted for accurate units +// (i.e., hour, minute, and seconds) in the last time component, +// which is permissible by ISO 8601-1:2019, section 5.5.2.3. +// +// - Both periods ('.') and commas (',') are supported as the separator +// between the integer part and fraction part of a number, +// as specified in ISO 8601-1:2019, section 3.2.6. +// While ISO 8601 recommends comma as the default separator, +// most formatters uses a period. +// +// - Leading zeros are ignored. This is not required by ISO 8601, +// but also not forbidden by the standard. Many parsers support this. +// +// - Lowercase designators are supported. This is not required by ISO 8601, +// but also not forbidden by the standard. Many parsers support this. +// +// If the nominal units of year, month, week, or day are present, +// this produces a best-effort value and also reports [errInaccurateDateUnits]. +// +// The accepted grammar is identical to JavaScript's Duration: +// +// https://tc39.es/proposal-temporal/#prod-Duration +// +// We follow JavaScript's grammar as JSON itself is derived from JavaScript. +// The Temporal.Duration.toJSON method is guaranteed to produce an output +// that can be parsed by this function so long as arithmetic in JavaScript +// do not use a largestUnit value higher than "hours" (which is the default). +// Even if it does, this will do a best-effort parsing with inaccurate units, +// but report [errInaccurateDateUnits]. +func parseDurationISO8601(b []byte) (time.Duration, error) { + var invalid, overflow, inaccurate, sawFrac bool + var sumNanos, n, co uint64 + + // cutBytes is like [bytes.Cut], but uses either c0 or c1 as the separator. + cutBytes := func(b []byte, c0, c1 byte) (prefix, suffix []byte, ok bool) { + for i, c := range b { + if c == c0 || c == c1 { + return b[:i], b[i+1:], true + } + } + return b, nil, false + } + + // mayParseUnit attempts to parse another date or time number + // identified by the desHi and desLo unit characters. + // If the part is absent for current unit, it returns b as is. + mayParseUnit := func(b []byte, desHi, desLo byte, unit time.Duration) []byte { + number, suffix, ok := cutBytes(b, desHi, desLo) + if !ok || sawFrac { + return b // designator is not present or already saw fraction, which can only be in the last component + } + + // Parse the number. + // A fraction allowed for the accurate units in the last part. + whole, frac, ok := cutBytes(number, '.', ',') + if ok { + sawFrac = true + invalid = invalid || len(frac) == len("") || unit > time.Hour + if unit == time.Second { + n, ok = parsePaddedBase10(frac, uint64(time.Second)) + invalid = invalid || !ok + } else { + f, err := strconv.ParseFloat("0."+string(frac), 64) + invalid = invalid || err != nil || len(bytes.Trim(frac[len("."):], "0123456789")) > 0 + n = uint64(math.Round(f * float64(unit))) // never overflows since f is within [0..1] + } + sumNanos, co = bits.Add64(sumNanos, n, 0) // overflow if co > 0 + overflow = overflow || co > 0 + } + for len(whole) > 1 && whole[0] == '0' { + whole = whole[len("0"):] // trim leading zeros + } + n, ok := jsonwire.ParseUint(whole) // overflow if !ok && MaxUint64 + hi, lo := bits.Mul64(n, uint64(unit)) // overflow if hi > 0 + sumNanos, co = bits.Add64(sumNanos, lo, 0) // overflow if co > 0 + invalid = invalid || (!ok && n != math.MaxUint64) + overflow = overflow || (!ok && n == math.MaxUint64) || hi > 0 || co > 0 + inaccurate = inaccurate || unit > time.Hour + return suffix + } + + suffix, neg := consumeSign(b, true) + prefix, suffix, okP := cutBytes(suffix, 'P', 'p') + durDate, durTime, okT := cutBytes(suffix, 'T', 't') + invalid = invalid || len(prefix) > 0 || !okP || (okT && len(durTime) == 0) || len(durDate)+len(durTime) == 0 + if len(durDate) > 0 { // nominal portion of the duration + durDate = mayParseUnit(durDate, 'Y', 'y', time.Duration(daysPerYear*24*60*60*1e9)) + durDate = mayParseUnit(durDate, 'M', 'm', time.Duration(daysPerYear/12*24*60*60*1e9)) + durDate = mayParseUnit(durDate, 'W', 'w', time.Duration(7*24*60*60*1e9)) + durDate = mayParseUnit(durDate, 'D', 'd', time.Duration(24*60*60*1e9)) + invalid = invalid || len(durDate) > 0 // unknown elements + } + if len(durTime) > 0 { // accurate portion of the duration + durTime = mayParseUnit(durTime, 'H', 'h', time.Duration(60*60*1e9)) + durTime = mayParseUnit(durTime, 'M', 'm', time.Duration(60*1e9)) + durTime = mayParseUnit(durTime, 'S', 's', time.Duration(1e9)) + invalid = invalid || len(durTime) > 0 // unknown elements + } + d := mayApplyDurationSign(sumNanos, neg) + overflow = overflow || (neg != (d < 0) && d != 0) // overflows signed duration + + switch { + case invalid: + return 0, fmt.Errorf("invalid ISO 8601 duration %q: %w", b, strconv.ErrSyntax) + case overflow: + return 0, fmt.Errorf("invalid ISO 8601 duration %q: %w", b, strconv.ErrRange) + case inaccurate: + return d, fmt.Errorf("invalid ISO 8601 duration %q: %w", b, errInaccurateDateUnits) + default: + return d, nil + } +} + // mayAppendDurationSign appends a negative sign if n is negative. func mayAppendDurationSign(b []byte, d time.Duration) ([]byte, uint64) { if d < 0 { @@ -471,7 +644,7 @@ func appendTimeUnix(b []byte, t time.Time, pow10 uint64) []byte { // parseTimeUnix parses t formatted as a decimal fractional number, // where pow10 is a power-of-10 used to scale down the number. func parseTimeUnix(b []byte, pow10 uint64) (time.Time, error) { - suffix, neg := consumeSign(b) // consume sign + suffix, neg := consumeSign(b, false) // consume sign wholeBytes, fracBytes := bytesCutByte(suffix, '.', true) // consume whole and frac fields whole, okWhole := jsonwire.ParseUint(wholeBytes) // parse whole field; may overflow frac, okFrac := parseFracBase10(fracBytes, 1e9/pow10) // parse frac field @@ -570,10 +743,14 @@ func parsePaddedBase10(b []byte, max10 uint64) (n uint64, ok bool) { return n, true } -// consumeSign consumes an optional leading negative sign. -func consumeSign(b []byte) ([]byte, bool) { - if len(b) > 0 && b[0] == '-' { - return b[len("-"):], true +// consumeSign consumes an optional leading negative or positive sign. +func consumeSign(b []byte, allowPlus bool) ([]byte, bool) { + if len(b) > 0 { + if b[0] == '-' { + return b[len("-"):], true + } else if b[0] == '+' && allowPlus { + return b[len("+"):], false + } } return b, false } diff --git a/src/encoding/json/v2/arshal_time_test.go b/src/encoding/json/v2/arshal_time_test.go index faa09de5098..6c08e124948 100644 --- a/src/encoding/json/v2/arshal_time_test.go +++ b/src/encoding/json/v2/arshal_time_test.go @@ -7,8 +7,10 @@ package json import ( + "errors" "fmt" "math" + "strconv" "testing" "time" @@ -28,63 +30,67 @@ var formatDurationTestdata = []struct { base10Milli string base10Micro string base10Nano string + iso8601 string }{ - {math.MaxInt64, "9223372036.854775807", "9223372036854.775807", "9223372036854775.807", "9223372036854775807"}, - {1e12 + 1e12, "2000", "2000000", "2000000000", "2000000000000"}, - {1e12 + 1e11, "1100", "1100000", "1100000000", "1100000000000"}, - {1e12 + 1e10, "1010", "1010000", "1010000000", "1010000000000"}, - {1e12 + 1e9, "1001", "1001000", "1001000000", "1001000000000"}, - {1e12 + 1e8, "1000.1", "1000100", "1000100000", "1000100000000"}, - {1e12 + 1e7, "1000.01", "1000010", "1000010000", "1000010000000"}, - {1e12 + 1e6, "1000.001", "1000001", "1000001000", "1000001000000"}, - {1e12 + 1e5, "1000.0001", "1000000.1", "1000000100", "1000000100000"}, - {1e12 + 1e4, "1000.00001", "1000000.01", "1000000010", "1000000010000"}, - {1e12 + 1e3, "1000.000001", "1000000.001", "1000000001", "1000000001000"}, - {1e12 + 1e2, "1000.0000001", "1000000.0001", "1000000000.1", "1000000000100"}, - {1e12 + 1e1, "1000.00000001", "1000000.00001", "1000000000.01", "1000000000010"}, - {1e12 + 1e0, "1000.000000001", "1000000.000001", "1000000000.001", "1000000000001"}, - {+(1e9 + 1), "1.000000001", "1000.000001", "1000000.001", "1000000001"}, - {+(1e9), "1", "1000", "1000000", "1000000000"}, - {+(1e9 - 1), "0.999999999", "999.999999", "999999.999", "999999999"}, - {+100000000, "0.1", "100", "100000", "100000000"}, - {+120000000, "0.12", "120", "120000", "120000000"}, - {+123000000, "0.123", "123", "123000", "123000000"}, - {+123400000, "0.1234", "123.4", "123400", "123400000"}, - {+123450000, "0.12345", "123.45", "123450", "123450000"}, - {+123456000, "0.123456", "123.456", "123456", "123456000"}, - {+123456700, "0.1234567", "123.4567", "123456.7", "123456700"}, - {+123456780, "0.12345678", "123.45678", "123456.78", "123456780"}, - {+123456789, "0.123456789", "123.456789", "123456.789", "123456789"}, - {+12345678, "0.012345678", "12.345678", "12345.678", "12345678"}, - {+1234567, "0.001234567", "1.234567", "1234.567", "1234567"}, - {+123456, "0.000123456", "0.123456", "123.456", "123456"}, - {+12345, "0.000012345", "0.012345", "12.345", "12345"}, - {+1234, "0.000001234", "0.001234", "1.234", "1234"}, - {+123, "0.000000123", "0.000123", "0.123", "123"}, - {+12, "0.000000012", "0.000012", "0.012", "12"}, - {+1, "0.000000001", "0.000001", "0.001", "1"}, - {0, "0", "0", "0", "0"}, - {-1, "-0.000000001", "-0.000001", "-0.001", "-1"}, - {-12, "-0.000000012", "-0.000012", "-0.012", "-12"}, - {-123, "-0.000000123", "-0.000123", "-0.123", "-123"}, - {-1234, "-0.000001234", "-0.001234", "-1.234", "-1234"}, - {-12345, "-0.000012345", "-0.012345", "-12.345", "-12345"}, - {-123456, "-0.000123456", "-0.123456", "-123.456", "-123456"}, - {-1234567, "-0.001234567", "-1.234567", "-1234.567", "-1234567"}, - {-12345678, "-0.012345678", "-12.345678", "-12345.678", "-12345678"}, - {-123456789, "-0.123456789", "-123.456789", "-123456.789", "-123456789"}, - {-123456780, "-0.12345678", "-123.45678", "-123456.78", "-123456780"}, - {-123456700, "-0.1234567", "-123.4567", "-123456.7", "-123456700"}, - {-123456000, "-0.123456", "-123.456", "-123456", "-123456000"}, - {-123450000, "-0.12345", "-123.45", "-123450", "-123450000"}, - {-123400000, "-0.1234", "-123.4", "-123400", "-123400000"}, - {-123000000, "-0.123", "-123", "-123000", "-123000000"}, - {-120000000, "-0.12", "-120", "-120000", "-120000000"}, - {-100000000, "-0.1", "-100", "-100000", "-100000000"}, - {-(1e9 - 1), "-0.999999999", "-999.999999", "-999999.999", "-999999999"}, - {-(1e9), "-1", "-1000", "-1000000", "-1000000000"}, - {-(1e9 + 1), "-1.000000001", "-1000.000001", "-1000000.001", "-1000000001"}, - {math.MinInt64, "-9223372036.854775808", "-9223372036854.775808", "-9223372036854775.808", "-9223372036854775808"}, + {math.MaxInt64, "9223372036.854775807", "9223372036854.775807", "9223372036854775.807", "9223372036854775807", "PT2562047H47M16.854775807S"}, + {123*time.Hour + 4*time.Minute + 56*time.Second, "443096", "443096000", "443096000000", "443096000000000", "PT123H4M56S"}, + {time.Hour, "3600", "3600000", "3600000000", "3600000000000", "PT1H"}, + {time.Minute, "60", "60000", "60000000", "60000000000", "PT1M"}, + {1e12 + 1e12, "2000", "2000000", "2000000000", "2000000000000", "PT33M20S"}, + {1e12 + 1e11, "1100", "1100000", "1100000000", "1100000000000", "PT18M20S"}, + {1e12 + 1e10, "1010", "1010000", "1010000000", "1010000000000", "PT16M50S"}, + {1e12 + 1e9, "1001", "1001000", "1001000000", "1001000000000", "PT16M41S"}, + {1e12 + 1e8, "1000.1", "1000100", "1000100000", "1000100000000", "PT16M40.1S"}, + {1e12 + 1e7, "1000.01", "1000010", "1000010000", "1000010000000", "PT16M40.01S"}, + {1e12 + 1e6, "1000.001", "1000001", "1000001000", "1000001000000", "PT16M40.001S"}, + {1e12 + 1e5, "1000.0001", "1000000.1", "1000000100", "1000000100000", "PT16M40.0001S"}, + {1e12 + 1e4, "1000.00001", "1000000.01", "1000000010", "1000000010000", "PT16M40.00001S"}, + {1e12 + 1e3, "1000.000001", "1000000.001", "1000000001", "1000000001000", "PT16M40.000001S"}, + {1e12 + 1e2, "1000.0000001", "1000000.0001", "1000000000.1", "1000000000100", "PT16M40.0000001S"}, + {1e12 + 1e1, "1000.00000001", "1000000.00001", "1000000000.01", "1000000000010", "PT16M40.00000001S"}, + {1e12 + 1e0, "1000.000000001", "1000000.000001", "1000000000.001", "1000000000001", "PT16M40.000000001S"}, + {+(1e9 + 1), "1.000000001", "1000.000001", "1000000.001", "1000000001", "PT1.000000001S"}, + {+(1e9), "1", "1000", "1000000", "1000000000", "PT1S"}, + {+(1e9 - 1), "0.999999999", "999.999999", "999999.999", "999999999", "PT0.999999999S"}, + {+100000000, "0.1", "100", "100000", "100000000", "PT0.1S"}, + {+120000000, "0.12", "120", "120000", "120000000", "PT0.12S"}, + {+123000000, "0.123", "123", "123000", "123000000", "PT0.123S"}, + {+123400000, "0.1234", "123.4", "123400", "123400000", "PT0.1234S"}, + {+123450000, "0.12345", "123.45", "123450", "123450000", "PT0.12345S"}, + {+123456000, "0.123456", "123.456", "123456", "123456000", "PT0.123456S"}, + {+123456700, "0.1234567", "123.4567", "123456.7", "123456700", "PT0.1234567S"}, + {+123456780, "0.12345678", "123.45678", "123456.78", "123456780", "PT0.12345678S"}, + {+123456789, "0.123456789", "123.456789", "123456.789", "123456789", "PT0.123456789S"}, + {+12345678, "0.012345678", "12.345678", "12345.678", "12345678", "PT0.012345678S"}, + {+1234567, "0.001234567", "1.234567", "1234.567", "1234567", "PT0.001234567S"}, + {+123456, "0.000123456", "0.123456", "123.456", "123456", "PT0.000123456S"}, + {+12345, "0.000012345", "0.012345", "12.345", "12345", "PT0.000012345S"}, + {+1234, "0.000001234", "0.001234", "1.234", "1234", "PT0.000001234S"}, + {+123, "0.000000123", "0.000123", "0.123", "123", "PT0.000000123S"}, + {+12, "0.000000012", "0.000012", "0.012", "12", "PT0.000000012S"}, + {+1, "0.000000001", "0.000001", "0.001", "1", "PT0.000000001S"}, + {0, "0", "0", "0", "0", "PT0S"}, + {-1, "-0.000000001", "-0.000001", "-0.001", "-1", "-PT0.000000001S"}, + {-12, "-0.000000012", "-0.000012", "-0.012", "-12", "-PT0.000000012S"}, + {-123, "-0.000000123", "-0.000123", "-0.123", "-123", "-PT0.000000123S"}, + {-1234, "-0.000001234", "-0.001234", "-1.234", "-1234", "-PT0.000001234S"}, + {-12345, "-0.000012345", "-0.012345", "-12.345", "-12345", "-PT0.000012345S"}, + {-123456, "-0.000123456", "-0.123456", "-123.456", "-123456", "-PT0.000123456S"}, + {-1234567, "-0.001234567", "-1.234567", "-1234.567", "-1234567", "-PT0.001234567S"}, + {-12345678, "-0.012345678", "-12.345678", "-12345.678", "-12345678", "-PT0.012345678S"}, + {-123456789, "-0.123456789", "-123.456789", "-123456.789", "-123456789", "-PT0.123456789S"}, + {-123456780, "-0.12345678", "-123.45678", "-123456.78", "-123456780", "-PT0.12345678S"}, + {-123456700, "-0.1234567", "-123.4567", "-123456.7", "-123456700", "-PT0.1234567S"}, + {-123456000, "-0.123456", "-123.456", "-123456", "-123456000", "-PT0.123456S"}, + {-123450000, "-0.12345", "-123.45", "-123450", "-123450000", "-PT0.12345S"}, + {-123400000, "-0.1234", "-123.4", "-123400", "-123400000", "-PT0.1234S"}, + {-123000000, "-0.123", "-123", "-123000", "-123000000", "-PT0.123S"}, + {-120000000, "-0.12", "-120", "-120000", "-120000000", "-PT0.12S"}, + {-100000000, "-0.1", "-100", "-100000", "-100000000", "-PT0.1S"}, + {-(1e9 - 1), "-0.999999999", "-999.999999", "-999999.999", "-999999999", "-PT0.999999999S"}, + {-(1e9), "-1", "-1000", "-1000000", "-1000000000", "-PT1S"}, + {-(1e9 + 1), "-1.000000001", "-1000.000001", "-1000000.001", "-1000000001", "-PT1.000000001S"}, + {math.MinInt64, "-9223372036.854775808", "-9223372036854.775808", "-9223372036854775.808", "-9223372036854775808", "-PT2562047H47M16.854775808S"}, } func TestFormatDuration(t *testing.T) { @@ -107,6 +113,7 @@ func TestFormatDuration(t *testing.T) { check(tt.td, tt.base10Milli, 1e6) check(tt.td, tt.base10Micro, 1e3) check(tt.td, tt.base10Nano, 1e0) + check(tt.td, tt.iso8601, 8601) } } @@ -114,31 +121,108 @@ var parseDurationTestdata = []struct { in string base uint64 want time.Duration - wantErr bool + wantErr error }{ - {"0", 1e0, 0, false}, - {"0.", 1e0, 0, true}, - {"0.0", 1e0, 0, false}, - {"0.00", 1e0, 0, false}, - {"00.0", 1e0, 0, true}, - {"+0", 1e0, 0, true}, - {"1e0", 1e0, 0, true}, - {"1.000000000x", 1e9, 0, true}, - {"1.000000x", 1e6, 0, true}, - {"1.000x", 1e3, 0, true}, - {"1.x", 1e0, 0, true}, - {"1.0000000009", 1e9, +time.Second, false}, - {"1.0000009", 1e6, +time.Millisecond, false}, - {"1.0009", 1e3, +time.Microsecond, false}, - {"1.9", 1e0, +time.Nanosecond, false}, - {"-9223372036854775809", 1e0, 0, true}, - {"9223372036854775.808", 1e3, 0, true}, - {"-9223372036854.775809", 1e6, 0, true}, - {"9223372036.854775808", 1e9, 0, true}, - {"-1.9", 1e0, -time.Nanosecond, false}, - {"-1.0009", 1e3, -time.Microsecond, false}, - {"-1.0000009", 1e6, -time.Millisecond, false}, - {"-1.0000000009", 1e9, -time.Second, false}, + {"0", 1e0, 0, nil}, + {"0.", 1e0, 0, strconv.ErrSyntax}, + {"0.0", 1e0, 0, nil}, + {"0.00", 1e0, 0, nil}, + {"00.0", 1e0, 0, strconv.ErrSyntax}, + {"+0", 1e0, 0, strconv.ErrSyntax}, + {"1e0", 1e0, 0, strconv.ErrSyntax}, + {"1.000000000x", 1e9, 0, strconv.ErrSyntax}, + {"1.000000x", 1e6, 0, strconv.ErrSyntax}, + {"1.000x", 1e3, 0, strconv.ErrSyntax}, + {"1.x", 1e0, 0, strconv.ErrSyntax}, + {"1.0000000009", 1e9, +time.Second, nil}, + {"1.0000009", 1e6, +time.Millisecond, nil}, + {"1.0009", 1e3, +time.Microsecond, nil}, + {"1.9", 1e0, +time.Nanosecond, nil}, + {"-9223372036854775809", 1e0, 0, strconv.ErrRange}, + {"9223372036854775.808", 1e3, 0, strconv.ErrRange}, + {"-9223372036854.775809", 1e6, 0, strconv.ErrRange}, + {"9223372036.854775808", 1e9, 0, strconv.ErrRange}, + {"-1.9", 1e0, -time.Nanosecond, nil}, + {"-1.0009", 1e3, -time.Microsecond, nil}, + {"-1.0000009", 1e6, -time.Millisecond, nil}, + {"-1.0000000009", 1e9, -time.Second, nil}, + {"", 8601, 0, strconv.ErrSyntax}, + {"P", 8601, 0, strconv.ErrSyntax}, + {"PT", 8601, 0, strconv.ErrSyntax}, + {"PT0", 8601, 0, strconv.ErrSyntax}, + {"DT0S", 8601, 0, strconv.ErrSyntax}, + {"PT0S", 8601, 0, nil}, + {" PT0S", 8601, 0, strconv.ErrSyntax}, + {"PT0S ", 8601, 0, strconv.ErrSyntax}, + {"+PT0S", 8601, 0, nil}, + {"PT0.M", 8601, 0, strconv.ErrSyntax}, + {"PT0.S", 8601, 0, strconv.ErrSyntax}, + {"PT0.0S", 8601, 0, nil}, + {"PT0.0_0H", 8601, 0, strconv.ErrSyntax}, + {"PT0.0_0M", 8601, 0, strconv.ErrSyntax}, + {"PT0.0_0S", 8601, 0, strconv.ErrSyntax}, + {"PT.0S", 8601, 0, strconv.ErrSyntax}, + {"PT00.0S", 8601, 0, nil}, + {"PT0S", 8601, 0, nil}, + {"PT1,5S", 8601, time.Second + 500*time.Millisecond, nil}, + {"PT1H", 8601, time.Hour, nil}, + {"PT1H0S", 8601, time.Hour, nil}, + {"PT0S", 8601, 0, nil}, + {"PT00S", 8601, 0, nil}, + {"PT000S", 8601, 0, nil}, + {"PTS", 8601, 0, strconv.ErrSyntax}, + {"PT1M", 8601, time.Minute, nil}, + {"PT01M", 8601, time.Minute, nil}, + {"PT001M", 8601, time.Minute, nil}, + {"PT1H59S", 8601, time.Hour + 59*time.Second, nil}, + {"PT123H4M56.789S", 8601, 123*time.Hour + 4*time.Minute + 56*time.Second + 789*time.Millisecond, nil}, + {"-PT123H4M56.789S", 8601, -123*time.Hour - 4*time.Minute - 56*time.Second - 789*time.Millisecond, nil}, + {"PT0H0S", 8601, 0, nil}, + {"PT0H", 8601, 0, nil}, + {"PT0M", 8601, 0, nil}, + {"-PT0S", 8601, 0, nil}, + {"PT1M0S", 8601, time.Minute, nil}, + {"PT0H1M0S", 8601, time.Minute, nil}, + {"PT01H02M03S", 8601, 1*time.Hour + 2*time.Minute + 3*time.Second, nil}, + {"PT0,123S", 8601, 123 * time.Millisecond, nil}, + {"PT1.S", 8601, 0, strconv.ErrSyntax}, + {"PT1.000S", 8601, time.Second, nil}, + {"PT0.025H", 8601, time.Minute + 30*time.Second, nil}, + {"PT0.025H0M", 8601, 0, strconv.ErrSyntax}, + {"PT1.5M", 8601, time.Minute + 30*time.Second, nil}, + {"PT1.5M0S", 8601, 0, strconv.ErrSyntax}, + {"PT60M", 8601, time.Hour, nil}, + {"PT3600S", 8601, time.Hour, nil}, + {"PT1H2M3.0S", 8601, 1*time.Hour + 2*time.Minute + 3*time.Second, nil}, + {"pt1h2m3,0s", 8601, 1*time.Hour + 2*time.Minute + 3*time.Second, nil}, + {"PT-1H-2M-3S", 8601, 0, strconv.ErrSyntax}, + {"P1Y", 8601, time.Duration(daysPerYear * 24 * 60 * 60 * 1e9), errInaccurateDateUnits}, + {"P1.0Y", 8601, 0, strconv.ErrSyntax}, + {"P1M", 8601, time.Duration(daysPerYear / 12 * 24 * 60 * 60 * 1e9), errInaccurateDateUnits}, + {"P1.0M", 8601, 0, strconv.ErrSyntax}, + {"P1W", 8601, 7 * 24 * time.Hour, errInaccurateDateUnits}, + {"P1.0W", 8601, 0, strconv.ErrSyntax}, + {"P1D", 8601, 24 * time.Hour, errInaccurateDateUnits}, + {"P1.0D", 8601, 0, strconv.ErrSyntax}, + {"P1W1S", 8601, 0, strconv.ErrSyntax}, + {"-P1Y2M3W4DT5H6M7.8S", 8601, -(time.Duration(14*daysPerYear/12*24*60*60*1e9) + time.Duration((3*7+4)*24*60*60*1e9) + 5*time.Hour + 6*time.Minute + 7*time.Second + 800*time.Millisecond), errInaccurateDateUnits}, + {"-p1y2m3w4dt5h6m7.8s", 8601, -(time.Duration(14*daysPerYear/12*24*60*60*1e9) + time.Duration((3*7+4)*24*60*60*1e9) + 5*time.Hour + 6*time.Minute + 7*time.Second + 800*time.Millisecond), errInaccurateDateUnits}, + {"P0Y0M0DT1H2M3S", 8601, 1*time.Hour + 2*time.Minute + 3*time.Second, errInaccurateDateUnits}, + {"PT0.0000000001S", 8601, 0, nil}, + {"PT0.0000000005S", 8601, 0, nil}, + {"PT0.000000000500000000S", 8601, 0, nil}, + {"PT0.000000000499999999S", 8601, 0, nil}, + {"PT2562047H47M16.854775808S", 8601, 0, strconv.ErrRange}, + {"-PT2562047H47M16.854775809S", 8601, 0, strconv.ErrRange}, + {"PT9223372036.854775807S", 8601, math.MaxInt64, nil}, + {"PT9223372036.854775808S", 8601, 0, strconv.ErrRange}, + {"-PT9223372036.854775808S", 8601, math.MinInt64, nil}, + {"-PT9223372036.854775809S", 8601, 0, strconv.ErrRange}, + {"PT18446744073709551616S", 8601, 0, strconv.ErrRange}, + {"PT5124096H", 8601, 0, strconv.ErrRange}, + {"PT2562047.7880152155019444H", 8601, math.MaxInt64, nil}, + {"PT2562047.7880152155022222H", 8601, 0, strconv.ErrRange}, + {"PT5124094H94M33.709551616S", 8601, 0, strconv.ErrRange}, } func TestParseDuration(t *testing.T) { @@ -147,10 +231,8 @@ func TestParseDuration(t *testing.T) { switch err := a.unmarshal([]byte(tt.in)); { case a.td != tt.want: t.Errorf("parseDuration(%q, %s) = %v, want %v", tt.in, baseLabel(tt.base), a.td, tt.want) - case (err == nil) && tt.wantErr: - t.Errorf("parseDuration(%q, %s) error is nil, want non-nil", tt.in, baseLabel(tt.base)) - case (err != nil) && !tt.wantErr: - t.Errorf("parseDuration(%q, %s) error is non-nil, want nil", tt.in, baseLabel(tt.base)) + case !errors.Is(err, tt.wantErr): + t.Errorf("parseDuration(%q, %s) error = %v, want %v", tt.in, baseLabel(tt.base), err, tt.wantErr) } } } @@ -161,7 +243,7 @@ func FuzzFormatDuration(f *testing.F) { } f.Fuzz(func(t *testing.T, want int64) { var buf []byte - for _, base := range [...]uint64{1e0, 1e3, 1e6, 1e9} { + for _, base := range [...]uint64{1e0, 1e3, 1e6, 1e9, 8601} { a := durationArshaler{td: time.Duration(want), base: base} buf, _ = a.appendMarshal(buf[:0]) switch err := a.unmarshal(buf); { @@ -179,9 +261,11 @@ func FuzzParseDuration(f *testing.F) { f.Add([]byte(tt.in)) } f.Fuzz(func(t *testing.T, in []byte) { - for _, base := range [...]uint64{1e0, 1e3, 1e6, 1e9, 60} { + for _, base := range [...]uint64{1e0, 1e3, 1e6, 1e9, 8601} { a := durationArshaler{base: base} - if err := a.unmarshal(in); err == nil && base != 60 { + switch err := a.unmarshal(in); { + case err != nil: // nothing else to check + case base != 8601: if n, err := jsonwire.ConsumeNumber(in); err != nil || n != len(in) { t.Fatalf("parseDuration(%q) error is nil for invalid JSON number", in) } @@ -239,26 +323,26 @@ var parseTimeTestdata = []struct { in string base uint64 want time.Time - wantErr bool + wantErr error }{ - {"0", 1e0, time.Unix(0, 0).UTC(), false}, - {"0.", 1e0, time.Time{}, true}, - {"0.0", 1e0, time.Unix(0, 0).UTC(), false}, - {"0.00", 1e0, time.Unix(0, 0).UTC(), false}, - {"00.0", 1e0, time.Time{}, true}, - {"+0", 1e0, time.Time{}, true}, - {"1e0", 1e0, time.Time{}, true}, - {"1234567890123456789012345678901234567890", 1e0, time.Time{}, true}, - {"9223372036854775808000.000000", 1e3, time.Time{}, true}, - {"9223372036854775807999999.9999", 1e6, time.Unix(math.MaxInt64, 1e9-1).UTC(), false}, - {"9223372036854775807999999999.9", 1e9, time.Unix(math.MaxInt64, 1e9-1).UTC(), false}, - {"9223372036854775807.999999999x", 1e0, time.Time{}, true}, - {"9223372036854775807000000000", 1e9, time.Unix(math.MaxInt64, 0).UTC(), false}, - {"-9223372036854775808", 1e0, time.Unix(math.MinInt64, 0).UTC(), false}, - {"-9223372036854775808000.000001", 1e3, time.Time{}, true}, - {"-9223372036854775808000000.0001", 1e6, time.Unix(math.MinInt64, 0).UTC(), false}, - {"-9223372036854775808000000000.x", 1e9, time.Time{}, true}, - {"-1234567890123456789012345678901234567890", 1e9, time.Time{}, true}, + {"0", 1e0, time.Unix(0, 0).UTC(), nil}, + {"0.", 1e0, time.Time{}, strconv.ErrSyntax}, + {"0.0", 1e0, time.Unix(0, 0).UTC(), nil}, + {"0.00", 1e0, time.Unix(0, 0).UTC(), nil}, + {"00.0", 1e0, time.Time{}, strconv.ErrSyntax}, + {"+0", 1e0, time.Time{}, strconv.ErrSyntax}, + {"1e0", 1e0, time.Time{}, strconv.ErrSyntax}, + {"1234567890123456789012345678901234567890", 1e0, time.Time{}, strconv.ErrRange}, + {"9223372036854775808000.000000", 1e3, time.Time{}, strconv.ErrRange}, + {"9223372036854775807999999.9999", 1e6, time.Unix(math.MaxInt64, 1e9-1).UTC(), nil}, + {"9223372036854775807999999999.9", 1e9, time.Unix(math.MaxInt64, 1e9-1).UTC(), nil}, + {"9223372036854775807.999999999x", 1e0, time.Time{}, strconv.ErrSyntax}, + {"9223372036854775807000000000", 1e9, time.Unix(math.MaxInt64, 0).UTC(), nil}, + {"-9223372036854775808", 1e0, time.Unix(math.MinInt64, 0).UTC(), nil}, + {"-9223372036854775808000.000001", 1e3, time.Time{}, strconv.ErrRange}, + {"-9223372036854775808000000.0001", 1e6, time.Unix(math.MinInt64, 0).UTC(), nil}, + {"-9223372036854775808000000000.x", 1e9, time.Time{}, strconv.ErrSyntax}, + {"-1234567890123456789012345678901234567890", 1e9, time.Time{}, strconv.ErrRange}, } func TestParseTime(t *testing.T) { @@ -267,10 +351,8 @@ func TestParseTime(t *testing.T) { switch err := a.unmarshal([]byte(tt.in)); { case a.tt != tt.want: t.Errorf("parseTime(%q, %s) = time.Unix(%d, %d), want time.Unix(%d, %d)", tt.in, baseLabel(tt.base), a.tt.Unix(), a.tt.Nanosecond(), tt.want.Unix(), tt.want.Nanosecond()) - case (err == nil) && tt.wantErr: - t.Errorf("parseTime(%q, %s) = (time.Unix(%d, %d), nil), want non-nil error", tt.in, baseLabel(tt.base), a.tt.Unix(), a.tt.Nanosecond()) - case (err != nil) && !tt.wantErr: - t.Errorf("parseTime(%q, %s) error is non-nil, want nil", tt.in, baseLabel(tt.base)) + case !errors.Is(err, tt.wantErr): + t.Errorf("parseTime(%q, %s) error = %v, want %v", tt.in, baseLabel(tt.base), err, tt.wantErr) } } } diff --git a/src/encoding/json/v2/bench_test.go b/src/encoding/json/v2/bench_test.go index a46f4ab5d37..ae4a5b20a5c 100644 --- a/src/encoding/json/v2/bench_test.go +++ b/src/encoding/json/v2/bench_test.go @@ -267,12 +267,13 @@ var arshalTestdata = []struct { new: func() any { return new(jsonArshalerV2) }, skipV1: true, }, { + /* TODO(https://go.dev/issue/71631): Re-enable this test case. name: "Duration", raw: []byte(`"1h1m1s"`), val: addr(time.Hour + time.Minute + time.Second), new: func() any { return new(time.Duration) }, skipV1: true, -}, { + }, { */ name: "Time", raw: []byte(`"2006-01-02T22:04:05Z"`), val: addr(time.Unix(1136239445, 0).UTC()), diff --git a/src/encoding/json/v2/doc.go b/src/encoding/json/v2/doc.go index 8dd0b138f5e..203139754c2 100644 --- a/src/encoding/json/v2/doc.go +++ b/src/encoding/json/v2/doc.go @@ -9,6 +9,11 @@ // primitive data types such as booleans, strings, and numbers, // in addition to structured data types such as objects and arrays. // +// This package (encoding/json/v2) is experimental, +// and not subject to the Go 1 compatibility promise. +// It only exists when building with the GOEXPERIMENT=jsonv2 environment variable set. +// Most users should use [encoding/json]. +// // [Marshal] and [Unmarshal] encode and decode Go values // to/from JSON text contained within a []byte. // [MarshalWrite] and [UnmarshalRead] operate on JSON text diff --git a/src/encoding/json/v2/example_test.go b/src/encoding/json/v2/example_test.go index fe40bff964c..c6bf0a864d8 100644 --- a/src/encoding/json/v2/example_test.go +++ b/src/encoding/json/v2/example_test.go @@ -402,27 +402,29 @@ func Example_unknownMembers() { // The "format" tag option can be used to alter the formatting of certain types. func Example_formatFlags() { value := struct { - BytesBase64 []byte `json:",format:base64"` - BytesHex [8]byte `json:",format:hex"` - BytesArray []byte `json:",format:array"` - FloatNonFinite float64 `json:",format:nonfinite"` - MapEmitNull map[string]any `json:",format:emitnull"` - SliceEmitNull []any `json:",format:emitnull"` - TimeDateOnly time.Time `json:",format:'2006-01-02'"` - TimeUnixSec time.Time `json:",format:unix"` - DurationSecs time.Duration `json:",format:sec"` - DurationNanos time.Duration `json:",format:nano"` + BytesBase64 []byte `json:",format:base64"` + BytesHex [8]byte `json:",format:hex"` + BytesArray []byte `json:",format:array"` + FloatNonFinite float64 `json:",format:nonfinite"` + MapEmitNull map[string]any `json:",format:emitnull"` + SliceEmitNull []any `json:",format:emitnull"` + TimeDateOnly time.Time `json:",format:'2006-01-02'"` + TimeUnixSec time.Time `json:",format:unix"` + DurationSecs time.Duration `json:",format:sec"` + DurationNanos time.Duration `json:",format:nano"` + DurationISO8601 time.Duration `json:",format:iso8601"` }{ - BytesBase64: []byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}, - BytesHex: [8]byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}, - BytesArray: []byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}, - FloatNonFinite: math.NaN(), - MapEmitNull: nil, - SliceEmitNull: nil, - TimeDateOnly: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC), - TimeUnixSec: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC), - DurationSecs: 12*time.Hour + 34*time.Minute + 56*time.Second + 7*time.Millisecond + 8*time.Microsecond + 9*time.Nanosecond, - DurationNanos: 12*time.Hour + 34*time.Minute + 56*time.Second + 7*time.Millisecond + 8*time.Microsecond + 9*time.Nanosecond, + BytesBase64: []byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}, + BytesHex: [8]byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}, + BytesArray: []byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}, + FloatNonFinite: math.NaN(), + MapEmitNull: nil, + SliceEmitNull: nil, + TimeDateOnly: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC), + TimeUnixSec: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC), + DurationSecs: 12*time.Hour + 34*time.Minute + 56*time.Second + 7*time.Millisecond + 8*time.Microsecond + 9*time.Nanosecond, + DurationNanos: 12*time.Hour + 34*time.Minute + 56*time.Second + 7*time.Millisecond + 8*time.Microsecond + 9*time.Nanosecond, + DurationISO8601: 12*time.Hour + 34*time.Minute + 56*time.Second + 7*time.Millisecond + 8*time.Microsecond + 9*time.Nanosecond, } b, err := json.Marshal(&value) @@ -452,7 +454,8 @@ func Example_formatFlags() { // "TimeDateOnly": "2000-01-01", // "TimeUnixSec": 946684800, // "DurationSecs": 45296.007008009, - // "DurationNanos": 45296007008009 + // "DurationNanos": 45296007008009, + // "DurationISO8601": "PT12H34M56.007008009S" // } } diff --git a/src/encoding/json/v2/fields.go b/src/encoding/json/v2/fields.go index 9413189c085..4a02be7327a 100644 --- a/src/encoding/json/v2/fields.go +++ b/src/encoding/json/v2/fields.go @@ -404,6 +404,7 @@ type fieldOptions struct { // the JSON member name and other features. func parseFieldOptions(sf reflect.StructField) (out fieldOptions, ignored bool, err error) { tag, hasTag := sf.Tag.Lookup("json") + tagOrig := tag // Check whether this field is explicitly ignored. if tag == "-" { @@ -453,6 +454,13 @@ func parseFieldOptions(sf reflect.StructField) (out fieldOptions, ignored bool, err = cmp.Or(err, fmt.Errorf("Go struct field %s has JSON object name %q with invalid UTF-8", sf.Name, name)) name = string([]rune(name)) // replace invalid UTF-8 with utf8.RuneError } + if name == "-" && tag[0] == '-' { + defer func() { // defer to let other errors take precedence + err = cmp.Or(err, fmt.Errorf("Go struct field %s has JSON object name %q; either "+ + "use `json:\"-\"` to ignore the field or "+ + "use `json:\"'-'%s` to specify %q as the name", sf.Name, out.name, strings.TrimPrefix(strconv.Quote(tagOrig), `"-`), name)) + }() + } if err2 == nil { out.hasName = true out.name = name diff --git a/src/encoding/json/v2/fields_test.go b/src/encoding/json/v2/fields_test.go index 1c36f809052..ae58182f298 100644 --- a/src/encoding/json/v2/fields_test.go +++ b/src/encoding/json/v2/fields_test.go @@ -502,6 +502,19 @@ func TestParseTagOptions(t *testing.T) { }{}, wantOpts: fieldOptions{hasName: true, name: "-", quotedName: `"-"`}, wantErr: errors.New("Go struct field V has malformed `json` tag: invalid trailing ',' character"), + }, { + name: jsontest.Name("DashCommaOmitEmpty"), + in: struct { + V int `json:"-,omitempty"` + }{}, + wantOpts: fieldOptions{hasName: true, name: "-", quotedName: `"-"`, omitempty: true}, + wantErr: errors.New("Go struct field V has JSON object name \"-\"; either use `json:\"-\"` to ignore the field or use `json:\"'-',omitempty\"` to specify \"-\" as the name"), + }, { + name: jsontest.Name("QuotedDashCommaOmitEmpty"), + in: struct { + V int `json:"'-',omitempty"` + }{}, + wantOpts: fieldOptions{hasName: true, name: "-", quotedName: `"-"`, omitempty: true}, }, { name: jsontest.Name("QuotedDashName"), in: struct { diff --git a/src/encoding/json/v2_decode_test.go b/src/encoding/json/v2_decode_test.go index fe814a3cfd5..3ab20e2b5d0 100644 --- a/src/encoding/json/v2_decode_test.go +++ b/src/encoding/json/v2_decode_test.go @@ -1195,6 +1195,27 @@ var unmarshalTests = []struct { out: []int{1, 2, 0, 4, 5}, err: &UnmarshalTypeError{Value: "bool", Type: reflect.TypeFor[int](), Field: "2", Offset: len64(`[1,2,`)}, }, + + { + CaseName: Name("DashComma"), + in: `{"-":"hello"}`, + ptr: new(struct { + F string `json:"-,"` + }), + out: struct { + F string `json:"-,"` + }{"hello"}, + }, + { + CaseName: Name("DashCommaOmitEmpty"), + in: `{"-":"hello"}`, + ptr: new(struct { + F string `json:"-,omitempty"` + }), + out: struct { + F string `json:"-,omitempty"` + }{"hello"}, + }, } func TestMarshal(t *testing.T) { diff --git a/src/encoding/json/v2_diff_test.go b/src/encoding/json/v2_diff_test.go index 871be497767..7a561732f4a 100644 --- a/src/encoding/json/v2_diff_test.go +++ b/src/encoding/json/v2_diff_test.go @@ -1038,6 +1038,7 @@ func TestMergeComposite(t *testing.T) { // // https://go.dev/issue/10275 func TestTimeDurations(t *testing.T) { + t.SkipNow() // TODO(https://go.dev/issue/71631): The default representation of time.Duration is still undecided. for _, json := range jsonPackages { t.Run(path.Join("Marshal", json.Version), func(t *testing.T) { got, err := json.Marshal(time.Minute) diff --git a/src/encoding/json/v2_encode.go b/src/encoding/json/v2_encode.go index c8f35d4281c..cbb167dbd0d 100644 --- a/src/encoding/json/v2_encode.go +++ b/src/encoding/json/v2_encode.go @@ -68,7 +68,10 @@ import ( // slice, map, or string of length zero. // // As a special case, if the field tag is "-", the field is always omitted. -// Note that a field with name "-" can still be generated using the tag "-,". +// JSON names containing commas or quotes, or names identical to "" or "-", +// can be specified using a single-quoted string literal, where the syntax +// is identical to the Go grammar for a double-quoted string literal, +// but instead uses single quotes as the delimiters. // // Examples of struct field tags and their meanings: // @@ -89,7 +92,7 @@ import ( // Field int `json:"-"` // // // Field appears in JSON as key "-". -// Field int `json:"-,"` +// Field int `json:"'-'"` // // The "omitzero" option specifies that the field should be omitted // from the encoding if the field has a zero value, according to rules: diff --git a/src/go/doc/testdata/issue62640.0.golden b/src/go/doc/testdata/issue62640.0.golden new file mode 100644 index 00000000000..90775fd283b --- /dev/null +++ b/src/go/doc/testdata/issue62640.0.golden @@ -0,0 +1,22 @@ +// +PACKAGE issue62640 + +IMPORTPATH + testdata/issue62640 + +FILENAMES + testdata/issue62640.go + +TYPES + // + type E struct{} + + // F should be hidden within S because of the S.F field. + func (E) F() + + // + type S struct { + E + F int + } + diff --git a/src/go/doc/testdata/issue62640.1.golden b/src/go/doc/testdata/issue62640.1.golden new file mode 100644 index 00000000000..90775fd283b --- /dev/null +++ b/src/go/doc/testdata/issue62640.1.golden @@ -0,0 +1,22 @@ +// +PACKAGE issue62640 + +IMPORTPATH + testdata/issue62640 + +FILENAMES + testdata/issue62640.go + +TYPES + // + type E struct{} + + // F should be hidden within S because of the S.F field. + func (E) F() + + // + type S struct { + E + F int + } + diff --git a/src/go/doc/testdata/issue62640.2.golden b/src/go/doc/testdata/issue62640.2.golden new file mode 100644 index 00000000000..6e871aa3700 --- /dev/null +++ b/src/go/doc/testdata/issue62640.2.golden @@ -0,0 +1,25 @@ +// +PACKAGE issue62640 + +IMPORTPATH + testdata/issue62640 + +FILENAMES + testdata/issue62640.go + +TYPES + // + type E struct{} + + // F should be hidden within S because of the S.F field. + func (E) F() + + // + type S struct { + E + F int + } + + // F should be hidden within S because of the S.F field. + func (S) F() + diff --git a/src/go/doc/testdata/issue62640.go b/src/go/doc/testdata/issue62640.go new file mode 100644 index 00000000000..f109de46fa9 --- /dev/null +++ b/src/go/doc/testdata/issue62640.go @@ -0,0 +1,15 @@ +// 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 issue62640 + +type E struct{} + +// F should be hidden within S because of the S.F field. +func (E) F() {} + +type S struct { + E + F int +} diff --git a/src/go/types/api_test.go b/src/go/types/api_test.go index f5a911306fb..4396b8ae89d 100644 --- a/src/go/types/api_test.go +++ b/src/go/types/api_test.go @@ -364,6 +364,11 @@ func TestTypesInfo(t *testing.T) { // go.dev/issue/47895 {`package p; import "unsafe"; type S struct { f int }; var s S; var _ = unsafe.Offsetof(s.f)`, `s.f`, `int`}, + // go.dev/issue/74303. Note that interface field types are synthetic, so + // even though `func()` doesn't appear in the source, it appears in the + // syntax tree. + {`package p; type T interface { M(int) }`, `func(int)`, `func(int)`}, + // go.dev/issue/50093 {`package u0a; func _[_ interface{int}]() {}`, `int`, `int`}, {`package u1a; func _[_ interface{~int}]() {}`, `~int`, `~int`}, diff --git a/src/go/types/interface.go b/src/go/types/interface.go index 6bcae7aef0e..5f9c88d8f5c 100644 --- a/src/go/types/interface.go +++ b/src/go/types/interface.go @@ -176,19 +176,17 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d name := f.Names[0] if name.Name == "_" { check.error(name, BlankIfaceMethod, "methods must have a unique non-blank name") - continue // ignore method + continue // ignore } - // Type-check method declaration. - // Note: Don't call check.typ(f.Type) as that would record - // the method incorrectly as a type expression in Info.Types. - ftyp, _ := f.Type.(*ast.FuncType) - if ftyp == nil { - check.errorf(f.Type, InvalidSyntaxTree, "%s is not a method signature", f.Type) - continue // ignore method + typ := check.typ(f.Type) + sig, _ := typ.(*Signature) + if sig == nil { + if isValid(typ) { + check.errorf(f.Type, InvalidSyntaxTree, "%s is not a method signature", typ) + } + continue // ignore } - sig := new(Signature) - check.funcType(sig, nil, ftyp) // The go/parser doesn't accept method type parameters but an ast.FuncType may have them. if sig.tparams != nil { diff --git a/src/internal/reflectlite/value.go b/src/internal/reflectlite/value.go index c38b498ea7e..7b231d554f5 100644 --- a/src/internal/reflectlite/value.go +++ b/src/internal/reflectlite/value.go @@ -43,17 +43,19 @@ type Value struct { ptr unsafe.Pointer // flag holds metadata about the value. - // The lowest bits are flag bits: + // + // The lowest five bits give the Kind of the value, mirroring typ.Kind(). + // + // The next set of bits are flag bits: // - flagStickyRO: obtained via unexported not embedded field, so read-only // - flagEmbedRO: obtained via unexported embedded field, so read-only // - flagIndir: val holds a pointer to the data - // - flagAddr: v.CanAddr is true (implies flagIndir) - // Value cannot represent method values. - // The next five bits give the Kind of the value. - // This repeats typ.Kind() except for method values. - // The remaining 23+ bits give a method number for method values. - // If flag.kind() != Func, code can assume that flagMethod is unset. + // - flagAddr: v.CanAddr is true (implies flagIndir and ptr is non-nil) + // - flagMethod: v is a method value. // If ifaceIndir(typ), code can assume that flagIndir is set. + // + // The remaining 22+ bits give a method number for method values. + // If flag.kind() != Func, code can assume that flagMethod is unset. flag // A method value represents a curried method invocation diff --git a/src/internal/synctest/synctest.go b/src/internal/synctest/synctest.go index 4d7fa3730c4..cb3333a6270 100644 --- a/src/internal/synctest/synctest.go +++ b/src/internal/synctest/synctest.go @@ -8,6 +8,7 @@ package synctest import ( + "internal/abi" "unsafe" ) @@ -22,14 +23,25 @@ func Wait() //go:linkname IsInBubble func IsInBubble() bool -// Associate associates p with the current bubble. -// It returns false if p has an existing association with a different bubble. -func Associate[T any](p *T) (ok bool) { - return associate(unsafe.Pointer(p)) +// Association is the state of a pointer's bubble association. +type Association int + +const ( + Unbubbled = Association(iota) // not associated with any bubble + CurrentBubble // associated with the current bubble + OtherBubble // associated with a different bubble +) + +// Associate attempts to associate p with the current bubble. +// It returns the new association status of p. +func Associate[T any](p *T) Association { + // Ensure p escapes to permit us to attach a special to it. + escapedP := abi.Escape(p) + return Association(associate(unsafe.Pointer(escapedP))) } //go:linkname associate -func associate(p unsafe.Pointer) bool +func associate(p unsafe.Pointer) int // Disassociate disassociates p from any bubble. func Disassociate[T any](p *T) { diff --git a/src/internal/synctest/synctest_test.go b/src/internal/synctest/synctest_test.go index fe6eb63702b..222cae2597f 100644 --- a/src/internal/synctest/synctest_test.go +++ b/src/internal/synctest/synctest_test.go @@ -731,6 +731,34 @@ func TestWaitGroupMovedBetweenBubblesAfterWait(t *testing.T) { }) } +var testWaitGroupLinkerAllocatedWG sync.WaitGroup + +func TestWaitGroupLinkerAllocated(t *testing.T) { + synctest.Run(func() { + // This WaitGroup is probably linker-allocated and has no span, + // so we won't be able to add a special to it associating it with + // this bubble. + // + // Operations on it may not be durably blocking, + // but they shouldn't fail. + testWaitGroupLinkerAllocatedWG.Go(func() {}) + testWaitGroupLinkerAllocatedWG.Wait() + }) +} + +var testWaitGroupHeapAllocatedWG = new(sync.WaitGroup) + +func TestWaitGroupHeapAllocated(t *testing.T) { + synctest.Run(func() { + // This package-scoped WaitGroup var should have been heap-allocated, + // so we can associate it with a bubble. + testWaitGroupHeapAllocatedWG.Add(1) + go testWaitGroupHeapAllocatedWG.Wait() + synctest.Wait() + testWaitGroupHeapAllocatedWG.Done() + }) +} + func TestHappensBefore(t *testing.T) { // Use two parallel goroutines accessing different vars to ensure that // we correctly account for multiple goroutines in the bubble. diff --git a/src/internal/syscall/windows/at_windows.go b/src/internal/syscall/windows/at_windows.go index 87e0195d30e..d48fce1c99d 100644 --- a/src/internal/syscall/windows/at_windows.go +++ b/src/internal/syscall/windows/at_windows.go @@ -192,6 +192,11 @@ func Mkdirat(dirfd syscall.Handle, name string, mode uint32) error { } func Deleteat(dirfd syscall.Handle, name string, options uint32) error { + if name == "." { + // NtOpenFile's documentation isn't explicit about what happens when deleting ".". + // Make this an error consistent with that of POSIX. + return syscall.EINVAL + } objAttrs := &OBJECT_ATTRIBUTES{} if err := objAttrs.init(dirfd, name); err != nil { return err diff --git a/src/internal/trace/event.go b/src/internal/trace/event.go index 21f1569f43f..f31412e35d8 100644 --- a/src/internal/trace/event.go +++ b/src/internal/trace/event.go @@ -8,6 +8,7 @@ import ( "fmt" "iter" "math" + "strconv" "strings" "time" @@ -812,6 +813,10 @@ func (e Event) String() string { switch kind := e.Kind(); kind { case EventMetric: m := e.Metric() + v := m.Value.String() + if m.Value.Kind() == ValueString { + v = strconv.Quote(v) + } fmt.Fprintf(&sb, " Name=%q Value=%s", m.Name, m.Value) case EventLabel: l := e.Label() diff --git a/src/internal/trace/gc.go b/src/internal/trace/gc.go index f5e8fe79f29..46890e784df 100644 --- a/src/internal/trace/gc.go +++ b/src/internal/trace/gc.go @@ -103,7 +103,7 @@ func MutatorUtilizationV2(events []Event, flags UtilFlags) [][]MutatorUtil { if m.Name != "/sched/gomaxprocs:threads" { break } - gomaxprocs := int(m.Value.ToUint64()) + gomaxprocs := int(m.Value.Uint64()) if len(ps) > gomaxprocs { if flags&UtilPerProc != 0 { // End each P's series. diff --git a/src/internal/trace/testdata/testprog/gc-stress.go b/src/internal/trace/testdata/testprog/gc-stress.go index 7979234c40a..74b63606d5e 100644 --- a/src/internal/trace/testdata/testprog/gc-stress.go +++ b/src/internal/trace/testdata/testprog/gc-stress.go @@ -13,6 +13,7 @@ import ( "log" "os" "runtime" + "runtime/debug" "runtime/trace" "time" ) @@ -36,11 +37,25 @@ func makeTree(depth int) *node { } } +func initTree(n *node) { + if n == nil { + return + } + for i := range n.data { + n.data[i] = 'a' + } + for i := range n.children { + initTree(n.children[i]) + } +} + var trees [16]*node var ballast *[16]*[1024]*node -var sink [][]byte +var sink []*node func main() { + debug.SetMemoryLimit(50 << 20) + for i := range trees { trees[i] = makeTree(6) } @@ -55,13 +70,17 @@ func main() { } procs := runtime.GOMAXPROCS(-1) - sink = make([][]byte, procs) + sink = make([]*node, procs) for i := 0; i < procs; i++ { i := i go func() { for { - sink[i] = make([]byte, 4<<10) + sink[i] = makeTree(3) + for range 5 { + initTree(sink[i]) + runtime.Gosched() + } } }() } diff --git a/src/internal/trace/testtrace/validation.go b/src/internal/trace/testtrace/validation.go index 3de1e1d4bdf..5edcf3a5b2d 100644 --- a/src/internal/trace/testtrace/validation.go +++ b/src/internal/trace/testtrace/validation.go @@ -135,7 +135,7 @@ func (v *Validator) Event(ev trace.Event) error { switch m.Value.Kind() { case trace.ValueUint64: // Just make sure it doesn't panic. - _ = m.Value.ToUint64() + _ = m.Value.Uint64() } case trace.EventLabel: l := ev.Label() diff --git a/src/internal/trace/trace_test.go b/src/internal/trace/trace_test.go index 7eb50d0f4ea..eaf194cf077 100644 --- a/src/internal/trace/trace_test.go +++ b/src/internal/trace/trace_test.go @@ -582,13 +582,30 @@ func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace testPath := filepath.Join("./testdata/testprog", progName) testName := progName runTest := func(t *testing.T, stress bool, extraGODEBUG string) { - // Run the program and capture the trace, which is always written to stdout. - cmd := testenv.Command(t, testenv.GoToolPath(t), "run") - if race.Enabled { - cmd.Args = append(cmd.Args, "-race") + // Build the program. + binFile, err := os.CreateTemp("", progName) + if err != nil { + t.Fatalf("failed to create temporary output file: %v", err) } - cmd.Args = append(cmd.Args, testPath) - cmd.Env = append(os.Environ(), "GOEXPERIMENT=rangefunc") + bin := binFile.Name() + binFile.Close() + t.Cleanup(func() { + os.Remove(bin) + }) + buildCmd := testenv.CommandContext(t, t.Context(), testenv.GoToolPath(t), "build", "-o", bin) + if race.Enabled { + buildCmd.Args = append(buildCmd.Args, "-race") + } + buildCmd.Args = append(buildCmd.Args, testPath) + buildCmd.Env = append(os.Environ(), "GOEXPERIMENT=rangefunc") + buildOutput, err := buildCmd.CombinedOutput() + if err != nil { + t.Fatalf("failed to build %s: %v: output:\n%s", testPath, err, buildOutput) + } + + // Run the program and capture the trace, which is always written to stdout. + cmd := testenv.CommandContext(t, t.Context(), bin) + // Add a stack ownership check. This is cheap enough for testing. godebug := "tracecheckstackownership=1" if stress { diff --git a/src/internal/trace/value.go b/src/internal/trace/value.go index bf396b6a9ee..fc2808e5975 100644 --- a/src/internal/trace/value.go +++ b/src/internal/trace/value.go @@ -35,24 +35,27 @@ func (v Value) Kind() ValueKind { return v.kind } -// ToUint64 returns the uint64 value for a ValueUint64. +// Uint64 returns the uint64 value for a ValueUint64. // // Panics if this Value's Kind is not ValueUint64. -func (v Value) ToUint64() uint64 { +func (v Value) Uint64() uint64 { if v.kind != ValueUint64 { - panic("ToUint64 called on Value of a different Kind") + panic("Uint64 called on Value of a different Kind") } return v.scalar } -// ToString returns the uint64 value for a ValueString. -// -// Panics if this Value's Kind is not ValueString. -func (v Value) ToString() string { - if v.kind != ValueString { - panic("ToString called on Value of a different Kind") +// String returns the string value for a ValueString, and otherwise +// a string representation of the value for other kinds of values. +func (v Value) String() string { + if v.kind == ValueString { + return unsafe.String((*byte)(v.pointer), int(v.scalar)) } - return unsafe.String((*byte)(v.pointer), int(v.scalar)) + switch v.kind { + case ValueUint64: + return fmt.Sprintf("Value{Uint64(%d)}", v.Uint64()) + } + return "Value{Bad}" } func uint64Value(x uint64) Value { @@ -62,14 +65,3 @@ func uint64Value(x uint64) Value { func stringValue(s string) Value { return Value{kind: ValueString, scalar: uint64(len(s)), pointer: unsafe.Pointer(unsafe.StringData(s))} } - -// String returns the string representation of the value. -func (v Value) String() string { - switch v.Kind() { - case ValueUint64: - return fmt.Sprintf("Value{Uint64(%d)}", v.ToUint64()) - case ValueString: - return fmt.Sprintf("Value{String(%s)}", v.ToString()) - } - return "Value{Bad}" -} diff --git a/src/net/http/csrf.go b/src/net/http/csrf.go index 8812a508ae2..5e1b686fd1c 100644 --- a/src/net/http/csrf.go +++ b/src/net/http/csrf.go @@ -136,7 +136,7 @@ func (c *CrossOriginProtection) Check(req *Request) error { if c.isRequestExempt(req) { return nil } - return errors.New("cross-origin request detected from Sec-Fetch-Site header") + return errCrossOriginRequest } origin := req.Header.Get("Origin") @@ -159,10 +159,15 @@ func (c *CrossOriginProtection) Check(req *Request) error { if c.isRequestExempt(req) { return nil } - return errors.New("cross-origin request detected, and/or browser is out of date: " + - "Sec-Fetch-Site is missing, and Origin does not match Host") + return errCrossOriginRequestFromOldBrowser } +var ( + errCrossOriginRequest = errors.New("cross-origin request detected from Sec-Fetch-Site header") + errCrossOriginRequestFromOldBrowser = errors.New("cross-origin request detected, and/or browser is out of date: " + + "Sec-Fetch-Site is missing, and Origin does not match Host") +) + // isRequestExempt checks the bypasses which require taking a lock, and should // be deferred until the last moment. func (c *CrossOriginProtection) isRequestExempt(req *Request) bool { diff --git a/src/net/iprawsock.go b/src/net/iprawsock.go index 4c06b1b5aca..76dded9ca16 100644 --- a/src/net/iprawsock.go +++ b/src/net/iprawsock.go @@ -24,9 +24,6 @@ import ( // BUG(mikio): On JS and Plan 9, methods and functions related // to IPConn are not implemented. -// BUG(mikio): On Windows, the File method of IPConn is not -// implemented. - // IPAddr represents the address of an IP end point. type IPAddr struct { IP IP diff --git a/src/net/tcpsock.go b/src/net/tcpsock.go index 1b11a03f65c..9d215db1b2e 100644 --- a/src/net/tcpsock.go +++ b/src/net/tcpsock.go @@ -14,7 +14,7 @@ import ( "time" ) -// BUG(mikio): On JS and Windows, the File method of TCPConn and +// BUG(mikio): On JS, the File method of TCPConn and // TCPListener is not implemented. // TCPAddr represents the address of a TCP end point. diff --git a/src/net/udpsock.go b/src/net/udpsock.go index 56aabffa318..35da018c307 100644 --- a/src/net/udpsock.go +++ b/src/net/udpsock.go @@ -14,9 +14,6 @@ import ( // BUG(mikio): On Plan 9, the ReadMsgUDP and // WriteMsgUDP methods of UDPConn are not implemented. -// BUG(mikio): On Windows, the File method of UDPConn is not -// implemented. - // BUG(mikio): On JS, methods and functions related to UDPConn are not // implemented. diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index fecfc97d138..91a6831b04f 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -17,7 +17,7 @@ // // Note that the examples in this package assume a Unix system. // They may not run on Windows, and they do not run in the Go Playground -// used by golang.org and godoc.org. +// used by go.dev and pkg.go.dev. // // # Executables in the current directory // diff --git a/src/os/root_noopenat.go b/src/os/root_noopenat.go index c4929623c48..59f1abe91b0 100644 --- a/src/os/root_noopenat.go +++ b/src/os/root_noopenat.go @@ -8,6 +8,7 @@ package os import ( "errors" + "internal/filepathlite" "internal/stringslite" "sync/atomic" "syscall" @@ -173,6 +174,12 @@ func rootRemove(r *Root, name string) error { if err := checkPathEscapesLstat(r, name); err != nil { return &PathError{Op: "removeat", Path: name, Err: err} } + if endsWithDot(name) { + // We don't want to permit removing the root itself, so check for that. + if filepathlite.Clean(name) == "." { + return &PathError{Op: "removeat", Path: name, Err: errPathEscapes} + } + } if err := Remove(joinPath(r.root.name, name)); err != nil { return &PathError{Op: "removeat", Path: name, Err: underlyingError(err)} } diff --git a/src/runtime/arena.go b/src/runtime/arena.go index 627c7cfdce5..e8079958105 100644 --- a/src/runtime/arena.go +++ b/src/runtime/arena.go @@ -1052,10 +1052,18 @@ func (h *mheap) allocUserArenaChunk() *mspan { h.initSpan(s, spanAllocHeap, spc, base, userArenaChunkPages) s.isUserArenaChunk = true s.elemsize -= userArenaChunkReserveBytes() - s.limit = s.base() + s.elemsize s.freeindex = 1 s.allocCount = 1 + // Adjust s.limit down to the object-containing part of the span. + // + // This is just to create a slightly tighter bound on the limit. + // It's totally OK if the garbage collector, in particular + // conservative scanning, can temporarily observes an inflated + // limit. It will simply mark the whole chunk or just skip it + // since we're in the mark phase anyway. + s.limit = s.base() + s.elemsize + // Adjust size to include redzone. if asanenabled { s.elemsize -= redZoneSize(s.elemsize) diff --git a/src/runtime/debug.go b/src/runtime/debug.go index bdaaa7196d3..c7592d33299 100644 --- a/src/runtime/debug.go +++ b/src/runtime/debug.go @@ -39,7 +39,7 @@ func GOMAXPROCS(n int) int { lock(&sched.lock) ret := int(gomaxprocs) - if n <= 0 || n == ret { + if n <= 0 { unlock(&sched.lock) return ret } @@ -52,6 +52,12 @@ func GOMAXPROCS(n int) int { lock(&computeMaxProcsLock) unlock(&computeMaxProcsLock) + if n == ret { + // sched.customGOMAXPROCS set, but no need to actually STW + // since the gomaxprocs itself isn't changing. + return ret + } + stw := stopTheWorldGC(stwGOMAXPROCS) // newprocs will be processed by startTheWorld diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index a9cc767e30e..83cf301be49 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -1911,3 +1911,9 @@ func (b BitCursor) Write(data *byte, cnt uintptr) { func (b BitCursor) Offset(cnt uintptr) BitCursor { return BitCursor{b: b.b.offset(cnt)} } + +const ( + BubbleAssocUnbubbled = bubbleAssocUnbubbled + BubbleAssocCurrentBubble = bubbleAssocCurrentBubble + BubbleAssocOtherBubble = bubbleAssocOtherBubble +) diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index 440120cdfe8..a1d04d2f8a2 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -253,6 +253,14 @@ func (c *mcache) allocLarge(size uintptr, noscan bool) *mspan { // Put the large span in the mcentral swept list so that it's // visible to the background sweeper. mheap_.central[spc].mcentral.fullSwept(mheap_.sweepgen).push(s) + + // Adjust s.limit down to the object-containing part of the span. + // + // This is just to create a slightly tighter bound on the limit. + // It's totally OK if the garbage collector, in particular + // conservative scanning, can temporarily observes an inflated + // limit. It will simply mark the whole object or just skip it + // since we're in the mark phase anyway. s.limit = s.base() + size s.initHeapBits() return s diff --git a/src/runtime/mcentral.go b/src/runtime/mcentral.go index c71ecbbcd54..ec27ce25a88 100644 --- a/src/runtime/mcentral.go +++ b/src/runtime/mcentral.go @@ -250,13 +250,10 @@ func (c *mcentral) uncacheSpan(s *mspan) { // grow allocates a new empty span from the heap and initializes it for c's size class. func (c *mcentral) grow() *mspan { npages := uintptr(gc.SizeClassToNPages[c.spanclass.sizeclass()]) - size := uintptr(gc.SizeClassToSize[c.spanclass.sizeclass()]) - s := mheap_.alloc(npages, c.spanclass) if s == nil { return nil } - s.limit = s.base() + size*uintptr(s.nelems) s.initHeapBits() return s } diff --git a/src/runtime/mem_sbrk.go b/src/runtime/mem_sbrk.go index 9d6842ae52c..05f0fdb5d74 100644 --- a/src/runtime/mem_sbrk.go +++ b/src/runtime/mem_sbrk.go @@ -231,6 +231,7 @@ func sysReserveAlignedSbrk(size, align uintptr) (unsafe.Pointer, uintptr) { memFree(unsafe.Pointer(end), endLen) } memCheck() + unlock(&memlock) return unsafe.Pointer(pAligned), size } diff --git a/src/runtime/mfinal.go b/src/runtime/mfinal.go index 44db1fb3562..2d4a54c9332 100644 --- a/src/runtime/mfinal.go +++ b/src/runtime/mfinal.go @@ -338,11 +338,6 @@ func blockUntilEmptyFinalizerQueue(timeout int64) bool { return false } -//go:linkname unique_runtime_blockUntilEmptyFinalizerQueue unique.runtime_blockUntilEmptyFinalizerQueue -func unique_runtime_blockUntilEmptyFinalizerQueue(timeout int64) bool { - return blockUntilEmptyFinalizerQueue(timeout) -} - // SetFinalizer sets the finalizer associated with obj to the provided // finalizer function. When the garbage collector finds an unreachable block // with an associated finalizer, it clears the association and runs diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 38f343164cc..f2df1a00e0c 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1048,7 +1048,7 @@ func gcMarkTermination(stw worldStop) { // N.B. The execution tracer is not aware of this status // transition and handles it specially based on the // wait reason. - casGToWaitingForGC(curgp, _Grunning, waitReasonGarbageCollection) + casGToWaitingForSuspendG(curgp, _Grunning, waitReasonGarbageCollection) // Run gc on the g0 stack. We do this so that the g stack // we're currently running on will no longer change. Cuts @@ -1522,7 +1522,8 @@ func gcBgMarkWorker(ready chan struct{}) { systemstack(func() { // Mark our goroutine preemptible so its stack - // can be scanned. This lets two mark workers + // can be scanned or observed by the execution + // tracer. This, for example, lets two mark workers // scan each other (otherwise, they would // deadlock). We must not modify anything on // the G stack. However, stack shrinking is @@ -1532,7 +1533,7 @@ func gcBgMarkWorker(ready chan struct{}) { // N.B. The execution tracer is not aware of this status // transition and handles it specially based on the // wait reason. - casGToWaitingForGC(gp, _Grunning, waitReasonGCWorkerActive) + casGToWaitingForSuspendG(gp, _Grunning, waitReasonGCWorkerActive) switch pp.gcMarkWorkerMode { default: throw("gcBgMarkWorker: unexpected gcMarkWorkerMode") diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go index 507aac74828..a136c7aeace 100644 --- a/src/runtime/mgcmark.go +++ b/src/runtime/mgcmark.go @@ -227,7 +227,7 @@ func markroot(gcw *gcWork, i uint32, flushBgCredit bool) int64 { userG := getg().m.curg selfScan := gp == userG && readgstatus(userG) == _Grunning if selfScan { - casGToWaitingForGC(userG, _Grunning, waitReasonGarbageCollectionScan) + casGToWaitingForSuspendG(userG, _Grunning, waitReasonGarbageCollectionScan) } // TODO: suspendG blocks (and spins) until gp @@ -682,7 +682,7 @@ func gcAssistAlloc1(gp *g, scanWork int64) { } // gcDrainN requires the caller to be preemptible. - casGToWaitingForGC(gp, _Grunning, waitReasonGCAssistMarking) + casGToWaitingForSuspendG(gp, _Grunning, waitReasonGCAssistMarking) // drain own cached work first in the hopes that it // will be more cache friendly. diff --git a/src/runtime/mgcmark_greenteagc.go b/src/runtime/mgcmark_greenteagc.go index 75c347b9e93..ac2b1732f93 100644 --- a/src/runtime/mgcmark_greenteagc.go +++ b/src/runtime/mgcmark_greenteagc.go @@ -111,6 +111,26 @@ func (o *spanScanOwnership) or(v spanScanOwnership) spanScanOwnership { } func (imb *spanInlineMarkBits) init(class spanClass) { + if imb == nil { + // This nil check and throw is almost pointless. Normally we would + // expect imb to never be nil. However, this is called on potentially + // freshly-allocated virtual memory. As of 2025, the compiler-inserted + // nil check is not a branch but a memory read that we expect to fault + // if the pointer really is nil. + // + // However, this causes a read of the page, and operating systems may + // take it as a hint to back the accessed memory with a read-only zero + // page. However, we immediately write to this memory, which can then + // force operating systems to have to update the page table and flush + // the TLB, causing a lot of churn for programs that are short-lived + // and monotonically grow in size. + // + // This nil check is thus an explicit branch instead of what the compiler + // would insert circa 2025, which is a memory read instruction. + // + // See go.dev/issue/74375 for details. + throw("runtime: span inline mark bits nil?") + } *imb = spanInlineMarkBits{} imb.class = class } diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 9361089b801..f25dbb429d7 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -312,8 +312,10 @@ type heapArena struct { // during marking. pageSpecials [pagesPerArena / 8]uint8 - // pageUseSpanDartboard is a bitmap that indicates which spans are - // heap spans and also gcUsesSpanDartboard. + // pageUseSpanInlineMarkBits is a bitmap where each bit corresponds + // to a span, as only spans one page in size can have inline mark bits. + // The bit indicates that the span has a spanInlineMarkBits struct + // stored directly at the top end of the span's memory. pageUseSpanInlineMarkBits [pagesPerArena / 8]uint8 // checkmarks stores the debug.gccheckmark state. It is only @@ -1445,7 +1447,6 @@ func (h *mheap) initSpan(s *mspan, typ spanAllocType, spanclass spanClass, base, if typ.manual() { s.manualFreeList = 0 s.nelems = 0 - s.limit = s.base() + s.npages*pageSize s.state.set(mSpanManual) } else { // We must set span properties before the span is published anywhere @@ -1486,6 +1487,9 @@ func (h *mheap) initSpan(s *mspan, typ spanAllocType, spanclass spanClass, base, s.gcmarkBits = newMarkBits(uintptr(s.nelems)) s.allocBits = newAllocBits(uintptr(s.nelems)) + // Adjust s.limit down to the object-containing part of the span. + s.limit = s.base() + uintptr(s.elemsize)*uintptr(s.nelems) + // It's safe to access h.sweepgen without the heap lock because it's // only ever updated with the world stopped and we run on the // systemstack which blocks a STW transition. @@ -1785,6 +1789,7 @@ func (span *mspan) init(base uintptr, npages uintptr) { span.list = nil span.startAddr = base span.npages = npages + span.limit = base + npages*gc.PageSize // see go.dev/issue/74288; adjusted later for heap spans span.allocCount = 0 span.spanclass = 0 span.elemsize = 0 diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go index a033e284795..b2ff257f65e 100644 --- a/src/runtime/mprof.go +++ b/src/runtime/mprof.go @@ -1431,10 +1431,6 @@ func tryRecordGoroutineProfile(gp1 *g, pcbuf []uintptr, yield func()) { // so here we check _Gdead first. return } - if isSystemGoroutine(gp1, false) { - // System goroutines should not appear in the profile. - return - } for { prev := gp1.goroutineProfiled.Load() @@ -1472,6 +1468,17 @@ func tryRecordGoroutineProfile(gp1 *g, pcbuf []uintptr, yield func()) { // stack), or from the scheduler in preparation to execute gp1 (running on the // system stack). func doRecordGoroutineProfile(gp1 *g, pcbuf []uintptr) { + if isSystemGoroutine(gp1, false) { + // System goroutines should not appear in the profile. + // Check this here and not in tryRecordGoroutineProfile because isSystemGoroutine + // may change on a goroutine while it is executing, so while the scheduler might + // see a system goroutine, goroutineProfileWithLabelsConcurrent might not, and + // this inconsistency could cause invariants to be violated, such as trying to + // record the stack of a running goroutine below. In short, we still want system + // goroutines to participate in the same state machine on gp1.goroutineProfiled as + // everything else, we just don't record the stack in the profile. + return + } if readgstatus(gp1) == _Grunning { print("doRecordGoroutineProfile gp1=", gp1.goid, "\n") throw("cannot read stack of running goroutine") diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index f2ee39dd496..5f83f37b507 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -1808,6 +1808,45 @@ func TestGoroutineProfileCoro(t *testing.T) { goroutineProf.WriteTo(io.Discard, 1) } +// This test tries to provoke a situation wherein the finalizer goroutine is +// erroneously inspected by the goroutine profiler in such a way that could +// cause a crash. See go.dev/issue/74090. +func TestGoroutineProfileIssue74090(t *testing.T) { + testenv.MustHaveParallelism(t) + + goroutineProf := Lookup("goroutine") + + // T is a pointer type so it won't be allocated by the tiny + // allocator, which can lead to its finalizer not being called + // during this test. + type T *byte + for range 10 { + // We use finalizers for this test because finalizers transition between + // system and user goroutine on each call, since there's substantially + // more work to do to set up a finalizer call. Cleanups, on the other hand, + // transition once for a whole batch, and so are less likely to trigger + // the failure. Under stress testing conditions this test fails approximately + // 5 times every 1000 executions on a 64 core machine without the appropriate + // fix, which is not ideal but if this test crashes at all, it's a clear + // signal that something is broken. + var objs []*T + for range 10000 { + obj := new(T) + runtime.SetFinalizer(obj, func(_ interface{}) {}) + objs = append(objs, obj) + } + objs = nil + + // Queue up all the finalizers. + runtime.GC() + + // Try to run a goroutine profile concurrently with finalizer execution + // to trigger the bug. + var w strings.Builder + goroutineProf.WriteTo(&w, 1) + } +} + func BenchmarkGoroutine(b *testing.B) { withIdle := func(n int, fn func(b *testing.B)) func(b *testing.B) { return func(b *testing.B) { diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 37a7b7f6849..98173084302 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1347,13 +1347,13 @@ func casGToWaiting(gp *g, old uint32, reason waitReason) { casgstatus(gp, old, _Gwaiting) } -// casGToWaitingForGC transitions gp from old to _Gwaiting, and sets the wait reason. -// The wait reason must be a valid isWaitingForGC wait reason. +// casGToWaitingForSuspendG transitions gp from old to _Gwaiting, and sets the wait reason. +// The wait reason must be a valid isWaitingForSuspendG wait reason. // // Use this over casgstatus when possible to ensure that a waitreason is set. -func casGToWaitingForGC(gp *g, old uint32, reason waitReason) { - if !reason.isWaitingForGC() { - throw("casGToWaitingForGC with non-isWaitingForGC wait reason") +func casGToWaitingForSuspendG(gp *g, old uint32, reason waitReason) { + if !reason.isWaitingForSuspendG() { + throw("casGToWaitingForSuspendG with non-isWaitingForSuspendG wait reason") } casGToWaiting(gp, old, reason) } @@ -1487,23 +1487,7 @@ func stopTheWorld(reason stwReason) worldStop { gp := getg() gp.m.preemptoff = reason.String() systemstack(func() { - // Mark the goroutine which called stopTheWorld preemptible so its - // stack may be scanned. - // This lets a mark worker scan us while we try to stop the world - // since otherwise we could get in a mutual preemption deadlock. - // We must not modify anything on the G stack because a stack shrink - // may occur. A stack shrink is otherwise OK though because in order - // to return from this function (and to leave the system stack) we - // must have preempted all goroutines, including any attempting - // to scan our stack, in which case, any stack shrinking will - // have already completed by the time we exit. - // - // N.B. The execution tracer is not aware of this status - // transition and handles it specially based on the - // wait reason. - casGToWaitingForGC(gp, _Grunning, waitReasonStoppingTheWorld) stopTheWorldContext = stopTheWorldWithSema(reason) // avoid write to stack - casgstatus(gp, _Gwaiting, _Grunning) }) return stopTheWorldContext } @@ -1592,7 +1576,30 @@ var gcsema uint32 = 1 // // Returns the STW context. When starting the world, this context must be // passed to startTheWorldWithSema. +// +//go:systemstack func stopTheWorldWithSema(reason stwReason) worldStop { + // Mark the goroutine which called stopTheWorld preemptible so its + // stack may be scanned by the GC or observed by the execution tracer. + // + // This lets a mark worker scan us or the execution tracer take our + // stack while we try to stop the world since otherwise we could get + // in a mutual preemption deadlock. + // + // We must not modify anything on the G stack because a stack shrink + // may occur, now that we switched to _Gwaiting, specifically if we're + // doing this during the mark phase (mark termination excepted, since + // we know that stack scanning is done by that point). A stack shrink + // is otherwise OK though because in order to return from this function + // (and to leave the system stack) we must have preempted all + // goroutines, including any attempting to scan our stack, in which + // case, any stack shrinking will have already completed by the time we + // exit. + // + // N.B. The execution tracer is not aware of this status transition and + // andles it specially based on the wait reason. + casGToWaitingForSuspendG(getg().m.curg, _Grunning, waitReasonStoppingTheWorld) + trace := traceAcquire() if trace.ok() { trace.STWStart(reason) @@ -1700,6 +1707,9 @@ func stopTheWorldWithSema(reason stwReason) worldStop { worldStopped() + // Switch back to _Grunning, now that the world is stopped. + casgstatus(getg().m.curg, _Gwaiting, _Grunning) + return worldStop{ reason: reason, startedStopping: start, @@ -2068,15 +2078,23 @@ found: func forEachP(reason waitReason, fn func(*p)) { systemstack(func() { gp := getg().m.curg - // Mark the user stack as preemptible so that it may be scanned. - // Otherwise, our attempt to force all P's to a safepoint could - // result in a deadlock as we attempt to preempt a worker that's - // trying to preempt us (e.g. for a stack scan). + // Mark the user stack as preemptible so that it may be scanned + // by the GC or observed by the execution tracer. Otherwise, our + // attempt to force all P's to a safepoint could result in a + // deadlock as we attempt to preempt a goroutine that's trying + // to preempt us (e.g. for a stack scan). + // + // We must not modify anything on the G stack because a stack shrink + // may occur. A stack shrink is otherwise OK though because in order + // to return from this function (and to leave the system stack) we + // must have preempted all goroutines, including any attempting + // to scan our stack, in which case, any stack shrinking will + // have already completed by the time we exit. // // N.B. The execution tracer is not aware of this status // transition and handles it specially based on the // wait reason. - casGToWaitingForGC(gp, _Grunning, reason) + casGToWaitingForSuspendG(gp, _Grunning, reason) forEachPInternal(fn) casgstatus(gp, _Gwaiting, _Grunning) }) diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index d1b31be172e..96720846b24 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -1163,17 +1163,17 @@ func (w waitReason) isMutexWait() bool { w == waitReasonSyncRWMutexLock } -func (w waitReason) isWaitingForGC() bool { - return isWaitingForGC[w] +func (w waitReason) isWaitingForSuspendG() bool { + return isWaitingForSuspendG[w] } -// isWaitingForGC indicates that a goroutine is only entering _Gwaiting and -// setting a waitReason because it needs to be able to let the GC take ownership -// of its stack. The G is always actually executing on the system stack, in -// these cases. +// isWaitingForSuspendG indicates that a goroutine is only entering _Gwaiting and +// setting a waitReason because it needs to be able to let the suspendG +// (used by the GC and the execution tracer) take ownership of its stack. +// The G is always actually executing on the system stack in these cases. // // TODO(mknyszek): Consider replacing this with a new dedicated G status. -var isWaitingForGC = [len(waitReasonStrings)]bool{ +var isWaitingForSuspendG = [len(waitReasonStrings)]bool{ waitReasonStoppingTheWorld: true, waitReasonGCMarkTermination: true, waitReasonGarbageCollection: true, diff --git a/src/runtime/stack.go b/src/runtime/stack.go index 7e69d65fbb7..4b647976f07 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -1212,14 +1212,14 @@ func isShrinkStackSafe(gp *g) bool { return false } // We also can't copy the stack while tracing is enabled, and - // gp is in _Gwaiting solely to make itself available to the GC. + // gp is in _Gwaiting solely to make itself available to suspendG. // In these cases, the G is actually executing on the system // stack, and the execution tracer may want to take a stack trace // of the G's stack. Note: it's safe to access gp.waitreason here. // We're only checking if this is true if we took ownership of the // G with the _Gscan bit. This prevents the goroutine from transitioning, // which prevents gp.waitreason from changing. - if traceEnabled() && readgstatus(gp)&^_Gscan == _Gwaiting && gp.waitreason.isWaitingForGC() { + if traceEnabled() && readgstatus(gp)&^_Gscan == _Gwaiting && gp.waitreason.isWaitingForSuspendG() { return false } return true diff --git a/src/runtime/synctest.go b/src/runtime/synctest.go index 08a0e5d444d..16af1209b4a 100644 --- a/src/runtime/synctest.go +++ b/src/runtime/synctest.go @@ -363,6 +363,13 @@ type specialBubble struct { bubbleid uint64 } +// Keep these in sync with internal/synctest. +const ( + bubbleAssocUnbubbled = iota // not associated with any bubble + bubbleAssocCurrentBubble // associated with the current bubble + bubbleAssocOtherBubble // associated with a different bubble +) + // getOrSetBubbleSpecial checks the special record for p's bubble membership. // // If add is true and p is not associated with any bubble, @@ -371,10 +378,12 @@ type specialBubble struct { // It returns ok==true if p is associated with bubbleid // (including if a new association was added), // and ok==false if not. -func getOrSetBubbleSpecial(p unsafe.Pointer, bubbleid uint64, add bool) (ok bool) { +func getOrSetBubbleSpecial(p unsafe.Pointer, bubbleid uint64, add bool) (assoc int) { span := spanOfHeap(uintptr(p)) if span == nil { - throw("getOrSetBubbleSpecial on invalid pointer") + // This is probably a package var. + // We can't attach a special to it, so always consider it unbubbled. + return bubbleAssocUnbubbled } // Ensure that the span is swept. @@ -393,7 +402,11 @@ func getOrSetBubbleSpecial(p unsafe.Pointer, bubbleid uint64, add bool) (ok bool // p is already associated with a bubble. // Return true iff it's the same bubble. s := (*specialBubble)((unsafe.Pointer)(*iter)) - ok = s.bubbleid == bubbleid + if s.bubbleid == bubbleid { + assoc = bubbleAssocCurrentBubble + } else { + assoc = bubbleAssocOtherBubble + } } else if add { // p is not associated with a bubble, // and we've been asked to add an association. @@ -404,23 +417,23 @@ func getOrSetBubbleSpecial(p unsafe.Pointer, bubbleid uint64, add bool) (ok bool s.special.next = *iter *iter = (*special)(unsafe.Pointer(s)) spanHasSpecials(span) - ok = true + assoc = bubbleAssocCurrentBubble } else { // p is not associated with a bubble. - ok = false + assoc = bubbleAssocUnbubbled } unlock(&span.speciallock) releasem(mp) - return ok + return assoc } // synctest_associate associates p with the current bubble. // It returns false if p is already associated with a different bubble. // //go:linkname synctest_associate internal/synctest.associate -func synctest_associate(p unsafe.Pointer) (ok bool) { +func synctest_associate(p unsafe.Pointer) int { return getOrSetBubbleSpecial(p, getg().bubble.id, true) } @@ -435,5 +448,5 @@ func synctest_disassociate(p unsafe.Pointer) { // //go:linkname synctest_isAssociated internal/synctest.isAssociated func synctest_isAssociated(p unsafe.Pointer) bool { - return getOrSetBubbleSpecial(p, getg().bubble.id, false) + return getOrSetBubbleSpecial(p, getg().bubble.id, false) == bubbleAssocCurrentBubble } diff --git a/src/runtime/synctest_test.go b/src/runtime/synctest_test.go index 0fdd032fc9d..1059d629d3b 100644 --- a/src/runtime/synctest_test.go +++ b/src/runtime/synctest_test.go @@ -5,6 +5,8 @@ package runtime_test import ( + "internal/synctest" + "runtime" "testing" ) @@ -15,3 +17,13 @@ func TestSynctest(t *testing.T) { t.Fatalf("output:\n%s\n\nwanted:\n%s", output, want) } } + +// TestSynctestAssocConsts verifies that constants defined +// in both runtime and internal/synctest match. +func TestSynctestAssocConsts(t *testing.T) { + if runtime.BubbleAssocUnbubbled != synctest.Unbubbled || + runtime.BubbleAssocCurrentBubble != synctest.CurrentBubble || + runtime.BubbleAssocOtherBubble != synctest.OtherBubble { + t.Fatal("mismatch: runtime.BubbleAssoc? != synctest.*") + } +} diff --git a/src/runtime/testdata/testprog/gomaxprocs.go b/src/runtime/testdata/testprog/gomaxprocs.go index 915e3c4dad4..99bc9f1dbb3 100644 --- a/src/runtime/testdata/testprog/gomaxprocs.go +++ b/src/runtime/testdata/testprog/gomaxprocs.go @@ -133,6 +133,20 @@ func UpdateGOMAXPROCS() { mustSetCPUMax(path, 200000) mustNotChangeMaxProcs(3) + // Re-enable updates. Change is immediately visible. + runtime.SetDefaultGOMAXPROCS() + procs = runtime.GOMAXPROCS(0) + println("GOMAXPROCS:", procs) + if procs != 2 { + panic(fmt.Sprintf("GOMAXPROCS got %d want %d", procs, 2)) + } + + // Setting GOMAXPROCS to itself also disables updates, despite not + // changing the value itself. + runtime.GOMAXPROCS(runtime.GOMAXPROCS(0)) + mustSetCPUMax(path, 300000) + mustNotChangeMaxProcs(2) + println("OK") } diff --git a/src/runtime/trace.go b/src/runtime/trace.go index 139cbba6a9f..b92e7b4e8e3 100644 --- a/src/runtime/trace.go +++ b/src/runtime/trace.go @@ -331,7 +331,7 @@ func StopTrace() { // // traceAdvanceSema must not be held. // -// traceAdvance is called by golang.org/x/exp/trace using linkname. +// traceAdvance is called by runtime/trace and golang.org/x/exp/trace using linkname. // //go:linkname traceAdvance func traceAdvance(stopTrace bool) { @@ -376,7 +376,7 @@ func traceAdvance(stopTrace bool) { me := getg().m.curg // We don't have to handle this G status transition because we // already eliminated ourselves from consideration above. - casGToWaitingForGC(me, _Grunning, waitReasonTraceGoroutineStatus) + casGToWaitingForSuspendG(me, _Grunning, waitReasonTraceGoroutineStatus) // We need to suspend and take ownership of the G to safely read its // goid. Note that we can't actually emit the event at this point // because we might stop the G in a window where it's unsafe to write @@ -956,7 +956,7 @@ func traceReader() *g { // scheduled and should be. Callers should first check that // (traceEnabled() || traceShuttingDown()) is true. func traceReaderAvailable() *g { - // There are three conditions under which we definitely want to schedule + // There are two conditions under which we definitely want to schedule // the reader: // - The reader is lagging behind in finishing off the last generation. // In this case, trace buffers could even be empty, but the trace @@ -965,12 +965,10 @@ func traceReaderAvailable() *g { // - The reader has pending work to process for it's reader generation // (assuming readerGen is not lagging behind). Note that we also want // to be careful *not* to schedule the reader if there's no work to do. - // - The trace is shutting down. The trace stopper blocks on the reader - // to finish, much like trace advancement. // // We also want to be careful not to schedule the reader if there's no // reason to. - if trace.flushedGen.Load() == trace.readerGen.Load() || trace.workAvailable.Load() || trace.shutdown.Load() { + if trace.flushedGen.Load() == trace.readerGen.Load() || trace.workAvailable.Load() { return trace.reader.Load() } return nil diff --git a/src/runtime/tracestatus.go b/src/runtime/tracestatus.go index 4dabc8e562f..03ec81fc026 100644 --- a/src/runtime/tracestatus.go +++ b/src/runtime/tracestatus.go @@ -126,11 +126,12 @@ func goStatusToTraceGoStatus(status uint32, wr waitReason) tracev2.GoStatus { // There are a number of cases where a G might end up in // _Gwaiting but it's actually running in a non-preemptive // state but needs to present itself as preempted to the - // garbage collector. In these cases, we're not going to - // emit an event, and we want these goroutines to appear in - // the final trace as if they're running, not blocked. + // garbage collector and traceAdvance (via suspendG). In + // these cases, we're not going to emit an event, and we + // want these goroutines to appear in the final trace as + // if they're running, not blocked. tgs = tracev2.GoWaiting - if status == _Gwaiting && wr.isWaitingForGC() { + if status == _Gwaiting && wr.isWaitingForSuspendG() { tgs = tracev2.GoRunning } case _Gdead: diff --git a/src/runtime/tracetime.go b/src/runtime/tracetime.go index 7ffab79badb..8be5c3d1306 100644 --- a/src/runtime/tracetime.go +++ b/src/runtime/tracetime.go @@ -51,7 +51,7 @@ type traceTime uint64 // nosplit because it's called from exitsyscall and various trace writing functions, // which are nosplit. // -// traceClockNow is called by golang.org/x/exp/trace using linkname. +// traceClockNow is called by runtime/trace and golang.org/x/exp/trace using linkname. // //go:linkname traceClockNow //go:nosplit diff --git a/src/sync/waitgroup.go b/src/sync/waitgroup.go index efc63be0990..0bd618a241b 100644 --- a/src/sync/waitgroup.go +++ b/src/sync/waitgroup.go @@ -83,13 +83,17 @@ func (wg *WaitGroup) Add(delta int) { race.Disable() defer race.Enable() } + bubbled := false if synctest.IsInBubble() { // If Add is called from within a bubble, then all Add calls must be made // from the same bubble. - if !synctest.Associate(wg) { + switch synctest.Associate(wg) { + case synctest.Unbubbled: + case synctest.OtherBubble: // wg is already associated with a different bubble. fatal("sync: WaitGroup.Add called from multiple synctest bubbles") - } else { + case synctest.CurrentBubble: + bubbled = true state := wg.state.Or(waitGroupBubbleFlag) if state != 0 && state&waitGroupBubbleFlag == 0 { // Add has been called from outside this bubble. @@ -98,7 +102,7 @@ func (wg *WaitGroup) Add(delta int) { } } state := wg.state.Add(uint64(delta) << 32) - if state&waitGroupBubbleFlag != 0 && !synctest.IsInBubble() { + if state&waitGroupBubbleFlag != 0 && !bubbled { // Add has been called from within a synctest bubble (and we aren't in one). fatal("sync: WaitGroup.Add called from inside and outside synctest bubble") } @@ -116,7 +120,7 @@ func (wg *WaitGroup) Add(delta int) { if w != 0 && delta > 0 && v == int32(delta) { panic("sync: WaitGroup misuse: Add called concurrently with Wait") } - if v == 0 && state&waitGroupBubbleFlag != 0 { + if v == 0 && bubbled { // Disassociate the WaitGroup from its bubble. synctest.Disassociate(wg) if w == 0 { diff --git a/src/testing/synctest/helper_test.go b/src/testing/synctest/helper_test.go new file mode 100644 index 00000000000..7547d3eac69 --- /dev/null +++ b/src/testing/synctest/helper_test.go @@ -0,0 +1,15 @@ +// 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 synctest_test + +import "testing" + +// helperLog is a t.Helper which logs. +// Since it is a helper, the log prefix should contain +// the caller's file, not helper_test.go. +func helperLog(t *testing.T, s string) { + t.Helper() + t.Log(s) +} diff --git a/src/testing/synctest/synctest.go b/src/testing/synctest/synctest.go index 57a6fbfbd61..0911519aab4 100644 --- a/src/testing/synctest/synctest.go +++ b/src/testing/synctest/synctest.go @@ -93,6 +93,11 @@ // A [sync.WaitGroup] becomes associated with a bubble on the first // call to Add or Go. Once a WaitGroup is associated with a bubble, // calling Add or Go from outside that bubble is a fatal error. +// (As a technical limitation, a WaitGroup defined as a package +// variable, such as "var wg sync.WaitGroup", cannot be associated +// with a bubble and operations on it may not be durably blocking. +// This limitation does not apply to a *WaitGroup stored in a +// package variable, such as "var wg = new(sync.WaitGroup)".) // // [sync.Cond.Wait] is durably blocking. Waking a goroutine in a bubble // blocked on Cond.Wait from outside the bubble is a fatal error. diff --git a/src/testing/synctest/synctest_test.go b/src/testing/synctest/synctest_test.go index 822fd6fe1c7..9c731787509 100644 --- a/src/testing/synctest/synctest_test.go +++ b/src/testing/synctest/synctest_test.go @@ -140,6 +140,18 @@ func TestRun(t *testing.T) { }) } +func TestHelper(t *testing.T) { + runTest(t, []string{"-test.v"}, func() { + synctest.Test(t, func(t *testing.T) { + helperLog(t, "log in helper") + }) + }, `^=== RUN TestHelper + synctest_test.go:.* log in helper +--- PASS: TestHelper.* +PASS +$`) +} + func wantPanic(t *testing.T, want string) { if e := recover(); e != nil { if got := fmt.Sprint(e); got != want { diff --git a/src/testing/testing.go b/src/testing/testing.go index b2d4c0c938a..3475bfca4a6 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -1261,6 +1261,9 @@ func (c *common) Skipped() bool { // When printing file and line information, that function will be skipped. // Helper may be called simultaneously from multiple goroutines. func (c *common) Helper() { + if c.isSynctest { + c = c.parent + } c.mu.Lock() defer c.mu.Unlock() if c.helperPCs == nil {