From ba93baa74a52d57ae79313313ea990cc791ef50e Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 7 Jul 2021 16:34:34 -0700 Subject: [release-branch.go1.15] net/http/httputil: close incoming ReverseProxy request body Reading from an incoming request body after the request handler aborts with a panic can cause a panic, becuse http.Server does not (contrary to its documentation) close the request body in this case. Always close the incoming request body in ReverseProxy.ServeHTTP to ensure that any in-flight outgoing requests using the body do not read from it. Fixes #47473 Updates #46866 Fixes CVE-2021-36221 Change-Id: I310df269200ad8732c5d9f1a2b00de68725831df Reviewed-on: https://go-review.googlesource.com/c/go/+/333191 Trust: Damien Neil Reviewed-by: Brad Fitzpatrick Reviewed-by: Filippo Valsorda (cherry picked from commit b7a85e0003cedb1b48a1fd3ae5b746ec6330102e) Reviewed-on: https://go-review.googlesource.com/c/go/+/338550 Trust: Filippo Valsorda Run-TryBot: Filippo Valsorda TryBot-Result: Go Bot Reviewed-by: Damien Neil --- src/net/http/httputil/reverseproxy.go | 9 +++++++ src/net/http/httputil/reverseproxy_test.go | 39 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index f49cefbb4f..68754cb088 100644 --- a/src/net/http/httputil/reverseproxy.go +++ b/src/net/http/httputil/reverseproxy.go @@ -234,6 +234,15 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { if req.ContentLength == 0 { outreq.Body = nil // Issue 16036: nil Body for http.Transport retries } + if outreq.Body != nil { + // Reading from the request body after returning from a handler is not + // allowed, and the RoundTrip goroutine that reads the Body can outlive + // this handler. This can lead to a crash if the handler panics (see + // Issue 46866). Although calling Close doesn't guarantee there isn't + // any Read in flight after the handle returns, in practice it's safe to + // read after closing it. + defer outreq.Body.Close() + } if outreq.Header == nil { outreq.Header = make(http.Header) // Issue 33142: historical behavior was to always allocate } diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go index 1f2dfb9867..aab94da9a1 100644 --- a/src/net/http/httputil/reverseproxy_test.go +++ b/src/net/http/httputil/reverseproxy_test.go @@ -1122,6 +1122,45 @@ func TestReverseProxy_PanicBodyError(t *testing.T) { rproxy.ServeHTTP(httptest.NewRecorder(), req) } +// Issue #46866: panic without closing incoming request body causes a panic +func TestReverseProxy_PanicClosesIncomingBody(t *testing.T) { + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + out := "this call was relayed by the reverse proxy" + // Coerce a wrong content length to induce io.ErrUnexpectedEOF + w.Header().Set("Content-Length", fmt.Sprintf("%d", len(out)*2)) + fmt.Fprintln(w, out) + })) + defer backend.Close() + backendURL, err := url.Parse(backend.URL) + if err != nil { + t.Fatal(err) + } + proxyHandler := NewSingleHostReverseProxy(backendURL) + proxyHandler.ErrorLog = log.New(ioutil.Discard, "", 0) // quiet for tests + frontend := httptest.NewServer(proxyHandler) + defer frontend.Close() + frontendClient := frontend.Client() + + var wg sync.WaitGroup + for i := 0; i < 2; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 10; j++ { + const reqLen = 6 * 1024 * 1024 + req, _ := http.NewRequest("POST", frontend.URL, &io.LimitedReader{R: neverEnding('x'), N: reqLen}) + req.ContentLength = reqLen + resp, _ := frontendClient.Transport.RoundTrip(req) + if resp != nil { + io.Copy(ioutil.Discard, resp.Body) + resp.Body.Close() + } + } + }() + } + wg.Wait() +} + func TestSelectFlushInterval(t *testing.T) { tests := []struct { name string -- cgit v1.2.3-54-g00ecf From b81d75fb42f3add3fefe7cf8fa220f88895920e5 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Fri, 12 Mar 2021 09:51:50 -0800 Subject: [release-branch.go1.15] cmd/go/internal/load: always set IsImportCycle when in a cycle When hitting an import cycle in reusePackage, and there is already an error set, make sure IsImportCycle is set so that we don't end up stuck in a loop. Updates #25830 Fixes #47347 Change-Id: Iba966aea4a637dfc34ee22782a477209ac48c9bd Reviewed-on: https://go-review.googlesource.com/c/go/+/301289 Trust: Roland Shoemaker Run-TryBot: Roland Shoemaker Reviewed-by: Bryan C. Mills Reviewed-by: Jay Conrod TryBot-Result: Go Bot (cherry picked from commit cdd08e615a9b92742b21a94443720b6d70452510) Reviewed-on: https://go-review.googlesource.com/c/go/+/336669 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills --- src/cmd/go/internal/load/pkg.go | 5 +++++ src/cmd/go/testdata/script/list_err_cycle.txt | 15 +++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 src/cmd/go/testdata/script/list_err_cycle.txt diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 2b5fbb1c5b..f111e6757d 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -1280,6 +1280,11 @@ func reusePackage(p *Package, stk *ImportStack) *Package { Err: errors.New("import cycle not allowed"), IsImportCycle: true, } + } else if !p.Error.IsImportCycle { + // If the error is already set, but it does not indicate that + // we are in an import cycle, set IsImportCycle so that we don't + // end up stuck in a loop down the road. + p.Error.IsImportCycle = true } p.Incomplete = true } diff --git a/src/cmd/go/testdata/script/list_err_cycle.txt b/src/cmd/go/testdata/script/list_err_cycle.txt new file mode 100644 index 0000000000..44b82a62b0 --- /dev/null +++ b/src/cmd/go/testdata/script/list_err_cycle.txt @@ -0,0 +1,15 @@ +# Check that we don't get infinite recursion when loading a package with +# an import cycle and another error. Verifies #25830. +! go list +stderr 'found packages a \(a.go\) and b \(b.go\)' + +-- go.mod -- +module errcycle + +go 1.16 +-- a.go -- +package a + +import _ "errcycle" +-- b.go -- +package b \ No newline at end of file -- cgit v1.2.3-54-g00ecf From 818c5f6fbe136286b7c76f38a11929a60eb327bc Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Fri, 28 Jun 2019 09:30:36 -0400 Subject: [release-branch.go1.15] cmd/compile: mark R16, R17 clobbered for non-standard calls on ARM64 On ARM64, (external) linker generated trampoline may clobber R16 and R17. In CL 183842 we change Duff's devices not to use those registers. However, this is not enough. The register allocator also needs to know that these registers may be clobbered in any calls that don't follow the standard Go calling convention. This include Duff's devices and the write barrier. Fixes #46927. Updates #32773. Change-Id: Ia52a891d9bbb8515c927617dd53aee5af5bd9aa4 Reviewed-on: https://go-review.googlesource.com/c/go/+/184437 Run-TryBot: Cherry Zhang TryBot-Result: Go Bot Reviewed-by: Meng Zhuo Reviewed-by: Keith Randall Trust: Meng Zhuo (cherry picked from commit 11b4aee05bfe83513cf08f83091e5aef8b33e766) Reviewed-on: https://go-review.googlesource.com/c/go/+/331030 Trust: Cherry Mui Run-TryBot: Cherry Mui --- src/cmd/compile/internal/ssa/gen/ARM64Ops.go | 9 ++++-- src/cmd/compile/internal/ssa/opGen.go | 6 ++-- src/runtime/asm_arm64.s | 47 +++++++++++++--------------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go index b402e35ea6..2208d26df9 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go @@ -498,13 +498,14 @@ func init() { // auxint = offset into duffzero code to start executing // returns mem // R20 changed as side effect + // R16 and R17 may be clobbered by linker trampoline. { name: "DUFFZERO", aux: "Int64", argLength: 2, reg: regInfo{ inputs: []regMask{buildReg("R20")}, - clobbers: buildReg("R20 R30"), + clobbers: buildReg("R16 R17 R20 R30"), }, faultOnNilArg0: true, }, @@ -537,13 +538,14 @@ func init() { // auxint = offset into duffcopy code to start executing // returns mem // R20, R21 changed as side effect + // R16 and R17 may be clobbered by linker trampoline. { name: "DUFFCOPY", aux: "Int64", argLength: 3, reg: regInfo{ inputs: []regMask{buildReg("R21"), buildReg("R20")}, - clobbers: buildReg("R20 R21 R26 R30"), + clobbers: buildReg("R16 R17 R20 R21 R26 R30"), }, faultOnNilArg0: true, faultOnNilArg1: true, @@ -664,7 +666,8 @@ func init() { // LoweredWB invokes runtime.gcWriteBarrier. arg0=destptr, arg1=srcptr, arg2=mem, aux=runtime.gcWriteBarrier // It saves all GP registers if necessary, // but clobbers R30 (LR) because it's a call. - {name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("R2"), buildReg("R3")}, clobbers: (callerSave &^ gpg) | buildReg("R30")}, clobberFlags: true, aux: "Sym", symEffect: "None"}, + // R16 and R17 may be clobbered by linker trampoline. + {name: "LoweredWB", argLength: 3, reg: regInfo{inputs: []regMask{buildReg("R2"), buildReg("R3")}, clobbers: (callerSave &^ gpg) | buildReg("R16 R17 R30")}, clobberFlags: true, aux: "Sym", symEffect: "None"}, // There are three of these functions so that they can have three different register inputs. // When we check 0 <= c <= cap (A), then 0 <= b <= c (B), then 0 <= a <= b (C), we want the diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go index 8a27c4bb2e..39320c8b03 100644 --- a/src/cmd/compile/internal/ssa/opGen.go +++ b/src/cmd/compile/internal/ssa/opGen.go @@ -20738,7 +20738,7 @@ var opcodeTable = [...]opInfo{ inputs: []inputInfo{ {0, 1048576}, // R20 }, - clobbers: 537919488, // R20 R30 + clobbers: 538116096, // R16 R17 R20 R30 }, }, { @@ -20765,7 +20765,7 @@ var opcodeTable = [...]opInfo{ {0, 2097152}, // R21 {1, 1048576}, // R20 }, - clobbers: 607125504, // R20 R21 R26 R30 + clobbers: 607322112, // R16 R17 R20 R21 R26 R30 }, }, { @@ -21090,7 +21090,7 @@ var opcodeTable = [...]opInfo{ {0, 4}, // R2 {1, 8}, // R3 }, - clobbers: 9223372035244163072, // R30 F0 F1 F2 F3 F4 F5 F6 F7 F8 F9 F10 F11 F12 F13 F14 F15 F16 F17 F18 F19 F20 F21 F22 F23 F24 F25 F26 F27 F28 F29 F30 F31 + clobbers: 9223372035244359680, // R16 R17 R30 F0 F1 F2 F3 F4 F5 F6 F7 F8 F9 F10 F11 F12 F13 F14 F15 F16 F17 F18 F19 F20 F21 F22 F23 F24 F25 F26 F27 F28 F29 F30 F31 }, }, { diff --git a/src/runtime/asm_arm64.s b/src/runtime/asm_arm64.s index 6b3d1e779e..141ed76624 100644 --- a/src/runtime/asm_arm64.s +++ b/src/runtime/asm_arm64.s @@ -1161,10 +1161,10 @@ TEXT ·checkASM(SB),NOSPLIT,$0-1 // It does not clobber any general-purpose registers, // but may clobber others (e.g., floating point registers) // The act of CALLing gcWriteBarrier will clobber R30 (LR). -TEXT runtime·gcWriteBarrier(SB),NOSPLIT,$216 +TEXT runtime·gcWriteBarrier(SB),NOSPLIT,$200 // Save the registers clobbered by the fast path. - MOVD R0, 200(RSP) - MOVD R1, 208(RSP) + MOVD R0, 184(RSP) + MOVD R1, 192(RSP) MOVD g_m(g), R0 MOVD m_p(R0), R0 MOVD (p_wbBuf+wbBuf_next)(R0), R1 @@ -1180,8 +1180,8 @@ TEXT runtime·gcWriteBarrier(SB),NOSPLIT,$216 // Is the buffer full? (flags set in CMP above) BEQ flush ret: - MOVD 200(RSP), R0 - MOVD 208(RSP), R1 + MOVD 184(RSP), R0 + MOVD 192(RSP), R1 // Do the write. MOVD R3, (R2) RET @@ -1205,17 +1205,16 @@ flush: MOVD R13, 96(RSP) MOVD R14, 104(RSP) MOVD R15, 112(RSP) - MOVD R16, 120(RSP) - MOVD R17, 128(RSP) + // R16, R17 may be clobbered by linker trampoline // R18 is unused. - MOVD R19, 136(RSP) - MOVD R20, 144(RSP) - MOVD R21, 152(RSP) - MOVD R22, 160(RSP) - MOVD R23, 168(RSP) - MOVD R24, 176(RSP) - MOVD R25, 184(RSP) - MOVD R26, 192(RSP) + MOVD R19, 120(RSP) + MOVD R20, 128(RSP) + MOVD R21, 136(RSP) + MOVD R22, 144(RSP) + MOVD R23, 152(RSP) + MOVD R24, 160(RSP) + MOVD R25, 168(RSP) + MOVD R26, 176(RSP) // R27 is temp register. // R28 is g. // R29 is frame pointer (unused). @@ -1239,16 +1238,14 @@ flush: MOVD 96(RSP), R13 MOVD 104(RSP), R14 MOVD 112(RSP), R15 - MOVD 120(RSP), R16 - MOVD 128(RSP), R17 - MOVD 136(RSP), R19 - MOVD 144(RSP), R20 - MOVD 152(RSP), R21 - MOVD 160(RSP), R22 - MOVD 168(RSP), R23 - MOVD 176(RSP), R24 - MOVD 184(RSP), R25 - MOVD 192(RSP), R26 + MOVD 120(RSP), R19 + MOVD 128(RSP), R20 + MOVD 136(RSP), R21 + MOVD 144(RSP), R22 + MOVD 152(RSP), R23 + MOVD 160(RSP), R24 + MOVD 168(RSP), R25 + MOVD 176(RSP), R26 JMP ret // Note: these functions use a special calling convention to save generated code space. -- cgit v1.2.3-54-g00ecf From 1b68e07a24618a74d534e3e20154b4de9d004c3d Mon Sep 17 00:00:00 2001 From: Yasuhiro Matsumoto Date: Wed, 23 Jun 2021 01:02:33 +0900 Subject: [release-branch.go1.15] cmd/go: use path.Dir instead of filepath.Dir for package paths in 'go mod vendor' copyMetadata walk-up to parent directory until the pkg become modPath. But pkg should be slash-separated paths. It have to use path.Dir instead of filepath.Dir. Updates #46867 Fixes #47014 Change-Id: I44cf1429fe52379a7415b94cc30ae3275cc430e8 Reviewed-on: https://go-review.googlesource.com/c/go/+/330149 Reviewed-by: Bryan C. Mills Trust: Bryan C. Mills Trust: Alexander Rakoczy Trust: Carlos Amedee Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot (cherry picked from commit 835d86a17ebf32a3cb081f66119c74363dbd8825) Reviewed-on: https://go-review.googlesource.com/c/go/+/332330 Reviewed-by: Jay Conrod Run-TryBot: Carlos Amedee --- src/cmd/go/internal/modcmd/vendor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/internal/modcmd/vendor.go b/src/cmd/go/internal/modcmd/vendor.go index 5a5bb943a5..d83b3de3e5 100644 --- a/src/cmd/go/internal/modcmd/vendor.go +++ b/src/cmd/go/internal/modcmd/vendor.go @@ -10,6 +10,7 @@ import ( "io" "io/ioutil" "os" + "path" "path/filepath" "sort" "strings" @@ -192,7 +193,7 @@ func copyMetadata(modPath, pkg, dst, src string) { if modPath == pkg { break } - pkg = filepath.Dir(pkg) + pkg = path.Dir(pkg) dst = filepath.Dir(dst) src = filepath.Dir(src) } -- cgit v1.2.3-54-g00ecf From 7921829a57360185500fa8f1be69b9b7a0700009 Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Thu, 17 Jun 2021 20:22:40 +0000 Subject: [release-branch.go1.15] runtime: fix crash during VDSO calls on PowerPC This patch reinstates a fix for PowerPC with regard to making VDSO calls while receiving a signal, and subsequently crashing. The crash happens because certain VDSO calls can modify the r30 register, which is where g is stored. This change was reverted for PowerPC because r30 is supposed to be a non-volatile register. This is true, but that only makes a guarantee across function calls, but not "within" a function call. This patch was seemingly fine before because the Linux kernel still had hand rolled assembly VDSO function calls, however with a recent change to C function calls it seems the compiler used can generate instructions which temporarily clobber r30. This means that when we receive a signal during one of these calls the value of r30 will not be the g as the runtime expects, causing a segfault. You can see from this assembly dump how the register is clobbered during the call: (the following is from a 5.13rc2 kernel) ``` Dump of assembler code for function __cvdso_clock_gettime_data: 0x00007ffff7ff0700 <+0>: cmplwi r4,15 0x00007ffff7ff0704 <+4>: bgt 0x7ffff7ff07f0 <__cvdso_clock_gettime_data+240> 0x00007ffff7ff0708 <+8>: li r9,1 0x00007ffff7ff070c <+12>: slw r9,r9,r4 0x00007ffff7ff0710 <+16>: andi. r10,r9,2179 0x00007ffff7ff0714 <+20>: beq 0x7ffff7ff0810 <__cvdso_clock_gettime_data+272> 0x00007ffff7ff0718 <+24>: rldicr r10,r4,4,59 0x00007ffff7ff071c <+28>: lis r9,32767 0x00007ffff7ff0720 <+32>: std r30,-16(r1) 0x00007ffff7ff0724 <+36>: std r31,-8(r1) 0x00007ffff7ff0728 <+40>: add r6,r3,r10 0x00007ffff7ff072c <+44>: ori r4,r9,65535 0x00007ffff7ff0730 <+48>: lwz r8,0(r3) 0x00007ffff7ff0734 <+52>: andi. r9,r8,1 0x00007ffff7ff0738 <+56>: bne 0x7ffff7ff07d0 <__cvdso_clock_gettime_data+208> 0x00007ffff7ff073c <+60>: lwsync 0x00007ffff7ff0740 <+64>: mftb r30 <---- RIGHT HERE => 0x00007ffff7ff0744 <+68>: ld r12,40(r6) ``` What I believe is happening is that the kernel changed the PowerPC VDSO calls to use standard C calls instead of using hand rolled assembly. The hand rolled assembly calls never touched r30, so this change was safe to roll back. That does not seem to be the case anymore as on the 5.13rc2 kernel the compiler *is* generating assembly which modifies r30, making this change again unsafe and causing a crash when the program receives a signal during these calls (which will happen often due to async preempt). This change happened here: https://lwn.net/ml/linux-kernel/235e5571959cfa89ced081d7e838ed5ff38447d2.1601365870.git.christophe.leroy@csgroup.eu/. I realize this was reverted due to unexplained hangs in PowerPC builders, but I think we should reinstate this change and investigate those issues separately: https://github.com/golang/go/commit/f4ca3c1e0a2066ca4f7bd6203866d282ed34acf2 Fixes #46857 Change-Id: Ib18d7bbfc80a1a9cb558f0098878d41081324b52 GitHub-Last-Rev: c3002bcfca3ef58b27485e31328e6297b7a9dfe7 GitHub-Pull-Request: golang/go#46767 Reviewed-on: https://go-review.googlesource.com/c/go/+/328110 Run-TryBot: Lynn Boger TryBot-Result: Go Bot Reviewed-by: Cherry Mui Trust: Lynn Boger (cherry picked from commit 16e82be454cbf41299e6a055d54d489ca4612ee0) Reviewed-on: https://go-review.googlesource.com/c/go/+/334411 Run-TryBot: Carlos Amedee --- src/runtime/signal_unix.go | 2 +- src/runtime/sys_linux_ppc64x.s | 86 +++++++++++++++++++++++++++++++++++------- 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index 003c7b0bc8..8e1eb9e186 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -393,7 +393,7 @@ func preemptM(mp *m) { //go:nosplit func sigFetchG(c *sigctxt) *g { switch GOARCH { - case "arm", "arm64": + case "arm", "arm64", "ppc64", "ppc64le": if !iscgo && inVDSOPage(c.sigpc()) { // When using cgo, we save the g on TLS and load it from there // in sigtramp. Just use that. diff --git a/src/runtime/sys_linux_ppc64x.s b/src/runtime/sys_linux_ppc64x.s index fd69ee70a5..7be8c4c724 100644 --- a/src/runtime/sys_linux_ppc64x.s +++ b/src/runtime/sys_linux_ppc64x.s @@ -215,15 +215,45 @@ TEXT runtime·walltime1(SB),NOSPLIT,$16-12 MOVD (g_sched+gobuf_sp)(R7), R1 // Set SP to g0 stack noswitch: - SUB $16, R1 // Space for results - RLDICR $0, R1, $59, R1 // Align for C code + SUB $16, R1 // Space for results + RLDICR $0, R1, $59, R1 // Align for C code MOVD R12, CTR MOVD R1, R4 - BL (CTR) // Call from VDSO - MOVD $0, R0 // Restore R0 - MOVD 0(R1), R3 // sec - MOVD 8(R1), R5 // nsec - MOVD R15, R1 // Restore SP + + // Store g on gsignal's stack, so if we receive a signal + // during VDSO code we can find the g. + // If we don't have a signal stack, we won't receive signal, + // so don't bother saving g. + // When using cgo, we already saved g on TLS, also don't save + // g here. + // Also don't save g if we are already on the signal stack. + // We won't get a nested signal. + MOVBZ runtime·iscgo(SB), R22 + CMP R22, $0 + BNE nosaveg + MOVD m_gsignal(R21), R22 // g.m.gsignal + CMP R22, $0 + BEQ nosaveg + + CMP g, R22 + BEQ nosaveg + MOVD (g_stack+stack_lo)(R22), R22 // g.m.gsignal.stack.lo + MOVD g, (R22) + + BL (CTR) // Call from VDSO + + MOVD $0, (R22) // clear g slot, R22 is unchanged by C code + + JMP finish + +nosaveg: + BL (CTR) // Call from VDSO + +finish: + MOVD $0, R0 // Restore R0 + MOVD 0(R1), R3 // sec + MOVD 8(R1), R5 // nsec + MOVD R15, R1 // Restore SP // Restore vdsoPC, vdsoSP // We don't worry about being signaled between the two stores. @@ -235,7 +265,7 @@ noswitch: MOVD 32(R1), R6 MOVD R6, m_vdsoPC(R21) -finish: +return: MOVD R3, sec+0(FP) MOVW R5, nsec+8(FP) RET @@ -246,7 +276,7 @@ fallback: SYSCALL $SYS_clock_gettime MOVD 32(R1), R3 MOVD 40(R1), R5 - JMP finish + JMP return TEXT runtime·nanotime1(SB),NOSPLIT,$16-8 MOVD $1, R3 // CLOCK_MONOTONIC @@ -282,7 +312,37 @@ noswitch: RLDICR $0, R1, $59, R1 // Align for C code MOVD R12, CTR MOVD R1, R4 - BL (CTR) // Call from VDSO + + // Store g on gsignal's stack, so if we receive a signal + // during VDSO code we can find the g. + // If we don't have a signal stack, we won't receive signal, + // so don't bother saving g. + // When using cgo, we already saved g on TLS, also don't save + // g here. + // Also don't save g if we are already on the signal stack. + // We won't get a nested signal. + MOVBZ runtime·iscgo(SB), R22 + CMP R22, $0 + BNE nosaveg + MOVD m_gsignal(R21), R22 // g.m.gsignal + CMP R22, $0 + BEQ nosaveg + + CMP g, R22 + BEQ nosaveg + MOVD (g_stack+stack_lo)(R22), R22 // g.m.gsignal.stack.lo + MOVD g, (R22) + + BL (CTR) // Call from VDSO + + MOVD $0, (R22) // clear g slot, R22 is unchanged by C code + + JMP finish + +nosaveg: + BL (CTR) // Call from VDSO + +finish: MOVD $0, R0 // Restore R0 MOVD 0(R1), R3 // sec MOVD 8(R1), R5 // nsec @@ -298,7 +358,7 @@ noswitch: MOVD 32(R1), R6 MOVD R6, m_vdsoPC(R21) -finish: +return: // sec is in R3, nsec in R5 // return nsec in R3 MOVD $1000000000, R4 @@ -313,7 +373,7 @@ fallback: SYSCALL $SYS_clock_gettime MOVD 32(R1), R3 MOVD 40(R1), R5 - JMP finish + JMP return TEXT runtime·rtsigprocmask(SB),NOSPLIT|NOFRAME,$0-28 MOVW how+0(FP), R3 @@ -366,7 +426,7 @@ TEXT sigtramp<>(SB),NOSPLIT,$64 // this might be called in external code context, // where g is not set. MOVBZ runtime·iscgo(SB), R6 - CMP R6, $0 + CMP R6, $0 BEQ 2(PC) BL runtime·load_g(SB) -- cgit v1.2.3-54-g00ecf From e90f40284eb02236b9efcfd6e8eec51082f4dd8b Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 3 Aug 2021 19:38:37 -0700 Subject: [release-branch.go1.15] net/http: speed up and deflake TestCancelRequestWhenSharingConnection This test made many requests over the same connection for 10 seconds, trusting that this will exercise the request cancelation race from #41600. Change the test to exhibit the specific race in a targeted fashion with only two requests. Fixes #47534. Updates #41600. Updates #47016. Change-Id: If99c9b9331ff645f6bb67fe9fb79b8aab8784710 Reviewed-on: https://go-review.googlesource.com/c/go/+/339594 Trust: Damien Neil Run-TryBot: Damien Neil TryBot-Result: Go Bot Reviewed-by: Heschi Kreinick (cherry picked from commit 6e738868a7a943d7d4fd6bb1963e7f6d78111726) Reviewed-on: https://go-review.googlesource.com/c/go/+/339829 --- src/net/http/transport_test.go | 77 ++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 26 deletions(-) diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index faa77e9ccd..83c85eae72 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -6290,10 +6290,11 @@ func TestTransportRejectsSignInContentLength(t *testing.T) { // Test that a new request which uses the connection of an active request // cannot cause it to be canceled as well. func TestCancelRequestWhenSharingConnection(t *testing.T) { - if testing.Short() { - t.Skip("skipping in short mode") - } + reqc := make(chan chan struct{}, 2) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, req *Request) { + ch := make(chan struct{}, 1) + reqc <- ch + <-ch w.Header().Add("Content-Length", "0") })) defer ts.Close() @@ -6305,34 +6306,58 @@ func TestCancelRequestWhenSharingConnection(t *testing.T) { var wg sync.WaitGroup - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + wg.Add(1) + putidlec := make(chan chan struct{}) + go func() { + defer wg.Done() + ctx := httptrace.WithClientTrace(context.Background(), &httptrace.ClientTrace{ + PutIdleConn: func(error) { + // Signal that the idle conn has been returned to the pool, + // and wait for the order to proceed. + ch := make(chan struct{}) + putidlec <- ch + <-ch + }, + }) + req, _ := NewRequestWithContext(ctx, "GET", ts.URL, nil) + res, err := client.Do(req) + if err == nil { + res.Body.Close() + } + if err != nil { + t.Errorf("request 1: got err %v, want nil", err) + } + }() - for i := 0; i < 10; i++ { - wg.Add(1) - go func() { - defer wg.Done() - for ctx.Err() == nil { - reqctx, reqcancel := context.WithCancel(ctx) - go reqcancel() - req, _ := NewRequestWithContext(reqctx, "GET", ts.URL, nil) - res, err := client.Do(req) - if err == nil { - res.Body.Close() - } - } - }() - } + // Wait for the first request to receive a response and return the + // connection to the idle pool. + r1c := <-reqc + close(r1c) + idlec := <-putidlec - for ctx.Err() == nil { - req, _ := NewRequest("GET", ts.URL, nil) - if res, err := client.Do(req); err != nil { - t.Errorf("unexpected: %p %v", req, err) - break - } else { + wg.Add(1) + cancelctx, cancel := context.WithCancel(context.Background()) + go func() { + defer wg.Done() + req, _ := NewRequestWithContext(cancelctx, "GET", ts.URL, nil) + res, err := client.Do(req) + if err == nil { res.Body.Close() } - } + if !errors.Is(err, context.Canceled) { + t.Errorf("request 2: got err %v, want Canceled", err) + } + }() + // Wait for the second request to arrive at the server, and then cancel + // the request context. + r2c := <-reqc cancel() + + // Give the cancelation a moment to take effect, and then unblock the first request. + time.Sleep(1 * time.Millisecond) + close(idlec) + + close(r2c) wg.Wait() } -- cgit v1.2.3-54-g00ecf From acbe242f8a2cae8ef4749806291a37d23089b572 Mon Sep 17 00:00:00 2001 From: David Chase Date: Wed, 4 Aug 2021 16:54:44 +0000 Subject: [release-branch.go1.15] go1.15.15 Change-Id: I42891b98d822d57aaead8815d2a32ea625e8437b Reviewed-on: https://go-review.googlesource.com/c/go/+/339450 Run-TryBot: David Chase TryBot-Result: Go Bot Reviewed-by: Damien Neil Reviewed-by: Alexander Rakoczy Trust: Damien Neil --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index cda321dacc..b035c3cf19 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -go1.15.14 \ No newline at end of file +go1.15.15 \ No newline at end of file -- cgit v1.2.3-54-g00ecf