From 0b6a10ef246ff55085d6ec88f68f5fd96677b141 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Thu, 27 Apr 2017 09:27:52 -0700 Subject: [PATCH] cmd/compile: dowidth more in the front end dowidth is fundamentally unsafe to call from the back end; it will cause data races. Replace all calls to dowidth in the backend with assertions that the width has been calculated. Then fix all the cases in which that was not so, including the cases from #20145. Fixes #20145. Change-Id: Idba3d19d75638851a30ec2ebcdb703c19da3e92b Reviewed-on: https://go-review.googlesource.com/41970 Run-TryBot: Josh Bleecher Snyder TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/ssa.go | 37 +++++++++++++++--------- src/cmd/compile/internal/gc/typecheck.go | 5 ++++ src/cmd/compile/internal/gc/walk.go | 2 +- src/cmd/compile/internal/types/type.go | 11 +++++-- test/fixedbugs/issue20145.go | 14 +++++++++ 5 files changed, 52 insertions(+), 17 deletions(-) create mode 100644 test/fixedbugs/issue20145.go diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index e9f31e68c4..08c96c3688 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -642,13 +642,6 @@ func (s *state) stmt(n *Node) { return } - var t *types.Type - if n.Right != nil { - t = n.Right.Type - } else { - t = n.Left.Type - } - // Evaluate RHS. rhs := n.Right if rhs != nil { @@ -682,6 +675,23 @@ func (s *state) stmt(n *Node) { } } } + + if isblank(n.Left) { + // _ = rhs + // Just evaluate rhs for side-effects. + if rhs != nil { + s.expr(rhs) + } + return + } + + var t *types.Type + if n.Right != nil { + t = n.Right.Type + } else { + t = n.Left.Type + } + var r *ssa.Value deref := !canSSAType(t) if deref { @@ -1509,8 +1519,8 @@ func (s *state) expr(n *Node) *ssa.Value { return v } - dowidth(from) - dowidth(to) + from.AssertWidthCalculated() + to.AssertWidthCalculated() if from.Width != to.Width { s.Fatalf("CONVNOP width mismatch %v (%d) -> %v (%d)\n", from, from.Width, to, to.Width) return nil @@ -2311,7 +2321,7 @@ func (s *state) assign(left *Node, right *ssa.Value, deref bool, skip skipMask) return } t := left.Type - dowidth(t) + t.AssertWidthCalculated() if s.canSSA(left) { if deref { s.Fatalf("can SSA LHS %v but not RHS %s", left, right) @@ -3085,7 +3095,7 @@ func (s *state) call(n *Node, k callKind) *ssa.Value { } rcvr = s.newValue1(ssa.OpIData, types.Types[TUINTPTR], i) } - dowidth(fn.Type) + fn.Type.AssertWidthCalculated() stksize := fn.Type.ArgWidth() // includes receiver // Run all argument assignments. The arg slots have already @@ -3341,7 +3351,7 @@ func (s *state) canSSA(n *Node) bool { // canSSA reports whether variables of type t are SSA-able. func canSSAType(t *types.Type) bool { - dowidth(t) + t.AssertWidthCalculated() if t.Width > int64(4*Widthptr) { // 4*Widthptr is an arbitrary constant. We want it // to be at least 3*Widthptr so slices can be registerized. @@ -4962,8 +4972,7 @@ func (e *ssafn) namedAuto(name string, typ ssa.Type, pos src.XPos) ssa.GCNode { n.Esc = EscNever n.Name.Curfn = e.curfn e.curfn.Func.Dcl = append(e.curfn.Func.Dcl, n) - - dowidth(t) + t.AssertWidthCalculated() return n } diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 3622b9af57..764374aa26 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -1621,6 +1621,7 @@ OpSwitch: continue } as[i] = assignconv(n, t.Elem(), "append") + checkwidth(as[i].Type) // ensure width is calculated for backend } } @@ -1691,6 +1692,7 @@ OpSwitch: case OCONV: ok |= Erv saveorignode(n) + checkwidth(n.Type) // ensure width is calculated for backend n.Left = typecheck(n.Left, Erv) n.Left = convlit1(n.Left, n.Type, true, noReuse) t := n.Left.Type @@ -3307,6 +3309,9 @@ func typecheckas(n *Node) { if n.Left.Typecheck() == 0 { n.Left = typecheck(n.Left, Erv|Easgn) } + if !isblank(n.Left) { + checkwidth(n.Left.Type) // ensure width is calculated for backend + } } func checkassignto(src *types.Type, dst *Node) { diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index cd41a24c09..1196421d7c 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -2611,7 +2611,7 @@ func vmatch1(l *Node, r *Node) bool { } // paramstoheap returns code to allocate memory for heap-escaped parameters -// and to copy non-result prameters' values from the stack. +// and to copy non-result parameters' values from the stack. func paramstoheap(params *types.Type) []*Node { var nn []*Node for _, t := range params.Fields().Slice() { diff --git a/src/cmd/compile/internal/types/type.go b/src/cmd/compile/internal/types/type.go index b0be122d0a..62041454ca 100644 --- a/src/cmd/compile/internal/types/type.go +++ b/src/cmd/compile/internal/types/type.go @@ -849,6 +849,13 @@ func (t *Type) WidthCalculated() bool { return t.Align > 0 } +// AssertWidthCalculated calls Fatalf if t's width has not yet been calculated. +func (t *Type) AssertWidthCalculated() { + if !t.WidthCalculated() { + Fatalf("width not calculated: %v", t) + } +} + // ArgWidth returns the total aligned argument size for a function. // It includes the receiver, parameters, and results. func (t *Type) ArgWidth() int64 { @@ -857,12 +864,12 @@ func (t *Type) ArgWidth() int64 { } func (t *Type) Size() int64 { - Dowidth(t) + t.AssertWidthCalculated() return t.Width } func (t *Type) Alignment() int64 { - Dowidth(t) + t.AssertWidthCalculated() return int64(t.Align) } diff --git a/test/fixedbugs/issue20145.go b/test/fixedbugs/issue20145.go new file mode 100644 index 0000000000..67ba5aee9a --- /dev/null +++ b/test/fixedbugs/issue20145.go @@ -0,0 +1,14 @@ +// compile + +// Copyright 2017 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. + +// Issue 20145: some func types weren't dowidth-ed by the front end, +// leading to races in the backend. + +package p + +func f() { + _ = (func())(nil) +}