go/analysis/passes/framepointer: support arm64

Add arm64 support to the framepointer vet check. Essentially use the
same logic as the amd64 check: in instruction order, look at functions
without frames, and fail if the functions write to the frame pointer
register before reading it. Stop looking at a function on the first
branch instruction.

For golang/go#69838

Change-Id: If69be8a6eb5f9275df602c2c2ff644c338deaef2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/635338
Reviewed-by: Cherry Mui <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>

go/analysis/passes/framepointer/framepointer.go

@@ -10,6 +10,7 @@ import (
 	"go/build"
 	"regexp"
 	"strings"
+	"unicode"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
@@ -24,15 +25,97 @@ var Analyzer = &analysis.Analyzer{
 	Run:  run,
 }
 
-var (
-	re             = regexp.MustCompile
-	asmWriteBP     = re(`,\s*BP$`) // TODO: can have false positive, e.g. for TESTQ BP,BP. Seems unlikely.
-	asmMentionBP   = re(`\bBP\b`)
-	asmControlFlow = re(`^(J|RET)`)
-)
+// Per-architecture checks for instructions.
+// Assume comments, leading and trailing spaces are removed.
+type arch struct {
+	isFPWrite func(string) bool
+	isFPRead  func(string) bool
+	isBranch  func(string) bool
+}
+
+var re = regexp.MustCompile
+
+func hasAnyPrefix(s string, prefixes ...string) bool {
+	for _, p := range prefixes {
+		if strings.HasPrefix(s, p) {
+			return true
+		}
+	}
+	return false
+}
+
+var arches = map[string]arch{
+	"amd64": {
+		isFPWrite: re(`,\s*BP$`).MatchString, // TODO: can have false positive, e.g. for TESTQ BP,BP. Seems unlikely.
+		isFPRead:  re(`\bBP\b`).MatchString,
+		isBranch: func(s string) bool {
+			return hasAnyPrefix(s, "J", "RET")
+		},
+	},
+	"arm64": {
+		isFPWrite: func(s string) bool {
+			if i := strings.LastIndex(s, ","); i > 0 && strings.HasSuffix(s[i:], "R29") {
+				return true
+			}
+			if hasAnyPrefix(s, "LDP", "LDAXP", "LDXP", "CASP") {
+				// Instructions which write to a pair of registers, e.g.
+				//	LDP 8(R0), (R26, R29)
+				//	CASPD (R2, R3), (R2), (R26, R29)
+				lp := strings.LastIndex(s, "(")
+				rp := strings.LastIndex(s, ")")
+				if lp > -1 && lp < rp {
+					return strings.Contains(s[lp:rp], ",") && strings.Contains(s[lp:rp], "R29")
+				}
+			}
+			return false
+		},
+		isFPRead: re(`\bR29\b`).MatchString,
+		isBranch: func(s string) bool {
+			// Get just the instruction
+			if i := strings.IndexFunc(s, unicode.IsSpace); i > 0 {
+				s = s[:i]
+			}
+			return arm64Branch[s]
+		},
+	},
+}
+
+// arm64 has many control flow instructions.
+// ^(B|RET) isn't sufficient or correct (e.g. BIC, BFI aren't control flow.)
+// It's easier to explicitly enumerate them in a map than to write a regex.
+// Borrowed from Go tree, cmd/asm/internal/arch/arm64.go
+var arm64Branch = map[string]bool{
+	"B":     true,
+	"BL":    true,
+	"BEQ":   true,
+	"BNE":   true,
+	"BCS":   true,
+	"BHS":   true,
+	"BCC":   true,
+	"BLO":   true,
+	"BMI":   true,
+	"BPL":   true,
+	"BVS":   true,
+	"BVC":   true,
+	"BHI":   true,
+	"BLS":   true,
+	"BGE":   true,
+	"BLT":   true,
+	"BGT":   true,
+	"BLE":   true,
+	"CBZ":   true,
+	"CBZW":  true,
+	"CBNZ":  true,
+	"CBNZW": true,
+	"JMP":   true,
+	"TBNZ":  true,
+	"TBZ":   true,
+	"RET":   true,
+}
 
 func run(pass *analysis.Pass) (interface{}, error) {
-	if build.Default.GOARCH != "amd64" { // TODO: arm64 also?
+	arch, ok := arches[build.Default.GOARCH]
+	if !ok {
 		return nil, nil
 	}
 	if build.Default.GOOS != "linux" && build.Default.GOOS != "darwin" {
@@ -63,6 +146,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
 				line = line[:i]
 			}
 			line = strings.TrimSpace(line)
+			if line == "" {
+				continue
+			}
 
 			// We start checking code at a TEXT line for a frameless function.
 			if strings.HasPrefix(line, "TEXT") && strings.Contains(line, "(SB)") && strings.Contains(line, "$0") {
@@ -73,16 +159,12 @@ func run(pass *analysis.Pass) (interface{}, error) {
 				continue
 			}
 
-			if asmWriteBP.MatchString(line) { // clobber of BP, function is not OK
+			if arch.isFPWrite(line) {
 				pass.Reportf(analysisutil.LineStart(tf, lineno), "frame pointer is clobbered before saving")
 				active = false
 				continue
 			}
-			if asmMentionBP.MatchString(line) { // any other use of BP might be a read, so function is OK
-				active = false
-				continue
-			}
-			if asmControlFlow.MatchString(line) { // give up after any branch instruction
+			if arch.isFPRead(line) || arch.isBranch(line) {
 				active = false
 				continue
 			}

go/analysis/passes/framepointer/testdata/src/a/asm_arm64.s

@@ -0,0 +1,42 @@
+// Copyright 2024 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.
+
+TEXT ·bad1(SB), 0, $0
+	MOVD	$0, R29 // want `frame pointer is clobbered before saving`
+	RET
+TEXT ·bad2(SB), 0, $0
+	MOVD	R1, R29 // want `frame pointer is clobbered before saving`
+	RET
+TEXT ·bad3(SB), 0, $0
+	MOVD	6(R2), R29 // want `frame pointer is clobbered before saving`
+	RET
+TEXT ·bad4(SB), 0, $0
+	LDP	0(R1), (R26, R29) // want `frame pointer is clobbered before saving`
+	RET
+TEXT ·bad5(SB), 0, $0
+	AND	$0x1, R3, R29 // want `frame pointer is clobbered before saving`
+	RET
+TEXT ·good1(SB), 0, $0
+	STPW 	(R29, R30), -32(RSP)
+	MOVD	$0, R29 // this is ok
+	LDPW	32(RSP), (R29, R30)
+	RET
+TEXT ·good2(SB), 0, $0
+	MOVD	R29, R1
+	MOVD	$0, R29 // this is ok
+	MOVD	R1, R29
+	RET
+TEXT ·good3(SB), 0, $0
+	CMP	R1, R2
+	BEQ	skip
+	MOVD	$0, R29 // this is ok
+skip:
+	RET
+TEXT ·good4(SB), 0, $0
+	RET
+	MOVD	$0, R29 // this is ok
+	RET
+TEXT ·good5(SB), 0, $8
+	MOVD	$0, R29 // this is ok
+	RET