Skip to content

Commit 48ef010

Browse files
committed
cmd/compile: handle new panicindex/slice names in optimizations
These new calls should not prevent NOSPLIT promotion, like the old ones. These new calls should not prevent racefuncenter/exit removal. (The latter was already true, as the new calls are not yet lowered to StaticCalls at the point where racefuncenter/exit removal is done.) Add tests to make sure we don't regress (again). Fixes #31219 Change-Id: I3fb6b17cdd32c425829f1e2498defa813a5a9ace Reviewed-on: https://go-review.googlesource.com/c/go/+/170639 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ilya Tocar <[email protected]>
1 parent 6073673 commit 48ef010

File tree

5 files changed

+52
-9
lines changed

5 files changed

+52
-9
lines changed

src/cmd/compile/internal/ssa/rewrite.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,17 +1129,20 @@ func needRaceCleanup(sym interface{}, v *Value) bool {
11291129
for _, v := range b.Values {
11301130
switch v.Op {
11311131
case OpStaticCall:
1132-
switch v.Aux.(fmt.Stringer).String() {
1133-
case "runtime.racefuncenter", "runtime.racefuncexit", "runtime.panicindex",
1134-
"runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap",
1135-
"runtime.panicshift":
11361132
// Check for racefuncenter will encounter racefuncexit and vice versa.
11371133
// Allow calls to panic*
1138-
default:
1139-
// If we encountered any call, we need to keep racefunc*,
1140-
// for accurate stacktraces.
1141-
return false
1134+
s := v.Aux.(fmt.Stringer).String()
1135+
switch s {
1136+
case "runtime.racefuncenter", "runtime.racefuncexit",
1137+
"runtime.panicdivide", "runtime.panicwrap",
1138+
"runtime.panicshift":
1139+
continue
11421140
}
1141+
// If we encountered any call, we need to keep racefunc*,
1142+
// for accurate stacktraces.
1143+
return false
1144+
case OpPanicBounds, OpPanicExtend:
1145+
// Note: these are panic generators that are ok (like the static calls above).
11431146
case OpClosureCall, OpInterCall:
11441147
// We must keep the race functions if there are any other call types.
11451148
return false

src/cmd/internal/obj/x86/obj6.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,13 @@ func isZeroArgRuntimeCall(s *obj.LSym) bool {
975975
return false
976976
}
977977
switch s.Name {
978-
case "runtime.panicindex", "runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap", "runtime.panicshift":
978+
case "runtime.panicdivide", "runtime.panicwrap", "runtime.panicshift":
979+
return true
980+
}
981+
if strings.HasPrefix(s.Name, "runtime.panicIndex") || strings.HasPrefix(s.Name, "runtime.panicSlice") {
982+
// These functions do take arguments (in registers),
983+
// but use no stack before they do a stack check. We
984+
// should include them. See issue 31219.
979985
return true
980986
}
981987
return false

test/codegen/race.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// asmcheck -race
2+
3+
// Copyright 2019 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package codegen
8+
9+
// Check that we elide racefuncenter/racefuncexit for
10+
// functions with no calls (but which might panic
11+
// in various ways). See issue 31219.
12+
// amd64:-"CALL.*racefuncenter.*"
13+
func RaceMightPanic(a []int, i, j, k, s int) {
14+
var b [4]int
15+
_ = b[i] // panicIndex
16+
_ = a[i:j] // panicSlice
17+
_ = a[i:j:k] // also panicSlice
18+
_ = i << s // panicShift
19+
_ = i / j // panicDivide
20+
}

test/codegen/stack.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,14 @@ func check_asmout(a, b int) int {
9898
// arm:`.*b\+4\(FP\)`
9999
return b
100100
}
101+
102+
// Check that simple functions get promoted to nosplit, even when
103+
// they might panic in various ways. See issue 31219.
104+
// amd64:"TEXT\t.*NOSPLIT.*"
105+
func MightPanic(a []int, i, j, k, s int) {
106+
_ = a[i] // panicIndex
107+
_ = a[i:j] // panicSlice
108+
_ = a[i:j:k] // also panicSlice
109+
_ = i << s // panicShift
110+
_ = i / j // panicDivide
111+
}

test/run.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,9 @@ func (t *test) run() {
660660
cmdline = append(cmdline, long)
661661
cmd := exec.Command(goTool(), cmdline...)
662662
cmd.Env = append(os.Environ(), env.Environ()...)
663+
if len(flags) > 0 && flags[0] == "-race" {
664+
cmd.Env = append(cmd.Env, "CGO_ENABLED=1")
665+
}
663666

664667
var buf bytes.Buffer
665668
cmd.Stdout, cmd.Stderr = &buf, &buf

0 commit comments

Comments
 (0)