1
0
mirror of https://github.com/golang/go synced 2024-11-26 23:11:24 -07:00

cmd/compile: make the stack allocator more careful about register args.

Assignment between input parameters causes them to have more than
one "Name", and running this backwards from names to values can end
up confusing (conflating) parameter spill slots.

Around 105a6e9518, this cases a stack overflow running
go test -race encoding/pem
because two slice parameters spill (incorrectly) into the same
stack slots (in the AB?I-defined parameter spill area).

This also tickles a failure in cue, which turned out to be
easier to isolate.

Fixes #45851.
Updates #40724.

Change-Id: I39c56815bd6abb652f1ccbe83c47f4f373a125c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/313212
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This commit is contained in:
David Chase 2021-04-23 21:49:08 -04:00
parent b584230889
commit 90ec257735
3 changed files with 112 additions and 1 deletions

View File

@ -6,6 +6,7 @@ package ssa
import ( import (
"cmd/compile/internal/abi" "cmd/compile/internal/abi"
"cmd/compile/internal/base"
"cmd/compile/internal/ir" "cmd/compile/internal/ir"
"cmd/compile/internal/types" "cmd/compile/internal/types"
"cmd/internal/src" "cmd/internal/src"
@ -1601,7 +1602,10 @@ func (x *expandState) newArgToMemOrRegs(baseArg, toReplace *Value, offset int64,
x.f.OwnAux.abiInfo.String()) x.f.OwnAux.abiInfo.String())
panic(fmt.Errorf("Op/Type mismatch, op=%s, type=%s", op.String(), t.String())) panic(fmt.Errorf("Op/Type mismatch, op=%s, type=%s", op.String(), t.String()))
} }
aux := &AuxNameOffset{baseArg.Aux.(*ir.Name), baseArg.AuxInt + offset} if baseArg.AuxInt != 0 {
base.Fatalf("BaseArg %s bound to registers has non-zero AuxInt", baseArg.LongString())
}
aux := &AuxNameOffset{baseArg.Aux.(*ir.Name), offset}
if toReplace != nil && toReplace.Block == baseArg.Block { if toReplace != nil && toReplace.Block == baseArg.Block {
toReplace.reset(op) toReplace.reset(op)
toReplace.Aux = aux toReplace.Aux = aux

View File

@ -145,6 +145,26 @@ func (s *stackAllocState) stackalloc() {
// Note: not "range f.NamedValues" above, because // Note: not "range f.NamedValues" above, because
// that would be nondeterministic. // that would be nondeterministic.
for _, v := range f.NamedValues[name] { for _, v := range f.NamedValues[name] {
if v.Op == OpArgIntReg || v.Op == OpArgFloatReg {
aux := v.Aux.(*AuxNameOffset)
// Never let an arg be bound to a differently named thing.
if name.N != aux.Name || name.Off != aux.Offset {
if f.pass.debug > stackDebug {
fmt.Printf("stackalloc register arg %s skipping name %s\n", v, name)
}
continue
}
} else if name.N.Class == ir.PPARAM && v.Op != OpArg {
// PPARAM's only bind to OpArg
if f.pass.debug > stackDebug {
fmt.Printf("stackalloc PPARAM name %s skipping non-Arg %s\n", name, v)
}
continue
}
if f.pass.debug > stackDebug {
fmt.Printf("stackalloc value %s to name %s\n", v, name)
}
names[v.ID] = name names[v.ID] = name
} }
} }
@ -165,6 +185,25 @@ func (s *stackAllocState) stackalloc() {
f.setHome(v, loc) f.setHome(v, loc)
continue continue
} }
// You might think this below would be the right idea, but you would be wrong.
// It almost works; as of 105a6e9518 - 2021-04-23,
// GOSSAHASH=11011011001011111 == cmd/compile/internal/noder.(*noder).embedded
// is compiled incorrectly. I believe the cause is one of those SSA-to-registers
// puzzles that the register allocator untangles; in the event that a register
// parameter does not end up bound to a name, "fixing" it is a bad idea.
//
//if f.DebugTest {
// if v.Op == OpArgIntReg || v.Op == OpArgFloatReg {
// aux := v.Aux.(*AuxNameOffset)
// loc := LocalSlot{N: aux.Name, Type: v.Type, Off: aux.Offset}
// if f.pass.debug > stackDebug {
// fmt.Printf("stackalloc Op%s %s to %s\n", v.Op, v, loc)
// }
// names[v.ID] = loc
// continue
// }
//}
} }
// For each type, we keep track of all the stack slots we // For each type, we keep track of all the stack slots we

View File

@ -0,0 +1,68 @@
// 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.
// This tickles a stack-allocation bug when the register ABI is enabled.
// The original report was from cue, internal/core/adt/equality.go,
// function equalVertex.
// In the failing case, something bad gets passed to equalTerminal.
package main
import "fmt"
type Kind uint16
type Flag uint16
const (
allKinds Kind = 1
TopKind Kind = (allKinds - 1)
)
type Value interface {
Kind() Kind
}
type Vertex struct {
BaseValue Value
name string
}
func (v *Vertex) Kind() Kind {
return TopKind
}
func main() {
vA := &Vertex{name:"vA",}
vB := &Vertex{name:"vB",}
vX := &Vertex{name:"vX",}
vA.BaseValue = vX
vB.BaseValue = vX
_ = equalVertex(vA, vB, Flag(1))
}
var foo string
//go:noinline
func (v *Vertex) IsClosedStruct() bool {
return true
}
func equalVertex(x *Vertex, v Value, flags Flag) bool {
y, ok := v.(*Vertex)
if !ok {
return false
}
v, ok1 := x.BaseValue.(Value)
w, ok2 := y.BaseValue.(Value)
if !ok1 && !ok2 {
return true // both are struct or list.
}
return equalTerminal(v, w, flags)
}
//go:noinline
func equalTerminal(x Value, y Value, flags Flag) bool {
foo = fmt.Sprintf("EQclosed %s %s %d\n", x.(*Vertex).name, y.(*Vertex).name, flags)
return true
}