aboutsummaryrefslogtreecommitdiff
path: root/test/fixedbugs
AgeCommit message (Collapse)Author
2025-06-26cmd/compile/internal/escape: evaluate any side effects when rewriting with ↵thepudds
literals CL 649035 and CL 649079 updated escape analysis to rewrite certain operands in OMAKE and OCONVIFACE nodes from non-constant expressions to basic literals that evaluate to the same value. However, when doing that rewriting, we need to evaluate any side effects prior to replacing the expression, which is what this CL now does. Issue #74379 reported a problem with OCONVIFACE nodes due to CL 649079. CL 649035 has essentially the same issue with OMAKE nodes. To illustrate that, we add a test for the OMAKE case in fixedbugs/issue74379b.go, which fails without this change. To avoid introducing an unnecessary temporary for OMAKE nodes, we also conditionalize the main work of CL 649035 on whether the OMAKE operand is already an OLITERAL. CL 649555 and CL 649078 were related changes that created read-only global storage for composite literals used in an interface conversion. This CL adds a test in fixedbugs/issue74379c.go to illustrate that they do not have the same problem. Updates #71359 Fixes #74379 Change-Id: I6645575ef34f1fe2b0241a22dc205875d66b7ada Reviewed-on: https://go-review.googlesource.com/c/go/+/684116 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Keith Randall <khr@google.com>
2025-06-04test: add another regression test for issue 73309Cuong Manh Le
Fixed #73309 Change-Id: Id715b9c71c95c92143a7fdb5a66b24305346dd3b Reviewed-on: https://go-review.googlesource.com/c/go/+/678415 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2025-05-28cmd/compile: do nil check before calling duff functions, on arm64 and amd64Keith Randall
On these platforms, we set up a frame pointer record below the current stack pointer, so when we're in duffcopy or duffzero, we get a reasonable traceback. See #73753. But because this frame pointer record is below SP, it is vulnerable. Anything that adds a new stack frame to the stack might clobber it. Which actually happens in #73748 on amd64. I have not yet come across a repro on arm64, but might as well be safe here. The only real situation this could happen is when duffzero or duffcopy is passed a nil pointer. So we can just avoid the problem by doing the nil check outside duffzero/duffcopy. That way we never add a frame below duffzero/duffcopy. (Most other ways to get a new frame below the current one, like async preempt or debugger-generated calls, don't apply to duffzero/duffcopy because they are runtime functions; we're not allowed to preempt there.) Longer term, we should stop putting stuff below SP. #73753 will include that as part of its remit. But that's not for 1.25, so we'll do the simple thing for 1.25 for this issue. Fixes #73748 Change-Id: I913c49ee46dcaee8fb439415a4531f7b59d0f612 Reviewed-on: https://go-review.googlesource.com/c/go/+/676916 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@google.com>
2025-05-27cmd/compile/internal/walk: use original type for composite literals in addrTempthepudds
When creating a new *ir.Name or *ir.LinksymOffsetExpr to represent a composite literal stored in the read-only data section, we should use the original type of the expression that was found via ir.ReassignOracle.StaticValue. (This is needed because the StaticValue method can traverse through OCONVNOP operations to find its final result.) Otherwise, the compilation may succeed, but the linker might erroneously conclude that a type is not used and prune an itab when it should not, leading to a call at execution-time to runtime.unreachableMethod, which throws "fatal error: unreachable method called. linker bug?". The tests exercise both the case of a zero value struct literal that can be represented by the read-only runtime.zeroVal, which was the case of the simplified example from #73888, and also modifies that example to test the non zero value struct literal case. This CL makes two similar changes for those two cases. We can get either of the tests we are adding to fail independently if we only make a single corresponding change. Fixes #73888 Updates #71359 Change-Id: Ifd91f445cc168ab895cc27f7964a6557d5cc32e5 Reviewed-on: https://go-review.googlesource.com/c/go/+/676517 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Auto-Submit: Keith Randall <khr@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2025-05-22cmd/compile: do not shapify when reading reshaping exprCuong Manh Le
Fixes #71184 Change-Id: I22e7ae5203311e86a90502bfe155b0597007887d Reviewed-on: https://go-review.googlesource.com/c/go/+/641955 Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
2025-05-22cmd/compile: fix ICE with recursive alias type parameterCuong Manh Le
CL 585399 fixed an initialization loop during IR contruction that involving alias type, by avoiding publishing alias declarations until the RHS type expression has been constructed. There's an assertion to ensure that the alias's type must be the same during the initialization. However, that assertion is too strict, since we may construct different instances of the same type, if the type is an instantination of generic type. To fix this, we could use types.IdenticalStrict to ensure that these types matching exactly. Updates #66873. Updates #73309. Change-Id: I2559bed37e21615854333fb1057d7349406e6a1b Reviewed-on: https://go-review.googlesource.com/c/go/+/668175 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
2025-05-22cmd/compile: fix ICE when transforming loopvarCuong Manh Le
When transforming for loop variables, the compiler does roughly following steps: (1) prebody = {z := z' for z in leaked} ... (4) init' = (init : s/z/z' for z in leaked) However, the definition of z is not updated to `z := z'` statement, causing ReassignOracle incorrectly use the new init statement with z' instead of z, trigger the ICE. Fixing this by updating the correct/new definition statement for z during the prebody initialization. Fixes #73823 Change-Id: Ice2a6741be7478506c58f4000f591d5582029136 Reviewed-on: https://go-review.googlesource.com/c/go/+/675475 Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
2025-05-21cmd/compile/internal/escape: propagate constants to interface conversions to ↵thepudds
avoid allocs Currently, the integer value in the following interface conversion gets heap allocated: v := 1000 fmt.Println(v) In contrast, this conversion does not currently cause the integer value to be heap allocated: fmt.Println(1000) The second example is able to avoid heap allocation because of an optimization in walk (by Josh in #18704 and related issues) that recognizes a literal is being used. In the first example, that optimization is currently thwarted by the literal getting assigned to a local variable prior to use in the interface conversion. This CL propagates constants to interface conversions like in the first example to avoid heap allocations, instead using a read-only global. The net effect is roughly turning the first example into the second. One place this comes up in practice currently is with logging or debug prints. For example, if we have something like: func conditionalDebugf(format string, args ...interface{}) { if debugEnabled { fmt.Fprintf(io.Discard, format, args...) } } Prior to this CL, this integer is heap allocated, even when the debugEnabled flag is false, and even when the compiler inlines conditionalDebugf: v := 1000 conditionalDebugf("hello %d", v) With this CL, the integer here is no longer heap allocated, even when the debugEnabled flag is enabled, because the compiler can now see that it can use a read-only global. See the writeup in #71359 for more details. CL 649076 (earlier in our stack) added most of the tests along with debug diagnostics in convert.go to make it easier to test this change. Updates #71359 Updates #62653 Updates #53465 Updates #8618 Change-Id: I19a51e74b36576ebb0b9cf599267cbd2bd847ce4 Reviewed-on: https://go-review.googlesource.com/c/go/+/649079 Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com>
2025-05-19cmd/compile: allocate backing store for append on the stackKeith Randall
When appending, if the backing store doesn't escape and a constant-sized backing store is big enough, use a constant-sized stack-allocated backing store instead of allocating it from the heap. cmd/go is <0.1% bigger. As an example of how this helps, if you edit strings/strings.go:FieldsFunc to replace spans := make([]span, 0, 32) with var spans []span then this CL removes the first 2 allocations that are part of the growth sequence: │ base │ exp │ │ allocs/op │ allocs/op vs base │ FieldsFunc/ASCII/16-24 3.000 ± ∞ ¹ 2.000 ± ∞ ¹ -33.33% (p=0.008 n=5) FieldsFunc/ASCII/256-24 7.000 ± ∞ ¹ 5.000 ± ∞ ¹ -28.57% (p=0.008 n=5) FieldsFunc/ASCII/4096-24 11.000 ± ∞ ¹ 9.000 ± ∞ ¹ -18.18% (p=0.008 n=5) FieldsFunc/ASCII/65536-24 18.00 ± ∞ ¹ 16.00 ± ∞ ¹ -11.11% (p=0.008 n=5) FieldsFunc/ASCII/1048576-24 30.00 ± ∞ ¹ 28.00 ± ∞ ¹ -6.67% (p=0.008 n=5) FieldsFunc/Mixed/16-24 2.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=5) FieldsFunc/Mixed/256-24 7.000 ± ∞ ¹ 5.000 ± ∞ ¹ -28.57% (p=0.008 n=5) FieldsFunc/Mixed/4096-24 11.000 ± ∞ ¹ 9.000 ± ∞ ¹ -18.18% (p=0.008 n=5) FieldsFunc/Mixed/65536-24 18.00 ± ∞ ¹ 16.00 ± ∞ ¹ -11.11% (p=0.008 n=5) FieldsFunc/Mixed/1048576-24 30.00 ± ∞ ¹ 28.00 ± ∞ ¹ -6.67% (p=0.008 n=5) (Of course, people have spotted and fixed a bunch of allocation sites like this, but now we're ~automatically doing it everywhere going forward.) No significant increases in frame sizes in cmd/go. Change-Id: I301c4d9676667eacdae0058960321041d173751a Reviewed-on: https://go-review.googlesource.com/c/go/+/664299 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Keith Randall <khr@golang.org>
2025-05-19go/types, types2: improve error message for init without bodyMark Freeman
Change-Id: I8a684965e88e0e33a6ff33a16e08d136e3267f7e Reviewed-on: https://go-review.googlesource.com/c/go/+/663636 TryBot-Bypass: Mark Freeman <mark@golang.org> Auto-Submit: Mark Freeman <mark@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
2025-05-15cmd/compile: create LSym for closures with type conversionYongyue Sun
Follow-up to #54959 with another failing case. The linker needs FuncInfo metadata for all inlined functions. CL 436240 explicitly creates LSym for direct closure calls to ensure we keep the FuncInfo metadata. However, CL 436240 won't work if the direct closure call is wrapped by a no-effect type conversion, even if that closure could be inlined. This commit should fix such case. Fixes #73716 Change-Id: Icda6024da54c8d933f87300e691334c080344695 GitHub-Last-Rev: e9aed02eb6ef343e4ed2d8a79f6426abf917ab0e GitHub-Pull-Request: golang/go#73718 Reviewed-on: https://go-review.googlesource.com/c/go/+/672855 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2025-05-08runtime: schedule cleanups across multiple goroutinesMichael Anthony Knyszek
This change splits the finalizer and cleanup queues and implements a new lock-free blocking queue for cleanups. The basic design is as follows: The cleanup queue is organized in fixed-sized blocks. Individual cleanup functions are queued, but only whole blocks are dequeued. Enqueuing cleanups places them in P-local cleanup blocks. These are flushed to the full list as they get full. Cleanups can only be enqueued by an active sweeper. Dequeuing cleanups always dequeues entire blocks from the full list. Cleanup blocks can be dequeued and executed at any time. The very last active sweeper in the sweep phase is responsible for flushing all local cleanup blocks to the full list. It can do this without any synchronization because the next GC can't start yet, so we can be very certain that nobody else will be accessing the local blocks. Cleanup blocks are stored off-heap because the need to be allocated by the sweeper, which is called from heap allocation paths. As a result, the GC treats cleanup blocks as roots, just like finalizer blocks. Flushes to the full list signal to the scheduler that cleanup goroutines should be awoken. Every time the scheduler goes to wake up a cleanup goroutine and there were more signals than goroutines to wake, it then forwards this signal to runtime.AddCleanup, so that it creates another goroutine the next time it is called, up to gomaxprocs goroutines. The signals here are a little convoluted, but exist because the sweeper and the scheduler cannot safely create new goroutines. For #71772. For #71825. Change-Id: Ie839fde2b67e1b79ac1426be0ea29a8d923a62cc Reviewed-on: https://go-review.googlesource.com/c/go/+/650697 Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Michael Knyszek <mknyszek@google.com>
2025-04-24cmd/compile: put constant value on node inside parenthesesKeith Randall
That's where the unified IR writer expects it. Fixes #73476 Change-Id: Ic22bd8dee5be5991e6d126ae3f6eccb2acdc0b19 Reviewed-on: https://go-review.googlesource.com/c/go/+/667415 Reviewed-by: Junyang Shao <shaojunyang@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Keith Randall <khr@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
2025-04-24cmd/compile: add cast in range loop final value computationKeith Randall
When replacing a loop where the iteration variable has a named type, we need to compute the last iteration value as i = T(len(a)-1), not just i = len(a)-1. Fixes #73491 Change-Id: Ic1cc3bdf8571a40c10060f929a9db8a888de2b70 Reviewed-on: https://go-review.googlesource.com/c/go/+/667815 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Keith Randall <khr@google.com> Reviewed-by: Junyang Shao <shaojunyang@google.com> Reviewed-by: Keith Randall <khr@google.com>
2025-04-23runtime: use precise bounds of Go data/bss for race detectorKeith Randall
We only want to call into the race detector for Go global variables. By rounding up the region bounds, we can include some C globals. Even worse, we can include only *part* of a C global, leading to race{read,write}range calls which straddle the end of shadow memory. That causes the race detector to barf. Fix some off-by-one errors in the assembly comparisons. We want to skip calling the race detector when addr == racedataend. Fixes #73483 Change-Id: I436b0f588d6165b61f30cb7653016ba9b7cbf585 Reviewed-on: https://go-review.googlesource.com/c/go/+/667655 Reviewed-by: Keith Randall <khr@google.com> Auto-Submit: Keith Randall <khr@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
2025-04-21cmd/compile: ensure we evaluate side effects of len() argKeith Randall
For any len() which requires the evaluation of its arg (according to the spec). Update #72844 Change-Id: Id2b0bcc78073a6d5051abd000131dafdf65e7f26 Reviewed-on: https://go-review.googlesource.com/c/go/+/658097 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
2025-04-07cmd/compile: add additional flag constant folding rulesKeith Randall
Fixes #73200 Change-Id: I77518d37acd838acf79ed113194bac5e2c30897f Reviewed-on: https://go-review.googlesource.com/c/go/+/663535 Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2025-04-06cmd/compile: be more conservative about arm64 insns that can take zero registerKeith Randall
It's really only needed for stores and store-like instructions (atomic exchange, compare-and-swap, ...). Fixes #73180 Change-Id: I8ecd833a301355adf0fa4bff43250091640c6226 Reviewed-on: https://go-review.googlesource.com/c/go/+/663155 Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
2025-03-19cmd/compile: remove implicit deref from len(p) where p is ptr-to-arrayKeith Randall
func f() *[4]int { return nil } _ = len(f()) should not panic. We evaluate f, but there isn't a dereference according to the spec (just "arg is evaluated"). Update #72844 Change-Id: Ia32cefc1b7aa091cd1c13016e015842b4d12d5b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/658096 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Keith Randall <khr@google.com>
2025-03-13cmd/compile: don't move nilCheck operations during tightenKeith Randall
Nil checks need to stay in their original blocks. They cannot be moved to a following conditionally-executed block. Fixes #72860 Change-Id: Ic2d66cdf030357d91f8a716a004152ba4c016f77 Reviewed-on: https://go-review.googlesource.com/c/go/+/657715 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2025-03-11go/types, types2: report better error messages for slice expressionsRobert Griesemer
Explicitly compute the common underlying type and while doing so report better slice-expression relevant error messages. Streamline message format for index and slice errors. This removes the last uses of the coreString and match functions. Delete them. Change-Id: I4b50dda1ef7e2ab5e296021458f7f0b6f6e229cd Reviewed-on: https://go-review.googlesource.com/c/go/+/655935 Reviewed-by: Robert Griesemer <gri@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Robert Griesemer <gri@google.com>
2025-03-10go/types, types2: better error messages for copy built-inRobert Griesemer
Rather than relying on coreString, use the new commonUnder function to determine the argument slice element types. Factor out this functionality, which is shared for append and copy, into a new helper function sliceElem (similar to chanElem). Use sliceElem for both the append and copy implementation. As a result, the error messages for invalid copy calls are now more detailed. While at it, handle the special cases for append and copy first because they don't need the slice element computation. Finally, share the same type recording code for the special and general cases. As an aside, in commonUnder, be clearer in the code that the result is either a nil type and an error, or a non-nil type and a nil error. This matches in style what we do in sliceElem. Change-Id: I318bafc0d2d31df04f33b1b464ad50d581918671 Reviewed-on: https://go-review.googlesource.com/c/go/+/655675 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
2025-03-06go/types, types2: better error messages for invalid callsRobert Griesemer
Rather than reporting "non-function" for an invalid type parameter, report which type in the type parameter's type set is not a function. Change-Id: I8beec25cc337bae8e03d23e62d97aa82db46bab4 Reviewed-on: https://go-review.googlesource.com/c/go/+/654475 Reviewed-by: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2025-03-06cmd/compile: remove no-longer-necessary recursive inlining checksDavid Chase
this does result in a little bit more inlining, cmd/compile text is 0.5% larger, bent-benchmark text geomeans grow by only 0.02%. some of our tests make assumptions about inlining. Change-Id: I999d1798aca5dc64a1928bd434258a61e702951a Reviewed-on: https://go-review.googlesource.com/c/go/+/655157 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2025-03-05cmd/compile: use inline-Pos-based recursion testDavid Chase
Look at the inlining stack of positions for a call site, if the line/col/file of the call site appears in that stack, do not inline. This subsumes all the other recently-added recursive inlining checks, but they are left in to make this easier+safer to backport. Fixes #72090 Change-Id: I0f487bb0d4c514015907c649312672b7be464abd Reviewed-on: https://go-review.googlesource.com/c/go/+/655155 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2025-03-04cmd/compile: fix out of memory when inlining closureCuong Manh Le
CL 629195 strongly favor closure inlining, allowing closures to be inlined more aggressively. However, if the closure body contains a call to a function, which itself is one of the call arguments, it causes the infinite inlining. Fixing this by prevent this kind of functions from being inlinable. Fixes #72063 Change-Id: I5fb5723a819b1e2c5aadb57c1023ec84ca9fa53c Reviewed-on: https://go-review.googlesource.com/c/go/+/654195 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2025-02-26cmd/compile: don't pull constant offsets out of pointer arithmeticKeith Randall
This could lead to manufacturing a pointer that points outside its original allocation. Bug was introduced in CL 629858. Fixes #71932 Change-Id: Ia86ab0b65ce5f80a8e0f4f4c81babd07c5904f8d Reviewed-on: https://go-review.googlesource.com/c/go/+/652078 Reviewed-by: Keith Randall <khr@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2025-02-25cmd/compile: ensure we don't reuse temporary registerkhr@golang.org
Before this CL, we could use the same register for both a temporary register and for moving a value in the output register out of the way. Fixes #71857 Change-Id: Iefbfd9d4139136174570d8aadf8a0fb391791ea9 Reviewed-on: https://go-review.googlesource.com/c/go/+/651221 Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
2025-02-20cmd/compile: don't report newLimit discovered when unsat happens multiple timesJorropo
Fixes #71852 Change-Id: I696fcb8fc8c0c2e5e5ae6ab50596f6bdb9b7d498 Reviewed-on: https://go-review.googlesource.com/c/go/+/650975 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org>
2025-02-19cmd/compile: determine static values of len and cap in make() callsMateusz Poliwczak
This change improves escape analysis by attempting to deduce static values for the len and cap parameters, allowing allocations to be made on the stack. Change-Id: I1161019aed9f60cf2c2fe4d405da94ad415231ac GitHub-Last-Rev: d78c1b4ca55fa53282e665009f689d0b013f1434 GitHub-Pull-Request: golang/go#71693 Reviewed-on: https://go-review.googlesource.com/c/go/+/649035 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@golang.org> Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
2025-02-19cmd/compile, runtime: use deferreturn as target PC for recover from ↵David Chase
deferrangefunc The existing code for recover from deferrangefunc was broken in several ways. 1. the code following a deferrangefunc call did not check the return value for an out-of-band value indicating "return now" (i.e., recover was called) 2. the returned value was delivered using a bespoke ABI that happened to match on register-ABI platforms, but not on older stack-based ABI. 3. the returned value was the wrong width (1 word versus 2) and type/value(integer 1, not a pointer to anything) for deferrangefunc's any-typed return value (in practice, the OOB value check could catch this, but still, it's sketchy). This -- using the deferreturn lookup method already in place for open-coded defers -- turned out to be a much-less-ugly way of obtaining the desired transfer of control for recover(). TODO: we also could do this for regular defer, and delete some code. Fixes #71675 Change-Id: If7d7ea789ad4320821aab3b443759a7d71647ff0 Reviewed-on: https://go-review.googlesource.com/c/go/+/650476 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2025-02-18cmd/compile: avoid infinite recursion when inlining closuresCuong Manh Le
CL 630696 changes budget for once-called closures, making them more inlinable. However, when recursive inlining involve both the closure and its parent, the inliner goes into an infinite loop: parent (a closure) -> closure -> parent -> ... The problem here dues to the closure name mangling, causing the inlined checking condition failed, since the closure name affects how the linker symbol generated. To fix this, just prevent the closure from inlining its parent into itself, avoid the infinite inlining loop. Fixes #71680 Change-Id: Ib27626d70f95e5f1c24a3eb1c8e6c3443b7d90c8 Reviewed-on: https://go-review.googlesource.com/c/go/+/649656 Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2025-02-15cmd/compile: fix sign extension of paired 32-bit loads on arm64Keith Randall
Fixes #71759 Change-Id: Iab05294ac933cc9972949158d3fe2bdc3073df5e Reviewed-on: https://go-review.googlesource.com/c/go/+/649895 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
2025-02-03test/issue71226: add cast to avoid clang errorIan Lance Taylor
Change-Id: I2d8ecb7b5f48943697d454d09947fdb1817809d0 Reviewed-on: https://go-review.googlesource.com/c/go/+/646295 TryBot-Bypass: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Auto-Submit: Ian Lance Taylor <iant@golang.org>
2025-02-03cmd/cgo: declare _GoString{Len,Ptr} in _cgo_export.hIan Lance Taylor
Fixes #71226 Change-Id: I91c46a4310a9c7a9fcd1e3a131ca16e46949edb3 Reviewed-on: https://go-review.googlesource.com/c/go/+/642235 Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2025-02-03cmd/cgo: add C declaration parameter unused attributeIan Lance Taylor
Fixes #71225 Change-Id: I3e60fdf632f2aa0e63b24225f13e4ace49906925 Reviewed-on: https://go-review.googlesource.com/c/go/+/642196 Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
2024-12-04sync/atomic: add missing leak tests for And & OrJorropo
Theses tests were forgot because when CL 462298 was originally written And & Or atomics were not available in go. Git were smart enough to rebase over And's & Or's addition. After most reviews and before merging it were pointed I should make theses new intrinsics noescape. When doing this last minute addition I forgot to add tests. Change-Id: I457f98315c0aee91d5743058ab76f256856cb782 Reviewed-on: https://go-review.googlesource.com/c/go/+/633416 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-11-22cmd/compile: modify CSE to remove redundant OpLocalAddrsYoulin Feng
Remove the OpLocalAddrs that are unnecessary in the CSE pass, so the following passes like DSE and memcombine can do its work better. Fixes #70300 Change-Id: I600025d49eeadb3ca4f092d614428399750f69bc Reviewed-on: https://go-review.googlesource.com/c/go/+/628075 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org>
2024-11-21cmd/compile: fix rewrite rules for multiply/addKeith Randall
x - (y - c) == (x - y) + c, not (x - y) - c. Oops. Fixes #70481 Change-Id: I0e54d8e65dd9843c6b92c543ac69d69ee21f617c Reviewed-on: https://go-review.googlesource.com/c/go/+/630397 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Jakub Ciolek <jakub@ciolek.dev> Auto-Submit: Keith Randall <khr@golang.org>
2024-11-19crypto/ecdh: move implementation to crypto/internal/fips/ecdhFilippo Valsorda
This intentionally gives up on the property of not computing the public key until requested. It was nice, but it was making the code too complex. The average use case is to call PublicKey immediately after GenerateKey anyway. Added support in the module for P-224, just in case we'd ever want to support it in crypto/ecdh. Tried various ways to fix test/fixedbugs/issue52193.go to be meaningful, but crypto/ecdh is pretty complex and all the solutions would end up locking in crypto/ecdh structure rather than compiler behavior. The rest of that test is good enough on its own anyway. If we do the work in the future of making crypto/ecdh zero-allocations using the affordances of the compiler, we can add a more robust TestAllocations on our side. For #69536 Change-Id: I68ac3955180cb31f6f96a0ef57604aaed88ab311 Reviewed-on: https://go-review.googlesource.com/c/go/+/628315 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Russ Cox <rsc@golang.org> Auto-Submit: Filippo Valsorda <filippo@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-11-18cmd/compile: remove gc programs from stack frame objectsKeith Randall
This is a two-pronged approach. First, try to keep large objects off the stack frame. Second, if they do manage to appear anyway, use straight bitmasks instead of gc programs. Generally probably a good idea to keep large objects out of stack frames. But particularly keeping gc programs off the stack simplifies runtime code a bit. This CL sets the limit of most stack objects to 131072 bytes (on 64-bit archs). There can still be large objects if allocated by a late pass, like order, or they are required to be on the stack, like function arguments. But the size for the bitmasks for these objects isn't a huge deal, as we have already have (probably several) bitmasks for the frame liveness map itself. Change-Id: I6d2bed0e9aa9ac7499955562c6154f9264061359 Reviewed-on: https://go-review.googlesource.com/c/go/+/542815 Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2024-11-15sync/atomic: make intrinsics noescape except 64bits op on 32bits arch and ↵Jorropo
unsafe.Pointer Fixes #16241 I made 64 bits op on 32 bits arches still leak since it was kinda promised. The promised leaks were wider than this but I don't belive it's effect can be observed in an breaking maner without using unsafe the way it's currently setup. Change-Id: I66d8df47bfe49bce3efa64ac668a2a55f70733a3 Reviewed-on: https://go-review.googlesource.com/c/go/+/462298 Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
2024-11-14cmd/compile: better error message when offending/missing token is a keywordRobert Griesemer
Prefix keywords (type, default, case, etc.) with "keyword" in error messages to make them less ambiguous. Fixes #68589. Change-Id: I1eb92d1382f621b934167b3a4c335045da26be9f Reviewed-on: https://go-review.googlesource.com/c/go/+/623819 Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Tim King <taking@google.com>
2024-11-12runtime: fix iterator returns map entries after clear (pre-swissmap)Youlin Feng
Fixes #70189 Fixes #59411 Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest-noswissmap Change-Id: I4ef7ecd7e996330189309cb2a658cf34bf9e1119 Reviewed-on: https://go-review.googlesource.com/c/go/+/625275 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-11-05cmd/compile: init limit for newly created value in prove passYoulin Feng
Fixes: #70156 Change-Id: I2e5dc2a39a8e54ec5f18c5f9d1644208cffb2e9a Reviewed-on: https://go-review.googlesource.com/c/go/+/624695 Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
2024-11-04cmd/compile: fix inlining name mangling for blank labelCuong Manh Le
Fixes #70175 Change-Id: I13767d951455854b03ad6707ff9292cfe9097ee9 Reviewed-on: https://go-review.googlesource.com/c/go/+/624377 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@golang.org> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Auto-Submit: Keith Randall <khr@golang.org>
2024-10-23go/types, types2: qualify named types in error messages with type kindRobert Griesemer
Change the description of an operand x that has a named type of sorts by providing a description of the type structure (array, struct, slice, pointer, etc). For instance, given a (variable) operand x of a struct type T, the operand is mentioned as (new): x (variable of struct type T) instead of (old): x (variable of type T) This approach is also used when a basic type is renamed, for instance as in: x (value of uint type big.Word) which makes it clear that big.Word is a uint. This change is expected to produce more informative error messages. Fixes #69955. Change-Id: I544b0698f753a522c3b6e1800a492a94974fbab7 Reviewed-on: https://go-review.googlesource.com/c/go/+/621458 Reviewed-by: Alan Donovan <adonovan@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-10-22go/types: improve recursive type error messageMax Neverov
This change improves error message for recursive types. Currently, compilation of the [following program](https://go.dev/play/p/3ef84ObpzfG): package main type T1[T T2] struct{} type T2[T T1] struct{} returns an error: ./prog.go:3:6: invalid recursive type T1 ./prog.go:3:6: T1 refers to ./prog.go:4:6: T2 refers to ./prog.go:3:6: T1 With the patch applied the error message looks like: ./prog.go:3:6: invalid recursive type T1 ./prog.go:3:6: T1 refers to T2 ./prog.go:4:6: T2 refers to T1 Change-Id: Ic07cdffcffb1483c672b241fede4e694269b5b79 GitHub-Last-Rev: cd042fdc384cf5591b3258ca80fdc002bb8c5e0d GitHub-Pull-Request: golang/go#69574 Reviewed-on: https://go-review.googlesource.com/c/go/+/614084 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Tim King <taking@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2024-10-21cmd/link: generate Mach-O UUID when -B flag is specifiedCherry Mui
Currently, on Mach-O, the Go linker doesn't generate LC_UUID in internal linking mode. This causes some macOS system tools unable to track the binary, as well as in some cases the binary unable to access local network on macOS 15. This CL makes the linker start generate LC_UUID. Currently, the UUID is generated if the -B flag is specified. And we'll make it generate UUID by default in a later CL. The -B flag is currently for generating GNU build ID on ELF, which is a similar concept to Mach-O's UUID. Instead of introducing another flag, we just use the same flag and the same setting. Specifically, "-B gobuildid" will generate a UUID based on the Go build ID. For #68678. Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64_14,gotip-darwin-arm64_13 Change-Id: I90089a78ba144110bf06c1c6836daf2d737ff10a Reviewed-on: https://go-review.googlesource.com/c/go/+/618595 Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Ingo Oeser <nightlyone@googlemail.com> Reviewed-by: Than McIntosh <thanm@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-10-14all: wire up swisstable mapsMichael Pratt
Use the new SwissTable-based map in internal/runtime/maps as the basis for the runtime map when GOEXPERIMENT=swissmap. Integration is complete enough to pass all.bash. Notable missing features: * Race integration / concurrent write detection * Stack-allocated maps * Specialized "fast" map variants * Indirect key / elem For #54766. Cq-Include-Trybots: luci.golang.try:gotip-linux-ppc64_power10,gotip-linux-amd64-longtest-swissmap Change-Id: Ie97b656b6d8e05c0403311ae08fef9f51756a639 Reviewed-on: https://go-review.googlesource.com/c/go/+/594596 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>