From c25b12fb815938d6fa894cd1552c3c78825c6254 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Fri, 15 Apr 2022 12:23:06 -0400 Subject: [release-branch.go1.17] runtime: use saved LR when unwinding through morestack On LR machine, consider F calling G calling H, which grows stack. The stack looks like ... G's frame: ... locals ... saved LR = return PC in F <- SP points here at morestack H's frame (to be created) At morestack, we save gp.sched.pc = H's morestack call gp.sched.sp = H's entry SP (the arrow above) gp.sched.lr = return PC in G Currently, when unwinding through morestack (if _TraceJumpStack is set), we switch PC and SP but not LR. We then have frame.pc = H's morestack call frame.sp = H's entry SP (the arrow above) As LR is not set, we load it from stack at *sp, so frame.lr = return PC in F As the SP hasn't decremented at the morestack call, frame.fp = frame.sp = H's entry SP Unwinding a frame, we have frame.pc = old frame.lr = return PC in F frame.sp = old frame.fp = H's entry SP a.k.a. G's SP The PC and SP don't match. The unwinding will go off if F and G have different frame sizes. Fix this by preserving the LR when switching stack. Also add code to detect infinite loop in unwinding. TODO: add some test. I can reproduce the infinite loop (or throw with added check) but the frequency is low. Fixes #53111. Updates #52116. Change-Id: I6e1294f1c6e55f664c962767a1cf6c466a0c0eff Reviewed-on: https://go-review.googlesource.com/c/go/+/400575 TryBot-Result: Gopher Robot Run-TryBot: Cherry Mui Reviewed-by: Eric Fang Reviewed-by: Benny Siegert (cherry picked from commit 74f00094220f26c80fbaab6eca28c3a664897d24) Reviewed-on: https://go-review.googlesource.com/c/go/+/408822 Reviewed-by: Austin Clements --- src/runtime/traceback.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 814c3236345..de593491a13 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -116,6 +116,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } waspanic := false cgoCtxt := gp.cgoCtxt + stack := gp.stack printing := pcbuf == nil && callback == nil // If the PC is zero, it's likely a nil function call. @@ -134,7 +135,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in if !f.valid() { if callback != nil || printing { print("runtime: unknown pc ", hex(frame.pc), "\n") - tracebackHexdump(gp.stack, &frame, 0) + tracebackHexdump(stack, &frame, 0) } if callback != nil { throw("unknown pc") @@ -194,12 +195,15 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in frame.fn = findfunc(frame.pc) f = frame.fn flag = f.flag + frame.lr = gp.m.curg.sched.lr frame.sp = gp.m.curg.sched.sp + stack = gp.m.curg.stack cgoCtxt = gp.m.curg.cgoCtxt case funcID_systemstack: // systemstack returns normally, so just follow the // stack transition. frame.sp = gp.m.curg.sched.sp + stack = gp.m.curg.stack cgoCtxt = gp.m.curg.cgoCtxt flag &^= funcFlag_SPWRITE } @@ -268,7 +272,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } if callback != nil || doPrint { print("runtime: unexpected return pc for ", funcname(f), " called from ", hex(frame.lr), "\n") - tracebackHexdump(gp.stack, &frame, lrPtr) + tracebackHexdump(stack, &frame, lrPtr) } if callback != nil { throw("unknown caller pc") @@ -497,6 +501,13 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in break } + if frame.pc == frame.lr && frame.sp == frame.fp { + // If the next frame is identical to the current frame, we cannot make progress. + print("runtime: traceback stuck. pc=", hex(frame.pc), " sp=", hex(frame.sp), "\n") + tracebackHexdump(stack, &frame, frame.sp) + throw("traceback stuck") + } + // Unwind to next frame. frame.fn = flr frame.pc = frame.lr -- cgit v1.2.3-54-g00ecf From 66c60f076c167f252b0d8f0ac37f4c5c0c2adb51 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Wed, 13 Jul 2022 11:48:04 -0400 Subject: [release-branch.go1.17] runtime: clear timerModifiedEarliest when last timer is deleted timerModifiedEarliest contains the lowest possible expiration for a modified earlier timer, which may be earlier than timer0When because we haven't yet updated the heap. Note "may", as the modified earlier timer that set timerModifiedEarliest may have since been modified later or deleted. We can clear timerModifiedEarliest when the last timer is deleted because by definition there must not be any modified earlier timers. Why does this matter? checkTimersNoP claims that there is work to do if timerModifiedEarliest has passed, causing findRunnable to loop back around to checkTimers. But the code to clean up timerModifiedEarliest in checkTimers (i.e., the call to adjusttimers) is conditional behind a check that len(pp.timers) > 0. Without clearing timerModifiedEarliest, a spinning M that would otherwise go to sleep will busy loop in findRunnable until some other work is available. Note that changing the condition on the call to adjusttimers would also be a valid fix. I took this approach because it feels a bit cleaner to clean up timerModifiedEarliest as soon as it is known to be irrelevant. For #51654. Fixes #53846. Change-Id: I3f3787c67781cac7ce87939c5706cef8db927dd5 Reviewed-on: https://go-review.googlesource.com/c/go/+/417434 Auto-Submit: Michael Pratt TryBot-Result: Gopher Robot Run-TryBot: Michael Pratt Reviewed-by: Michael Knyszek Reviewed-by: Ian Lance Taylor (cherry picked from commit c006b7ac2765252f397dec40fef610a3c17d956d) Reviewed-on: https://go-review.googlesource.com/c/go/+/417476 --- src/runtime/time.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/runtime/time.go b/src/runtime/time.go index 517a493a700..70e21d4482e 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -390,7 +390,11 @@ func dodeltimer(pp *p, i int) int { if i == 0 { updateTimer0When(pp) } - atomic.Xadd(&pp.numTimers, -1) + n := atomic.Xadd(&pp.numTimers, -1) + if n == 0 { + // If there are no timers, then clearly none are modified. + atomic.Store64(&pp.timerModifiedEarliest, 0) + } return smallestChanged } @@ -414,7 +418,11 @@ func dodeltimer0(pp *p) { siftdownTimer(pp.timers, 0) } updateTimer0When(pp) - atomic.Xadd(&pp.numTimers, -1) + n := atomic.Xadd(&pp.numTimers, -1) + if n == 0 { + // If there are no timers, then clearly none are modified. + atomic.Store64(&pp.timerModifiedEarliest, 0) + } } // modtimer modifies an existing timer. -- cgit v1.2.3-54-g00ecf From 489c1485780f6326fb5f446bd9d97dff2ff0abcf Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 29 Jun 2022 13:22:59 -0700 Subject: [release-branch.go1.17] cmd/compile: fix prove pass when upper condition is <= maxint When the terminating condition is <= X, we need to make sure that X+step doesn't overflow. Fixes #53617 Change-Id: I36e5384d05b4d7168e48db6094200fcae409bfe5 Reviewed-on: https://go-review.googlesource.com/c/go/+/415219 Reviewed-by: Than McIntosh Run-TryBot: David Chase Reviewed-by: David Chase TryBot-Result: Gopher Robot Run-TryBot: Keith Randall (cherry picked from commit 31b8c23c5702f129aca9241bbb2132c90b1929cc) Reviewed-on: https://go-review.googlesource.com/c/go/+/415415 Reviewed-by: Keith Randall --- src/cmd/compile/internal/ssa/loopbce.go | 7 ++++++ test/fixedbugs/issue53600.go | 42 +++++++++++++++++++++++++++++++++ test/fixedbugs/issue53600.out | 8 +++++++ 3 files changed, 57 insertions(+) create mode 100644 test/fixedbugs/issue53600.go create mode 100644 test/fixedbugs/issue53600.out diff --git a/src/cmd/compile/internal/ssa/loopbce.go b/src/cmd/compile/internal/ssa/loopbce.go index 5a4bc1d60aa..fd03efb417d 100644 --- a/src/cmd/compile/internal/ssa/loopbce.go +++ b/src/cmd/compile/internal/ssa/loopbce.go @@ -159,6 +159,13 @@ func findIndVar(f *Func) []indVar { step = -step } + if flags&indVarMaxInc != 0 && max.Op == OpConst64 && max.AuxInt+step < max.AuxInt { + // For a <= comparison, we need to make sure that a value equal to + // max can be incremented without overflowing. + // (For a < comparison, the %step check below ensures no overflow.) + continue + } + // Up to now we extracted the induction variable (ind), // the increment delta (inc), the temporary sum (nxt), // the mininum value (min) and the maximum value (max). diff --git a/test/fixedbugs/issue53600.go b/test/fixedbugs/issue53600.go new file mode 100644 index 00000000000..fd3a9e5e470 --- /dev/null +++ b/test/fixedbugs/issue53600.go @@ -0,0 +1,42 @@ +// run + +// Copyright 2022 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 "math" + +func main() { + f() + g() + h() +} +func f() { + for i := int64(math.MaxInt64); i <= math.MaxInt64; i++ { + if i < 0 { + println("done") + return + } + println(i, i < 0) + } +} +func g() { + for i := int64(math.MaxInt64) - 1; i <= math.MaxInt64; i++ { + if i < 0 { + println("done") + return + } + println(i, i < 0) + } +} +func h() { + for i := int64(math.MaxInt64) - 2; i <= math.MaxInt64; i += 2 { + if i < 0 { + println("done") + return + } + println(i, i < 0) + } +} diff --git a/test/fixedbugs/issue53600.out b/test/fixedbugs/issue53600.out new file mode 100644 index 00000000000..5590c7dcfb7 --- /dev/null +++ b/test/fixedbugs/issue53600.out @@ -0,0 +1,8 @@ +9223372036854775807 false +done +9223372036854775806 false +9223372036854775807 false +done +9223372036854775805 false +9223372036854775807 false +done -- cgit v1.2.3-54-g00ecf From d9242f7a8c29aa17201cd66d29cdd20916c2de60 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Wed, 18 May 2022 11:58:53 -0400 Subject: [release-branch.go1.17] cmd/compile: do not use special literal assignment if LHS is address-taken A composite literal assignment x = T{field: v} may be compiled to x = T{} x.field = v We already do not use this form is RHS uses LHS. If LHS is address-taken, RHS may uses LHS implicitly, e.g. v = &x.field x = T{field: *v} The lowering above would change the value of RHS (*v). Updates #52953. Fixes #52960. Change-Id: I3f798e00598aaa550b8c17182c7472fef440d483 Reviewed-on: https://go-review.googlesource.com/c/go/+/407014 Reviewed-by: Cuong Manh Le Run-TryBot: Cherry Mui TryBot-Result: Gopher Robot Reviewed-by: Michael Knyszek (cherry picked from commit 1c77137d4fdfbb3e7e8d9efaab3bab5ee736a19d) Reviewed-on: https://go-review.googlesource.com/c/go/+/419451 Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/walk/complit.go | 8 +++++++- test/fixedbugs/issue52953.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue52953.go diff --git a/src/cmd/compile/internal/walk/complit.go b/src/cmd/compile/internal/walk/complit.go index abd920d6461..1f288d21c18 100644 --- a/src/cmd/compile/internal/walk/complit.go +++ b/src/cmd/compile/internal/walk/complit.go @@ -621,6 +621,12 @@ func oaslit(n *ir.AssignStmt, init *ir.Nodes) bool { // not a special composite literal assignment return false } + if x.Addrtaken() { + // If x is address-taken, the RHS may (implicitly) uses LHS. + // Not safe to do a special composite literal assignment + // (which may expand to multiple assignments). + return false + } switch n.Y.Op() { default: @@ -629,7 +635,7 @@ func oaslit(n *ir.AssignStmt, init *ir.Nodes) bool { case ir.OSTRUCTLIT, ir.OARRAYLIT, ir.OSLICELIT, ir.OMAPLIT: if ir.Any(n.Y, func(y ir.Node) bool { return ir.Uses(y, x) }) { - // not a special composite literal assignment + // not safe to do a special composite literal assignment if RHS uses LHS. return false } anylit(n.Y, n.X, init) diff --git a/test/fixedbugs/issue52953.go b/test/fixedbugs/issue52953.go new file mode 100644 index 00000000000..2085e4e3fe3 --- /dev/null +++ b/test/fixedbugs/issue52953.go @@ -0,0 +1,29 @@ +// run + +// Copyright 2022 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 52953: miscompilation for composite literal assignment +// when LHS is address-taken. + +package main + +type T struct { + Field1 bool +} + +func main() { + var ret T + ret.Field1 = true + var v *bool = &ret.Field1 + ret = T{Field1: *v} + check(ret.Field1) +} + +//go:noinline +func check(b bool) { + if !b { + panic("FAIL") + } +} -- cgit v1.2.3-54-g00ecf From 703c8ab7e5ba75c95553d4e249309297abad7102 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Fri, 15 Jul 2022 10:43:44 -0700 Subject: [release-branch.go1.17] math/big: check buffer lengths in GobDecode In Float.GobDecode and Rat.GobDecode, check buffer sizes before indexing slices. Updates #53871 Fixes #54094 Change-Id: I1b652c32c2bc7a0e8aa7620f7be9b2740c568b0a Reviewed-on: https://go-review.googlesource.com/c/go/+/417774 TryBot-Result: Gopher Robot Reviewed-by: Tatiana Bradley Run-TryBot: Roland Shoemaker (cherry picked from commit 055113ef364337607e3e72ed7d48df67fde6fc66) Reviewed-on: https://go-review.googlesource.com/c/go/+/419814 Reviewed-by: Julie Qiu --- src/math/big/floatmarsh.go | 7 +++++++ src/math/big/floatmarsh_test.go | 12 ++++++++++++ src/math/big/ratmarsh.go | 6 ++++++ src/math/big/ratmarsh_test.go | 12 ++++++++++++ 4 files changed, 37 insertions(+) diff --git a/src/math/big/floatmarsh.go b/src/math/big/floatmarsh.go index d1c1dab0691..990e085abe8 100644 --- a/src/math/big/floatmarsh.go +++ b/src/math/big/floatmarsh.go @@ -8,6 +8,7 @@ package big import ( "encoding/binary" + "errors" "fmt" ) @@ -67,6 +68,9 @@ func (z *Float) GobDecode(buf []byte) error { *z = Float{} return nil } + if len(buf) < 6 { + return errors.New("Float.GobDecode: buffer too small") + } if buf[0] != floatGobVersion { return fmt.Errorf("Float.GobDecode: encoding version %d not supported", buf[0]) @@ -83,6 +87,9 @@ func (z *Float) GobDecode(buf []byte) error { z.prec = binary.BigEndian.Uint32(buf[2:]) if z.form == finite { + if len(buf) < 10 { + return errors.New("Float.GobDecode: buffer too small for finite form float") + } z.exp = int32(binary.BigEndian.Uint32(buf[6:])) z.mant = z.mant.setBytes(buf[10:]) } diff --git a/src/math/big/floatmarsh_test.go b/src/math/big/floatmarsh_test.go index c056d78b800..401f45a51fe 100644 --- a/src/math/big/floatmarsh_test.go +++ b/src/math/big/floatmarsh_test.go @@ -137,3 +137,15 @@ func TestFloatJSONEncoding(t *testing.T) { } } } + +func TestFloatGobDecodeShortBuffer(t *testing.T) { + for _, tc := range [][]byte{ + []byte{0x1, 0x0, 0x0, 0x0}, + []byte{0x1, 0xfa, 0x0, 0x0, 0x0, 0x0}, + } { + err := NewFloat(0).GobDecode(tc) + if err == nil { + t.Error("expected GobDecode to return error for malformed input") + } + } +} diff --git a/src/math/big/ratmarsh.go b/src/math/big/ratmarsh.go index fbc7b6002d9..56102e845b7 100644 --- a/src/math/big/ratmarsh.go +++ b/src/math/big/ratmarsh.go @@ -45,12 +45,18 @@ func (z *Rat) GobDecode(buf []byte) error { *z = Rat{} return nil } + if len(buf) < 5 { + return errors.New("Rat.GobDecode: buffer too small") + } b := buf[0] if b>>1 != ratGobVersion { return fmt.Errorf("Rat.GobDecode: encoding version %d not supported", b>>1) } const j = 1 + 4 i := j + binary.BigEndian.Uint32(buf[j-4:j]) + if len(buf) < int(i) { + return errors.New("Rat.GobDecode: buffer too small") + } z.a.neg = b&1 != 0 z.a.abs = z.a.abs.setBytes(buf[j:i]) z.b.abs = z.b.abs.setBytes(buf[i:]) diff --git a/src/math/big/ratmarsh_test.go b/src/math/big/ratmarsh_test.go index 351d109f8d8..55a9878bb87 100644 --- a/src/math/big/ratmarsh_test.go +++ b/src/math/big/ratmarsh_test.go @@ -123,3 +123,15 @@ func TestRatXMLEncoding(t *testing.T) { } } } + +func TestRatGobDecodeShortBuffer(t *testing.T) { + for _, tc := range [][]byte{ + []byte{0x2}, + []byte{0x2, 0x0, 0x0, 0x0, 0xff}, + } { + err := NewRat(1, 2).GobDecode(tc) + if err == nil { + t.Error("expected GobDecode to return error for malformed input") + } + } +} -- cgit v1.2.3-54-g00ecf From 15da892a4950a4caac987ee72c632436329f62d5 Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Mon, 1 Aug 2022 14:51:55 +0000 Subject: [release-branch.go1.17] go1.17.13 Change-Id: Id21203787dc0bbca2548044d7bcc442204dfdd7d Reviewed-on: https://go-review.googlesource.com/c/go/+/420554 Auto-Submit: Gopher Robot Reviewed-by: Heschi Kreinick Run-TryBot: Gopher Robot Reviewed-by: Cherry Mui Reviewed-by: Dmitri Shuralyov TryBot-Result: Gopher Robot --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 760e3832356..1db9985a169 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -go1.17.12 \ No newline at end of file +go1.17.13 \ No newline at end of file -- cgit v1.2.3-54-g00ecf