cmd/compile: when merging instructions, prefer line number of faulting insn

Normally this happens when combining a sign extension and a load.  We
want the resulting combo-instruction to get the line number of the
load, not the line number of the sign extension.

For each rule, compute where we should get its line number by finding
a value on the match side that can fault.  Use that line number for
all the new values created on the right-hand side.

Fixes #27201

Change-Id: I19b3c6f468fff1a3c0bfbce2d6581828557064a3
Reviewed-on: https://go-review.googlesource.com/c/156937Reviewed-by: David Chase <drchase@google.com>

src/cmd/compile/internal/ssa/gen/rulegen.go

@@ -209,7 +209,11 @@ func genRules(arch arch) {
 
 				canFail = false
 				fmt.Fprintf(buf, "for {\n")
-				if genMatch(buf, arch, match, rule.loc) {
+				pos, matchCanFail := genMatch(buf, arch, match, rule.loc)
+				if pos == "" {
+					pos = "v.Pos"
+				}
+				if matchCanFail {
 					canFail = true
 				}
 
@@ -221,7 +225,7 @@ func genRules(arch arch) {
 					log.Fatalf("unconditional rule %s is followed by other rules", match)
 				}
 
-				genResult(buf, arch, result, rule.loc)
+				genResult(buf, arch, result, rule.loc, pos)
 				if *genLog {
 					fmt.Fprintf(buf, "logRule(\"%s\")\n", rule.loc)
 				}
@@ -291,10 +295,11 @@ func genRules(arch arch) {
 			_, _, _, aux, s := extract(match) // remove parens, then split
 
 			// check match of control value
+			pos := ""
 			if s[0] != "nil" {
 				fmt.Fprintf(w, "v := b.Control\n")
 				if strings.Contains(s[0], "(") {
-					genMatch0(w, arch, s[0], "v", map[string]struct{}{}, false, rule.loc)
+					pos, _ = genMatch0(w, arch, s[0], "v", map[string]struct{}{}, false, rule.loc)
 				} else {
 					fmt.Fprintf(w, "_ = v\n") // in case we don't use v
 					fmt.Fprintf(w, "%s := b.Control\n", s[0])
@@ -335,7 +340,10 @@ func genRules(arch arch) {
 			if t[0] == "nil" {
 				fmt.Fprintf(w, "b.SetControl(nil)\n")
 			} else {
-				fmt.Fprintf(w, "b.SetControl(%s)\n", genResult0(w, arch, t[0], new(int), false, false, rule.loc))
+				if pos == "" {
+					pos = "v.Pos"
+				}
+				fmt.Fprintf(w, "b.SetControl(%s)\n", genResult0(w, arch, t[0], new(int), false, false, rule.loc, pos))
 			}
 			if aux != "" {
 				fmt.Fprintf(w, "b.Aux = %s\n", aux)
@@ -386,15 +394,17 @@ func genRules(arch arch) {
 	}
 }
 
-// genMatch reports whether the match can fail.
-func genMatch(w io.Writer, arch arch, match string, loc string) bool {
+// genMatch returns the variable whose source position should be used for the
+// result (or "" if no opinion), and a boolean that reports whether the match can fail.
+func genMatch(w io.Writer, arch arch, match string, loc string) (string, bool) {
 	return genMatch0(w, arch, match, "v", map[string]struct{}{}, true, loc)
 }
 
-func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, top bool, loc string) bool {
+func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, top bool, loc string) (string, bool) {
 	if match[0] != '(' || match[len(match)-1] != ')' {
 		panic("non-compound expr in genMatch0: " + match)
 	}
+	pos := ""
 	canFail := false
 
 	op, oparch, typ, auxint, aux, args := parseValue(match, arch, loc)
@@ -404,6 +414,10 @@ func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, t
 		fmt.Fprintf(w, "if %s.Op != Op%s%s {\nbreak\n}\n", v, oparch, op.name)
 		canFail = true
 	}
+	if op.faultOnNilArg0 || op.faultOnNilArg1 {
+		// Prefer the position of an instruction which could fault.
+		pos = v + ".Pos"
+	}
 
 	if typ != "" {
 		if !isVariable(typ) {
@@ -494,7 +508,16 @@ func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, t
 			argname = fmt.Sprintf("%s_%d", v, i)
 		}
 		fmt.Fprintf(w, "%s := %s.Args[%d]\n", argname, v, i)
-		if genMatch0(w, arch, arg, argname, m, false, loc) {
+		argPos, argCanFail := genMatch0(w, arch, arg, argname, m, false, loc)
+		if argPos != "" {
+			// Keep the argument in preference to the parent, as the
+			// argument is normally earlier in program flow.
+			// Keep the argument in preference to an earlier argument,
+			// as that prefers the memory argument which is also earlier
+			// in the program flow.
+			pos = argPos
+		}
+		if argCanFail {
 			canFail = true
 		}
 	}
@@ -503,10 +526,10 @@ func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, t
 		fmt.Fprintf(w, "if len(%s.Args) != %d {\nbreak\n}\n", v, len(args))
 		canFail = true
 	}
-	return canFail
+	return pos, canFail
 }
 
-func genResult(w io.Writer, arch arch, result string, loc string) {
+func genResult(w io.Writer, arch arch, result string, loc string, pos string) {
 	move := false
 	if result[0] == '@' {
 		// parse @block directive
@@ -515,9 +538,9 @@ func genResult(w io.Writer, arch arch, result string, loc string) {
 		result = s[1]
 		move = true
 	}
-	genResult0(w, arch, result, new(int), true, move, loc)
+	genResult0(w, arch, result, new(int), true, move, loc, pos)
 }
-func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move bool, loc string) string {
+func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move bool, loc string, pos string) string {
 	// TODO: when generating a constant result, use f.constVal to avoid
 	// introducing copies just to clean them up again.
 	if result[0] != '(' {
@@ -554,7 +577,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move boo
 		}
 		v = fmt.Sprintf("v%d", *alloc)
 		*alloc++
-		fmt.Fprintf(w, "%s := b.NewValue0(v.Pos, Op%s%s, %s)\n", v, oparch, op.name, typ)
+		fmt.Fprintf(w, "%s := b.NewValue0(%s, Op%s%s, %s)\n", v, pos, oparch, op.name, typ)
 		if move && top {
 			// Rewrite original into a copy
 			fmt.Fprintf(w, "v.reset(OpCopy)\n")
@@ -569,7 +592,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move boo
 		fmt.Fprintf(w, "%s.Aux = %s\n", v, aux)
 	}
 	for _, arg := range args {
-		x := genResult0(w, arch, arg, alloc, false, move, loc)
+		x := genResult0(w, arch, arg, alloc, false, move, loc, pos)
 		fmt.Fprintf(w, "%s.AddArg(%s)\n", v, x)
 	}
 

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

@@ -3197,7 +3197,7 @@ func rewriteValue386_Op386CMPB_0(v *Value) bool {
 			break
 		}
 		v.reset(Op386InvertFlags)
-		v0 := b.NewValue0(v.Pos, Op386CMPBload, types.TypeFlags)
+		v0 := b.NewValue0(l.Pos, Op386CMPBload, types.TypeFlags)
 		v0.AuxInt = off
 		v0.Aux = sym
 		v0.AddArg(ptr)
@@ -3381,7 +3381,7 @@ func rewriteValue386_Op386CMPBconst_0(v *Value) bool {
 			break
 		}
 		b = l.Block
-		v0 := b.NewValue0(v.Pos, Op386CMPBconstload, types.TypeFlags)
+		v0 := b.NewValue0(l.Pos, Op386CMPBconstload, types.TypeFlags)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = makeValAndOff(c, off)
@@ -3501,7 +3501,7 @@ func rewriteValue386_Op386CMPL_0(v *Value) bool {
 			break
 		}
 		v.reset(Op386InvertFlags)
-		v0 := b.NewValue0(v.Pos, Op386CMPLload, types.TypeFlags)
+		v0 := b.NewValue0(l.Pos, Op386CMPLload, types.TypeFlags)
 		v0.AuxInt = off
 		v0.Aux = sym
 		v0.AddArg(ptr)
@@ -3704,7 +3704,7 @@ func rewriteValue386_Op386CMPLconst_10(v *Value) bool {
 			break
 		}
 		b = l.Block
-		v0 := b.NewValue0(v.Pos, Op386CMPLconstload, types.TypeFlags)
+		v0 := b.NewValue0(l.Pos, Op386CMPLconstload, types.TypeFlags)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = makeValAndOff(c, off)
@@ -3824,7 +3824,7 @@ func rewriteValue386_Op386CMPW_0(v *Value) bool {
 			break
 		}
 		v.reset(Op386InvertFlags)
-		v0 := b.NewValue0(v.Pos, Op386CMPWload, types.TypeFlags)
+		v0 := b.NewValue0(l.Pos, Op386CMPWload, types.TypeFlags)
 		v0.AuxInt = off
 		v0.Aux = sym
 		v0.AddArg(ptr)
@@ -4008,7 +4008,7 @@ func rewriteValue386_Op386CMPWconst_0(v *Value) bool {
 			break
 		}
 		b = l.Block
-		v0 := b.NewValue0(v.Pos, Op386CMPWconstload, types.TypeFlags)
+		v0 := b.NewValue0(l.Pos, Op386CMPWconstload, types.TypeFlags)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = makeValAndOff(c, off)
@@ -4957,7 +4957,7 @@ func rewriteValue386_Op386MOVBLSX_0(v *Value) bool {
 			break
 		}
 		b = x.Block
-		v0 := b.NewValue0(v.Pos, Op386MOVBLSXload, v.Type)
+		v0 := b.NewValue0(x.Pos, Op386MOVBLSXload, v.Type)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = off
@@ -5062,7 +5062,7 @@ func rewriteValue386_Op386MOVBLZX_0(v *Value) bool {
 			break
 		}
 		b = x.Block
-		v0 := b.NewValue0(v.Pos, Op386MOVBload, v.Type)
+		v0 := b.NewValue0(x.Pos, Op386MOVBload, v.Type)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = off
@@ -10726,7 +10726,7 @@ func rewriteValue386_Op386MOVWLSX_0(v *Value) bool {
 			break
 		}
 		b = x.Block
-		v0 := b.NewValue0(v.Pos, Op386MOVWLSXload, v.Type)
+		v0 := b.NewValue0(x.Pos, Op386MOVWLSXload, v.Type)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = off
@@ -10831,7 +10831,7 @@ func rewriteValue386_Op386MOVWLZX_0(v *Value) bool {
 			break
 		}
 		b = x.Block
-		v0 := b.NewValue0(v.Pos, Op386MOVWload, v.Type)
+		v0 := b.NewValue0(x.Pos, Op386MOVWload, v.Type)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = off
@@ -14209,7 +14209,7 @@ func rewriteValue386_Op386ORL_10(v *Value) bool {
 			break
 		}
 		b = mergePoint(b, x0, x1)
-		v0 := b.NewValue0(v.Pos, Op386MOVWload, typ.UInt16)
+		v0 := b.NewValue0(x1.Pos, Op386MOVWload, typ.UInt16)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = i0
@@ -14258,7 +14258,7 @@ func rewriteValue386_Op386ORL_10(v *Value) bool {
 			break
 		}
 		b = mergePoint(b, x0, x1)
-		v0 := b.NewValue0(v.Pos, Op386MOVWload, typ.UInt16)
+		v0 := b.NewValue0(x0.Pos, Op386MOVWload, typ.UInt16)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = i0
@@ -14334,7 +14334,7 @@ func rewriteValue386_Op386ORL_10(v *Value) bool {
 			break
 		}
 		b = mergePoint(b, x0, x1, x2)
-		v0 := b.NewValue0(v.Pos, Op386MOVLload, typ.UInt32)
+		v0 := b.NewValue0(x2.Pos, Op386MOVLload, typ.UInt32)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = i0
@@ -14410,7 +14410,7 @@ func rewriteValue386_Op386ORL_10(v *Value) bool {
 			break
 		}
 		b = mergePoint(b, x0, x1, x2)
-		v0 := b.NewValue0(v.Pos, Op386MOVLload, typ.UInt32)
+		v0 := b.NewValue0(x2.Pos, Op386MOVLload, typ.UInt32)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = i0
@@ -14486,7 +14486,7 @@ func rewriteValue386_Op386ORL_10(v *Value) bool {
 			break
 		}
 		b = mergePoint(b, x0, x1, x2)
-		v0 := b.NewValue0(v.Pos, Op386MOVLload, typ.UInt32)
+		v0 := b.NewValue0(x1.Pos, Op386MOVLload, typ.UInt32)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = i0
@@ -14562,7 +14562,7 @@ func rewriteValue386_Op386ORL_10(v *Value) bool {
 			break
 		}
 		b = mergePoint(b, x0, x1, x2)
-		v0 := b.NewValue0(v.Pos, Op386MOVLload, typ.UInt32)
+		v0 := b.NewValue0(x0.Pos, Op386MOVLload, typ.UInt32)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = i0

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

@@ -3502,7 +3502,7 @@ func rewriteValueMIPS_OpMIPSMOVBUreg_0(v *Value) bool {
 			break
 		}
 		b = x.Block
-		v0 := b.NewValue0(v.Pos, OpMIPSMOVBUload, t)
+		v0 := b.NewValue0(x.Pos, OpMIPSMOVBUload, t)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = off
@@ -3663,7 +3663,7 @@ func rewriteValueMIPS_OpMIPSMOVBreg_0(v *Value) bool {
 			break
 		}
 		b = x.Block
-		v0 := b.NewValue0(v.Pos, OpMIPSMOVBload, t)
+		v0 := b.NewValue0(x.Pos, OpMIPSMOVBload, t)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = off
@@ -4359,7 +4359,7 @@ func rewriteValueMIPS_OpMIPSMOVHUreg_0(v *Value) bool {
 			break
 		}
 		b = x.Block
-		v0 := b.NewValue0(v.Pos, OpMIPSMOVHUload, t)
+		v0 := b.NewValue0(x.Pos, OpMIPSMOVHUload, t)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = off
@@ -4570,7 +4570,7 @@ func rewriteValueMIPS_OpMIPSMOVHreg_0(v *Value) bool {
 			break
 		}
 		b = x.Block
-		v0 := b.NewValue0(v.Pos, OpMIPSMOVHload, t)
+		v0 := b.NewValue0(x.Pos, OpMIPSMOVHload, t)
 		v.reset(OpCopy)
 		v.AddArg(v0)
 		v0.AuxInt = off

test/fixedbugs/issue27201.go

+// run
+
+// Copyright 2019 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.
+
+package main
+
+import (
+	"runtime"
+	"strings"
+)
+
+func main() {
+	f(nil)
+}
+
+func f(p *int32) {
+	defer checkstack()
+	v := *p         // panic should happen here, line 20
+	sink = int64(v) // not here, line 21
+}
+
+var sink int64
+
+func checkstack() {
+	_ = recover()
+	var buf [1024]byte
+	n := runtime.Stack(buf[:], false)
+	s := string(buf[:n])
+	if strings.Contains(s, "issue27201.go:21 ") {
+		panic("panic at wrong location")
+	}
+	if !strings.Contains(s, "issue27201.go:20 ") {
+		panic("no panic at correct location")
+	}
+}