cmd/compile: disallow rewrite rules from declaring reserved names

If I change a rule in ARM64.rules to use the variable name "b" in a
conflicting way, rulegen would previously not complain, and the compiler
would later give a confusing error:

	$ go run *.go && go build cmd/compile/internal/ssa
	# cmd/compile/internal/ssa
	../rewriteARM64.go:24236:10: b.NewValue0 undefined (type int64 has no field or method NewValue0)

Make rulegen complain early about those cases. Sometimes they might
happen to be harmless, but in general they can easily cause confusion or
unintended effect due to shadowing.

After the change, with the same conflicting rule:

	$ go run *.go && go build cmd/compile/internal/ssa
	2021/03/22 11:31:49 rule ARM64.rules:495 uses the reserved name b
	exit status 1

Note that 24 existing rules were using reserved names. It seems like the
shadowing was harmless, as it wasn't causing typechecking issues nor did
it seem to cause unintended behavior when the rule rewrite code ran.

The bool values "b" were renamed "t", since that seems to have a
precedent in other rules and in the fmt package.

Sequential values like "a b c" were renamed to "x y z", since "b" is
reserved.

Finally, "typ" was renamed to "_typ", since there doesn't seem to be an
obviously better answer.

Passes all three of:

	$ GOARCH=amd64 go build -toolexec 'toolstash -cmp' -a std
	$ GOARCH=arm64 go build -toolexec 'toolstash -cmp' -a std
	$ GOARCH=mips64 go build -toolexec 'toolstash -cmp' -a std

Fixes #45154.

Change-Id: I1cce194dc7b477886a9c218c17973e996bcedccf
Reviewed-on: https://go-review.googlesource.com/c/go/+/303549
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

8 files changed, 68 insertions, 44 deletions

diff --git a/src/cmd/compile/internal/ssa/gen/ARM.rules b/src/cmd/compile/internal/ssa/gen/ARM.rules
index f46f4238f7..5c6438a986 100644
--- a/src/cmd/compile/internal/ssa/gen/ARM.rules
+++ b/src/cmd/compile/internal/ssa/gen/ARM.rules
@@ -173,7 +173,7 @@
 (Const(8|16|32) [val]) => (MOVWconst [int32(val)])
 (Const(32|64)F [val]) => (MOV(F|D)const [float64(val)])
 (ConstNil) => (MOVWconst [0])
-(ConstBool [b]) => (MOVWconst [b2i32(b)])
+(ConstBool [t]) => (MOVWconst [b2i32(t)])
 
 // truncations
 // Because we ignore high parts of registers, truncates are just copies.
diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules
index 6f30c11bd1..7f9f8298de 100644
--- a/src/cmd/compile/internal/ssa/gen/ARM64.rules
+++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules
@@ -198,7 +198,7 @@
 (Const(64|32|16|8) [val]) => (MOVDconst [int64(val)])
 (Const(32F|64F) [val]) => (FMOV(S|D)const [float64(val)])
 (ConstNil) => (MOVDconst [0])
-(ConstBool [b]) => (MOVDconst [b2i(b)])
+(ConstBool [t]) => (MOVDconst [b2i(t)])
 
 (Slicemask <t> x) => (SRAconst (NEG <t> x) [63])
 
diff --git a/src/cmd/compile/internal/ssa/gen/MIPS.rules b/src/cmd/compile/internal/ssa/gen/MIPS.rules
index 6b59555cbe..4ac9668ea9 100644
--- a/src/cmd/compile/internal/ssa/gen/MIPS.rules
+++ b/src/cmd/compile/internal/ssa/gen/MIPS.rules
@@ -144,7 +144,7 @@
 (Const(32|16|8) [val]) => (MOVWconst [int32(val)])
 (Const(32|64)F ...) => (MOV(F|D)const ...)
 (ConstNil) => (MOVWconst [0])
-(ConstBool [b]) => (MOVWconst [b2i32(b)])
+(ConstBool [t]) => (MOVWconst [b2i32(t)])
 
 // truncations
 // Because we ignore high parts of registers, truncates are just copies.
diff --git a/src/cmd/compile/internal/ssa/gen/MIPS64.rules b/src/cmd/compile/internal/ssa/gen/MIPS64.rules
index bb91dcd5ee..fd04a6c3a8 100644
--- a/src/cmd/compile/internal/ssa/gen/MIPS64.rules
+++ b/src/cmd/compile/internal/ssa/gen/MIPS64.rules
@@ -134,7 +134,7 @@
 (Const(64|32|16|8) [val]) => (MOVVconst [int64(val)])
 (Const(32|64)F [val]) => (MOV(F|D)const [float64(val)])
 (ConstNil) => (MOVVconst [0])
-(ConstBool [b]) => (MOVVconst [int64(b2i(b))])
+(ConstBool [t]) => (MOVVconst [int64(b2i(t))])
 
 (Slicemask <t> x) => (SRAVconst (NEGV <t> x) [63])
 
diff --git a/src/cmd/compile/internal/ssa/gen/PPC64.rules b/src/cmd/compile/internal/ssa/gen/PPC64.rules
index b618cde529..f83ff75761 100644
--- a/src/cmd/compile/internal/ssa/gen/PPC64.rules
+++ b/src/cmd/compile/internal/ssa/gen/PPC64.rules
@@ -101,7 +101,7 @@
 (Const(64|32|16|8) [val]) => (MOVDconst [int64(val)])
 (Const(32|64)F ...) => (FMOV(S|D)const ...)
 (ConstNil) => (MOVDconst [0])
-(ConstBool [b]) => (MOVDconst [b2i(b)])
+(ConstBool [t]) => (MOVDconst [b2i(t)])
 
 // Constant folding
 (FABS (FMOVDconst [x])) => (FMOVDconst [math.Abs(x)])
diff --git a/src/cmd/compile/internal/ssa/gen/S390X.rules b/src/cmd/compile/internal/ssa/gen/S390X.rules
index 0fdd231d71..88762f7045 100644
--- a/src/cmd/compile/internal/ssa/gen/S390X.rules
+++ b/src/cmd/compile/internal/ssa/gen/S390X.rules
@@ -428,7 +428,7 @@
 (Const(64|32|16|8) [val]) => (MOVDconst [int64(val)])
 (Const(32|64)F ...) => (FMOV(S|D)const ...)
 (ConstNil) => (MOVDconst [0])
-(ConstBool [b]) => (MOVDconst [b2i(b)])
+(ConstBool [t]) => (MOVDconst [b2i(t)])
 
 // Lowering calls
 (StaticCall ...) => (CALLstatic ...)
diff --git a/src/cmd/compile/internal/ssa/gen/generic.rules b/src/cmd/compile/internal/ssa/gen/generic.rules
index 9dd20a7cfa..6b5fd99c7e 100644
--- a/src/cmd/compile/internal/ssa/gen/generic.rules
+++ b/src/cmd/compile/internal/ssa/gen/generic.rules
@@ -768,7 +768,7 @@
 	=> mem
 
 // Collapse OffPtr
-(OffPtr (OffPtr p [b]) [a]) => (OffPtr p [a+b])
+(OffPtr (OffPtr p [y]) [x]) => (OffPtr p [x+y])
 (OffPtr p [0]) && v.Type.Compare(p.Type) == types.CMPeq => p
 
 // indexing operations
@@ -847,7 +847,7 @@
             f0 mem))))
 
 // Putting struct{*byte} and similar into direct interfaces.
-(IMake typ (StructMake1 val)) => (IMake typ val)
+(IMake _typ (StructMake1 val)) => (IMake _typ val)
 (StructSelect [0] (IData x)) => (IData x)
 
 // un-SSAable values use mem->mem copies
@@ -869,7 +869,7 @@
 (Store dst (ArrayMake1 e) mem) => (Store {e.Type} dst e mem)
 
 // Putting [1]*byte and similar into direct interfaces.
-(IMake typ (ArrayMake1 val)) => (IMake typ val)
+(IMake _typ (ArrayMake1 val)) => (IMake _typ val)
 (ArraySelect [0] (IData x)) => (IData x)
 
 // string ops
@@ -1974,9 +1974,9 @@
 (Sqrt (Const64F [c])) && !math.IsNaN(math.Sqrt(c)) => (Const64F [math.Sqrt(c)])
 
 // for rewriting results of some late-expanded rewrites (below)
-(SelectN [0] (MakeResult a ___)) => a
-(SelectN [1] (MakeResult a b ___)) => b
-(SelectN [2] (MakeResult a b c ___)) => c
+(SelectN [0] (MakeResult x ___)) => x
+(SelectN [1] (MakeResult x y ___)) => y
+(SelectN [2] (MakeResult x y z ___)) => z
 
 // for late-expanded calls, recognize newobject and remove zeroing and nilchecks
 (Zero (SelectN [0] call:(StaticLECall _ _)) mem:(SelectN [1] call))
@@ -2013,18 +2013,18 @@
 // Evaluate constant address comparisons.
 (EqPtr  x x) => (ConstBool [true])
 (NeqPtr x x) => (ConstBool [false])
-(EqPtr  (Addr {a} _) (Addr {b} _)) => (ConstBool [a == b])
-(EqPtr  (Addr {a} _) (OffPtr [o] (Addr {b} _))) => (ConstBool [a == b && o == 0])
-(EqPtr  (OffPtr [o1] (Addr {a} _)) (OffPtr [o2] (Addr {b} _))) => (ConstBool [a == b && o1 == o2])
-(NeqPtr (Addr {a} _) (Addr {b} _)) => (ConstBool [a != b])
-(NeqPtr (Addr {a} _) (OffPtr [o] (Addr {b} _))) => (ConstBool [a != b || o != 0])
-(NeqPtr (OffPtr [o1] (Addr {a} _)) (OffPtr [o2] (Addr {b} _))) => (ConstBool [a != b || o1 != o2])
-(EqPtr  (LocalAddr {a} _ _) (LocalAddr {b} _ _)) => (ConstBool [a == b])
-(EqPtr  (LocalAddr {a} _ _) (OffPtr [o] (LocalAddr {b} _ _))) => (ConstBool [a == b && o == 0])
-(EqPtr  (OffPtr [o1] (LocalAddr {a} _ _)) (OffPtr [o2] (LocalAddr {b} _ _))) => (ConstBool [a == b && o1 == o2])
-(NeqPtr (LocalAddr {a} _ _) (LocalAddr {b} _ _)) => (ConstBool [a != b])
-(NeqPtr (LocalAddr {a} _ _) (OffPtr [o] (LocalAddr {b} _ _))) => (ConstBool [a != b || o != 0])
-(NeqPtr (OffPtr [o1] (LocalAddr {a} _ _)) (OffPtr [o2] (LocalAddr {b} _ _))) => (ConstBool [a != b || o1 != o2])
+(EqPtr  (Addr {x} _) (Addr {y} _)) => (ConstBool [x == y])
+(EqPtr  (Addr {x} _) (OffPtr [o] (Addr {y} _))) => (ConstBool [x == y && o == 0])
+(EqPtr  (OffPtr [o1] (Addr {x} _)) (OffPtr [o2] (Addr {y} _))) => (ConstBool [x == y && o1 == o2])
+(NeqPtr (Addr {x} _) (Addr {y} _)) => (ConstBool [x != y])
+(NeqPtr (Addr {x} _) (OffPtr [o] (Addr {y} _))) => (ConstBool [x != y || o != 0])
+(NeqPtr (OffPtr [o1] (Addr {x} _)) (OffPtr [o2] (Addr {y} _))) => (ConstBool [x != y || o1 != o2])
+(EqPtr  (LocalAddr {x} _ _) (LocalAddr {y} _ _)) => (ConstBool [x == y])
+(EqPtr  (LocalAddr {x} _ _) (OffPtr [o] (LocalAddr {y} _ _))) => (ConstBool [x == y && o == 0])
+(EqPtr  (OffPtr [o1] (LocalAddr {x} _ _)) (OffPtr [o2] (LocalAddr {y} _ _))) => (ConstBool [x == y && o1 == o2])
+(NeqPtr (LocalAddr {x} _ _) (LocalAddr {y} _ _)) => (ConstBool [x != y])
+(NeqPtr (LocalAddr {x} _ _) (OffPtr [o] (LocalAddr {y} _ _))) => (ConstBool [x != y || o != 0])
+(NeqPtr (OffPtr [o1] (LocalAddr {x} _ _)) (OffPtr [o2] (LocalAddr {y} _ _))) => (ConstBool [x != y || o1 != o2])
 (EqPtr  (OffPtr [o1] p1) p2) && isSamePtr(p1, p2) => (ConstBool [o1 == 0])
 (NeqPtr (OffPtr [o1] p1) p2) && isSamePtr(p1, p2) => (ConstBool [o1 != 0])
 (EqPtr  (OffPtr [o1] p1) (OffPtr [o2] p2)) && isSamePtr(p1, p2) => (ConstBool [o1 == o2])
diff --git a/src/cmd/compile/internal/ssa/gen/rulegen.go b/src/cmd/compile/internal/ssa/gen/rulegen.go
index a3ce726dc3..fd672b2f74 100644
--- a/src/cmd/compile/internal/ssa/gen/rulegen.go
+++ b/src/cmd/compile/internal/ssa/gen/rulegen.go
@@ -218,10 +218,10 @@ func genRulesSuffix(arch arch, suff string) {
 			Suffix: fmt.Sprintf("_%s", op),
 			ArgLen: opByName(arch, op).argLength,
 		}
-		fn.add(declf("b", "v.Block"))
-		fn.add(declf("config", "b.Func.Config"))
-		fn.add(declf("fe", "b.Func.fe"))
-		fn.add(declf("typ", "&b.Func.Config.Types"))
+		fn.add(declReserved("b", "v.Block"))
+		fn.add(declReserved("config", "b.Func.Config"))
+		fn.add(declReserved("fe", "b.Func.fe"))
+		fn.add(declReserved("typ", "&b.Func.Config.Types"))
 		for _, rule := range rules {
 			if rr != nil && !rr.CanFail {
 				log.Fatalf("unconditional rule %s is followed by other rules", rr.Match)
@@ -250,8 +250,8 @@ func genRulesSuffix(arch arch, suff string) {
 	// Generate block rewrite function. There are only a few block types
 	// so we can make this one function with a switch.
 	fn = &Func{Kind: "Block"}
-	fn.add(declf("config", "b.Func.Config"))
-	fn.add(declf("typ", "&b.Func.Config.Types"))
+	fn.add(declReserved("config", "b.Func.Config"))
+	fn.add(declReserved("typ", "&b.Func.Config.Types"))
 
 	sw = &Switch{Expr: exprf("b.Kind")}
 	ops = ops[:0]
@@ -828,12 +828,36 @@ func stmtf(format string, a ...interface{}) Statement {
 	return file.Decls[0].(*ast.FuncDecl).Body.List[0]
 }
 
-// declf constructs a simple "name := value" declaration, using exprf for its
-// value.
-func declf(name, format string, a ...interface{}) *Declare {
+var reservedNames = map[string]bool{
+	"v":      true, // Values[i], etc
+	"b":      true, // v.Block
+	"config": true, // b.Func.Config
+	"fe":     true, // b.Func.fe
+	"typ":    true, // &b.Func.Config.Types
+}
+
+// declf constructs a simple "name := value" declaration,
+// using exprf for its value.
+//
+// name must not be one of reservedNames.
+// This helps prevent unintended shadowing and name clashes.
+// To declare a reserved name, use declReserved.
+func declf(loc, name, format string, a ...interface{}) *Declare {
+	if reservedNames[name] {
+		log.Fatalf("rule %s uses the reserved name %s", loc, name)
+	}
 	return &Declare{name, exprf(format, a...)}
 }
 
+// declReserved is like declf, but the name must be one of reservedNames.
+// Calls to declReserved should generally be static and top-level.
+func declReserved(name, value string) *Declare {
+	if !reservedNames[name] {
+		panic(fmt.Sprintf("declReserved call does not use a reserved name: %q", name))
+	}
+	return &Declare{name, exprf(value)}
+}
+
 // breakf constructs a simple "if cond { break }" statement, using exprf for its
 // condition.
 func breakf(format string, a ...interface{}) *CondBreak {
@@ -858,7 +882,7 @@ func genBlockRewrite(rule Rule, arch arch, data blockData) *RuleRewrite {
 			if vname == "" {
 				vname = fmt.Sprintf("v_%v", i)
 			}
-			rr.add(declf(vname, cname))
+			rr.add(declf(rr.Loc, vname, cname))
 			p, op := genMatch0(rr, arch, expr, vname, nil, false) // TODO: pass non-nil cnt?
 			if op != "" {
 				check := fmt.Sprintf("%s.Op == %s", cname, op)
@@ -873,7 +897,7 @@ func genBlockRewrite(rule Rule, arch arch, data blockData) *RuleRewrite {
 			}
 			pos[i] = p
 		} else {
-			rr.add(declf(arg, cname))
+			rr.add(declf(rr.Loc, arg, cname))
 			pos[i] = arg + ".Pos"
 		}
 	}
@@ -893,7 +917,7 @@ func genBlockRewrite(rule Rule, arch arch, data blockData) *RuleRewrite {
 		if !token.IsIdentifier(e.name) || rr.declared(e.name) {
 			rr.add(breakf("%sTo%s(b.%s) != %s", unTitle(e.field), title(e.dclType), e.field, e.name))
 		} else {
-			rr.add(declf(e.name, "%sTo%s(b.%s)", unTitle(e.field), title(e.dclType), e.field))
+			rr.add(declf(rr.Loc, e.name, "%sTo%s(b.%s)", unTitle(e.field), title(e.dclType), e.field))
 		}
 	}
 	if rr.Cond != "" {
@@ -1043,11 +1067,11 @@ func genMatch0(rr *RuleRewrite, arch arch, match, v string, cnt map[string]int,
 		} else {
 			switch e.field {
 			case "Aux":
-				rr.add(declf(e.name, "auxTo%s(%s.%s)", title(e.dclType), v, e.field))
+				rr.add(declf(rr.Loc, e.name, "auxTo%s(%s.%s)", title(e.dclType), v, e.field))
 			case "AuxInt":
-				rr.add(declf(e.name, "auxIntTo%s(%s.%s)", title(e.dclType), v, e.field))
+				rr.add(declf(rr.Loc, e.name, "auxIntTo%s(%s.%s)", title(e.dclType), v, e.field))
 			case "Type":
-				rr.add(declf(e.name, "%s.%s", v, e.field))
+				rr.add(declf(rr.Loc, e.name, "%s.%s", v, e.field))
 			}
 		}
 	}
@@ -1077,7 +1101,7 @@ func genMatch0(rr *RuleRewrite, arch arch, match, v string, cnt map[string]int,
 				continue
 			}
 			if !rr.declared(a) && token.IsIdentifier(a) && !(commutative && len(args) == 2) {
-				rr.add(declf(a, "%s.Args[%d]", v, n))
+				rr.add(declf(rr.Loc, a, "%s.Args[%d]", v, n))
 				// delete the last argument so it is not reprocessed
 				args = args[:n]
 			} else {
@@ -1089,7 +1113,7 @@ func genMatch0(rr *RuleRewrite, arch arch, match, v string, cnt map[string]int,
 	if commutative && !pregenTop {
 		for i := 0; i <= 1; i++ {
 			vname := fmt.Sprintf("%s_%d", v, i)
-			rr.add(declf(vname, "%s.Args[%d]", v, i))
+			rr.add(declf(rr.Loc, vname, "%s.Args[%d]", v, i))
 		}
 	}
 	if commutative {
@@ -1116,7 +1140,7 @@ func genMatch0(rr *RuleRewrite, arch arch, match, v string, cnt map[string]int,
 				rr.add(breakf("%s != %s", arg, rhs))
 			} else {
 				if arg != rhs {
-					rr.add(declf(arg, "%s", rhs))
+					rr.add(declf(rr.Loc, arg, "%s", rhs))
 				}
 			}
 			continue
@@ -1131,7 +1155,7 @@ func genMatch0(rr *RuleRewrite, arch arch, match, v string, cnt map[string]int,
 		}
 
 		if argname != rhs {
-			rr.add(declf(argname, "%s", rhs))
+			rr.add(declf(rr.Loc, argname, "%s", rhs))
 		}
 		bexpr := exprf("%s.Op != addLater", argname)
 		rr.add(&CondBreak{Cond: bexpr})
@@ -1208,7 +1232,7 @@ func genResult0(rr *RuleRewrite, arch arch, result string, top, move bool, pos s
 			v = resname
 		}
 		rr.Alloc++
-		rr.add(declf(v, "b.NewValue0(%s, Op%s%s, %s)", pos, oparch, op.name, typ))
+		rr.add(declf(rr.Loc, v, "b.NewValue0(%s, Op%s%s, %s)", pos, oparch, op.name, typ))
 		if move && top {
 			// Rewrite original into a copy
 			rr.add(stmtf("v.copyOf(%s)", v))