From bac709817371c349d17a2ee245f29411849babda Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 29 Oct 2013 11:07:09 -0400 Subject: [PATCH] go.tools/ssa: fix crash on (new)(T) due to missing unparen() call. Audited codebase for other occurrences, found two more. Added test coverage for all of them. R=gri CC=golang-dev https://golang.org/cl/14698043 --- ssa/builder.go | 8 ++-- ssa/emit.go | 3 +- ssa/interp/testdata/coverage.go | 7 ++++ ssa/ssa.go | 5 ++- ssa/testdata/valueforexpr.go | 71 +++++++++++++++++++++++++++++++-- 5 files changed, 85 insertions(+), 9 deletions(-) diff --git a/ssa/builder.go b/ssa/builder.go index 6474be6bb9..327cbc383d 100644 --- a/ssa/builder.go +++ b/ssa/builder.go @@ -457,7 +457,7 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue { // in an addressable location. // func (b *builder) exprInPlace(fn *Function, loc lvalue, e ast.Expr) { - if e, ok := e.(*ast.CompositeLit); ok { + if e, ok := unparen(e).(*ast.CompositeLit); ok { // A CompositeLit never evaluates to a pointer, // so if the type of the location is a pointer, // an &-operation is implied. @@ -476,7 +476,9 @@ func (b *builder) exprInPlace(fn *Function, loc lvalue, e ast.Expr) { // Can't in-place initialize an interface value. // Fall back to copying. } else { - b.compLit(fn, loc.address(fn), e, typ) // in place + addr := loc.address(fn) + b.compLit(fn, addr, e, typ) // in place + emitDebugRef(fn, e, addr, true) return } } @@ -551,7 +553,7 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { return y } // Call to "intrinsic" built-ins, e.g. new, make, panic. - if id, ok := e.Fun.(*ast.Ident); ok { + if id, ok := unparen(e.Fun).(*ast.Ident); ok { if obj, ok := fn.Pkg.objectOf(id).(*types.Builtin); ok { if v := b.builtin(fn, obj, e.Args, typ, e.Lparen); v != nil { return v diff --git a/ssa/emit.go b/ssa/emit.go index 983115606a..900fb0e724 100644 --- a/ssa/emit.go +++ b/ssa/emit.go @@ -45,6 +45,7 @@ func emitDebugRef(f *Function, e ast.Expr, v Value, isAddr bool) { panic("nil") } var obj types.Object + e = unparen(e) if id, ok := e.(*ast.Ident); ok { if isBlankIdent(id) { return @@ -57,7 +58,7 @@ func emitDebugRef(f *Function, e ast.Expr, v Value, isAddr bool) { } f.emit(&DebugRef{ X: v, - Expr: unparen(e), + Expr: e, IsAddr: isAddr, object: obj, }) diff --git a/ssa/interp/testdata/coverage.go b/ssa/interp/testdata/coverage.go index 4743f0a569..cef30bdf10 100644 --- a/ssa/interp/testdata/coverage.go +++ b/ssa/interp/testdata/coverage.go @@ -279,6 +279,13 @@ func main() { _ = map[int]*struct{}{0: {}} } +// Parens should not prevent intrinsic treatment of built-ins. +// (Regression test for a crash.) +func init() { + _ = (new)(int) + _ = (make)([]int, 0) +} + type mybool bool func (mybool) f() {} diff --git a/ssa/ssa.go b/ssa/ssa.go index 6d50846432..2d132a2fa9 100644 --- a/ssa/ssa.go +++ b/ssa/ssa.go @@ -99,7 +99,7 @@ type Value interface { // Instruction. // // This is the same as the source name for Parameters, - // Builtins, Functions, Captures, Globals and some Allocs. + // Builtins, Functions, Captures, Globals. // For constants, it is a representation of the constant's value // and type. For all other Values this is the name of the // virtual register defined by the instruction. @@ -115,7 +115,7 @@ type Value interface { String() string // Type returns the type of this value. Many instructions - // (e.g. IndexAddr) change the behaviour depending on the + // (e.g. IndexAddr) change their behaviour depending on the // types of their operands. Type() types.Type @@ -186,6 +186,7 @@ type Instruction interface { // SetBlock sets the basic block to which this instruction // belongs. + // TODO(adonovan): unexport this. SetBlock(*BasicBlock) // Operands returns the operands of this instruction: the diff --git a/ssa/testdata/valueforexpr.go b/ssa/testdata/valueforexpr.go index 2f719b96c2..70906cac4e 100644 --- a/ssa/testdata/valueforexpr.go +++ b/ssa/testdata/valueforexpr.go @@ -33,12 +33,12 @@ func f(spilled, unspilled int) { _ = /*@Phi*/ (y) map1 := /*@MakeMap*/ (make(map[string]string)) _ = map1 - _ = /*@MakeMap*/ (map[string]string{"": ""}) _ = /*@MakeSlice*/ (make([]int, 0)) _ = /*@MakeClosure*/ (func() { print(spilled) }) - sl := /*@Slice*/ ([]int{}) - _ = /*@Alloc*/ (&struct{}{}) + + sl := []int{} _ = /*@Slice*/ (sl[:0]) + _ = /*@*/ (new(int)) // optimized away tmp := /*@Alloc*/ (new(int)) _ = tmp @@ -66,6 +66,71 @@ func f(spilled, unspilled int) { /*@UnOp*/ (n) = /*@UnOp*/ (**n) } +func complit() { + // Composite literals. + // We get different results for + // - composite literal as value (e.g. operand to print) + // - composite literal initializer for addressable value + // - composite literal value assigned to blank var + + // 1. Slices + print( /*@Slice*/ ([]int{})) + print( /*@Alloc*/ (&[]int{})) + print(& /*@Alloc*/ ([]int{})) + + sl1 := /*@Slice*/ ([]int{}) + sl2 := /*@Alloc*/ (&[]int{}) + sl3 := & /*@Alloc*/ ([]int{}) + _, _, _ = sl1, sl2, sl3 + + _ = /*@Slice*/ ([]int{}) + _ = /*@*/ (& /*@Slice*/ ([]int{})) // & optimized away + _ = & /*@Slice*/ ([]int{}) + + // 2. Arrays + print( /*@UnOp*/ ([1]int{})) + print( /*@Alloc*/ (&[1]int{})) + print(& /*@Alloc*/ ([1]int{})) + + arr1 := /*@Alloc*/ ([1]int{}) + arr2 := /*@Alloc*/ (&[1]int{}) + arr3 := & /*@Alloc*/ ([1]int{}) + _, _, _ = arr1, arr2, arr3 + + _ = /*@UnOp*/ ([1]int{}) + _ = /*@Alloc*/ (& /*@Alloc*/ ([1]int{})) // & optimized away + _ = & /*@Alloc*/ ([1]int{}) + + // 3. Maps + type M map[int]int + print( /*@MakeMap*/ (M{})) + print( /*@Alloc*/ (&M{})) + print(& /*@Alloc*/ (M{})) + + m1 := /*@MakeMap*/ (M{}) + m2 := /*@Alloc*/ (&M{}) + m3 := & /*@Alloc*/ (M{}) + _, _, _ = m1, m2, m3 + + _ = /*@MakeMap*/ (M{}) + _ = /*@*/ (& /*@MakeMap*/ (M{})) // & optimized away + _ = & /*@MakeMap*/ (M{}) + + // 4. Structs + print( /*@UnOp*/ (struct{}{})) + print( /*@Alloc*/ (&struct{}{})) + print(& /*@Alloc*/ (struct{}{})) + + s1 := /*@Alloc*/ (struct{}{}) + s2 := /*@Alloc*/ (&struct{}{}) + s3 := & /*@Alloc*/ (struct{}{}) + _, _, _ = s1, s2, s3 + + _ = /*@UnOp*/ (struct{}{}) + _ = /*@Alloc*/ (& /*@Alloc*/ (struct{}{})) + _ = & /*@Alloc*/ (struct{}{}) +} + type t struct{ x int } // Ensure we can locate methods of named types.