Skip to content

Commit a1eb5fd

Browse files
committed
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 <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
1 parent 9c087d9 commit a1eb5fd

File tree

2 files changed

+137
-13
lines changed

2 files changed

+137
-13
lines changed

go/analysis/passes/framepointer/framepointer.go

+95-13
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"go/build"
1111
"regexp"
1212
"strings"
13+
"unicode"
1314

1415
"golang.org/x/tools/go/analysis"
1516
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
@@ -24,15 +25,97 @@ var Analyzer = &analysis.Analyzer{
2425
Run: run,
2526
}
2627

27-
var (
28-
re = regexp.MustCompile
29-
asmWriteBP = re(`,\s*BP$`) // TODO: can have false positive, e.g. for TESTQ BP,BP. Seems unlikely.
30-
asmMentionBP = re(`\bBP\b`)
31-
asmControlFlow = re(`^(J|RET)`)
32-
)
28+
// Per-architecture checks for instructions.
29+
// Assume comments, leading and trailing spaces are removed.
30+
type arch struct {
31+
isFPWrite func(string) bool
32+
isFPRead func(string) bool
33+
isBranch func(string) bool
34+
}
35+
36+
var re = regexp.MustCompile
37+
38+
func hasAnyPrefix(s string, prefixes ...string) bool {
39+
for _, p := range prefixes {
40+
if strings.HasPrefix(s, p) {
41+
return true
42+
}
43+
}
44+
return false
45+
}
46+
47+
var arches = map[string]arch{
48+
"amd64": {
49+
isFPWrite: re(`,\s*BP$`).MatchString, // TODO: can have false positive, e.g. for TESTQ BP,BP. Seems unlikely.
50+
isFPRead: re(`\bBP\b`).MatchString,
51+
isBranch: func(s string) bool {
52+
return hasAnyPrefix(s, "J", "RET")
53+
},
54+
},
55+
"arm64": {
56+
isFPWrite: func(s string) bool {
57+
if i := strings.LastIndex(s, ","); i > 0 && strings.HasSuffix(s[i:], "R29") {
58+
return true
59+
}
60+
if hasAnyPrefix(s, "LDP", "LDAXP", "LDXP", "CASP") {
61+
// Instructions which write to a pair of registers, e.g.
62+
// LDP 8(R0), (R26, R29)
63+
// CASPD (R2, R3), (R2), (R26, R29)
64+
lp := strings.LastIndex(s, "(")
65+
rp := strings.LastIndex(s, ")")
66+
if lp > -1 && lp < rp {
67+
return strings.Contains(s[lp:rp], ",") && strings.Contains(s[lp:rp], "R29")
68+
}
69+
}
70+
return false
71+
},
72+
isFPRead: re(`\bR29\b`).MatchString,
73+
isBranch: func(s string) bool {
74+
// Get just the instruction
75+
if i := strings.IndexFunc(s, unicode.IsSpace); i > 0 {
76+
s = s[:i]
77+
}
78+
return arm64Branch[s]
79+
},
80+
},
81+
}
82+
83+
// arm64 has many control flow instructions.
84+
// ^(B|RET) isn't sufficient or correct (e.g. BIC, BFI aren't control flow.)
85+
// It's easier to explicitly enumerate them in a map than to write a regex.
86+
// Borrowed from Go tree, cmd/asm/internal/arch/arm64.go
87+
var arm64Branch = map[string]bool{
88+
"B": true,
89+
"BL": true,
90+
"BEQ": true,
91+
"BNE": true,
92+
"BCS": true,
93+
"BHS": true,
94+
"BCC": true,
95+
"BLO": true,
96+
"BMI": true,
97+
"BPL": true,
98+
"BVS": true,
99+
"BVC": true,
100+
"BHI": true,
101+
"BLS": true,
102+
"BGE": true,
103+
"BLT": true,
104+
"BGT": true,
105+
"BLE": true,
106+
"CBZ": true,
107+
"CBZW": true,
108+
"CBNZ": true,
109+
"CBNZW": true,
110+
"JMP": true,
111+
"TBNZ": true,
112+
"TBZ": true,
113+
"RET": true,
114+
}
33115

34116
func run(pass *analysis.Pass) (interface{}, error) {
35-
if build.Default.GOARCH != "amd64" { // TODO: arm64 also?
117+
arch, ok := arches[build.Default.GOARCH]
118+
if !ok {
36119
return nil, nil
37120
}
38121
if build.Default.GOOS != "linux" && build.Default.GOOS != "darwin" {
@@ -63,6 +146,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
63146
line = line[:i]
64147
}
65148
line = strings.TrimSpace(line)
149+
if line == "" {
150+
continue
151+
}
66152

67153
// We start checking code at a TEXT line for a frameless function.
68154
if strings.HasPrefix(line, "TEXT") && strings.Contains(line, "(SB)") && strings.Contains(line, "$0") {
@@ -73,16 +159,12 @@ func run(pass *analysis.Pass) (interface{}, error) {
73159
continue
74160
}
75161

76-
if asmWriteBP.MatchString(line) { // clobber of BP, function is not OK
162+
if arch.isFPWrite(line) {
77163
pass.Reportf(analysisutil.LineStart(tf, lineno), "frame pointer is clobbered before saving")
78164
active = false
79165
continue
80166
}
81-
if asmMentionBP.MatchString(line) { // any other use of BP might be a read, so function is OK
82-
active = false
83-
continue
84-
}
85-
if asmControlFlow.MatchString(line) { // give up after any branch instruction
167+
if arch.isFPRead(line) || arch.isBranch(line) {
86168
active = false
87169
continue
88170
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
TEXT ·bad1(SB), 0, $0
6+
MOVD $0, R29 // want `frame pointer is clobbered before saving`
7+
RET
8+
TEXT ·bad2(SB), 0, $0
9+
MOVD R1, R29 // want `frame pointer is clobbered before saving`
10+
RET
11+
TEXT ·bad3(SB), 0, $0
12+
MOVD 6(R2), R29 // want `frame pointer is clobbered before saving`
13+
RET
14+
TEXT ·bad4(SB), 0, $0
15+
LDP 0(R1), (R26, R29) // want `frame pointer is clobbered before saving`
16+
RET
17+
TEXT ·bad5(SB), 0, $0
18+
AND $0x1, R3, R29 // want `frame pointer is clobbered before saving`
19+
RET
20+
TEXT ·good1(SB), 0, $0
21+
STPW (R29, R30), -32(RSP)
22+
MOVD $0, R29 // this is ok
23+
LDPW 32(RSP), (R29, R30)
24+
RET
25+
TEXT ·good2(SB), 0, $0
26+
MOVD R29, R1
27+
MOVD $0, R29 // this is ok
28+
MOVD R1, R29
29+
RET
30+
TEXT ·good3(SB), 0, $0
31+
CMP R1, R2
32+
BEQ skip
33+
MOVD $0, R29 // this is ok
34+
skip:
35+
RET
36+
TEXT ·good4(SB), 0, $0
37+
RET
38+
MOVD $0, R29 // this is ok
39+
RET
40+
TEXT ·good5(SB), 0, $8
41+
MOVD $0, R29 // this is ok
42+
RET

0 commit comments

Comments
 (0)