From: Matthew Dempsky Date: Mon, 18 Jan 2021 00:14:48 +0000 (-0800) Subject: [dev.regabi] cmd/compile: convert OPANIC argument to interface{} during typecheck X-Git-Tag: go1.17beta1~1539^2~75 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=6113db0bb4;p=gostls13.git [dev.regabi] cmd/compile: convert OPANIC argument to interface{} during typecheck Currently, typecheck leaves arguments to OPANIC as their original type. This CL changes it to insert implicit OCONVIFACE operations to convert arguments to `interface{}` like how any other function call would be handled. No immediate benefits, other than getting to remove a tiny bit of special-case logic in order.go's handling of OPANICs. Instead, the generic code path for handling OCONVIFACE is used, if necessary. Longer term, this should be marginally helpful for #43753, as it reduces the number of cases where we need values to be addressable for runtime calls. However, this does require adding some hacks to appease existing tests: 1. We need yet another kludge in inline budgeting, to ensure that reflect.flag.mustBe stays inlinable for cmd/compile/internal/test's TestIntendedInlining. 2. Since the OCONVIFACE expressions are now being introduced during typecheck, they're now visible to escape analysis. So expressions like "panic(1)" are now seen as "panic(interface{}(1))", and escape analysis warns that the "interface{}(1)" escapes to the heap. These have always escaped to heap, just now we're accurately reporting about it. (Also, unfortunately fmt.go hides implicit conversions by default in diagnostics messages, so instead of reporting "interface{}(1) escapes to heap", it actually reports "1 escapes to heap", which is confusing. However, this confusing messaging also isn't new.) Change-Id: Icedf60e1d2e464e219441b8d1233a313770272af Reviewed-on: https://go-review.googlesource.com/c/go/+/284412 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Cuong Manh Le Trust: Matthew Dempsky --- diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 143fbe9efe..aa194ebab2 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -346,6 +346,13 @@ func (v *hairyVisitor) doNode(n ir.Node) error { v.budget -= v.extraCallCost case ir.OPANIC: + n := n.(*ir.UnaryExpr) + if n.X.Op() == ir.OCONVIFACE && n.X.(*ir.ConvExpr).Implicit() { + // Hack to keep reflect.flag.mustBe inlinable for TestIntendedInlining. + // Before CL 284412, these conversions were introduced later in the + // compiler, so they didn't count against inlining budget. + v.budget++ + } v.budget -= inlineExtraPanicCost case ir.ORECOVER: diff --git a/src/cmd/compile/internal/typecheck/func.go b/src/cmd/compile/internal/typecheck/func.go index c832d9700f..b576590d4d 100644 --- a/src/cmd/compile/internal/typecheck/func.go +++ b/src/cmd/compile/internal/typecheck/func.go @@ -896,7 +896,7 @@ func tcNew(n *ir.UnaryExpr) ir.Node { // tcPanic typechecks an OPANIC node. func tcPanic(n *ir.UnaryExpr) ir.Node { n.X = Expr(n.X) - n.X = DefaultLit(n.X, types.Types[types.TINTER]) + n.X = AssignConv(n.X, types.Types[types.TINTER], "argument to panic") if n.X.Type() == nil { n.SetType(nil) return n diff --git a/src/cmd/compile/internal/walk/order.go b/src/cmd/compile/internal/walk/order.go index e1e9f168bb..fe0b6a0eff 100644 --- a/src/cmd/compile/internal/walk/order.go +++ b/src/cmd/compile/internal/walk/order.go @@ -768,14 +768,12 @@ func (o *orderState) stmt(n ir.Node) { orderBlock(&n.Else, o.free) o.out = append(o.out, n) - // Special: argument will be converted to interface using convT2E - // so make sure it is an addressable temporary. case ir.OPANIC: n := n.(*ir.UnaryExpr) t := o.markTemp() n.X = o.expr(n.X, nil) - if !n.X.Type().IsInterface() { - n.X = o.addrTemp(n.X) + if !n.X.Type().IsEmptyInterface() { + base.FatalfAt(n.Pos(), "bad argument to panic: %L", n.X) } o.out = append(o.out, n) o.cleanTemp(t) diff --git a/test/closure3.dir/main.go b/test/closure3.dir/main.go index 5694673f1e..e8e1e99860 100644 --- a/test/closure3.dir/main.go +++ b/test/closure3.dir/main.go @@ -285,5 +285,5 @@ func main() { //go:noinline func ppanic(s string) { // ERROR "leaking param: s" - panic(s) + panic(s) // ERROR "s escapes to heap" } diff --git a/test/escape2.go b/test/escape2.go index 5c6eb559fa..b9b723d866 100644 --- a/test/escape2.go +++ b/test/escape2.go @@ -1547,7 +1547,7 @@ func foo153(v interface{}) *int { // ERROR "v does not escape" case int: // ERROR "moved to heap: x$" return &x } - panic(0) + panic(0) // ERROR "0 escapes to heap" } // issue 8185 - &result escaping into result diff --git a/test/escape2n.go b/test/escape2n.go index 46e58f8566..7c8208aa73 100644 --- a/test/escape2n.go +++ b/test/escape2n.go @@ -1547,7 +1547,7 @@ func foo153(v interface{}) *int { // ERROR "v does not escape" case int: // ERROR "moved to heap: x$" return &x } - panic(0) + panic(0) // ERROR "0 escapes to heap" } // issue 8185 - &result escaping into result diff --git a/test/escape4.go b/test/escape4.go index a4a9c14a3e..4e50231bf9 100644 --- a/test/escape4.go +++ b/test/escape4.go @@ -35,14 +35,14 @@ func f1() { func f2() {} // ERROR "can inline f2" // No inline for recover; panic now allowed to inline. -func f3() { panic(1) } // ERROR "can inline f3" +func f3() { panic(1) } // ERROR "can inline f3" "1 escapes to heap" func f4() { recover() } func f5() *byte { type T struct { x [1]byte } - t := new(T) // ERROR "new.T. escapes to heap" + t := new(T) // ERROR "new.T. escapes to heap" return &t.x[0] } @@ -52,6 +52,6 @@ func f6() *byte { y byte } } - t := new(T) // ERROR "new.T. escapes to heap" + t := new(T) // ERROR "new.T. escapes to heap" return &t.x.y } diff --git a/test/fixedbugs/issue13799.go b/test/fixedbugs/issue13799.go index fbdd4c32bc..c8ecfc54e4 100644 --- a/test/fixedbugs/issue13799.go +++ b/test/fixedbugs/issue13799.go @@ -60,7 +60,7 @@ func test1(iter int) { } if len(m) != maxI { - panic(fmt.Sprintf("iter %d: maxI = %d, len(m) = %d", iter, maxI, len(m))) // ERROR "iter escapes to heap$" "len\(m\) escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" + panic(fmt.Sprintf("iter %d: maxI = %d, len(m) = %d", iter, maxI, len(m))) // ERROR "iter escapes to heap$" "len\(m\) escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" "fmt.Sprintf\(.*\) escapes to heap" } } @@ -84,7 +84,7 @@ func test2(iter int) { } if len(m) != maxI { - panic(fmt.Sprintf("iter %d: maxI = %d, len(m) = %d", iter, maxI, len(m))) // ERROR "iter escapes to heap$" "len\(m\) escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" + panic(fmt.Sprintf("iter %d: maxI = %d, len(m) = %d", iter, maxI, len(m))) // ERROR "iter escapes to heap$" "len\(m\) escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" "fmt.Sprintf\(.*\) escapes to heap" } } @@ -110,7 +110,7 @@ func test3(iter int) { } if *m != maxI { - panic(fmt.Sprintf("iter %d: maxI = %d, *m = %d", iter, maxI, *m)) // ERROR "\*m escapes to heap$" "iter escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" + panic(fmt.Sprintf("iter %d: maxI = %d, *m = %d", iter, maxI, *m)) // ERROR "\*m escapes to heap$" "iter escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" "fmt.Sprintf\(.*\) escapes to heap" } } @@ -136,7 +136,7 @@ func test4(iter int) { } if *m != maxI { - panic(fmt.Sprintf("iter %d: maxI = %d, *m = %d", iter, maxI, *m)) // ERROR "\*m escapes to heap$" "iter escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" + panic(fmt.Sprintf("iter %d: maxI = %d, *m = %d", iter, maxI, *m)) // ERROR "\*m escapes to heap$" "iter escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" "fmt.Sprintf\(.*\) escapes to heap" } } @@ -167,7 +167,7 @@ func test5(iter int) { } if *m != maxI { - panic(fmt.Sprintf("iter %d: maxI = %d, *m = %d", iter, maxI, *m)) // ERROR "\*m escapes to heap$" "iter escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" + panic(fmt.Sprintf("iter %d: maxI = %d, *m = %d", iter, maxI, *m)) // ERROR "\*m escapes to heap$" "iter escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" "fmt.Sprintf\(.*\) escapes to heap" } } @@ -185,6 +185,6 @@ func test6(iter int) { } if *m != maxI { - panic(fmt.Sprintf("iter %d: maxI = %d, *m = %d", iter, maxI, *m)) // ERROR "\*m escapes to heap$" "iter escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" + panic(fmt.Sprintf("iter %d: maxI = %d, *m = %d", iter, maxI, *m)) // ERROR "\*m escapes to heap$" "iter escapes to heap$" "maxI escapes to heap$" "... argument does not escape$" "fmt.Sprintf\(.*\) escapes to heap" } } diff --git a/test/fixedbugs/issue7921.go b/test/fixedbugs/issue7921.go index a4e7b246d4..65be4b5bbe 100644 --- a/test/fixedbugs/issue7921.go +++ b/test/fixedbugs/issue7921.go @@ -41,7 +41,7 @@ func bufferNoEscape3(xs []string) string { // ERROR "xs does not escape$" func bufferNoEscape4() []byte { var b bytes.Buffer - b.Grow(64) // ERROR "bufferNoEscape4 ignoring self-assignment in bytes.b.buf = bytes.b.buf\[:bytes.m\]$" "inlining call to bytes.\(\*Buffer\).Grow$" + b.Grow(64) // ERROR "bufferNoEscape4 ignoring self-assignment in bytes.b.buf = bytes.b.buf\[:bytes.m\]$" "inlining call to bytes.\(\*Buffer\).Grow$" "string\(.*\) escapes to heap" useBuffer(&b) return b.Bytes() // ERROR "inlining call to bytes.\(\*Buffer\).Bytes$" }