From c498b9fa2223244c57dfb0656bf89f7513c8f26a Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Fri, 31 Jul 2026 13:15:44 -0400 Subject: [PATCH 01/13] [release-branch.go1.27] all: update x/net Pull in the fix of x/net/dns/dnsmessage panic (#79795). Fixes #79795. Change-Id: I3e9a6c7f2e934bf174bbc9e64eedc595adcfb39b Reviewed-on: https://go-review.googlesource.com/c/go/+/808660 Reviewed-by: Nicholas Husin LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Nicholas Husin --- src/go.mod | 2 +- src/go.sum | 4 +-- .../golang.org/x/net/dns/dnsmessage/svcb.go | 25 +++++++++++-------- src/vendor/modules.txt | 2 +- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/go.mod b/src/go.mod index bb6abc93792f39..111f99e60629e1 100644 --- a/src/go.mod +++ b/src/go.mod @@ -4,7 +4,7 @@ go 1.27 require ( golang.org/x/crypto v0.52.1-0.20260526024921-9beb694f9766 - golang.org/x/net v0.55.1-0.20260526154343-657eb1317b5d + golang.org/x/net v0.55.1-0.20260731170536-c1d18010be90 ) require ( diff --git a/src/go.sum b/src/go.sum index ab34844da17757..20681d37cbe14c 100644 --- a/src/go.sum +++ b/src/go.sum @@ -1,7 +1,7 @@ golang.org/x/crypto v0.52.1-0.20260526024921-9beb694f9766 h1:ABD+jVg0H4Hwu2sGcUtKeb3T8mlS+jS3uWrkTAPcXjs= golang.org/x/crypto v0.52.1-0.20260526024921-9beb694f9766/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= -golang.org/x/net v0.55.1-0.20260526154343-657eb1317b5d h1:G6GZDsxGyGK2SxMEqnPJfBWRKGCNpWheup5btZYkYpw= -golang.org/x/net v0.55.1-0.20260526154343-657eb1317b5d/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= +golang.org/x/net v0.55.1-0.20260731170536-c1d18010be90 h1:v8JYc8J0G5tszb4H3rnr1UU9fjQFKQLtPr8U6HjvVqI= +golang.org/x/net v0.55.1-0.20260731170536-c1d18010be90/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= diff --git a/src/vendor/golang.org/x/net/dns/dnsmessage/svcb.go b/src/vendor/golang.org/x/net/dns/dnsmessage/svcb.go index 4840516a7f88e5..252401b3cecd0c 100644 --- a/src/vendor/golang.org/x/net/dns/dnsmessage/svcb.go +++ b/src/vendor/golang.org/x/net/dns/dnsmessage/svcb.go @@ -205,7 +205,7 @@ func unpackSVCBResource(msg []byte, off int, length uint16) (SVCBResource, error off = paramsOff var previousKey uint16 for off < bodyEnd { - var key, len uint16 + var key, size uint16 if key, off, err = unpackUint16(msg, off); err != nil { return SVCBResource{}, &nestedError{"Params key", err} } @@ -214,14 +214,14 @@ func unpackSVCBResource(msg []byte, off int, length uint16) (SVCBResource, error // consider the RR malformed if the SvcParamKeys are not in strictly increasing numeric order return SVCBResource{}, &nestedError{"Params", errParamOutOfOrder} } - if len, off, err = unpackUint16(msg, off); err != nil { + if size, off, err = unpackUint16(msg, off); err != nil { return SVCBResource{}, &nestedError{"Params value length", err} } - if off+int(len) > bodyEnd { + if off+int(size) > bodyEnd { return SVCBResource{}, errResourceLen } - totalValueLen += len - off += int(len) + totalValueLen += size + off += int(size) n++ } if off != bodyEnd { @@ -236,20 +236,23 @@ func unpackSVCBResource(msg []byte, off int, length uint16) (SVCBResource, error off = paramsOff for i := 0; i < n; i++ { p := &r.Params[i] - var key, len uint16 + var key, size uint16 if key, off, err = unpackUint16(msg, off); err != nil { return SVCBResource{}, &nestedError{"param key", err} } p.Key = SVCParamKey(key) - if len, off, err = unpackUint16(msg, off); err != nil { + if size, off, err = unpackUint16(msg, off); err != nil { return SVCBResource{}, &nestedError{"param length", err} } - if copy(valuesBuf, msg[off:off+int(len)]) != int(len) { + if len(msg[off:]) < int(size) { return SVCBResource{}, &nestedError{"param value", errCalcLen} } - p.Value = valuesBuf[:len:len] - valuesBuf = valuesBuf[len:] - off += int(len) + if copy(valuesBuf, msg[off:][:int(size)]) != int(size) { + return SVCBResource{}, &nestedError{"param value", errCalcLen} + } + p.Value = valuesBuf[:size:size] + valuesBuf = valuesBuf[size:] + off += int(size) } return r, nil diff --git a/src/vendor/modules.txt b/src/vendor/modules.txt index 54fcbab6a221c0..b34527fdb12010 100644 --- a/src/vendor/modules.txt +++ b/src/vendor/modules.txt @@ -7,7 +7,7 @@ golang.org/x/crypto/cryptobyte/asn1 golang.org/x/crypto/hkdf golang.org/x/crypto/internal/alias golang.org/x/crypto/internal/poly1305 -# golang.org/x/net v0.55.1-0.20260526154343-657eb1317b5d +# golang.org/x/net v0.55.1-0.20260731170536-c1d18010be90 ## explicit; go 1.25.0 golang.org/x/net/dns/dnsmessage golang.org/x/net/http/httpguts From 95d89e6588acffafba2025318d9921f72af650c6 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 31 Jul 2026 15:44:04 +0000 Subject: [PATCH 02/13] [release-branch.go1.27] cmd/compile: use pointer-size alignment for stack object records This fixes mips64 under qemu-user. And actually makes amd64/arm64 binaries smaller, which was the opposite of what I expected. Fixes #80668 Change-Id: I501f0557b12c82987a7ef60e3f91625690686379 Reviewed-on: https://go-review.googlesource.com/c/go/+/808560 Reviewed-by: Jorropo LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Russ Cox Reviewed-by: Cherry Mui Reviewed-by: Nicholas Husin (cherry picked from commit 5d29d80b6c9960c3fc39e14e3e8b9a0f52041fed) Reviewed-on: https://go-review.googlesource.com/c/go/+/808800 Reviewed-by: David Chase TryBot-Bypass: Cherry Mui Reviewed-by: Ian Lance Taylor Reviewed-by: Michael Pratt --- src/cmd/compile/internal/liveness/plive.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/compile/internal/liveness/plive.go b/src/cmd/compile/internal/liveness/plive.go index 198f6418d6363a..02643fa352efde 100644 --- a/src/cmd/compile/internal/liveness/plive.go +++ b/src/cmd/compile/internal/liveness/plive.go @@ -1478,7 +1478,7 @@ func (lv *Liveness) emitStackObjects() *obj.LSym { // Format must match runtime/stack.go:stackObjectRecord. x := base.Ctxt.Lookup(lv.fn.LSym.Name + ".stkobj") x.Set(obj.AttrContentAddressable, true) - x.Align = 4 + x.Align = int16(types.PtrSize) // see https://go.dev/issue/80668 lv.fn.LSym.Func().StackObjects = x off := 0 off = objw.Uintptr(x, off, uint64(len(vars))) From 2cd21e94dc376d0e5088788d57f483d3a4fb4786 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Mon, 3 Aug 2026 18:35:46 -0400 Subject: [PATCH 03/13] [release-branch.go1.27] runtime/cgo: add back indirections to C functions CL 746300 refactored runtime/cgo and removed a layer of indirection to the C functions. This causes Go text section directly references the addresses of C functions. In certain build settings, this can cause dynamic relocations in the text segment, and may lead to build failures, e.g. when linking with -Bsymbolic-functions in c-shared build mode on Linux. This CL adds back the indirections, so Go text does not reference C functions directly, instead, through a Go variable. On Windows, as of CL 746300 these functions are not defined in C, so the variables remain nil. Fixes #80632. Change-Id: I261d23fdf6361f84d8ee10e8b850f5c78593e0b2 Reviewed-on: https://go-review.googlesource.com/c/go/+/809940 Reviewed-by: Michael Pratt Reviewed-by: Quim Muntal LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com (cherry picked from commit 29a84921605d7049be6e81778291cad968eb22b6) Reviewed-on: https://go-review.googlesource.com/c/go/+/810300 --- .../cgo/internal/testcshared/cshared_test.go | 26 +++++++++ src/runtime/cgo/callbacks.go | 32 ---------- src/runtime/cgo/callbacks_unix.go | 58 +++++++++++++++++++ src/runtime/cgo/gcc_libinit_unix.c | 4 -- src/runtime/cgo/gcc_libinit_windows.c | 5 -- src/runtime/cgo/gcc_unix.c | 2 - src/runtime/cgo/pthread_unix.c | 4 -- src/runtime/cgo/windows.go | 19 +++++- 8 files changed, 102 insertions(+), 48 deletions(-) create mode 100644 src/runtime/cgo/callbacks_unix.go diff --git a/src/cmd/cgo/internal/testcshared/cshared_test.go b/src/cmd/cgo/internal/testcshared/cshared_test.go index 144cf22e6c78cb..abaa75587eb3a1 100644 --- a/src/cmd/cgo/internal/testcshared/cshared_test.go +++ b/src/cmd/cgo/internal/testcshared/cshared_test.go @@ -942,3 +942,29 @@ func TestIssue68411(t *testing.T) { t.Error("missing functions") } } + +func TestSymbolicFunctions(t *testing.T) { + // Test that we can build a c-shared library with -Wl,-Bsymbolic-functions, + // see issue 80632. + globalSkip(t) + testenv.MustHaveGoBuild(t) + testenv.MustHaveCGO(t) + testenv.MustHaveBuildMode(t, "c-shared") + if GOOS != "linux" { + t.Skip("Skipping on non-Linux OS") + } + + t.Parallel() + + tmpdir := t.TempDir() + libname := filepath.Join(tmpdir, "libbsymbolic.a") + + run(t, + nil, + "go", "build", + "-buildmode=c-shared", + "-installsuffix", "testcshared", + "-ldflags=-extldflags=-Wl,-Bsymbolic-functions", + "-o", libname, "./libgo", + ) +} diff --git a/src/runtime/cgo/callbacks.go b/src/runtime/cgo/callbacks.go index 41907b2d421060..d782a6b9d5bea2 100644 --- a/src/runtime/cgo/callbacks.go +++ b/src/runtime/cgo/callbacks.go @@ -47,24 +47,6 @@ func _cgo_panic(a *struct{ cstr *byte }) { _runtime_cgo_panic_internal(a.cstr) } -//go:cgo_import_static _cgo_init -//go:linkname _cgo_init _cgo_init -var _cgo_init unsafe.Pointer - -//go:cgo_import_static _cgo_thread_start -//go:linkname _cgo_thread_start _cgo_thread_start -var _cgo_thread_start unsafe.Pointer - -// Creates a new system thread without updating any Go state. -// -// This method is invoked during shared library loading to create a new OS -// thread to perform the runtime initialization. This method is similar to -// x_cgo_thread_start except that it doesn't update any Go state. - -//go:cgo_import_static _cgo_sys_thread_create -//go:linkname _cgo_sys_thread_create _cgo_sys_thread_create -var _cgo_sys_thread_create unsafe.Pointer - // Indicates whether a dummy thread key has been created or not. // // When calling go exported function from C, we register a destructor @@ -92,13 +74,6 @@ func set_crosscall2() //go:linkname _set_crosscall2 runtime.set_crosscall2 var _set_crosscall2 = set_crosscall2 -// Store the g into the thread-specific value. -// So that pthread_key_destructor will dropm when the thread is exiting. - -//go:cgo_import_static _cgo_bindm -//go:linkname _cgo_bindm _cgo_bindm -var _cgo_bindm unsafe.Pointer - // Notifies that the runtime has been initialized. // // We currently block at every CGO entry point (via _cgo_wait_runtime_init_done) @@ -150,10 +125,3 @@ var _cgo_yield unsafe.Pointer //go:cgo_export_static _cgo_topofstack //go:cgo_export_dynamic _cgo_topofstack - -// x_cgo_getstackbound gets the thread's C stack size and -// set the G's stack bound based on the stack size. - -//go:cgo_import_static _cgo_getstackbound -//go:linkname _cgo_getstackbound _cgo_getstackbound -var _cgo_getstackbound unsafe.Pointer diff --git a/src/runtime/cgo/callbacks_unix.go b/src/runtime/cgo/callbacks_unix.go new file mode 100644 index 00000000000000..311f549e348a2e --- /dev/null +++ b/src/runtime/cgo/callbacks_unix.go @@ -0,0 +1,58 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build unix + +package cgo + +import _ "unsafe" // for go:linkname + +// We need to reference the addresses of the C functions in Go code, +// but we don't want to reference them directly in text. Otherwise it +// may cause dynamic relocations in the text segment, or even build +// failures in some settings, e.g. linking with -Bsymbolic-functions. +// Reference them via variables with an extra level of indirection to +// avoid that. + +//go:cgo_import_static x_cgo_init +//go:linkname x_cgo_init x_cgo_init +//go:linkname _cgo_init _cgo_init +var x_cgo_init byte +var _cgo_init = &x_cgo_init + +//go:cgo_import_static x_cgo_thread_start +//go:linkname x_cgo_thread_start x_cgo_thread_start +//go:linkname _cgo_thread_start _cgo_thread_start +var x_cgo_thread_start byte +var _cgo_thread_start = &x_cgo_thread_start + +// Creates a new system thread without updating any Go state. +// +// This method is invoked during shared library loading to create a new OS +// thread to perform the runtime initialization. This method is similar to +// x_cgo_thread_start except that it doesn't update any Go state. + +//go:cgo_import_static x_cgo_sys_thread_create +//go:linkname x_cgo_sys_thread_create x_cgo_sys_thread_create +//go:linkname _cgo_sys_thread_create _cgo_sys_thread_create +var x_cgo_sys_thread_create byte +var _cgo_sys_thread_create = &x_cgo_sys_thread_create + +// Store the g into the thread-specific value. +// So that pthread_key_destructor will dropm when the thread is exiting. + +//go:cgo_import_static x_cgo_bindm +//go:linkname x_cgo_bindm x_cgo_bindm +//go:linkname _cgo_bindm _cgo_bindm +var x_cgo_bindm byte +var _cgo_bindm = &x_cgo_bindm + +// x_cgo_getstackbound gets the thread's C stack size and +// set the G's stack bound based on the stack size. + +//go:cgo_import_static x_cgo_getstackbound +//go:linkname x_cgo_getstackbound x_cgo_getstackbound +//go:linkname _cgo_getstackbound _cgo_getstackbound +var x_cgo_getstackbound byte +var _cgo_getstackbound = &x_cgo_getstackbound diff --git a/src/runtime/cgo/gcc_libinit_unix.c b/src/runtime/cgo/gcc_libinit_unix.c index 58cd32b8844925..8024fa8dc2f730 100644 --- a/src/runtime/cgo/gcc_libinit_unix.c +++ b/src/runtime/cgo/gcc_libinit_unix.c @@ -93,8 +93,6 @@ void x_cgo_bindm(void* g) { pthread_setspecific(pthread_g, g); } -void (* _cgo_bindm)(void*) = x_cgo_bindm; - void x_cgo_notify_runtime_init_done(void* dummy __attribute__ ((unused))) { pthread_mutex_lock(&runtime_init_mu); @@ -196,5 +194,3 @@ x_cgo_thread_start(ThreadStart *arg) _cgo_sys_thread_start(ts); /* OS-dependent half */ } - -void (* _cgo_thread_start)(ThreadStart*) = x_cgo_thread_start; diff --git a/src/runtime/cgo/gcc_libinit_windows.c b/src/runtime/cgo/gcc_libinit_windows.c index ed30878c3c7f66..606fc1865aae0c 100644 --- a/src/runtime/cgo/gcc_libinit_windows.c +++ b/src/runtime/cgo/gcc_libinit_windows.c @@ -48,11 +48,6 @@ static int runtime_init_done; // No pthreads on Windows, these are always zero. uintptr_t x_cgo_pthread_key_created; void (*x_crosscall2_ptr)(void (*fn)(void *), void *, int, size_t); -void (*_cgo_init)(G*, void (*)(void*), void **, void **); -void (*_cgo_thread_start)(ThreadStart *); -void (*_cgo_sys_thread_create)(void* (*func)(void*)); -void (*_cgo_getstackbound)(uintptr[2]); -void (*_cgo_bindm)(void*); // Pre-initialize the runtime synchronization objects void diff --git a/src/runtime/cgo/gcc_unix.c b/src/runtime/cgo/gcc_unix.c index bb3d7001bcb322..f606675339911c 100644 --- a/src/runtime/cgo/gcc_unix.c +++ b/src/runtime/cgo/gcc_unix.c @@ -44,8 +44,6 @@ x_cgo_init(G *g, void (*setg)(void*), void **tlsg, void **tlsbase) } } -void (* _cgo_init)(G*, void (*)(void*), void **, void **) = x_cgo_init; - void* threadentry(void *v) { diff --git a/src/runtime/cgo/pthread_unix.c b/src/runtime/cgo/pthread_unix.c index 438c599f1cce3f..d38045fdaa3060 100644 --- a/src/runtime/cgo/pthread_unix.c +++ b/src/runtime/cgo/pthread_unix.c @@ -73,8 +73,6 @@ x_cgo_sys_thread_create(void* (*func)(void*)) { } } -void (* _cgo_sys_thread_create)(void* (*func)(void*)) = x_cgo_sys_thread_create; - void x_cgo_getstackbound(uintptr bounds[2]) { @@ -117,8 +115,6 @@ x_cgo_getstackbound(uintptr bounds[2]) _cgo_tsan_release(); } -void (* _cgo_getstackbound)(uintptr[2]) = x_cgo_getstackbound; - // _cgo_try_pthread_create retries pthread_create if it fails with EAGAIN. int _cgo_try_pthread_create(pthread_t* thread, const pthread_attr_t* attr, void* (*pfn)(void*), void* arg) { diff --git a/src/runtime/cgo/windows.go b/src/runtime/cgo/windows.go index 7ba61753dffda2..5160066ec037cf 100644 --- a/src/runtime/cgo/windows.go +++ b/src/runtime/cgo/windows.go @@ -6,7 +6,7 @@ package cgo -import _ "unsafe" // for go:linkname +import "unsafe" // _cgo_stub_export is only used to ensure there's at least one symbol // in the .def file passed to the external linker. @@ -20,3 +20,20 @@ import _ "unsafe" // for go:linkname //go:cgo_export_static _cgo_stub_export //go:linkname _cgo_stub_export _cgo_stub_export var _cgo_stub_export uintptr + +// No pthreads on Windows, these are always zero. + +//go:linkname _cgo_init _cgo_init +var _cgo_init unsafe.Pointer + +//go:linkname _cgo_thread_start _cgo_thread_start +var _cgo_thread_start unsafe.Pointer + +//go:linkname _cgo_sys_thread_create _cgo_sys_thread_create +var _cgo_sys_thread_create unsafe.Pointer + +//go:linkname _cgo_bindm _cgo_bindm +var _cgo_bindm unsafe.Pointer + +//go:linkname _cgo_getstackbound _cgo_getstackbound +var _cgo_getstackbound unsafe.Pointer From ea9653fb9caf537360a956a22a3cbe2aff3093c7 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 12 May 2026 08:47:15 -0700 Subject: [PATCH 04/13] [release-branch.go1.27] net/http: enable HTTP/2 on http2.ConfigureServer The modern way to enable/disable HTTP protocols is via the Server.Protocols field. Fix an unintentional change in behavior when not using Server.Protocols: - User sets Server.TLSNextProto to a non-nil map which does not contain an "h2" key. - User calls http2.ConfigureServer to add HTTP/2 support to the Server. Historically, ConfigureServer would set the "h2" key in TLSNextProto. After the move of HTTP/2 into std, it no longer changes TLSNextProto. This results in the net/http Server interpreting the missing "h2" key in the above scenario as the user intentionally disabling HTTP/2. Fix this by initializing the server's HTTP/2 support when ConfigureServer is called, which sets TLSNextProto's "h2" key to a dummy value. Fixes #80482 Change-Id: I497881423bc76deed2a59e355b59672f6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/803740 Reviewed-by: Nicholas Husin Reviewed-by: Nicholas Husin LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com (cherry picked from commit 857fa809d2d1b613f0a18f2dcf93e5963ac2d239) Reviewed-on: https://go-review.googlesource.com/c/go/+/810540 Reviewed-by: Carlos Amedee Reviewed-by: Damien Neil Auto-Submit: Damien Neil --- src/net/http/http2.go | 1 + src/net/http/transport_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/net/http/http2.go b/src/net/http/http2.go index 301aff7a6f27af..3c886a83819984 100644 --- a/src/net/http/http2.go +++ b/src/net/http/http2.go @@ -88,6 +88,7 @@ func (s *Server) setHTTP2Config(conf http2ExternalServerConfig) { } s.h2Config = conf s.h2Config.ServeConnFunc(s.serveHTTP2Conn) + s.configureHTTP2() } func (s *Server) serveHTTP2Conn(ctx context.Context, nc net.Conn, h Handler, sawClientPreface bool, upgradeReq *Request, settings []byte) { diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 5f3343a77de276..c70b5855184b2d 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -7734,6 +7734,19 @@ func TestTransportServerProtocols(t *testing.T) { srv.Protocols.SetHTTP2(true) }, want: "error", + }, { + // https://go.dev/issue/80482 + name: "ConfigureServer updates TLSNextProto", + scheme: "https", + transport: func(tr *Transport) { + tr.Protocols = &Protocols{} + tr.Protocols.SetHTTP2(true) + }, + server: func(srv *Server) { + srv.TLSNextProto = map[string]func(*http.Server, *tls.Conn, http.Handler){} + testHTTP2ConfigureServer(srv) + }, + want: "HTTP/2.0", }} { t.Run(test.name, func(t *testing.T) { // We don't use httptest here because it makes its own decisions @@ -7796,6 +7809,24 @@ func TestTransportServerProtocols(t *testing.T) { } } +// testHTTP2ConfigureServer is a stripped-down version of http2.ConfigureServer. +func testHTTP2ConfigureServer(s *Server) { + s.Serve(testHTTP2ServerConfig{}) +} + +type testHTTP2ServerConfig struct { + net.Listener +} + +func (testHTTP2ServerConfig) HTTP2Config() HTTP2Config { + return HTTP2Config{} +} +func (testHTTP2ServerConfig) IdleTimeout() time.Duration { + return 0 +} +func (testHTTP2ServerConfig) ServeConnFunc(func(ctx context.Context, nc net.Conn, h Handler, sawClientPreface bool, upgradeReq *Request, settings []byte)) { +} + func TestIssue61474(t *testing.T) { run(t, testIssue61474, []testMode{http2Mode}) } From 0d586ce4c23f1e5e025ae354c833a5bb39d00416 Mon Sep 17 00:00:00 2001 From: Nick Ripley Date: Fri, 31 Jul 2026 12:59:40 -0400 Subject: [PATCH 05/13] [release-branch.go1.27] runtime: fix frame pointer adjustment around injected calls This CL addresses two bugs related to moving goroutine stacks during injected function calls. The runtime injects calls during asynchronous preemption, when handling signals that get turned into panics (sigpanic), and for calls injected by a debugger. If a function triggers a sigpanic, and there is no way to resume execution in the function (such as through a deferred function), then that function's call frame is skipped during stack copying. The reasoning is that its locals, arguments, etc are dead. However, the call frame might contain a frame pointer. We can visit that frame pointer with the execution tracer or block/mutex profilers. If we visit that frame pointer after stack movement, it can point into the old stack. Following it will crash. This CL fixes this by adjusting the frame pointer if there is one. But it still skips the rest of the work, which is unnecessary. For arm64, there is an additonal frame pointer adjustment we're missing. Injected call frames are placed 16 bytes below the stack pointer at the point of call injection. This gap is needed both to avoid clobbering the frame pointer saved below the original function's call frame, and to have space to save the link register so it can be restored when the injected call returns. Normally when function A calls function B, the frame pointer saved below function A's frame is fixed when adjusting function B's frame. But because of the gap in the case of injection, the frame pointer saved by the original function isn't inside any call frame. We need to fix it when visiting the injected call frame. The regression test uses stackPoisonCopy so that the old stack is filled with garbage and frame pointer unwinding will reliably crash. Otherwise we'd only see a crash if something else happens to reuse the old stack space. For #73664 Change-Id: I600d7942521e90852c67e379679b07e96a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/730200 Reviewed-by: Mark Freeman LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Cherry Mui (cherry picked from commit 40fd497af5a5eb360e63393d87b12622c6fc788e) Reviewed-on: https://go-review.googlesource.com/c/go/+/809780 Auto-Submit: Nick Ripley --- src/runtime/callers_test.go | 24 +++++++++++++++++++ src/runtime/export_test.go | 10 ++++++++ src/runtime/stack.go | 47 ++++++++++++++++++++++++++++++------- src/runtime/traceback.go | 6 ++++- 4 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/runtime/callers_test.go b/src/runtime/callers_test.go index 9429442fc08d39..5a7369be2d5ce2 100644 --- a/src/runtime/callers_test.go +++ b/src/runtime/callers_test.go @@ -487,3 +487,27 @@ func TestFPUnwindAfterRecovery(t *testing.T) { }() panic(1) } + +//go:noinline +func deref() int { + var i *int + runtime.KeepAlive(&i) + return *i +} + +func TestFPUnwindStackGrowthAfterRecovery(t *testing.T) { + if !runtime.FramePointerEnabled { + t.Skip("frame pointers not supported for this architecture") + } + state := runtime.StackPoisonCopy() + defer state.Restore() + defer func() { + if recover() == nil { + t.Fatal("did not recover from panic") + } + growStack(nil) + var pcs [32]uintptr + runtime.FPCallers(pcs[:]) + }() + deref() +} diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index c0f1d979061948..00a3c095059c8f 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -453,6 +453,16 @@ func ShrinkStackAndVerifyFramePointers() { FPCallers(make([]uintptr, 1024)) } +type StackPoisonCopyRestore int + +func (s StackPoisonCopyRestore) Restore() { stackPoisonCopy = int(s) } + +func StackPoisonCopy() StackPoisonCopyRestore { + before := stackPoisonCopy + stackPoisonCopy = 1 + return StackPoisonCopyRestore(before) +} + // BlockOnSystemStack switches to the system stack, prints "x\n" to // stderr, and blocks in a stack containing // "runtime.blockOnSystemStackInternal". diff --git a/src/runtime/stack.go b/src/runtime/stack.go index 6f89cc142c39f0..838090067a1769 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -699,15 +699,6 @@ func adjustpointers(scanp unsafe.Pointer, bv *bitvector, adjinfo *adjustinfo, f // Note: the argument/return area is adjusted by the callee. func adjustframe(frame *stkframe, adjinfo *adjustinfo) { - if frame.continpc == 0 { - // Frame is dead. - return - } - f := frame.fn - if stackDebug >= 2 { - print(" adjusting ", funcname(f), " frame=[", hex(frame.sp), ",", hex(frame.fp), "] pc=", hex(frame.pc), " continpc=", hex(frame.continpc), "\n") - } - // Adjust saved frame pointer if there is one. if (goarch.ArchFamily == goarch.AMD64 || goarch.ArchFamily == goarch.ARM64) && frame.argp-frame.varp == 2*goarch.PtrSize { if stackDebug >= 3 { @@ -729,6 +720,44 @@ func adjustframe(frame *stkframe, adjinfo *adjustinfo) { // by the caller in its frame (one word below its SP). adjustpointer(adjinfo, unsafe.Pointer(frame.varp)) } + if goarch.ArchFamily == goarch.ARM64 && isInjectedCall(frame.fn.funcID) { + // If this is an injected call on arm64, then we need to adjust + // the frame pointer saved by the original function into which + // the call was injected. Normally this would be handled when + // adjusting the callee's frame or in adjustctxt. But when a + // call is injected, the frame is placed 16 bytes below the + // original stack pointer to make room to save the link + // register, and the frame pointer saved by the original + // function isn't inside any call frame. We can adjust that + // saved frame pointer here by looking just above frame.fp. + // + // ^ original call ^ + // | frame above... | + // +-------------------+ <- stack pointer at the time of injection + // : FP saved by : + // : original func : + // :···················: <- frame pointer register from original function + // : LR saved during : + // : injection : + // +-------------------+ <- frame.fp (injection decrements SP by 16 bytes) + // | FP saved | + // | during injection | + // +-------------------+ + // | injected call | + // V frame below... V + adjustpointer(adjinfo, unsafe.Pointer(frame.fp+goarch.PtrSize)) + } + + if frame.continpc == 0 { + // Frame is dead. The program might still see the frame pointer + // saved in the frame, adjusted above, but we don't need to + // adjust the rest of the frame. + return + } + f := frame.fn + if stackDebug >= 2 { + print(" adjusting ", funcname(f), " frame=[", hex(frame.sp), ",", hex(frame.fp), "] pc=", hex(frame.pc), " continpc=", hex(frame.continpc), "\n") + } locals, args, objs := frame.getStackMap(true) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index e05075432df93a..58161dee06bddb 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -438,6 +438,10 @@ func (u *unwinder) resolveInternal(innermost, isSyscall bool) { } } +func isInjectedCall(id abi.FuncID) bool { + return id == abi.FuncID_sigpanic || id == abi.FuncID_asyncPreempt || id == abi.FuncID_debugCallV2 +} + func (u *unwinder) next() { frame := &u.frame f := frame.fn @@ -482,7 +486,7 @@ func (u *unwinder) next() { throw("traceback stuck") } - injectedCall := f.funcID == abi.FuncID_sigpanic || f.funcID == abi.FuncID_asyncPreempt || f.funcID == abi.FuncID_debugCallV2 + injectedCall := isInjectedCall(f.funcID) if injectedCall { u.flags |= unwindTrap } else { From 83223a99fff949d21582db4699ee54e1f41f6f96 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Tue, 28 Jul 2026 23:34:44 +0200 Subject: [PATCH 06/13] [release-branch.go1.27] cmd/compile: fix mips64le bigger than 32bits pointer offsets This was a latent bug revealed by CL 787720 which adds a bigger than 32bits slice pointer bump in it's test suite. Updates #80613 Fixes #80740 Change-Id: I47c49f6bd5ea83821de750b0c47b17a5e038d119 Reviewed-on: https://go-review.googlesource.com/c/go/+/807120 Reviewed-by: Keith Randall Reviewed-by: Keith Randall Reviewed-by: Cherry Mui Auto-Submit: Jorropo LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Julian Zhu Reviewed-on: https://go-review.googlesource.com/c/go/+/810040 Reviewed-by: Carlos Amedee --- .../compile/internal/ssa/_gen/MIPS64.rules | 3 ++- src/cmd/compile/internal/ssa/rewriteMIPS64.go | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/cmd/compile/internal/ssa/_gen/MIPS64.rules b/src/cmd/compile/internal/ssa/_gen/MIPS64.rules index a36f7076b19524..0198874f222239 100644 --- a/src/cmd/compile/internal/ssa/_gen/MIPS64.rules +++ b/src/cmd/compile/internal/ssa/_gen/MIPS64.rules @@ -227,7 +227,8 @@ (Leq64U x y) => (XOR (MOVVconst [1]) (SGTU x y)) (OffPtr [off] ptr:(SP)) && is32Bit(off) => (MOVVaddr [int32(off)] ptr) -(OffPtr [off] ptr) => (ADDVconst [off] ptr) +(OffPtr [off] ptr) && is32Bit(off) => (ADDVconst [off] ptr) +(OffPtr [off] ptr) && !is32Bit(off) => (ADDV ptr (MOVVconst [off])) (Addr {sym} base) => (MOVVaddr {sym} base) (LocalAddr {sym} base mem) && t.Elem().HasPointers() => (MOVVaddr {sym} (SPanchored base mem)) diff --git a/src/cmd/compile/internal/ssa/rewriteMIPS64.go b/src/cmd/compile/internal/ssa/rewriteMIPS64.go index c4b08435dcb002..60a22646d6a885 100644 --- a/src/cmd/compile/internal/ssa/rewriteMIPS64.go +++ b/src/cmd/compile/internal/ssa/rewriteMIPS64.go @@ -6138,6 +6138,8 @@ func rewriteValueMIPS64_OpNot(v *Value) bool { } func rewriteValueMIPS64_OpOffPtr(v *Value) bool { v_0 := v.Args[0] + b := v.Block + typ := &b.Func.Config.Types // match: (OffPtr [off] ptr:(SP)) // cond: is32Bit(off) // result: (MOVVaddr [int32(off)] ptr) @@ -6153,15 +6155,35 @@ func rewriteValueMIPS64_OpOffPtr(v *Value) bool { return true } // match: (OffPtr [off] ptr) + // cond: is32Bit(off) // result: (ADDVconst [off] ptr) for { off := auxIntToInt64(v.AuxInt) ptr := v_0 + if !(is32Bit(off)) { + break + } v.reset(OpMIPS64ADDVconst) v.AuxInt = int64ToAuxInt(off) v.AddArg(ptr) return true } + // match: (OffPtr [off] ptr) + // cond: !is32Bit(off) + // result: (ADDV ptr (MOVVconst [off])) + for { + off := auxIntToInt64(v.AuxInt) + ptr := v_0 + if !(!is32Bit(off)) { + break + } + v.reset(OpMIPS64ADDV) + v0 := b.NewValue0(v.Pos, OpMIPS64MOVVconst, typ.UInt64) + v0.AuxInt = int64ToAuxInt(off) + v.AddArg2(ptr, v0) + return true + } + return false } func rewriteValueMIPS64_OpRotateLeft16(v *Value) bool { v_1 := v.Args[1] From 434ebde9e6c753539fa1b9fd26c76bb4fa6144d8 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Mon, 27 Jul 2026 13:44:06 +0200 Subject: [PATCH 07/13] [release-branch.go1.27] cmd/compile: do not elide riscv64 sign extension of unsigned values The riscv64 lowering rules remove sign extensions after 32-bit instructions, which architecturally sign-extend their result. However regalloc restores spilled values with an extension chosen from their type: unsigned 4-byte values are restored zero-extended, losing the sign extension the rules removed. Unlike what I've assumed in 80577 where I thought lower were correct and regalloc were wrongly ignoring lower's assumptions, it's much easier to fix the issue the other way around and make lower follows the existing assumptions (shared by arm64). Only remove the extension when a spill/restore preserves it, that is when the extended value's type is 8 bytes wide, or 4 bytes wide and signed. Updates #80577 Fixes #80738 Change-Id: I3cf5cff3f9026c6501f794114e6afd537506fa63 Reviewed-on: https://go-review.googlesource.com/c/go/+/806340 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Michael Pratt Auto-Submit: Jorropo Reviewed-by: Keith Randall Reviewed-by: Keith Randall Reviewed-on: https://go-review.googlesource.com/c/go/+/810441 Reviewed-by: Carlos Amedee Reviewed-by: Mark Freeman --- .../compile/internal/ssa/_gen/RISCV64.rules | 24 ++++--- .../compile/internal/ssa/rewriteRISCV64.go | 33 ++++++--- test/fixedbugs/issue80577.go | 68 +++++++++++++++++++ 3 files changed, 103 insertions(+), 22 deletions(-) create mode 100644 test/fixedbugs/issue80577.go diff --git a/src/cmd/compile/internal/ssa/_gen/RISCV64.rules b/src/cmd/compile/internal/ssa/_gen/RISCV64.rules index ed7e142d0881bb..853cd2fbeca39a 100644 --- a/src/cmd/compile/internal/ssa/_gen/RISCV64.rules +++ b/src/cmd/compile/internal/ssa/_gen/RISCV64.rules @@ -622,17 +622,19 @@ (MOVBUreg x:(Select0 (LoweredAtomicCas64 _ _ _ _))) => (MOVDreg x) // Avoid sign extension after word arithmetic. -(MOVWreg x:(ADDIW _)) => (MOVDreg x) -(MOVWreg x:(SUBW _ _)) => (MOVDreg x) -(MOVWreg x:(NEGW _)) => (MOVDreg x) -(MOVWreg x:(MULW _ _)) => (MOVDreg x) -(MOVWreg x:(DIVW _ _)) => (MOVDreg x) -(MOVWreg x:(DIVUW _ _)) => (MOVDreg x) -(MOVWreg x:(REMW _ _)) => (MOVDreg x) -(MOVWreg x:(REMUW _ _)) => (MOVDreg x) -(MOVWreg x:(ROLW _ _)) => (MOVDreg x) -(MOVWreg x:(RORW _ _)) => (MOVDreg x) -(MOVWreg x:(RORIW _)) => (MOVDreg x) +// Careful to not omit the sign extension in cases where regalloc +// might restore without it, see issue 80577. +(MOVWreg x:(ADDIW _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(SUBW _ _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(NEGW _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(MULW _ _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(DIVW _ _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(DIVUW _ _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(REMW _ _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(REMUW _ _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(ROLW _ _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(RORW _ _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) +(MOVWreg x:(RORIW _)) && (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) => (MOVDreg x) // Fold double extensions. (MOVBreg x:(MOVBreg _)) => (MOVDreg x) diff --git a/src/cmd/compile/internal/ssa/rewriteRISCV64.go b/src/cmd/compile/internal/ssa/rewriteRISCV64.go index 43df9db6bc8445..e516259cc09d0c 100644 --- a/src/cmd/compile/internal/ssa/rewriteRISCV64.go +++ b/src/cmd/compile/internal/ssa/rewriteRISCV64.go @@ -6670,10 +6670,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(ADDIW _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64ADDIW { + if x.Op != OpRISCV64ADDIW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6681,10 +6682,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(SUBW _ _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64SUBW { + if x.Op != OpRISCV64SUBW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6692,10 +6694,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(NEGW _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64NEGW { + if x.Op != OpRISCV64NEGW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6703,10 +6706,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(MULW _ _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64MULW { + if x.Op != OpRISCV64MULW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6714,10 +6718,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(DIVW _ _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64DIVW { + if x.Op != OpRISCV64DIVW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6725,10 +6730,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(DIVUW _ _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64DIVUW { + if x.Op != OpRISCV64DIVUW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6736,10 +6742,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(REMW _ _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64REMW { + if x.Op != OpRISCV64REMW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6747,10 +6754,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(REMUW _ _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64REMUW { + if x.Op != OpRISCV64REMUW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6758,10 +6766,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(ROLW _ _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64ROLW { + if x.Op != OpRISCV64ROLW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6769,10 +6778,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(RORW _ _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64RORW { + if x.Op != OpRISCV64RORW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) @@ -6780,10 +6790,11 @@ func rewriteValueRISCV64_OpRISCV64MOVWreg(v *Value) bool { return true } // match: (MOVWreg x:(RORIW _)) + // cond: (x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) // result: (MOVDreg x) for { x := v_0 - if x.Op != OpRISCV64RORIW { + if x.Op != OpRISCV64RORIW || !(x.Type.Size() == 8 || (x.Type.Size() == 4 && x.Type.IsSigned())) { break } v.reset(OpRISCV64MOVDreg) diff --git a/test/fixedbugs/issue80577.go b/test/fixedbugs/issue80577.go new file mode 100644 index 00000000000000..4ca0c5de26561e --- /dev/null +++ b/test/fixedbugs/issue80577.go @@ -0,0 +1,68 @@ +// run + +// Copyright 2026 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. + +// Issue 80577: on riscv64, sign extensions were elided after +// 32-bit instructions that architecturally sign-extend their +// result, but an unsigned-typed value spilled across a call is +// restored with a zero-extending load, losing the elided +// sign extension. + +package main + +import "math/bits" + +var sink uint32 + +//go:noinline +func use(x uint32) { sink = x } + +//go:noinline +func mul(a, b uint32) int64 { + p := a * b + use(p) // p live across the call, forcing a spill + return int64(int32(p)) +} + +//go:noinline +func div(a, b uint32) int64 { + p := a / b + use(p) + return int64(int32(p)) +} + +//go:noinline +func rem(a, b uint32) int64 { + p := a % b + use(p) + return int64(int32(p)) +} + +//go:noinline +func rot(a uint32, k int) int64 { + p := bits.RotateLeft32(a, k) + use(p) + return int64(int32(p)) +} + +func main() { + const want = -2147483648 + if got := mul(0x8000, 0x10000); got != want { + println("mul: got", got, "want", want) + panic("bad mul") + } + if got := div(0x80000000, 1); got != want { + println("div: got", got, "want", want) + panic("bad div") + } + if got := rem(0x80000000, 0xffffffff); got != want { + println("rem: got", got, "want", want) + panic("bad rem") + } + if got := rot(1, 31); got != want { + println("rot: got", got, "want", want) + panic("bad rot") + } +} From 14e2c88dd57af43a799e3ea19e07106ac99a0a81 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Sat, 6 Jun 2026 06:16:09 +0200 Subject: [PATCH 08/13] [release-branch.go1.27] cmd/compile: fix prove to generate Const64 for 64bits slicemasks Updates #79874 Fixes #80741 Change-Id: I155b6571db5b3c3537c549eadfe87a14b7d5836a Reviewed-on: https://go-review.googlesource.com/c/go/+/787720 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Carlos Amedee Reviewed-by: Keith Randall Auto-Submit: Jorropo Reviewed-by: Keith Randall (cherry picked from commit 395f4f5e3f016edff5143ab67479fcb6bca0886e) Reviewed-on: https://go-review.googlesource.com/c/go/+/807223 Reviewed-by: Mark Freeman --- src/cmd/compile/internal/ssa/prove.go | 4 +- test/fixedbugs/issue79874.go | 57 +++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 test/fixedbugs/issue79874.go diff --git a/src/cmd/compile/internal/ssa/prove.go b/src/cmd/compile/internal/ssa/prove.go index 6f2143271115ff..9cf0d3eae0f378 100644 --- a/src/cmd/compile/internal/ssa/prove.go +++ b/src/cmd/compile/internal/ssa/prove.go @@ -2742,7 +2742,7 @@ func simplifyBlock(sdom SparseTree, ft *factsTable, b *Block) { // if x is larger than -y (y is negative), then slicemask is -1. lim := ft.limits[x.ID] if lim.umin > uint64(-delta) { - if cap.Op == OpAdd64 { + if v.Type.Size() == 8 { v.reset(OpConst64) } else { v.reset(OpConst32) @@ -2756,7 +2756,7 @@ func simplifyBlock(sdom SparseTree, ft *factsTable, b *Block) { } lim := ft.limits[cap.ID] if lim.umin > 0 { - if cap.Type.Size() == 8 { + if v.Type.Size() == 8 { v.reset(OpConst64) } else { v.reset(OpConst32) diff --git a/test/fixedbugs/issue79874.go b/test/fixedbugs/issue79874.go new file mode 100644 index 00000000000000..03ab5acdc7179f --- /dev/null +++ b/test/fixedbugs/issue79874.go @@ -0,0 +1,57 @@ +// run + +//go:build (linux || darwin) && !(386 || arm || mips || mipsle) + +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "encoding/binary" + "fmt" + "syscall" + "unsafe" +) + +const l = 1 << 34 + +//go:noinline +func bug(s []byte) []byte { + if len(s) < l+8 { + panic("too short") + } + return s[min(l, len(s)):] +} + +func main() { + // This code is a bit tricky because I have two contradictory constraints: + // 1. I need a slice >4GB big, ideally more (to test with non byte size element). + // 2. I can't allocate 4GB of ram in a test, let alone the 16GB I ended up using for real. + pageSize := syscall.Getpagesize() + + // Allocate a bunch of zeros, because this MAP_ANON mapping lack the PROT_WRITE permission + // the kernel will use a single shared aliased zero page to back up this memory. + // We still pay on the order of 32MB for the page table entries but it's acceptable. + s, err := syscall.Mmap(-1, 0, l+pageSize, syscall.PROT_READ, syscall.MAP_ANON|syscall.MAP_PRIVATE) + if err != nil { + panic(err) + } + + // Make the tail page writable. Use unsafe.Slice rather than s[l:] because s[l:] goes through the slicemask path under test. + if err := syscall.Mprotect(unsafe.Slice(&s[l], pageSize), syscall.PROT_READ|syscall.PROT_WRITE); err != nil { + panic(err) + } + + // Write without using s[l:] otherwise the same bug happens here and in bug making the test pass even if the bug is present. + const sentinel uint64 = 0x1122334455667788 + for i := 0; i < 8; i++ { + s[l+i] = byte(sentinel >> (8 * i)) + } + + // Finally test the bug. + if v := binary.LittleEndian.Uint64(bug(s)); v != sentinel { + panic(fmt.Sprintf("got %x, want %x", v, sentinel)) + } +} From e781256af37c2ef9e8dccaee88627f25f1005913 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Wed, 22 Jul 2026 16:03:17 +0200 Subject: [PATCH 09/13] [release-branch.go1.27] cmd/compile: fix time traveling proofs in prove So we had an issue with some of the slices proofs. They ran in addLocalFacts which learned all values's proofs. The problem is that some proofs are only valid after executing certain values. This happens when a value adds relations in the factsTable about exclusively the value's arguments. But due to the descend / simplify separation simplify would run with all the proofs of the whole block. This CL include a huge amount of retabulation of code, all the meaningfull part is the changes to the prove function. Now we run: 1. flowLimit 2. constantFoldArguments 3. addValueFact 4. simplifyValue inside the same value loop. So thanks to toposorting, facts are learned in a valid execution order. And thanks to interleaving simplify for value v can only see v's facts. The tests from CL 807182 are included in this backport. Updates #80517 Fixes #80742 Change-Id: I36f5bce5102df55a9e5eca400dfd931c46323d71 Reviewed-on: https://go-review.googlesource.com/c/go/+/804220 Reviewed-by: Keith Randall Reviewed-by: Keith Randall Reviewed-by: Carlos Amedee Auto-Submit: Jorropo LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com (cherry picked from commit a595f29385b4ef82d346ed96824258cb5cbba112) Reviewed-on: https://go-review.googlesource.com/c/go/+/807224 Reviewed-by: Cherry Mui --- src/cmd/compile/internal/ssa/prove.go | 868 +++++++++++++------------- test/fixedbugs/issue80517_1.go | 41 ++ test/fixedbugs/issue80517_2.go | 33 + test/fixedbugs/issue80517_3.go | 33 + test/prove.go | 2 +- 5 files changed, 542 insertions(+), 435 deletions(-) create mode 100644 test/fixedbugs/issue80517_1.go create mode 100644 test/fixedbugs/issue80517_2.go create mode 100644 test/fixedbugs/issue80517_3.go diff --git a/src/cmd/compile/internal/ssa/prove.go b/src/cmd/compile/internal/ssa/prove.go index 9cf0d3eae0f378..49925904792531 100644 --- a/src/cmd/compile/internal/ssa/prove.go +++ b/src/cmd/compile/internal/ssa/prove.go @@ -1639,7 +1639,7 @@ func prove(f *Func) { type walkState int const ( descend walkState = iota - simplify + restore ) // work maintains the DFS stack. type bp struct { @@ -1702,12 +1702,23 @@ func prove(f *Func) { // taking this branch. We'll restore // ft when we unwind. - // Add facts about the values in the current block. - addLocalFacts(ft, node.block) + ft.topoSortValuesInBlock(node.block) + + for _, v := range node.block.Values { + ft.flowLimit(v) + // constant fold arguments before addValueFact to avoid v's v.Args learned facts time traveling into v's arguments. + // in other words if v teaches us something about it's arguments, + // we can't use that to optimize v's arguments since v hasn't ran yet. + ft.constantFoldArguments(v) + ft.addValueFact(node.block, v) + ft.simplifyValue(node.block, v) + } + + ft.simplifyBlock(sdom, node.block) work = append(work, bp{ block: node.block, - state: simplify, + state: restore, }) for s := sdom.Child(node.block); s != nil; s = sdom.Sibling(s) { work = append(work, bp{ @@ -1716,8 +1727,7 @@ func prove(f *Func) { }) } - case simplify: - simplifyBlock(sdom, ft, node.block) + case restore: ft.restore() } } @@ -2391,180 +2401,172 @@ func checkForChunkedIndexBounds(ft *factsTable, b *Block, index, bound *Value, i return false } -func addLocalFacts(ft *factsTable, b *Block) { - ft.topoSortValuesInBlock(b) - - for _, v := range b.Values { - // Propagate constant ranges before relative relations to get - // the most up-to-date constant bounds for isNonNegative calls. - ft.flowLimit(v) - - switch v.Op { - case OpAdd64, OpAdd32, OpAdd16, OpAdd8: - x := ft.limits[v.Args[0].ID] - y := ft.limits[v.Args[1].ID] - if !unsignedAddOverflows(x.umax, y.umax, v.Type) { - r := gt - if x.maybeZero() { - r |= eq - } - ft.update(b, v, v.Args[1], unsigned, r) - r = gt - if y.maybeZero() { - r |= eq - } - ft.update(b, v, v.Args[0], unsigned, r) +func (ft *factsTable) addValueFact(b *Block, v *Value) { + switch v.Op { + case OpAdd64, OpAdd32, OpAdd16, OpAdd8: + x := ft.limits[v.Args[0].ID] + y := ft.limits[v.Args[1].ID] + if !unsignedAddOverflows(x.umax, y.umax, v.Type) { + r := gt + if x.maybeZero() { + r |= eq } - if x.min >= 0 && !signedAddOverflowsOrUnderflows(x.max, y.max, v.Type) { - r := gt - if x.maybeZero() { - r |= eq - } - ft.update(b, v, v.Args[1], signed, r) + ft.update(b, v, v.Args[1], unsigned, r) + r = gt + if y.maybeZero() { + r |= eq } - if y.min >= 0 && !signedAddOverflowsOrUnderflows(x.max, y.max, v.Type) { - r := gt - if y.maybeZero() { - r |= eq - } - ft.update(b, v, v.Args[0], signed, r) + ft.update(b, v, v.Args[0], unsigned, r) + } + if x.min >= 0 && !signedAddOverflowsOrUnderflows(x.max, y.max, v.Type) { + r := gt + if x.maybeZero() { + r |= eq } - if x.max <= 0 && !signedAddOverflowsOrUnderflows(x.min, y.min, v.Type) { - r := lt - if x.maybeZero() { - r |= eq - } - ft.update(b, v, v.Args[1], signed, r) + ft.update(b, v, v.Args[1], signed, r) + } + if y.min >= 0 && !signedAddOverflowsOrUnderflows(x.max, y.max, v.Type) { + r := gt + if y.maybeZero() { + r |= eq } - if y.max <= 0 && !signedAddOverflowsOrUnderflows(x.min, y.min, v.Type) { - r := lt - if y.maybeZero() { - r |= eq - } - ft.update(b, v, v.Args[0], signed, r) - } - case OpSub64, OpSub32, OpSub16, OpSub8: - x := ft.limits[v.Args[0].ID] - y := ft.limits[v.Args[1].ID] - if !unsignedSubUnderflows(x.umin, y.umax) { - r := lt - if y.maybeZero() { - r |= eq - } - ft.update(b, v, v.Args[0], unsigned, r) - } - // FIXME: we could also do signed facts but the overflow checks are much trickier and I don't need it yet. - case OpAnd64, OpAnd32, OpAnd16, OpAnd8: - ft.update(b, v, v.Args[0], unsigned, lt|eq) - ft.update(b, v, v.Args[1], unsigned, lt|eq) - if ft.isNonNegative(v.Args[0]) { - ft.update(b, v, v.Args[0], signed, lt|eq) - } - if ft.isNonNegative(v.Args[1]) { - ft.update(b, v, v.Args[1], signed, lt|eq) - } - case OpOr64, OpOr32, OpOr16, OpOr8: - // TODO: investigate how to always add facts without much slowdown, see issue #57959 - //ft.update(b, v, v.Args[0], unsigned, gt|eq) - //ft.update(b, v, v.Args[1], unsigned, gt|eq) - case OpDiv64, OpDiv32, OpDiv16, OpDiv8: - if !ft.isNonNegative(v.Args[1]) { - break + ft.update(b, v, v.Args[0], signed, r) + } + if x.max <= 0 && !signedAddOverflowsOrUnderflows(x.min, y.min, v.Type) { + r := lt + if x.maybeZero() { + r |= eq } - fallthrough - case OpRsh8x64, OpRsh8x32, OpRsh8x16, OpRsh8x8, - OpRsh16x64, OpRsh16x32, OpRsh16x16, OpRsh16x8, - OpRsh32x64, OpRsh32x32, OpRsh32x16, OpRsh32x8, - OpRsh64x64, OpRsh64x32, OpRsh64x16, OpRsh64x8: - if !ft.isNonNegative(v.Args[0]) { - break + ft.update(b, v, v.Args[1], signed, r) + } + if y.max <= 0 && !signedAddOverflowsOrUnderflows(x.min, y.min, v.Type) { + r := lt + if y.maybeZero() { + r |= eq } - fallthrough - case OpDiv64u, OpDiv32u, OpDiv16u, OpDiv8u, - OpRsh8Ux64, OpRsh8Ux32, OpRsh8Ux16, OpRsh8Ux8, - OpRsh16Ux64, OpRsh16Ux32, OpRsh16Ux16, OpRsh16Ux8, - OpRsh32Ux64, OpRsh32Ux32, OpRsh32Ux16, OpRsh32Ux8, - OpRsh64Ux64, OpRsh64Ux32, OpRsh64Ux16, OpRsh64Ux8: - switch add := v.Args[0]; add.Op { - // round-up division pattern; given: - // v = (x + y) / z - // if y < z then v <= x - case OpAdd64, OpAdd32, OpAdd16, OpAdd8: - z := v.Args[1] - zl := ft.limits[z.ID] - var uminDivisor uint64 - switch v.Op { - case OpDiv64u, OpDiv32u, OpDiv16u, OpDiv8u, - OpDiv64, OpDiv32, OpDiv16, OpDiv8: - uminDivisor = zl.umin - case OpRsh8Ux64, OpRsh8Ux32, OpRsh8Ux16, OpRsh8Ux8, - OpRsh16Ux64, OpRsh16Ux32, OpRsh16Ux16, OpRsh16Ux8, - OpRsh32Ux64, OpRsh32Ux32, OpRsh32Ux16, OpRsh32Ux8, - OpRsh64Ux64, OpRsh64Ux32, OpRsh64Ux16, OpRsh64Ux8, - OpRsh8x64, OpRsh8x32, OpRsh8x16, OpRsh8x8, - OpRsh16x64, OpRsh16x32, OpRsh16x16, OpRsh16x8, - OpRsh32x64, OpRsh32x32, OpRsh32x16, OpRsh32x8, - OpRsh64x64, OpRsh64x32, OpRsh64x16, OpRsh64x8: - uminDivisor = 1 << zl.umin - default: - panic("unreachable") - } - - x := add.Args[0] - xl := ft.limits[x.ID] - y := add.Args[1] - yl := ft.limits[y.ID] - if !unsignedAddOverflows(xl.umax, yl.umax, add.Type) { - if xl.umax < uminDivisor { - ft.update(b, v, y, unsigned, lt|eq) - } - if yl.umax < uminDivisor { - ft.update(b, v, x, unsigned, lt|eq) - } - } + ft.update(b, v, v.Args[0], signed, r) + } + case OpSub64, OpSub32, OpSub16, OpSub8: + x := ft.limits[v.Args[0].ID] + y := ft.limits[v.Args[1].ID] + if !unsignedSubUnderflows(x.umin, y.umax) { + r := lt + if y.maybeZero() { + r |= eq } - ft.update(b, v, v.Args[0], unsigned, lt|eq) - case OpMod64, OpMod32, OpMod16, OpMod8: - if !ft.isNonNegative(v.Args[0]) || !ft.isNonNegative(v.Args[1]) { - break + ft.update(b, v, v.Args[0], unsigned, r) + } + // FIXME: we could also do signed facts but the overflow checks are much trickier and I don't need it yet. + case OpAnd64, OpAnd32, OpAnd16, OpAnd8: + ft.update(b, v, v.Args[0], unsigned, lt|eq) + ft.update(b, v, v.Args[1], unsigned, lt|eq) + if ft.isNonNegative(v.Args[0]) { + ft.update(b, v, v.Args[0], signed, lt|eq) + } + if ft.isNonNegative(v.Args[1]) { + ft.update(b, v, v.Args[1], signed, lt|eq) + } + case OpOr64, OpOr32, OpOr16, OpOr8: + // TODO: investigate how to always add facts without much slowdown, see issue #57959 + //ft.update(b, v, v.Args[0], unsigned, gt|eq) + //ft.update(b, v, v.Args[1], unsigned, gt|eq) + case OpDiv64, OpDiv32, OpDiv16, OpDiv8: + if !ft.isNonNegative(v.Args[1]) { + break + } + fallthrough + case OpRsh8x64, OpRsh8x32, OpRsh8x16, OpRsh8x8, + OpRsh16x64, OpRsh16x32, OpRsh16x16, OpRsh16x8, + OpRsh32x64, OpRsh32x32, OpRsh32x16, OpRsh32x8, + OpRsh64x64, OpRsh64x32, OpRsh64x16, OpRsh64x8: + if !ft.isNonNegative(v.Args[0]) { + break + } + fallthrough + case OpDiv64u, OpDiv32u, OpDiv16u, OpDiv8u, + OpRsh8Ux64, OpRsh8Ux32, OpRsh8Ux16, OpRsh8Ux8, + OpRsh16Ux64, OpRsh16Ux32, OpRsh16Ux16, OpRsh16Ux8, + OpRsh32Ux64, OpRsh32Ux32, OpRsh32Ux16, OpRsh32Ux8, + OpRsh64Ux64, OpRsh64Ux32, OpRsh64Ux16, OpRsh64Ux8: + switch add := v.Args[0]; add.Op { + // round-up division pattern; given: + // v = (x + y) / z + // if y < z then v <= x + case OpAdd64, OpAdd32, OpAdd16, OpAdd8: + z := v.Args[1] + zl := ft.limits[z.ID] + var uminDivisor uint64 + switch v.Op { + case OpDiv64u, OpDiv32u, OpDiv16u, OpDiv8u, + OpDiv64, OpDiv32, OpDiv16, OpDiv8: + uminDivisor = zl.umin + case OpRsh8Ux64, OpRsh8Ux32, OpRsh8Ux16, OpRsh8Ux8, + OpRsh16Ux64, OpRsh16Ux32, OpRsh16Ux16, OpRsh16Ux8, + OpRsh32Ux64, OpRsh32Ux32, OpRsh32Ux16, OpRsh32Ux8, + OpRsh64Ux64, OpRsh64Ux32, OpRsh64Ux16, OpRsh64Ux8, + OpRsh8x64, OpRsh8x32, OpRsh8x16, OpRsh8x8, + OpRsh16x64, OpRsh16x32, OpRsh16x16, OpRsh16x8, + OpRsh32x64, OpRsh32x32, OpRsh32x16, OpRsh32x8, + OpRsh64x64, OpRsh64x32, OpRsh64x16, OpRsh64x8: + uminDivisor = 1 << zl.umin + default: + panic("unreachable") } - fallthrough - case OpMod64u, OpMod32u, OpMod16u, OpMod8u: - ft.update(b, v, v.Args[0], unsigned, lt|eq) - // Note: we have to be careful that this doesn't imply - // that the modulus is >0, which isn't true until *after* - // the mod instruction executes (and thus panics if the - // modulus is 0). See issue 67625. - ft.update(b, v, v.Args[1], unsigned, lt) - case OpStringLen: - if v.Args[0].Op == OpStringMake { - ft.update(b, v, v.Args[0].Args[1], signed, eq) - } - case OpSliceLen: - if v.Args[0].Op == OpSliceMake { - ft.update(b, v, v.Args[0].Args[1], signed, eq) - } - case OpSliceCap: - if v.Args[0].Op == OpSliceMake { - ft.update(b, v, v.Args[0].Args[2], signed, eq) - } - case OpIsInBounds: - if checkForChunkedIndexBounds(ft, b, v.Args[0], v.Args[1], false) { - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved %s for blocked indexing", v.Op) + + x := add.Args[0] + xl := ft.limits[x.ID] + y := add.Args[1] + yl := ft.limits[y.ID] + if !unsignedAddOverflows(xl.umax, yl.umax, add.Type) { + if xl.umax < uminDivisor { + ft.update(b, v, y, unsigned, lt|eq) } - ft.booleanTrue(v) - } - case OpIsSliceInBounds: - if checkForChunkedIndexBounds(ft, b, v.Args[0], v.Args[1], true) { - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved %s for blocked reslicing", v.Op) + if yl.umax < uminDivisor { + ft.update(b, v, x, unsigned, lt|eq) } - ft.booleanTrue(v) } - case OpPhi: - addLocalFactsPhi(ft, v) } + ft.update(b, v, v.Args[0], unsigned, lt|eq) + case OpMod64, OpMod32, OpMod16, OpMod8: + if !ft.isNonNegative(v.Args[0]) || !ft.isNonNegative(v.Args[1]) { + break + } + fallthrough + case OpMod64u, OpMod32u, OpMod16u, OpMod8u: + ft.update(b, v, v.Args[0], unsigned, lt|eq) + // Note: we have to be careful that this doesn't imply + // that the modulus is >0, which isn't true until *after* + // the mod instruction executes (and thus panics if the + // modulus is 0). See issue 67625. + ft.update(b, v, v.Args[1], unsigned, lt) + case OpStringLen: + if v.Args[0].Op == OpStringMake { + ft.update(b, v, v.Args[0].Args[1], signed, eq) + } + case OpSliceLen: + if v.Args[0].Op == OpSliceMake { + ft.update(b, v, v.Args[0].Args[1], signed, eq) + } + case OpSliceCap: + if v.Args[0].Op == OpSliceMake { + ft.update(b, v, v.Args[0].Args[2], signed, eq) + } + case OpIsInBounds: + if checkForChunkedIndexBounds(ft, b, v.Args[0], v.Args[1], false) { + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved %s for blocked indexing", v.Op) + } + ft.booleanTrue(v) + } + case OpIsSliceInBounds: + if checkForChunkedIndexBounds(ft, b, v.Args[0], v.Args[1], true) { + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved %s for blocked reslicing", v.Op) + } + ft.booleanTrue(v) + } + case OpPhi: + addLocalFactsPhi(ft, v) } } @@ -2717,305 +2719,303 @@ var invertEqNeqOp = map[Op]Op{ OpNeq64: OpEq64, } -// simplifyBlock simplifies some constant values in b and evaluates -// branches to non-uniquely dominated successors of b. -func simplifyBlock(sdom SparseTree, ft *factsTable, b *Block) { - for _, v := range b.Values { - switch v.Op { - case OpStaticLECall: - if b.Func.pass.debug > 0 && len(v.Args) == 2 { - fn := auxToCall(v.Aux).Fn - if fn != nil && strings.Contains(fn.String(), "prove") { - // Print bounds of any argument to single-arg function with "prove" in name, - // for debugging and especially for test/prove.go. - // (v.Args[1] is mem). - x := v.Args[0] - b.Func.Warnl(v.Pos, "Proved %v (%v)", ft.limits[x.ID], x) - } - } - case OpSlicemask: - // Replace OpSlicemask operations in b with constants where possible. - cap := v.Args[0] - x, delta := isConstDelta(cap) - if x != nil { - // slicemask(x + y) - // if x is larger than -y (y is negative), then slicemask is -1. - lim := ft.limits[x.ID] - if lim.umin > uint64(-delta) { - if v.Type.Size() == 8 { - v.reset(OpConst64) - } else { - v.reset(OpConst32) - } - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved slicemask not needed") - } - v.AuxInt = -1 - } - break - } - lim := ft.limits[cap.ID] - if lim.umin > 0 { +func (ft *factsTable) simplifyValue(b *Block, v *Value) { + switch v.Op { + case OpStaticLECall: + if b.Func.pass.debug > 0 && len(v.Args) == 2 { + fn := auxToCall(v.Aux).Fn + if fn != nil && strings.Contains(fn.String(), "prove") { + // Print bounds of any argument to single-arg function with "prove" in name, + // for debugging and especially for test/prove.go. + // (v.Args[1] is mem). + x := v.Args[0] + b.Func.Warnl(v.Pos, "Proved %v (%v)", ft.limits[x.ID], x) + } + } + case OpSlicemask: + // Replace OpSlicemask operations in b with constants where possible. + cap := v.Args[0] + x, delta := isConstDelta(cap) + if x != nil { + // slicemask(x + y) + // if x is larger than -y (y is negative), then slicemask is -1. + lim := ft.limits[x.ID] + if lim.umin > uint64(-delta) { if v.Type.Size() == 8 { v.reset(OpConst64) } else { v.reset(OpConst32) } if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved slicemask not needed (by limit)") + b.Func.Warnl(v.Pos, "Proved slicemask not needed") } v.AuxInt = -1 } + break + } + lim := ft.limits[cap.ID] + if lim.umin > 0 { + if v.Type.Size() == 8 { + v.reset(OpConst64) + } else { + v.reset(OpConst32) + } + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved slicemask not needed (by limit)") + } + v.AuxInt = -1 + } - case OpCtz8, OpCtz16, OpCtz32, OpCtz64: - // On some architectures, notably amd64, we can generate much better - // code for CtzNN if we know that the argument is non-zero. - // Capture that information here for use in arch-specific optimizations. - x := v.Args[0] - lim := ft.limits[x.ID] - if lim.umin > 0 || lim.min > 0 || lim.max < 0 { - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved %v non-zero", v.Op) - } - v.Op = ctzNonZeroOp[v.Op] + case OpCtz8, OpCtz16, OpCtz32, OpCtz64: + // On some architectures, notably amd64, we can generate much better + // code for CtzNN if we know that the argument is non-zero. + // Capture that information here for use in arch-specific optimizations. + x := v.Args[0] + lim := ft.limits[x.ID] + if lim.umin > 0 || lim.min > 0 || lim.max < 0 { + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved %v non-zero", v.Op) } - case OpRsh8x8, OpRsh8x16, OpRsh8x32, OpRsh8x64, - OpRsh16x8, OpRsh16x16, OpRsh16x32, OpRsh16x64, - OpRsh32x8, OpRsh32x16, OpRsh32x32, OpRsh32x64, - OpRsh64x8, OpRsh64x16, OpRsh64x32, OpRsh64x64: - if ft.isNonNegative(v.Args[0]) { - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved %v is unsigned", v.Op) - } - v.Op = unsignedOp[v.Op] + v.Op = ctzNonZeroOp[v.Op] + } + case OpRsh8x8, OpRsh8x16, OpRsh8x32, OpRsh8x64, + OpRsh16x8, OpRsh16x16, OpRsh16x32, OpRsh16x64, + OpRsh32x8, OpRsh32x16, OpRsh32x32, OpRsh32x64, + OpRsh64x8, OpRsh64x16, OpRsh64x32, OpRsh64x64: + if ft.isNonNegative(v.Args[0]) { + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved %v is unsigned", v.Op) } - fallthrough - case OpLsh8x8, OpLsh8x16, OpLsh8x32, OpLsh8x64, - OpLsh16x8, OpLsh16x16, OpLsh16x32, OpLsh16x64, - OpLsh32x8, OpLsh32x16, OpLsh32x32, OpLsh32x64, - OpLsh64x8, OpLsh64x16, OpLsh64x32, OpLsh64x64, - OpRsh8Ux8, OpRsh8Ux16, OpRsh8Ux32, OpRsh8Ux64, - OpRsh16Ux8, OpRsh16Ux16, OpRsh16Ux32, OpRsh16Ux64, - OpRsh32Ux8, OpRsh32Ux16, OpRsh32Ux32, OpRsh32Ux64, - OpRsh64Ux8, OpRsh64Ux16, OpRsh64Ux32, OpRsh64Ux64: - // Check whether, for a << b, we know that b - // is strictly less than the number of bits in a. - by := v.Args[1] - lim := ft.limits[by.ID] - bits := 8 * v.Args[0].Type.Size() - if lim.umax < uint64(bits) || (lim.max < bits && ft.isNonNegative(by)) { - v.AuxInt = 1 // see shiftIsBounded - if b.Func.pass.debug > 0 && !by.isGenericIntConst() { - b.Func.Warnl(v.Pos, "Proved %v bounded", v.Op) - } + v.Op = unsignedOp[v.Op] + } + fallthrough + case OpLsh8x8, OpLsh8x16, OpLsh8x32, OpLsh8x64, + OpLsh16x8, OpLsh16x16, OpLsh16x32, OpLsh16x64, + OpLsh32x8, OpLsh32x16, OpLsh32x32, OpLsh32x64, + OpLsh64x8, OpLsh64x16, OpLsh64x32, OpLsh64x64, + OpRsh8Ux8, OpRsh8Ux16, OpRsh8Ux32, OpRsh8Ux64, + OpRsh16Ux8, OpRsh16Ux16, OpRsh16Ux32, OpRsh16Ux64, + OpRsh32Ux8, OpRsh32Ux16, OpRsh32Ux32, OpRsh32Ux64, + OpRsh64Ux8, OpRsh64Ux16, OpRsh64Ux32, OpRsh64Ux64: + // Check whether, for a << b, we know that b + // is strictly less than the number of bits in a. + by := v.Args[1] + lim := ft.limits[by.ID] + bits := 8 * v.Args[0].Type.Size() + if lim.umax < uint64(bits) || (lim.max < bits && ft.isNonNegative(by)) { + v.AuxInt = 1 // see shiftIsBounded + if b.Func.pass.debug > 0 && !by.isGenericIntConst() { + b.Func.Warnl(v.Pos, "Proved %v bounded", v.Op) + } + } + case OpDiv8, OpDiv16, OpDiv32, OpDiv64, OpMod8, OpMod16, OpMod32, OpMod64: + p, q := ft.limits[v.Args[0].ID], ft.limits[v.Args[1].ID] // p/q + if p.nonnegative() && q.nonnegative() { + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved %v is unsigned", v.Op) + } + v.Op = unsignedOp[v.Op] + v.AuxInt = 0 + break + } + // Fixup code can be avoided on x86 if we know + // the divisor is not -1 or the dividend > MinIntNN. + if v.Op != OpDiv8 && v.Op != OpMod8 && (q.max < -1 || q.min > -1 || p.min > mostNegativeDividend[v.Op]) { + // See DivisionNeedsFixUp in rewrite.go. + // v.AuxInt = 1 means we have proved that the divisor is not -1 + // or that the dividend is not the most negative integer, + // so we do not need to add fix-up code. + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved %v does not need fix-up", v.Op) + } + // Only usable on amd64 and 386, and only for ≥ 16-bit ops. + // Don't modify AuxInt on other architectures, as that can interfere with CSE. + // (Print the debug info above always, so that test/prove.go can be + // checked on non-x86 systems.) + // TODO: add other architectures? + if b.Func.Config.arch == "386" || b.Func.Config.arch == "amd64" { + v.AuxInt = 1 } - case OpDiv8, OpDiv16, OpDiv32, OpDiv64, OpMod8, OpMod16, OpMod32, OpMod64: - p, q := ft.limits[v.Args[0].ID], ft.limits[v.Args[1].ID] // p/q - if p.nonnegative() && q.nonnegative() { - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved %v is unsigned", v.Op) - } - v.Op = unsignedOp[v.Op] - v.AuxInt = 0 - break + } + case OpMul64, OpMul32, OpMul16, OpMul8: + if vl := ft.limits[v.ID]; vl.min == vl.max || vl.umin == vl.umax { + // v is going to be constant folded away; don't "optimize" it. + break + } + x := v.Args[0] + xl := ft.limits[x.ID] + y := v.Args[1] + yl := ft.limits[y.ID] + if xl.umin == xl.umax && isPowerOfTwo(xl.umin) || + xl.min == xl.max && isPowerOfTwo(xl.min) || + yl.umin == yl.umax && isPowerOfTwo(yl.umin) || + yl.min == yl.max && isPowerOfTwo(yl.min) { + // 0,1 * a power of two is better done as a shift + break + } + switch xOne, yOne := xl.umax <= 1, yl.umax <= 1; { + case xOne && yOne: + v.Op = bytesizeToAnd[v.Type.Size()] + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Rewrote Mul %v into And", v) } - // Fixup code can be avoided on x86 if we know - // the divisor is not -1 or the dividend > MinIntNN. - if v.Op != OpDiv8 && v.Op != OpMod8 && (q.max < -1 || q.min > -1 || p.min > mostNegativeDividend[v.Op]) { - // See DivisionNeedsFixUp in rewrite.go. - // v.AuxInt = 1 means we have proved that the divisor is not -1 - // or that the dividend is not the most negative integer, - // so we do not need to add fix-up code. - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved %v does not need fix-up", v.Op) - } - // Only usable on amd64 and 386, and only for ≥ 16-bit ops. - // Don't modify AuxInt on other architectures, as that can interfere with CSE. - // (Print the debug info above always, so that test/prove.go can be - // checked on non-x86 systems.) - // TODO: add other architectures? - if b.Func.Config.arch == "386" || b.Func.Config.arch == "amd64" { - v.AuxInt = 1 - } + case yOne && b.Func.Config.haveCondSelect: + x, y = y, x + fallthrough + case xOne && b.Func.Config.haveCondSelect: + if !canCondSelect(v, b.Func.Config.arch, nil) { + break } - case OpMul64, OpMul32, OpMul16, OpMul8: - if vl := ft.limits[v.ID]; vl.min == vl.max || vl.umin == vl.umax { - // v is going to be constant folded away; don't "optimize" it. + zero := b.Func.constVal(bytesizeToConst[v.Type.Size()], v.Type, 0, true) + ft.initLimitForNewValue(zero) + check := b.NewValue2(v.Pos, bytesizeToNeq[v.Type.Size()], types.Types[types.TBOOL], zero, x) + ft.initLimitForNewValue(check) + v.reset(OpCondSelect) + v.AddArg3(y, zero, check) + + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Rewrote Mul %v into CondSelect; %v is bool", v, x) + } + } + case OpEq64, OpEq32, OpEq16, OpEq8, + OpNeq64, OpNeq32, OpNeq16, OpNeq8: + // Canonicalize: + // [0,1] != 1 → [0,1] == 0 + // [0,1] == 1 → [0,1] != 0 + // Comparison with zero often encode smaller. + xPos, yPos := 0, 1 + x, y := v.Args[xPos], v.Args[yPos] + xl, yl := ft.limits[x.ID], ft.limits[y.ID] + xConst, xIsConst := xl.constValue() + yConst, yIsConst := yl.constValue() + switch { + case xIsConst && yIsConst: + case xIsConst: + xPos, yPos = yPos, xPos + x, y = y, x + xl, yl = yl, xl + xConst, yConst = yConst, xConst + fallthrough + case yIsConst: + if yConst != 1 || + xl.umax > 1 { break } - x := v.Args[0] - xl := ft.limits[x.ID] - y := v.Args[1] - yl := ft.limits[y.ID] - if xl.umin == xl.umax && isPowerOfTwo(xl.umin) || - xl.min == xl.max && isPowerOfTwo(xl.min) || - yl.umin == yl.umax && isPowerOfTwo(yl.umin) || - yl.min == yl.max && isPowerOfTwo(yl.min) { - // 0,1 * a power of two is better done as a shift + zero := b.Func.constVal(bytesizeToConst[x.Type.Size()], x.Type, 0, true) + ft.initLimitForNewValue(zero) + oldOp := v.Op + v.Op = invertEqNeqOp[v.Op] + v.SetArg(yPos, zero) + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Rewrote %v (%v) %v argument is boolean-like; rewrote to %v against 0", v, oldOp, x, v.Op) + } + } + case OpAnd64, OpAnd32, OpAnd16, OpAnd8: + x, y := v.Args[0], v.Args[1] + xl, yl := ft.limits[x.ID], ft.limits[y.ID] + xConst, xIsConst := xl.constValue() + yConst, yIsConst := yl.constValue() + // Remove no-op Ands + switch { + case xIsConst && yIsConst: + case xIsConst: + x, y = y, x + xl, yl = yl, xl + xConst, yConst = yConst, xConst + fallthrough + case yIsConst: + knownBits, fixedLen := xl.unsignedFixedLeadingBits() + varyingLen := 64 - fixedLen + wantBits := knownBits | (uint64(1)< 0 { - b.Func.Warnl(v.Pos, "Rewrote Mul %v into And", v) - } - case yOne && b.Func.Config.haveCondSelect: - x, y = y, x - fallthrough - case xOne && b.Func.Config.haveCondSelect: - if !canCondSelect(v, b.Func.Config.arch, nil) { - break - } - zero := b.Func.constVal(bytesizeToConst[v.Type.Size()], v.Type, 0, true) - ft.initLimitForNewValue(zero) - check := b.NewValue2(v.Pos, bytesizeToNeq[v.Type.Size()], types.Types[types.TBOOL], zero, x) - ft.initLimitForNewValue(check) - v.reset(OpCondSelect) - v.AddArg3(y, zero, check) - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Rewrote Mul %v into CondSelect; %v is bool", v, x) - } + oldOp := v.Op + v.copyOf(x) + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved %v is a no-op %v", v, oldOp) } - case OpEq64, OpEq32, OpEq16, OpEq8, - OpNeq64, OpNeq32, OpNeq16, OpNeq8: - // Canonicalize: - // [0,1] != 1 → [0,1] == 0 - // [0,1] == 1 → [0,1] != 0 - // Comparison with zero often encode smaller. - xPos, yPos := 0, 1 - x, y := v.Args[xPos], v.Args[yPos] - xl, yl := ft.limits[x.ID], ft.limits[y.ID] - xConst, xIsConst := xl.constValue() - yConst, yIsConst := yl.constValue() - switch { - case xIsConst && yIsConst: - case xIsConst: - xPos, yPos = yPos, xPos - x, y = y, x - xl, yl = yl, xl - xConst, yConst = yConst, xConst - fallthrough - case yIsConst: - if yConst != 1 || - xl.umax > 1 { - break - } - zero := b.Func.constVal(bytesizeToConst[x.Type.Size()], x.Type, 0, true) - ft.initLimitForNewValue(zero) - oldOp := v.Op - v.Op = invertEqNeqOp[v.Op] - v.SetArg(yPos, zero) - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Rewrote %v (%v) %v argument is boolean-like; rewrote to %v against 0", v, oldOp, x, v.Op) - } - } - case OpAnd64, OpAnd32, OpAnd16, OpAnd8: - x, y := v.Args[0], v.Args[1] - xl, yl := ft.limits[x.ID], ft.limits[y.ID] - xConst, xIsConst := xl.constValue() - yConst, yIsConst := yl.constValue() - // Remove no-op Ands - switch { - case xIsConst && yIsConst: - case xIsConst: - x, y = y, x - xl, yl = yl, xl - xConst, yConst = yConst, xConst - fallthrough - case yIsConst: - knownBits, fixedLen := xl.unsignedFixedLeadingBits() - varyingLen := 64 - fixedLen - wantBits := knownBits | (uint64(1)< 0 { - b.Func.Warnl(v.Pos, "Proved %v is a no-op %v", v, oldOp) - } + } + case OpOr64, OpOr32, OpOr16, OpOr8: + x, y := v.Args[0], v.Args[1] + xl, yl := ft.limits[x.ID], ft.limits[y.ID] + xConst, xIsConst := xl.constValue() + yConst, yIsConst := yl.constValue() + // Remove no-op Ors + switch { + case xIsConst && yIsConst: + case xIsConst: + x, y = y, x + xl, yl = yl, xl + xConst, yConst = yConst, xConst + fallthrough + case yIsConst: + wantBits, _ := xl.unsignedFixedLeadingBits() + // wantBits has the fixed bits and the worst case bits (unset) for the varying bits + // if after oring it with y it isn't modified we know the or is always a no-op. + if wantBits|uint64(yConst) != wantBits { + break } - case OpOr64, OpOr32, OpOr16, OpOr8: - x, y := v.Args[0], v.Args[1] - xl, yl := ft.limits[x.ID], ft.limits[y.ID] - xConst, xIsConst := xl.constValue() - yConst, yIsConst := yl.constValue() - // Remove no-op Ors - switch { - case xIsConst && yIsConst: - case xIsConst: - x, y = y, x - xl, yl = yl, xl - xConst, yConst = yConst, xConst - fallthrough - case yIsConst: - wantBits, _ := xl.unsignedFixedLeadingBits() - // wantBits has the fixed bits and the worst case bits (unset) for the varying bits - // if after oring it with y it isn't modified we know the or is always a no-op. - if wantBits|uint64(yConst) != wantBits { - break - } - oldOp := v.Op - v.copyOf(x) - if b.Func.pass.debug > 0 { - b.Func.Warnl(v.Pos, "Proved %v is a no-op %v", v, oldOp) - } + oldOp := v.Op + v.copyOf(x) + if b.Func.pass.debug > 0 { + b.Func.Warnl(v.Pos, "Proved %v is a no-op %v", v, oldOp) } } + } +} - // Fold provable constant results. - // Helps in cases where we reuse a value after branching on its equality. - for i, arg := range v.Args { - lim := ft.limits[arg.ID] - constValue, ok := lim.constValue() - if !ok { - continue - } - switch arg.Op { - case OpConst64, OpConst32, OpConst16, OpConst8, OpConstBool, OpConstNil: - continue - } - typ := arg.Type - f := b.Func - var c *Value - switch { - case typ.IsBoolean(): - c = f.ConstBool(typ, constValue != 0) - case typ.IsInteger() && typ.Size() == 1: - c = f.ConstInt8(typ, int8(constValue)) - case typ.IsInteger() && typ.Size() == 2: - c = f.ConstInt16(typ, int16(constValue)) - case typ.IsInteger() && typ.Size() == 4: - c = f.ConstInt32(typ, int32(constValue)) - case typ.IsInteger() && typ.Size() == 8: - c = f.ConstInt64(typ, constValue) - case typ.IsPtrShaped(): - if constValue == 0 { - c = f.ConstNil(typ) - } else { - // Not sure how this might happen, but if it - // does, just skip it. - continue - } - default: +func (ft *factsTable) constantFoldArguments(v *Value) { + for i, arg := range v.Args { + lim := ft.limits[arg.ID] + constValue, ok := lim.constValue() + if !ok { + continue + } + switch arg.Op { + case OpConst64, OpConst32, OpConst16, OpConst8, OpConstBool, OpConstNil: + continue + } + typ := arg.Type + f := v.Block.Func + var c *Value + switch { + case typ.IsBoolean(): + c = f.ConstBool(typ, constValue != 0) + case typ.IsInteger() && typ.Size() == 1: + c = f.ConstInt8(typ, int8(constValue)) + case typ.IsInteger() && typ.Size() == 2: + c = f.ConstInt16(typ, int16(constValue)) + case typ.IsInteger() && typ.Size() == 4: + c = f.ConstInt32(typ, int32(constValue)) + case typ.IsInteger() && typ.Size() == 8: + c = f.ConstInt64(typ, constValue) + case typ.IsPtrShaped(): + if constValue == 0 { + c = f.ConstNil(typ) + } else { // Not sure how this might happen, but if it // does, just skip it. continue } - v.SetArg(i, c) - ft.initLimitForNewValue(c) - if b.Func.pass.debug > 1 { - b.Func.Warnl(v.Pos, "Proved %v's arg %d (%v) is constant %d", v, i, arg, constValue) - } + default: + // Not sure how this might happen, but if it + // does, just skip it. + continue + } + v.SetArg(i, c) + ft.initLimitForNewValue(c) + if f.pass.debug > 1 { + f.Warnl(v.Pos, "Proved %v's arg %d (%v) is constant %d", v, i, arg, constValue) } } +} +func (ft *factsTable) simplifyBlock(sdom SparseTree, b *Block) { if b.Kind != BlockIf { return } diff --git a/test/fixedbugs/issue80517_1.go b/test/fixedbugs/issue80517_1.go new file mode 100644 index 00000000000000..b8285ab6767fa7 --- /dev/null +++ b/test/fixedbugs/issue80517_1.go @@ -0,0 +1,41 @@ +// run + +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +const targetPC = uintptr(0xdeadbeef) + +type payload struct { + x uintptr + y *uintptr + fn [2]func() +} + +var p payload +var v []byte + +func init() { + p.x = targetPC + p.y = &p.x + p.fn[0] = func() {} + p.fn[1] = func() {} +} + +//go:noinline +func trigger(n int) { + defer func() { recover() }() + + if n < len(p.fn) { + p.fn[n&1]() + + s := make([]byte, n) + v = s + } +} + +func main() { + trigger(-1) +} diff --git a/test/fixedbugs/issue80517_2.go b/test/fixedbugs/issue80517_2.go new file mode 100644 index 00000000000000..a364f1d8a22c21 --- /dev/null +++ b/test/fixedbugs/issue80517_2.go @@ -0,0 +1,33 @@ +// run + +// Copyright 2026 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. + +// The prove pass must not use a fact that only becomes valid after a +// later value executes to simplify an earlier value. Here make([]byte, n) +// teaches prove that n >= 0, but that is only true after the make runs. +// A buggy prove lets that fact travel back in time and rewrites the +// earlier signed shift n>>1 into an unsigned shift, corrupting the result +// for negative n. + +package main + +var sink []byte + +//go:noinline +func trigger(n int) (res int) { + defer func() { recover() }() + if n < 100 { + res = n >> 1 // signed arithmetic shift right + sink = make([]byte, n) // only asserts n >= 0 after this point + } + return +} + +func main() { + if got := trigger(-2); got != -1 { + println("n>>1 =", got, "want -1") + panic("prove miscompiled a signed shift") + } +} diff --git a/test/fixedbugs/issue80517_3.go b/test/fixedbugs/issue80517_3.go new file mode 100644 index 00000000000000..c0a9792cefd7a6 --- /dev/null +++ b/test/fixedbugs/issue80517_3.go @@ -0,0 +1,33 @@ +// run + +// Copyright 2026 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. + +// Same time-traveling prove bug as issue80517_2.go, but the victims are a +// signed division and a signed modulo. make([]byte, n) teaches prove that +// n >= 0 only after it runs; a buggy prove lets that fact travel back and +// rewrites the earlier n/4 and n%3 into unsigned operations, corrupting +// the result for negative n. + +package main + +var sink []byte + +//go:noinline +func trigger(n int) (q, r int) { + defer func() { recover() }() + if n < 100 { + q = n / 4 // signed division + r = n % 3 // signed modulo + sink = make([]byte, n) // only asserts n >= 0 after this point + } + return +} + +func main() { + if q, r := trigger(-8); q != -2 || r != -2 { + println("n/4 =", q, "want -2; n%3 =", r, "want -2") + panic("prove miscompiled a signed div/mod") + } +} diff --git a/test/prove.go b/test/prove.go index 30f5e77e76ca9c..0badf9cda68a8b 100644 --- a/test/prove.go +++ b/test/prove.go @@ -703,7 +703,7 @@ func suffix(s, suffix string) bool { } func constsuffix(s string) bool { - return suffix(s, "abc") // ERROR "Proved IsSliceInBounds$" "Proved slicemask not needed$" "Proved Eq64$" + return suffix(s, "abc") // ERROR "Proved IsSliceInBounds$" "Proved slicemask not needed \(by limit\)$" "Proved Eq64$" } func atexit(foobar []func()) { From bd62f0c26450224a26857a6d38a738d31f8fbaf6 Mon Sep 17 00:00:00 2001 From: Ian Alexander Date: Mon, 20 Jul 2026 22:48:04 -0400 Subject: [PATCH 10/13] [release-branch.go1.27] net/url: avoid quadratic complexity in resolvePath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operate on a []byte with index-based backtracking instead of calling dst.String() and dst.WriteString() per iteration. goos: linux goarch: amd64 pkg: net/url cpu: AMD EPYC 7B13 │ b/gotip │ b/fixed │ │ sec/op │ sec/op vs base │ ResolvePath/Simple 123.85n ± 1% 94.56n ± 0% -23.65% (p=0.000 n=10) ResolvePath/Deep 1.819µ ± 1% 1.611µ ± 1% -11.41% (p=0.000 n=10) ResolvePath/Backtrack 6.439µ ± 2% 2.280µ ± 1% -64.59% (p=0.000 n=10) geomean 1.132µ 702.9n -37.90% │ b/gotip │ b/fixed │ │ B/op │ B/op vs base │ ResolvePath/Simple 16.000 ± 0% 4.000 ± 0% -75.00% (p=0.000 n=10) ResolvePath/Deep 712.0 ± 0% 624.0 ± 0% -12.36% (p=0.000 n=10) ResolvePath/Backtrack 9064.0 ± 0% 816.0 ± 0% -91.00% (p=0.000 n=10) geomean 469.1 126.8 -72.98% │ b/gotip │ b/fixed │ │ allocs/op │ allocs/op vs base │ ResolvePath/Simple 2.000 ± 0% 1.000 ± 0% -50.00% (p=0.000 n=10) ResolvePath/Deep 7.000 ± 0% 3.000 ± 0% -57.14% (p=0.000 n=10) ResolvePath/Backtrack 107.000 ± 0% 3.000 ± 0% -97.20% (p=0.000 n=10) geomean 11.44 2.080 -81.82% Updates #80494 Fixes CVE-2026-56860 Change-Id: I0aada41a0a10e2ce77c6476a65a49abc796dff3f Reviewed-on: https://go-review.googlesource.com/c/go/+/803681 Reviewed-by: Neal Patel LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com (cherry picked from commit 594b58aa32c4a5535c1ae8ab6e4387418316f950) Reviewed-on: https://go-review.googlesource.com/c/go/+/807700 Auto-Submit: Ian Alexander Commit-Queue: Mark Freeman --- src/net/url/url.go | 48 ++++++++++++++++------------------------- src/net/url/url_test.go | 25 +++++++++++++++++---- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/net/url/url.go b/src/net/url/url.go index 77c2dd2b5ae93f..ebc0ff4520b7c6 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -15,6 +15,7 @@ package url // Unit tests should also contain references to issue numbers with details. import ( + "bytes" "errors" "fmt" "internal/godebug" @@ -1056,54 +1057,43 @@ func resolvePath(base, ref string) string { return "" } - var ( - elem string - dst strings.Builder - ) - first := true + dst := make([]byte, 0, len(full)+1) + dst = append(dst, '/') + elem := "" remaining := full - // We want to return a leading '/', so write it now. - dst.WriteByte('/') found := true + first := true for found { elem, remaining, found = strings.Cut(remaining, "/") - if elem == "." { + switch elem { + case ".": first = false - // drop continue - } - - if elem == ".." { - // Ignore the leading '/' we already wrote. - str := dst.String()[1:] - index := strings.LastIndexByte(str, '/') - - dst.Reset() - dst.WriteByte('/') - if index == -1 { - first = true + case "..": + if i := bytes.LastIndexByte(dst[1:], '/'); i >= 0 { + dst = dst[:i+1] } else { - dst.WriteString(str[:index]) + dst = dst[:1] } - } else { + first = len(dst) == 1 + default: if !first { - dst.WriteByte('/') + dst = append(dst, '/') } - dst.WriteString(elem) + dst = append(dst, elem...) first = false } } if elem == "." || elem == ".." { - dst.WriteByte('/') + dst = append(dst, '/') } // We wrote an initial '/', but we don't want two. - r := dst.String() - if len(r) > 1 && r[1] == '/' { - r = r[1:] + if len(dst) > 1 && dst[1] == '/' { + return string(dst[1:]) } - return r + return string(dst) } // IsAbs reports whether the [URL] is absolute. diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index dd093e6942357d..a2e3451444128c 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -1214,10 +1214,27 @@ func TestResolvePath(t *testing.T) { } func BenchmarkResolvePath(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - resolvePath("a/b/c", ".././d") - } + b.Run("Simple", func(b *testing.B) { + for i := 0; i < b.N; i++ { + resolvePath("a/b/c", ".././d") + } + }) + b.Run("Deep", func(b *testing.B) { + base := strings.Repeat("a/", 100) + "b" + ref := "c" + b.ResetTimer() + for b.Loop() { + resolvePath(base, ref) + } + }) + b.Run("Backtrack", func(b *testing.B) { + base := strings.Repeat("a/", 100) + "b" + ref := strings.Repeat("../", 50) + "c" + b.ResetTimer() + for b.Loop() { + resolvePath(base, ref) + } + }) } var resolveReferenceTests = []struct { From bcdba48a6adcaceabb3696e46b50be21dc739b5e Mon Sep 17 00:00:00 2001 From: Neal Patel Date: Tue, 28 Jul 2026 17:06:12 -0400 Subject: [PATCH 11/13] [release-branch.go1.27] html/template: fix JavaScript regexp tracking This change prevents pathological inputs from closing an unescaped `/` early, allowing for attacker-controlled data to inject arbitrary unscaped content. Additionally, CL 532595 hints at the invariant in TestEscapeText potentially getting the update this change makes. For #80435 Fixes CVE-2026-56858 Change-Id: I502b8960249fa9a2827b44b64d081d224ac57cbc Reviewed-on: https://go-review.googlesource.com/c/go/+/807100 Reviewed-by: Neal Patel LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Roland Shoemaker (cherry picked from commit 0157ee9c8d501bac60d2ec8689fac1eea740215a) Reviewed-on: https://go-review.googlesource.com/c/go/+/811040 --- src/html/template/escape_test.go | 56 ++++++++++++++++++++++++++++++-- src/html/template/transition.go | 4 +++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go index 5ff173420207dd..3b26f7815fee37 100644 --- a/src/html/template/escape_test.go +++ b/src/html/template/escape_test.go @@ -985,7 +985,6 @@ func TestEscapeSet(t *testing.T) { t.Errorf("want\n\t%q\ngot\n\t%q", test.want, got) } } - } func TestErrors(t *testing.T) { @@ -1259,7 +1258,6 @@ func TestErrors(t *testing.T) { // Check that we get the same error if we call Execute again. if err := tmpl.Execute(buf, nil); err == nil || err.Error() != got { t.Errorf("input=%q: unexpected error on second call %q", test.input, err) - } } } @@ -1864,7 +1862,7 @@ func TestEscapeText(t *testing.T) { }, { "`, + input: "a.b", + want: ``, + }, + { + name: "regexp after close brace", + tmpl: ``, + input: "a.b", + want: ``, + }, + { + name: "regexp pathological attacker input", + tmpl: ``, + input: `./;alert(1);var q=/.`, + want: ``, + }, + { + name: "regexp after open brace in template literal", + tmpl: "", + input: "a.b", + want: "", + }, + { + name: "regexp after close brace in template literal", + tmpl: "", + input: "a.b", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpl := Must(New("test").Parse(tt.tmpl)) + var buf bytes.Buffer + if err := tmpl.Execute(&buf, tt.input); err != nil { + t.Fatalf("Execute: %v", err) + } + if got := buf.String(); got != tt.want { + t.Errorf("got: %s\nwant: %s", got, tt.want) + } + }) + } +} diff --git a/src/html/template/transition.go b/src/html/template/transition.go index 05b6abd03d1772..d9d4f63beba807 100644 --- a/src/html/template/transition.go +++ b/src/html/template/transition.go @@ -336,11 +336,14 @@ func tJS(c context, s []byte) (context, int) { // We only care about tracking brace depth if we are inside of a // template literal. if len(c.jsBraceDepth) == 0 { + c.jsCtx = nextJSCtx(s[i:i+1], c.jsCtx) return c, i + 1 } c.jsBraceDepth[len(c.jsBraceDepth)-1]++ + c.jsCtx = nextJSCtx(s[i:i+1], c.jsCtx) case '}': if len(c.jsBraceDepth) == 0 { + c.jsCtx = nextJSCtx(s[i:i+1], c.jsCtx) return c, i + 1 } // There are no cases where a brace can be escaped in the JS context @@ -349,6 +352,7 @@ func tJS(c context, s []byte) (context, int) { // fully fledged parsers will just fail anyway. c.jsBraceDepth[len(c.jsBraceDepth)-1]-- if c.jsBraceDepth[len(c.jsBraceDepth)-1] >= 0 { + c.jsCtx = nextJSCtx(s[i:i+1], c.jsCtx) return c, i + 1 } c.jsBraceDepth = c.jsBraceDepth[:len(c.jsBraceDepth)-1] From 528b8718ac6d230a60bb1e4f308c4d70ff2ac867 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 29 Jul 2026 03:45:18 +0000 Subject: [PATCH 12/13] net/http/internal/http2: let Transport's request-write goroutine exit early The goroutine spawned per Transport.RoundTrip to write the request previously parked until the stream ended, even after the request was fully sent, just to wait for the stream-end events and run cleanupWriteRequest. For clients with many concurrent long-lived response streams (long polls, event streams), that's a parked goroutine and its stack per stream doing nothing, which adds up to a large fraction of such a client's memory use (as seen in a production load balancer). Once the request is fully sent, detach: the goroutine exits, and cleanupWriteRequest instead runs (on a short-lived goroutine) from whichever stream-ending event fires first. The only case where the goroutine stays running is if the caller uses the deprecated Request.Cancel channel, but basically nobody uses that anymore. Updates #80735 Change-Id: I5500c691194457195b0869c1612bf93718ca96c1 Reviewed-on: https://go-review.googlesource.com/c/go/+/810780 Reviewed-by: Damien Neil Reviewed-by: Nicholas Husin Reviewed-by: Nicholas Husin TryBot-Bypass: Brad Fitzpatrick (cherry picked from commit 3c0665e551be23e62167f6257bede071bfbdcacb) --- src/net/http/internal/http2/transport.go | 136 +++++++++++++++++- src/net/http/internal/http2/transport_test.go | 126 ++++++++++++++++ 2 files changed, 261 insertions(+), 1 deletion(-) diff --git a/src/net/http/internal/http2/transport.go b/src/net/http/internal/http2/transport.go index 0b32ea72da0246..9d63ed26c85cfc 100644 --- a/src/net/http/internal/http2/transport.go +++ b/src/net/http/internal/http2/transport.go @@ -247,6 +247,27 @@ type clientStream struct { donec chan struct{} // closed after the stream is in the closed state on100 chan struct{} // buffered; written to if a 100 is received + // detached, guarded by cc.mu, indicates that the writeRequest + // goroutine has exited without waiting for the stream to end, and + // that cleanupWriteRequest should instead be run (on a new goroutine) + // by whichever of abortStreamLocked or clientConnReadLoop.endStream + // ends the stream. It is cleared when that cleanup is scheduled. + // See clientStream.detach. + detached bool + + // stopCtxWatch, if non-nil, cancels the context.AfterFunc watching + // for request context cancellation on behalf of a detached stream. + // It is set (under cc.mu) at most once, by detach, before detached + // is set, and is called by cleanupWriteRequest. + stopCtxWatch func() bool + + // respHeaderTimeoutTimer, guarded by cc.mu, is a timer enforcing + // Transport.ResponseHeaderTimeout on behalf of a detached stream. + // It is armed by detach if response headers haven't yet arrived, and + // stopped when they do (clientConnReadLoop.processHeaders) or when + // the stream ends (cleanupWriteRequest). + respHeaderTimeoutTimer *time.Timer + respHeaderRecv chan struct{} // closed when headers are received res *ClientResponse // set if respHeaderRecv is closed @@ -299,6 +320,10 @@ func (cs *clientStream) abortStreamLocked(err error) { cs.abortErr = err close(cs.abort) }) + if cs.detached { + cs.detached = false + go cs.cleanupWriteRequest(cs.abortErr) + } if cs.reqBody != nil { cs.closeReqBodyLocked() } @@ -1211,12 +1236,83 @@ func (cc *ClientConn) roundTrip(req *ClientRequest, streamf func(*clientStream)) // doRequest runs for the duration of the request lifetime. // -// It sends the request and performs post-request cleanup (closing Request.Body, etc.). +// It sends the request and performs post-request cleanup (closing Request.Body, etc.), +// except when writeRequest detaches from the stream, in which case cleanup is +// performed at stream end by whoever ends it. See clientStream.detach. func (cs *clientStream) doRequest(req *ClientRequest, streamf func(*clientStream)) { err := cs.writeRequest(req, streamf) + if err == errStreamDetached { + return + } cs.cleanupWriteRequest(err) } +// errStreamDetached is a sentinel returned by writeRequest to tell doRequest +// that the stream detached and cleanupWriteRequest will be called at stream +// end by whoever ends it. It is never returned to users. +var errStreamDetached = errors.New("http2: internal sentinel; stream detached from writeRequest goroutine") + +// detach arranges for cleanupWriteRequest to run when the stream ends (the +// peer half-closes it, it's aborted, or the request context is canceled), +// letting the writeRequest goroutine exit instead of parking until then. +// +// This matters for servers and proxies with many concurrent long-lived +// response streams (long polls): without it, each in-flight request pins a +// goroutine and its stack for the stream's lifetime doing nothing but +// waiting. +// +// respHeaderTimeout, if non-zero, gives the Transport.ResponseHeaderTimeout +// to enforce on the detached stream if response headers haven't arrived yet. +// +// It reports whether the stream was detached. It returns false if the stream +// has already ended, in which case the caller should wait for the stream end +// events itself (they're already pending). +func (cs *clientStream) detach(respHeaderTimeout time.Duration) bool { + cc := cs.cc + cc.mu.Lock() + defer cc.mu.Unlock() + select { + case <-cs.peerClosed: + return false + case <-cs.abort: + return false + default: + } + if respHeaderTimeout != 0 { + select { + case <-cs.respHeaderRecv: + // Headers already arrived; nothing to enforce. + default: + cs.respHeaderTimeoutTimer = time.AfterFunc(respHeaderTimeout, func() { + cc.mu.Lock() + defer cc.mu.Unlock() + select { + case <-cs.respHeaderRecv: + // Headers arrived after all; we lost a race + // with the Stop in processHeaders. Not a + // timeout. + return + default: + } + cs.abortStreamLocked(errTimeout) + }) + } + } + // Watch for request context cancellation without parking a goroutine + // on ctx.Done(). If the context was canceled already, AfterFunc runs + // the func in a new goroutine, which blocks acquiring cc.mu until we + // return. + // + // stopCtxWatch must be assigned before detached is set: once detached + // is set, an abort or peer close can schedule cleanupWriteRequest + // (which calls stopCtxWatch) as soon as we release cc.mu. + cs.stopCtxWatch = context.AfterFunc(cs.ctx, func() { + cs.abortStream(cs.ctx.Err()) + }) + cs.detached = true + return true +} + var errExtendedConnectNotSupported = errors.New("net/http: extended connect not supported by peer") // writeRequest sends a request. @@ -1340,6 +1436,24 @@ func (cs *clientStream) writeRequest(req *ClientRequest, streamf func(*clientStr traceWroteRequest(cs.trace, err) + // If the request is fully sent and there's nothing left for this + // goroutine to do but wait for the stream to end, detach from the + // stream and exit rather than pinning this goroutine (and its stack) + // for the lifetime of what may be a very long-lived response stream. + // The remaining cases below then run cleanupWriteRequest from the + // stream-end event sites instead: + // - peerClosed and abort schedule it directly + // (abortStreamLocked, clientConnReadLoop.endStream) + // - ctx.Done is handled via context.AfterFunc in detach + // - ResponseHeaderTimeout is enforced by a time.AfterFunc timer, + // armed in detach and stopped when headers arrive + // The deprecated Request.Cancel channel can only be watched by a + // goroutine, so that (rare) case keeps the historical behavior of + // waiting here. + if cs.sentEndStream && cs.reqCancel == nil && cs.detach(cc.responseHeaderTimeout()) { + return errStreamDetached + } + var respHeaderTimer <-chan time.Time var respHeaderRecv chan struct{} if d := cc.responseHeaderTimeout(); d != 0 { @@ -1348,6 +1462,7 @@ func (cs *clientStream) writeRequest(req *ClientRequest, streamf func(*clientStr respHeaderTimer = timer.C respHeaderRecv = cs.respHeaderRecv } + // Wait until the peer half-closes its end of the stream, // or until the request is aborted (via context, error, or otherwise), // whichever comes first. @@ -1433,6 +1548,10 @@ func encodeRequestHeaders(req *ClientRequest, addGzipHeader bool, peerMaxHeaderL func (cs *clientStream) cleanupWriteRequest(err error) { cc := cs.cc + if cs.stopCtxWatch != nil { + cs.stopCtxWatch() + } + if cs.ID == 0 { // We were canceled before creating the stream, so return our reservation. cc.decrStreamReservations() @@ -1443,6 +1562,10 @@ func (cs *clientStream) cleanupWriteRequest(err error) { // and in multiple cases: server replies <=299 and >299 // while still writing request body cc.mu.Lock() + if t := cs.respHeaderTimeoutTimer; t != nil { + t.Stop() + cs.respHeaderTimeoutTimer = nil + } mustCloseBody := false if cs.reqBody != nil && cs.reqBodyClosed == nil { mustCloseBody = true @@ -2154,6 +2277,13 @@ func (rl *clientConnReadLoop) processHeaders(f *MetaHeadersFrame) error { } cs.res = res close(cs.respHeaderRecv) + // Stop a detached stream's response header timeout, if armed. + rl.cc.mu.Lock() + if t := cs.respHeaderTimeoutTimer; t != nil { + t.Stop() + cs.respHeaderTimeoutTimer = nil + } + rl.cc.mu.Unlock() if f.StreamEnded() { rl.endStream(cs) } @@ -2566,6 +2696,10 @@ func (rl *clientConnReadLoop) endStream(cs *clientStream) { defer rl.cc.mu.Unlock() cs.bufPipe.closeWithErrorAndCode(io.EOF, cs.copyTrailers) close(cs.peerClosed) + if cs.detached { + cs.detached = false + go cs.cleanupWriteRequest(nil) + } } } diff --git a/src/net/http/internal/http2/transport_test.go b/src/net/http/internal/http2/transport_test.go index e3093f64e39380..6cbd8cb676a431 100644 --- a/src/net/http/internal/http2/transport_test.go +++ b/src/net/http/internal/http2/transport_test.go @@ -26,6 +26,7 @@ import ( "net/url" "os" "reflect" + "runtime" "sort" "strconv" "strings" @@ -5651,3 +5652,128 @@ func testExtendedConnectReadFrameError(t *testing.T) { t.Fatalf("after connection closed: RoundTrip succeeded; want error") } } + +// TestTransportRequestGoroutineExits verifies that the goroutine spawned to +// write a request exits once the request has been fully sent, rather than +// parking for the lifetime of the response stream. For clients with many +// concurrent long-lived streams (long polls), a parked goroutine and its +// stack per stream is a significant memory cost. +func TestTransportRequestGoroutineExits(t *testing.T) { + synctest.Test(t, testTransportRequestGoroutineExits) +} +func testTransportRequestGoroutineExits(t *testing.T) { + tc := newTestClientConn(t) + tc.greet() + + // Count the goroutines net/http has parked (the connection's read + // loop, etc.) before any request is in flight. + synctest.Wait() + base := bubbleNetHTTPGoroutines(t) + + req, _ := http.NewRequest("GET", "https://dummy.tld/", nil) + rt := tc.roundTrip(req) + + tc.wantFrameType(FrameHeaders) + tc.writeHeaders(HeadersFrameParam{ + StreamID: rt.streamID(), + EndHeaders: true, + EndStream: false, + BlockFragment: tc.makeHeaderBlockFragment(":status", "200"), + }) + rt.wantStatus(200) + + // The request is fully sent and the response is streaming with no + // end in sight. The request-writing goroutine should be gone, + // leaving only the goroutines that predate the request. + synctest.Wait() + if n := bubbleNetHTTPGoroutines(t); n != base { + t.Errorf("got %d net/http goroutines parked during long-lived response stream; want %d (the pre-request baseline)", n, base) + } + + // The stream still works and still cleans up at END_STREAM. + tc.writeData(rt.streamID(), false, []byte("hello, ")) + tc.writeData(rt.streamID(), true, []byte("world")) + rt.wantBody([]byte("hello, world")) +} + +// bubbleNetHTTPGoroutines returns the number of goroutines in the calling +// test's synctest bubble that were created by non-test functions under +// net/http. The caller must be running in a synctest bubble. +func bubbleNetHTTPGoroutines(t *testing.T) int { + buf := make([]byte, 1<<20) + buf = buf[:runtime.Stack(buf, true)] + // The first record is the calling goroutine, whose header names the + // test's bubble: "goroutine 8 [running, synctest bubble 3]:". + head, _, _ := strings.Cut(string(buf), "\n") + _, id, ok := strings.Cut(head, ", synctest bubble ") + if !ok { + t.Fatalf("calling goroutine is not in a synctest bubble: %s", head) + } + bubble := ", synctest bubble " + strings.TrimSuffix(id, "]:") + "]:" + n := 0 + for g := range strings.SplitSeq(string(buf), "\n\n") { + header, _, _ := strings.Cut(g, "\n") + if !strings.HasSuffix(header, bubble) { + continue + } + i := strings.LastIndex(g, "\ncreated by ") + if i < 0 { + continue + } + fn, loc, _ := strings.Cut(g[i+len("\ncreated by "):], "\n") + if strings.HasPrefix(fn, "net/http") && !strings.Contains(loc, "_test.go:") { + n++ + } + } + return n +} + +// TestTransportRequestGoroutineExitsRespHeaderTimeout is like +// TestTransportRequestGoroutineExits, but with a ResponseHeaderTimeout +// configured: the timeout is enforced by a timer rather than a parked +// goroutine, and once response headers arrive the timer is disarmed and +// must not fire even long after the timeout elapses. +func TestTransportRequestGoroutineExitsRespHeaderTimeout(t *testing.T) { + synctest.Test(t, testTransportRequestGoroutineExitsRespHeaderTimeout) +} +func testTransportRequestGoroutineExitsRespHeaderTimeout(t *testing.T) { + const timeout = 1 * time.Second + tc := newTestClientConn(t, func(t1 *http.Transport) { + t1.ResponseHeaderTimeout = timeout + }) + tc.greet() + + // Count the goroutines net/http has parked (the connection's read + // loop, etc.) before any request is in flight. + synctest.Wait() + base := bubbleNetHTTPGoroutines(t) + + req, _ := http.NewRequest("GET", "https://dummy.tld/", nil) + rt := tc.roundTrip(req) + + tc.wantFrameType(FrameHeaders) + + // The request-writing goroutine should be gone even before response + // headers arrive; the response header timeout is enforced by a timer. + synctest.Wait() + if n := bubbleNetHTTPGoroutines(t); n != base { + t.Errorf("got %d net/http goroutines parked awaiting response headers; want %d (the pre-request baseline)", n, base) + } + + // Response headers arrive within the timeout. + time.Sleep(timeout / 2) + tc.writeHeaders(HeadersFrameParam{ + StreamID: rt.streamID(), + EndHeaders: true, + EndStream: false, + BlockFragment: tc.makeHeaderBlockFragment(":status", "200"), + }) + rt.wantStatus(200) + + // Long after the response header timeout has elapsed, the + // still-streaming response must be unaffected. + time.Sleep(10 * timeout) + synctest.Wait() + tc.writeData(rt.streamID(), true, []byte("hello")) + rt.wantBody([]byte("hello")) +} From 42b0e69d31215fa759e47e2500e09768e59fef40 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 6 Aug 2026 15:03:03 +0000 Subject: [PATCH 13/13] net/http/internal/http2: release handler write buffers when idle Each in-flight server response held a 4KB bufio.Writer for the lifetime of its handler, including handlers that sit idle mid-response for a very long time between writes, as when streaming long polls or server-sent events. For servers with many such concurrent streams, that's 4KB of dead weight per stream. (e.g. 40 GB of RAM at 10M streaming conns) Instead of tying the buffer to the responseWriterState for the whole response, acquire it from a pool on the first buffered write and return it to the pool whenever a Flush leaves it empty. Handlers that never flush keep the buffer until the handler completes, as before. Updates #80735 Change-Id: Icf7cdb5c21abb1126f6571cb00bbe2f00bf23b3c Reviewed-on: https://go-review.googlesource.com/c/go/+/811680 Reviewed-by: Nicholas Husin Reviewed-by: Damien Neil Reviewed-by: Nicholas Husin LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com (cherry picked from commit 128a36cf0367c46daff2528de89da4225f001dcd) --- src/net/http/internal/http2/export_test.go | 13 ++++ src/net/http/internal/http2/server.go | 56 +++++++++++++--- src/net/http/internal/http2/server_test.go | 74 ++++++++++++++++++++++ 3 files changed, 134 insertions(+), 9 deletions(-) diff --git a/src/net/http/internal/http2/export_test.go b/src/net/http/internal/http2/export_test.go index 570fd3f8e5b20e..e9d7c6bead5d9d 100644 --- a/src/net/http/internal/http2/export_test.go +++ b/src/net/http/internal/http2/export_test.go @@ -25,6 +25,7 @@ func init() { const ( DefaultMaxReadFrameSize = defaultMaxReadFrameSize DefaultMaxStreams = defaultMaxStreams + HandlerChunkWriteSize = handlerChunkWriteSize InflowMinRefresh = inflowMinRefresh InitialHeaderTableSize = initialHeaderTableSize InitialMaxConcurrentStreams = initialMaxConcurrentStreams @@ -225,3 +226,15 @@ func InvalidHTTP1LookingFrameHeader() FrameHeader { func EncodeRequestHeaders(req *ClientRequest, addGzipHeader bool, peerMaxHeaderListSize uint64, headerf func(name, value string)) (httpcommon.EncodeHeadersResult, error) { return encodeRequestHeaders(req, addGzipHeader, peerMaxHeaderListSize, headerf) } + +func (w *responseWriter) hasWriteBuffer() bool { + return w.rws.bw != nil +} + +// ResponseWriterHasWriteBufferForTesting reports whether w (which must +// be or embed this package's responseWriter) currently holds a write +// buffer. It is for testing that Flush releases the buffer while a +// handler is parked mid-response. +func ResponseWriterHasWriteBufferForTesting(w any) bool { + return w.(interface{ hasWriteBuffer() bool }).hasWriteBuffer() +} diff --git a/src/net/http/internal/http2/server.go b/src/net/http/internal/http2/server.go index 12b85096f18333..a0381e9a5f671e 100644 --- a/src/net/http/internal/http2/server.go +++ b/src/net/http/internal/http2/server.go @@ -75,9 +75,20 @@ var ( var responseWriterStatePool = sync.Pool{ New: func() any { - rws := &responseWriterState{} - rws.bw = bufio.NewWriterSize(chunkWriter{rws}, handlerChunkWriteSize) - return rws + return &responseWriterState{} + }, +} + +// handlerWriterPool is a pool of the bufio.Writers used by +// responseWriterState (rws.bw) to buffer handler response writes. +// +// The buffers are acquired from the pool lazily on the first buffered +// write and, notably, are returned to it by Flush when empty, so that a +// handler that's parked mid-response for a long time (e.g. streaming a +// long poll) doesn't pin a buffer per stream. +var handlerWriterPool = sync.Pool{ + New: func() any { + return bufio.NewWriterSize(nil, handlerChunkWriteSize) }, } @@ -2246,11 +2257,8 @@ func (sc *serverConn) newWriterAndRequestNoBody(st *stream, rp httpcommon.Server func (sc *serverConn) newResponseWriter(st *stream) *responseWriter { rws := responseWriterStatePool.Get().(*responseWriterState) - bwSave := rws.bw *rws = responseWriterState{} // zero all the fields rws.conn = sc - rws.bw = bwSave - rws.bw.Reset(chunkWriter{rws}) rws.stream = st return &responseWriter{rws: rws} } @@ -2501,7 +2509,13 @@ type responseWriterState struct { conn *serverConn // TODO: adjust buffer writing sizes based on server config, frame size updates from peer, etc - bw *bufio.Writer // writing to a chunkWriter{this *responseWriterState} + // + // bw buffers handler writes, writing to a chunkWriter{this + // *responseWriterState}. It is nil until the first buffered write + // (see responseWriter.write) and is returned to handlerWriterPool + // (and set nil again) whenever a Flush leaves it empty, so a + // handler parked mid-response doesn't pin a buffer. + bw *bufio.Writer // mutated by http.Handler goroutine: handlerHeader Header // nil until called @@ -2783,12 +2797,19 @@ func (w *responseWriter) Flush() { func (w *responseWriter) FlushError() error { rws := w.rws if rws == nil { - panic("Header called after Handler finished") + panic("Flush called after Handler finished") } var err error - if rws.bw.Buffered() > 0 { + if rws.bw != nil && rws.bw.Buffered() > 0 { err = rws.bw.Flush() + if err == nil { + rws.releaseWriteBuffer() + } } else { + if rws.bw != nil { + // If a >4KB write allocated a bufio before it flushed itself, release. + rws.releaseWriteBuffer() + } // The bufio.Writer won't call chunkWriter.Write // (writeChunk with zero bytes), so we have to do it // ourselves to force the HTTP response header and/or @@ -2950,6 +2971,10 @@ func (w *responseWriter) write(lenData int, dataB []byte, dataS string) (n int, return 0, errors.New("http2: handler wrote more than declared Content-Length") } + if rws.bw == nil { + rws.bw = handlerWriterPool.Get().(*bufio.Writer) + rws.bw.Reset(chunkWriter{rws}) + } if dataB != nil { return rws.bw.Write(dataB) } else { @@ -2957,10 +2982,23 @@ func (w *responseWriter) write(lenData int, dataB []byte, dataS string) (n int, } } +// releaseWriteBuffer returns rws.bw to handlerWriterPool. +func (rws *responseWriterState) releaseWriteBuffer() { + bw := rws.bw + rws.bw = nil + bw.Reset(nil) // don't retain the chunkWriter's rws pointer in the pool + handlerWriterPool.Put(bw) +} + func (w *responseWriter) handlerDone() { rws := w.rws rws.handlerDone = true w.Flush() + if rws.bw != nil { + // A failed Flush left data (and a sticky error) behind; + // discard both and recycle the buffer. + rws.releaseWriteBuffer() + } w.rws = nil responseWriterStatePool.Put(rws) } diff --git a/src/net/http/internal/http2/server_test.go b/src/net/http/internal/http2/server_test.go index 3d42cd4a4ac404..9e1374b6e05d20 100644 --- a/src/net/http/internal/http2/server_test.go +++ b/src/net/http/internal/http2/server_test.go @@ -2368,6 +2368,80 @@ func testServer_Response_Empty_Data_Not_FlowControlled(t *testing.T) { }) } +// TestServer_Response_FlushReleasesWriteBuffer verifies that a handler's +// write buffer is released back to the pool by an empty-leaving Flush and +// lazily reacquired by the next write, so that handlers parked mid-response +// (long polls) don't pin a 4KB buffer per stream. +func TestServer_Response_FlushReleasesWriteBuffer(t *testing.T) { + synctest.Test(t, testServer_Response_FlushReleasesWriteBuffer) +} +func testServer_Response_FlushReleasesWriteBuffer(t *testing.T) { + const msg = "hello, " + const msg2 = "world" + largeMsg := bytes.Repeat([]byte("a"), HandlerChunkWriteSize*2) + testServerResponse(t, func(w http.ResponseWriter, r *http.Request) error { + if ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer allocated before first write") + } + io.WriteString(w, msg) + if !ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not allocated after buffered write") + } + w.(http.Flusher).Flush() + if ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not released by Flush") + } + io.WriteString(w, msg2) + if !ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not reacquired by write after Flush") + } + w.(http.Flusher).Flush() + if ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not released by second Flush") + } + // A []byte write larger than the 4KB write buffer bypasses + // the buffer entirely, going directly to the chunkWriter and + // leaving the buffer allocated but empty. Flush must + // release it in that case too. + w.Write(largeMsg) + if !ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not allocated by large write") + } + w.(http.Flusher).Flush() + if ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not released by Flush after buffer-bypassing write") + } + return nil + }, func(st *serverTester) { + getSlash(st) + st.wantHeaders(wantHeader{ + streamID: 1, + endStream: false, + }) + st.wantData(wantData{ + streamID: 1, + endStream: false, + data: []byte(msg), + }) + st.wantData(wantData{ + streamID: 1, + endStream: false, + data: []byte(msg2), + }) + st.wantData(wantData{ + streamID: 1, + endStream: false, + data: largeMsg, + multiple: true, + }) + st.wantData(wantData{ + streamID: 1, + endStream: true, + size: 0, + }) + }) +} + func TestServer_Response_Automatic100Continue(t *testing.T) { synctest.Test(t, testServer_Response_Automatic100Continue) }