Skip to content

G
go
Commit c0cfe968 authored by Matthew Dempsky's avatar Matthew Dempsky
Browse files
Options

cmd/compile: rewrite f(g()) for multi-value g() during typecheck 

This is a re-attempt at CL 153841, which caused two regressions:

1. crypto/ecdsa failed to build with -gcflags=-l=4. This was because
when "t1, t2, ... := g(); f(t1, t2, ...)" was exported, we were losing
the first assignment from the call's Ninit field.

2. net/http/pprof failed to run with -gcflags=-N. This is due to a
conflict with CL 159717: as of that CL, package-scope initialization
statements are executed within the "init.ializer" function, rather
than the "init" function, and the generated temp variables need to be
moved accordingly too.

[Rest of description is as before.]

This CL moves order.go's copyRet logic for rewriting f(g()) into t1,
t2, ... := g(); f(t1, t2, ...) earlier into typecheck. This allows the
rest of the compiler to stop worrying about multi-value functions
appearing outside of OAS2FUNC nodes.

This changes compiler behavior in a few observable ways:

1. Typechecking error messages for builtin functions now use general
case error messages rather than unnecessarily differing ones.

2. Because f(g()) is rewritten before inlining, saved inline bodies
now see the rewritten form too. This could be addressed, but doesn't
seem worthwhile.

3. Most notably, this simplifies escape analysis and fixes a memory
corruption issue in esc.go. See #29197 for details.

Fixes #15992.
Fixes #29197.

Change-Id: I930b10f7e27af68a0944d6c9bfc8707c3fab27a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/166983
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
parent 032acb3a
Hide whitespace changes
Inline Side-by-side
Showing with 167 additions and 268 deletions
src/cmd/compile/internal/gc/esc.go
View file @ c0cfe968
......@@ -1604,13 +1604,6 @@ func (e *EscState) esccall(call *Node, parent *Node) {
}
argList := call.List
if argList.Len() == 1 {
arg := argList.First()
if arg.Type.IsFuncArgStruct() { // f(g())
argList = e.nodeEscState(arg).Retval
}
}
args := argList.Slice()
if indirect {
......
src/cmd/compile/internal/gc/fmt.go
View file @ c0cfe968
......@@ -1404,14 +1404,11 @@ func (n *Node) exprfmt(s fmt.State, prec int, mode fmtMode) {
}
mode.Fprintf(s, "sliceheader{%v,%v,%v}", n.Left, n.List.First(), n.List.Second())
case OCOPY:
mode.Fprintf(s, "%#v(%v, %v)", n.Op, n.Left, n.Right)
case OCOMPLEX:
if n.List.Len() == 1 {
mode.Fprintf(s, "%#v(%v)", n.Op, n.List.First())
} else {
case OCOMPLEX, OCOPY:
if n.Left != nil {
mode.Fprintf(s, "%#v(%v, %v)", n.Op, n.Left, n.Right)
} else {
mode.Fprintf(s, "%#v(%.v)", n.Op, n.List)
}
case OCONV,
......@@ -1540,6 +1537,8 @@ func (n *Node) nodefmt(s fmt.State, flag FmtFlag, mode fmtMode) {
if flag&FmtLong != 0 && t != nil {
if t.Etype == TNIL {
fmt.Fprint(s, "nil")
} else if n.Op == ONAME && n.Name.AutoTemp() {
mode.Fprintf(s, "%v value", t)
} else {
mode.Fprintf(s, "%v (type %v)", n, t)
}
......
src/cmd/compile/internal/gc/iexport.go
View file @ c0cfe968
......@@ -1277,6 +1277,7 @@ func (w *exportWriter) expr(n *Node) {
case OCALL, OCALLFUNC, OCALLMETH, OCALLINTER, OGETG:
w.op(OCALL)
w.pos(n.Pos)
w.stmtList(n.Ninit)
w.expr(n.Left)
w.exprList(n.List)
w.bool(n.IsDDD())
......@@ -1387,7 +1388,8 @@ func (w *exportWriter) localIdent(s *types.Sym, v int32) {
return
}
if i := strings.LastIndex(name, "."); i >= 0 {
// TODO(mdempsky): Fix autotmp hack.
if i := strings.LastIndex(name, "."); i >= 0 && !strings.HasPrefix(name, ".autotmp_") {
Fatalf("unexpected dot in identifier: %v", name)
}
......
src/cmd/compile/internal/gc/iimport.go
View file @ c0cfe968
......@@ -907,7 +907,9 @@ func (r *importReader) node() *Node {
// unreachable - mapped to OCALL case below by exporter
case OCALL:
n := nodl(r.pos(), OCALL, r.expr(), nil)
n := nodl(r.pos(), OCALL, nil, nil)
n.Ninit.Set(r.stmtList())
n.Left = r.expr()
n.List.Set(r.exprList())
n.SetIsDDD(r.bool())
return n
......
src/cmd/compile/internal/gc/init.go
View file @ c0cfe968
......@@ -14,6 +14,9 @@ import (
// the name, normally "pkg.init", is altered to "pkg.init.0".
var renameinitgen int
// Dummy function for autotmps generated during typechecking.
var dummyInitFn = nod(ODCLFUNC, nil, nil)
func renameinit() *types.Sym {
s := lookupN("init.", renameinitgen)
renameinitgen++
......@@ -93,6 +96,12 @@ func fninit(n []*Node) {
initializers = lookup("init.ializers")
disableExport(initializers)
fn := dclfunc(initializers, nod(OTFUNC, nil, nil))
for _, dcl := range dummyInitFn.Func.Dcl {
dcl.Name.Curfn = fn
}
fn.Func.Dcl = append(fn.Func.Dcl, dummyInitFn.Func.Dcl...)
dummyInitFn.Func.Dcl = nil
fn.Nbody.Set(nf)
funcbody()
......@@ -103,6 +112,12 @@ func fninit(n []*Node) {
funccompile(fn)
lineno = autogeneratedPos
}
if dummyInitFn.Func.Dcl != nil {
// We only generate temps using dummyInitFn if there
// are package-scope initialization statements, so
// something's weird if we get here.
Fatalf("dummyInitFn still has declarations")
}
var r []*Node
......
src/cmd/compile/internal/gc/inl.go
View file @ c0cfe968
......@@ -589,24 +589,13 @@ func inlnode(n *Node, maxCost int32) *Node {
}
inlnodelist(n.List, maxCost)
switch n.Op {
case OBLOCK:
if n.Op == OBLOCK {
for _, n2 := range n.List.Slice() {
if n2.Op == OINLCALL {
inlconv2stmt(n2)
}
}
case ORETURN, OCALLFUNC, OCALLMETH, OCALLINTER, OAPPEND, OCOMPLEX:
// if we just replaced arg in f(arg()) or return arg with an inlined call
// and arg returns multiple values, glue as list
if n.List.Len() == 1 && n.List.First().Op == OINLCALL && n.List.First().Rlist.Len() > 1 {
n.List.Set(inlconv2list(n.List.First()))
break
}
fallthrough
default:
} else {
s := n.List.Slice()
for i1, n1 := range s {
if n1 != nil && n1.Op == OINLCALL {
......@@ -1016,9 +1005,6 @@ func mkinlcall(n, fn *Node, maxCost int32) *Node {
// to pass as a slice.
numvals := n.List.Len()
if numvals == 1 && n.List.First().Type.IsFuncArgStruct() {
numvals = n.List.First().Type.NumFields()
}
x := as.List.Len()
for as.List.Len() < numvals {
......
src/cmd/compile/internal/gc/order.go
View file @ c0cfe968
......@@ -380,66 +380,12 @@ func (o *Order) init(n *Node) {
n.Ninit.Set(nil)
}
// Ismulticall reports whether the list l is f() for a multi-value function.
// Such an f() could appear as the lone argument to a multi-arg function.
func ismulticall(l Nodes) bool {
// one arg only
if l.Len() != 1 {
return false
}
n := l.First()
// must be call
switch n.Op {
default:
return false
case OCALLFUNC, OCALLMETH, OCALLINTER:
// call must return multiple values
return n.Left.Type.NumResults() > 1
}
}
// copyRet emits t1, t2, ... = n, where n is a function call,
// and then returns the list t1, t2, ....
func (o *Order) copyRet(n *Node) []*Node {
if !n.Type.IsFuncArgStruct() {
Fatalf("copyret %v %d", n.Type, n.Left.Type.NumResults())
}
slice := n.Type.Fields().Slice()
l1 := make([]*Node, len(slice))
l2 := make([]*Node, len(slice))
for i, t := range slice {
tmp := temp(t.Type)
l1[i] = tmp
l2[i] = tmp
}
as := nod(OAS2, nil, nil)
as.List.Set(l1)
as.Rlist.Set1(n)
as = typecheck(as, ctxStmt)
o.stmt(as)
return l2
}
// callArgs orders the list of call arguments *l.
func (o *Order) callArgs(l *Nodes) {
if ismulticall(*l) {
// return f() where f() is multiple values.
l.Set(o.copyRet(l.First()))
} else {
o.exprList(*l)
}
}
// call orders the call expression n.
// n.Op is OCALLMETH/OCALLFUNC/OCALLINTER or a builtin like OCOPY.
func (o *Order) call(n *Node) {
n.Left = o.expr(n.Left, nil)
n.Right = o.expr(n.Right, nil) // ODDDARG temp
o.callArgs(&n.List)
o.exprList(n.List)
if n.Op != OCALLFUNC {
return
......@@ -811,7 +757,7 @@ func (o *Order) stmt(n *Node) {
o.cleanTemp(t)
case ORETURN:
o.callArgs(&n.List)
o.exprList(n.List)
o.out = append(o.out, n)
// Special: clean case temporaries in each block entry.
......@@ -1200,7 +1146,7 @@ func (o *Order) expr(n, lhs *Node) *Node {
n.List.SetFirst(o.expr(n.List.First(), nil)) // order x
n.List.Second().Left = o.expr(n.List.Second().Left, nil) // order y
} else {
o.callArgs(&n.List)
o.exprList(n.List)
}
if lhs == nil || lhs.Op != ONAME && !samesafeexpr(lhs, n.List.First()) {
......
src/cmd/compile/internal/gc/typecheck.go
View file @ c0cfe968
......@@ -1285,11 +1285,7 @@ func typecheck1(n *Node, top int) (res *Node) {
return n
}
if n.List.Len() == 1 && !n.IsDDD() {
n.List.SetFirst(typecheck(n.List.First(), ctxExpr|ctxMultiOK))
} else {
typecheckslice(n.List.Slice(), ctxExpr)
}
typecheckargs(n)
t := l.Type
if t == nil {
n.Type = nil
......@@ -1433,51 +1429,24 @@ func typecheck1(n *Node, top int) (res *Node) {
case OCOMPLEX:
ok |= ctxExpr
var r *Node
var l *Node
if n.List.Len() == 1 {
typecheckslice(n.List.Slice(), ctxMultiOK)
if n.List.First().Op != OCALLFUNC && n.List.First().Op != OCALLMETH {
yyerror("invalid operation: complex expects two arguments")
n.Type = nil
return n
}
t := n.List.First().Left.Type
if !t.IsKind(TFUNC) {
// Bail. This error will be reported elsewhere.
return n
}
if t.NumResults() != 2 {
yyerror("invalid operation: complex expects two arguments, %v returns %d results", n.List.First(), t.NumResults())
n.Type = nil
return n
}
t = n.List.First().Type
l = asNode(t.Field(0).Nname)
r = asNode(t.Field(1).Nname)
} else {
if !twoarg(n) {
n.Type = nil
return n
}
n.Left = typecheck(n.Left, ctxExpr)
n.Right = typecheck(n.Right, ctxExpr)
l = n.Left
r = n.Right
if l.Type == nil || r.Type == nil {
n.Type = nil
return n
}
l, r = defaultlit2(l, r, false)
if l.Type == nil || r.Type == nil {
n.Type = nil
return n
}
n.Left = l
n.Right = r
typecheckargs(n)
if !twoarg(n) {
n.Type = nil
return n
}
l := n.Left
r := n.Right
if l.Type == nil || r.Type == nil {
n.Type = nil
return n
}
l, r = defaultlit2(l, r, false)
if l.Type == nil || r.Type == nil {
n.Type = nil
return n
}
n.Left = l
n.Right = r
if !types.Identical(l.Type, r.Type) {
yyerror("invalid operation: %v (mismatched types %v and %v)", n, l.Type, r.Type)
......@@ -1531,6 +1500,8 @@ func typecheck1(n *Node, top int) (res *Node) {
ok |= ctxStmt
case ODELETE:
ok |= ctxStmt
typecheckargs(n)
args := n.List
if args.Len() == 0 {
yyerror("missing arguments to delete")
......@@ -1550,8 +1521,6 @@ func typecheck1(n *Node, top int) (res *Node) {
return n
}
ok |= ctxStmt
typecheckslice(args.Slice(), ctxExpr)
l := args.First()
r := args.Second()
if l.Type != nil && !l.Type.IsMap() {
......@@ -1564,6 +1533,7 @@ func typecheck1(n *Node, top int) (res *Node) {
case OAPPEND:
ok |= ctxExpr
typecheckargs(n)
args := n.List
if args.Len() == 0 {
yyerror("missing arguments to append")
......@@ -1571,25 +1541,12 @@ func typecheck1(n *Node, top int) (res *Node) {
return n
}
if args.Len() == 1 && !n.IsDDD() {
args.SetFirst(typecheck(args.First(), ctxExpr|ctxMultiOK))
} else {
typecheckslice(args.Slice(), ctxExpr)
}
t := args.First().Type
if t == nil {
n.Type = nil
return n
}
// Unpack multiple-return result before type-checking.
var funarg *types.Type
if t.IsFuncArgStruct() {
funarg = t
t = t.Field(0).Type
}
n.Type = t
if !t.IsSlice() {
if Isconst(args.First(), CTNIL) {
......@@ -1625,44 +1582,23 @@ func typecheck1(n *Node, top int) (res *Node) {
break
}
if funarg != nil {
for _, t := range funarg.FieldSlice()[1:] {
if assignop(t.Type, n.Type.Elem(), nil) == 0 {
yyerror("cannot append %v value to []%v", t.Type, n.Type.Elem())
}
}
} else {
as := args.Slice()[1:]
for i, n := range as {
if n.Type == nil {
continue
}
as[i] = assignconv(n, t.Elem(), "append")
checkwidth(as[i].Type) // ensure width is calculated for backend
as := args.Slice()[1:]
for i, n := range as {
if n.Type == nil {
continue
}
as[i] = assignconv(n, t.Elem(), "append")
checkwidth(as[i].Type) // ensure width is calculated for backend
}
case OCOPY:
ok |= ctxStmt | ctxExpr
args := n.List
if args.Len() < 2 {
yyerror("missing arguments to copy")
typecheckargs(n)
if !twoarg(n) {
n.Type = nil
return n
}
if args.Len() > 2 {
yyerror("too many arguments to copy")
n.Type = nil
return n
}
n.Left = args.First()
n.Right = args.Second()
n.List.Set(nil)
n.Type = types.Types[TINT]
n.Left = typecheck(n.Left, ctxExpr)
n.Right = typecheck(n.Right, ctxExpr)
if n.Left.Type == nil || n.Right.Type == nil {
n.Type = nil
return n
......@@ -2055,11 +1991,7 @@ func typecheck1(n *Node, top int) (res *Node) {
case ORETURN:
ok |= ctxStmt
if n.List.Len() == 1 {
typecheckslice(n.List.Slice(), ctxExpr|ctxMultiOK)
} else {
typecheckslice(n.List.Slice(), ctxExpr)
}
typecheckargs(n)
if Curfn == nil {
yyerror("return outside function")
n.Type = nil
......@@ -2163,6 +2095,51 @@ func typecheck1(n *Node, top int) (res *Node) {
return n
}
func typecheckargs(n *Node) {
if n.List.Len() != 1 || n.IsDDD() {
typecheckslice(n.List.Slice(), ctxExpr)
return
}
typecheckslice(n.List.Slice(), ctxExpr|ctxMultiOK)
t := n.List.First().Type
if t == nil || !t.IsFuncArgStruct() {
return
}
// Rewrite f(g()) into t1, t2, ... = g(); f(t1, t2, ...).
// Save n as n.Orig for fmt.go.
if n.Orig == n {
n.Orig = n.sepcopy()
}
as := nod(OAS2, nil, nil)
as.Rlist.AppendNodes(&n.List)
// If we're outside of function context, then this call will
// be executed during the generated init function. However,
// init.go hasn't yet created it. Instead, associate the
// temporary variables with dummyInitFn for now, and init.go
// will reassociate them later when it's appropriate.
static := Curfn == nil
if static {
Curfn = dummyInitFn
}
for _, f := range t.FieldSlice() {
t := temp(f.Type)
as.Ninit.Append(nod(ODCL, t, nil))
as.List.Append(t)
n.List.Append(t)
}
if static {
Curfn = nil
}
as = typecheck(as, ctxStmt)
n.Ninit.Append(as)
}
func checksliceindex(l *Node, r *Node, tp *types.Type) bool {
t := r.Type
if t == nil {
......@@ -2302,24 +2279,15 @@ func twoarg(n *Node) bool {
if n.Left != nil {
return true
}
if n.List.Len() == 0 {
yyerror("missing argument to %v - %v", n.Op, n)
if n.List.Len() != 2 {
if n.List.Len() < 2 {
yyerror("not enough arguments in call to %v", n)
} else {
yyerror("too many arguments in call to %v", n)
}
return false
}
n.Left = n.List.First()
if n.List.Len() == 1 {
yyerror("missing argument to %v - %v", n.Op, n)
n.List.Set(nil)
return false
}
if n.List.Len() > 2 {
yyerror("too many arguments to %v - %v", n.Op, n)
n.List.Set(nil)
return false
}
n.Right = n.List.Second()
n.List.Set(nil)
return true
......@@ -2579,8 +2547,6 @@ func hasddd(t *types.Type) bool {
// typecheck assignment: type list = expression list
func typecheckaste(op Op, call *Node, isddd bool, tstruct *types.Type, nl Nodes, desc func() string) {
var t *types.Type
var n1 int
var n2 int
var i int
lno := lineno
......@@ -2593,57 +2559,10 @@ func typecheckaste(op Op, call *Node, isddd bool, tstruct *types.Type, nl Nodes,
var n *Node
if nl.Len() == 1 {
n = nl.First()
if n.Type != nil && n.Type.IsFuncArgStruct() {
if !hasddd(tstruct) {
n1 := tstruct.NumFields()
n2 := n.Type.NumFields()
if n2 > n1 {
goto toomany
}
if n2 < n1 {
goto notenough
}
}
lfs := tstruct.FieldSlice()
rfs := n.Type.FieldSlice()
var why string
for i, tl := range lfs {
if tl.IsDDD() {
for _, tn := range rfs[i:] {
if assignop(tn.Type, tl.Type.Elem(), &why) == 0 {
if call != nil {
yyerror("cannot use %v as type %v in argument to %v%s", tn.Type, tl.Type.Elem(), call, why)
} else {
yyerror("cannot use %v as type %v in %s%s", tn.Type, tl.Type.Elem(), desc(), why)
}
}
}
return
}
if i >= len(rfs) {
goto notenough
}
tn := rfs[i]
if assignop(tn.Type, tl.Type, &why) == 0 {
if call != nil {
yyerror("cannot use %v as type %v in argument to %v%s", tn.Type, tl.Type, call, why)
} else {
yyerror("cannot use %v as type %v in %s%s", tn.Type, tl.Type, desc(), why)
}
}
}
if len(rfs) > len(lfs) {
goto toomany
}
return
}
}
n1 = tstruct.NumFields()
n2 = nl.Len()
n1 := tstruct.NumFields()
n2 := nl.Len()
if !hasddd(tstruct) {
if n2 > n1 {
goto toomany
......@@ -2685,6 +2604,7 @@ func typecheckaste(op Op, call *Node, isddd bool, tstruct *types.Type, nl Nodes,
return
}
// TODO(mdempsky): Make into ... call with implicit slice.
for ; i < nl.Len(); i++ {
n = nl.Index(i)
setlineno(n)
......@@ -2792,14 +2712,8 @@ func (nl Nodes) retsigerr(isddd bool) string {
}
var typeStrings []string
if nl.Len() == 1 && nl.First().Type != nil && nl.First().Type.IsFuncArgStruct() {
for _, f := range nl.First().Type.Fields().Slice() {
typeStrings = append(typeStrings, sigrepr(f.Type))
}
} else {
for _, n := range nl.Slice() {
typeStrings = append(typeStrings, sigrepr(n.Type))
}
for _, n := range nl.Slice() {
typeStrings = append(typeStrings, sigrepr(n.Type))
}
ddd := ""
......
test/cmplx.go
View file @ c0cfe968
......@@ -49,10 +49,10 @@ func main() {
_ = complex(f64, F64) // ERROR "complex"
_ = complex(F64, f64) // ERROR "complex"
_ = complex(F1()) // ERROR "expects two arguments.*returns 1"
_ = complex(F3()) // ERROR "expects two arguments.*returns 3"
_ = complex(F1()) // ERROR "not enough arguments"
_ = complex(F3()) // ERROR "too many arguments"
_ = complex() // ERROR "missing argument"
_ = complex() // ERROR "not enough arguments"
c128 = complex(f32, f32) // ERROR "cannot use"
c64 = complex(f64, f64) // ERROR "cannot use"
......
test/copy1.go
View file @ c0cfe968
......@@ -14,7 +14,7 @@ func main() {
si := make([]int, 8)
sf := make([]float64, 8)
_ = copy() // ERROR "missing arguments"
_ = copy() // ERROR "not enough arguments"
_ = copy(1, 2, 3) // ERROR "too many arguments"
_ = copy(si, "hi") // ERROR "have different element types.*int.*string"
......
test/fixedbugs/issue15992.go 0 → 100644
View file @ c0cfe968
// run
// Copyright 2018 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 (
"fmt"
)
func f(a []byte) ([]byte, []byte) {
return a, []byte("abc")
}
func g(a []byte) ([]byte, string) {
return a, "abc"
}
func h(m map[int]int) (map[int]int, int) {
return m, 0
}
func main() {
a := []byte{1, 2, 3}
n := copy(f(a))
fmt.Println(n, a)
b := []byte{1, 2, 3}
n = copy(f(b))
fmt.Println(n, b)
m := map[int]int{0: 0}
fmt.Println(len(m))
delete(h(m))
fmt.Println(len(m))
}
test/fixedbugs/issue15992.out 0 → 100644
View file @ c0cfe968
3 [97 98 99]
3 [97 98 99]
1
0
test/fixedbugs/issue17038.go
View file @ c0cfe968
......@@ -6,4 +6,4 @@
package main
const A = complex(0()) // ERROR "cannot call non-function"
const A = complex(0()) // ERROR "cannot call non-function" "not enough arguments"
test/fixedbugs/issue9521.go
View file @ c0cfe968
......@@ -13,6 +13,6 @@ func f() (_, _ []int) { return }
func g() (x []int, y float64) { return }
func main() {
_ = append(f()) // ERROR "cannot append \[\]int value to \[\]int"
_ = append(g()) // ERROR "cannot append float64 value to \[\]int"
_ = append(f()) // ERROR "cannot use \[\]int value as type int in append"
_ = append(g()) // ERROR "cannot use float64 value as type int in append"
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7