From 14bdcc76fd9aa3edda64ef07a526fbeeed8b4326 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Fri, 28 Dec 2018 12:43:48 -0800 Subject: [PATCH] cmd/compile: fix racewalk{enter,exit} removal We can't remove race instrumentation unless there are no calls, not just no static calls. Closure and interface calls also count. The problem in issue 29329 is that there was a racefuncenter, an InterCall, and a racefuncexit. The racefuncenter was removed, then the InterCall was rewritten to a StaticCall. That prevented the racefuncexit from being removed. That caused an imbalance in racefuncenter/racefuncexit calls, which made the race detector barf. Bug introduced at CL 121235 Fixes #29329 Change-Id: I2c94ac6cf918dd910b74b2a0de5dc2480d236f16 Reviewed-on: https://go-review.googlesource.com/c/155917 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/compile/internal/ssa/rewrite.go | 6 +- test/fixedbugs/issue29329.go | 106 ++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue29329.go diff --git a/src/cmd/compile/internal/ssa/rewrite.go b/src/cmd/compile/internal/ssa/rewrite.go index 69365c4e60..a154249371 100644 --- a/src/cmd/compile/internal/ssa/rewrite.go +++ b/src/cmd/compile/internal/ssa/rewrite.go @@ -1111,7 +1111,8 @@ func needRaceCleanup(sym interface{}, v *Value) bool { } for _, b := range f.Blocks { for _, v := range b.Values { - if v.Op == OpStaticCall { + switch v.Op { + case OpStaticCall: switch v.Aux.(fmt.Stringer).String() { case "runtime.racefuncenter", "runtime.racefuncexit", "runtime.panicindex", "runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap": @@ -1122,6 +1123,9 @@ func needRaceCleanup(sym interface{}, v *Value) bool { // for accurate stacktraces. return false } + case OpClosureCall, OpInterCall: + // We must keep the race functions if there are any other call types. + return false } } } diff --git a/test/fixedbugs/issue29329.go b/test/fixedbugs/issue29329.go new file mode 100644 index 0000000000..1c2825e3bc --- /dev/null +++ b/test/fixedbugs/issue29329.go @@ -0,0 +1,106 @@ +// run -race + +// 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. + +// +build linux,amd64 + +package main + +import ( + "fmt" +) + +type LineString []Point +type Point [2]float64 + +//go:noinline +func benchmarkData() LineString { + return LineString{{1.0, 2.0}} +} + +func (ls LineString) Clone() LineString { + ps := MultiPoint(ls) + return LineString(ps.Clone()) +} + +type MultiPoint []Point + +func (mp MultiPoint) Clone() MultiPoint { + if mp == nil { + return nil + } + + points := make([]Point, len(mp)) + copy(points, mp) + + return MultiPoint(points) +} + +func F1() { + cases := []struct { + threshold float64 + length int + }{ + {0.1, 1118}, + {0.5, 257}, + {1.0, 144}, + {1.5, 95}, + {2.0, 71}, + {3.0, 46}, + {4.0, 39}, + {5.0, 33}, + } + + ls := benchmarkData() + + for k := 0; k < 100; k++ { + for i, tc := range cases { + r := DouglasPeucker(tc.threshold).LineString(ls.Clone()) + if len(r) == tc.length { + fmt.Printf("%d: unexpected\n", i) + } + } + } +} + +// A DouglasPeuckerSimplifier wraps the DouglasPeucker function. +type DouglasPeuckerSimplifier struct { + Threshold float64 +} + +// DouglasPeucker creates a new DouglasPeuckerSimplifier. +func DouglasPeucker(threshold float64) *DouglasPeuckerSimplifier { + return &DouglasPeuckerSimplifier{ + Threshold: threshold, + } +} + +func (s *DouglasPeuckerSimplifier) LineString(ls LineString) LineString { + return lineString(s, ls) +} + +type simplifier interface { + simplify(LineString, bool) (LineString, []int) +} + +func lineString(s simplifier, ls LineString) LineString { + return runSimplify(s, ls) +} + +func runSimplify(s simplifier, ls LineString) LineString { + if len(ls) <= 2 { + return ls + } + ls, _ = s.simplify(ls, false) + return ls +} + +func (s *DouglasPeuckerSimplifier) simplify(ls LineString, wim bool) (LineString, []int) { + return nil, nil +} + +func main() { + F1() +}