]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.typeparams] test: add regression test for go/defer wrapper
authorCuong Manh Le <cuong.manhle.vn@gmail.com>
Fri, 16 Jul 2021 17:15:07 +0000 (00:15 +0700)
committerCuong Manh Le <cuong.manhle.vn@gmail.com>
Wed, 21 Jul 2021 00:01:12 +0000 (00:01 +0000)
CL 330330 moved logic for wrapping go/defer from order to esacpe
analysis. It introduced a bug involves go/defer statement with ABI0
functions.

Consider this following code:

package p

//go:cgo_unsafe_args
func g(*int) (r1 struct{}) {
return
}

func f() {
defer g(new(int))
}

g is a cgo-like generated function with ABI0. While compiling g, we set
the offset per ABI0.

The function f is rewritten into:

func f() {
_0, _1 := g, new(int)
defer func() { _0(_1) }()
}

The temporary _0 hold function value with the same type as g, but with
class PAUTO. Thus ssagen/ssa.go:state.call cannot handle it and use
ABIDefault to set the offset, causes the offset of r1 changed

CL 330332 intended to optimize code generated for wrapping function, by
rewriting the wrapper function into:

func f() {
_0 := new(int)
defer func() { g(_0) }()
}

So it fixed the bug unintentionally.

This CL add regression test for this bug, and also add a comment to
explain while not wrapping declared function is important.

Updates #47227

Change-Id: I75c83d1d9cc7fd4699e6b218a295d0c0a10ef471
Reviewed-on: https://go-review.googlesource.com/c/go/+/334882
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
src/cmd/compile/internal/escape/call.go
test/fixedbugs/issue47227.go [new file with mode: 0644]

index 9e5abed59142edaf7b6dba120d733f765e4cf6cb..65c76d6870b3988067621e08c01c553dda0efb99 100644 (file)
@@ -320,6 +320,8 @@ func (e *escape) rewriteArgument(argp *ir.Node, init *ir.Nodes, call ir.Node, fn
                        return
                case ir.ONAME:
                        if arg.(*ir.Name).Class == ir.PFUNC {
+                               // TODO(cuonglm): figure it why this is necessary, we should not depend on this to make
+                               //                ABI analyze works correctly (see #47227 and discussion in CL 334882).
                                return
                        }
                }
diff --git a/test/fixedbugs/issue47227.go b/test/fixedbugs/issue47227.go
new file mode 100644 (file)
index 0000000..a14efc9
--- /dev/null
@@ -0,0 +1,23 @@
+// run fake-arg-to-force-use-of-go-run
+
+// Copyright 2021 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 cgo
+// +build cgo
+
+package main
+
+// void f(int *p) { *p = 0x12345678; }
+import "C"
+
+func main() {
+       var x C.int
+       func() {
+               defer C.f(&x)
+       }()
+       if x != 0x12345678 {
+               panic("FAIL")
+       }
+}