close_issue_1
/
tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cmd/vet: check return moves, support power64x, various fixes 

This adds support for checking moves to the return value stack
slot (from rsc), adds support for checking power64x assembly,
fixes argument offset checking and leaf function support on
platforms with a link register (arm and power64).

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/166920043
This commit is contained in:
Austin Clements 2014-10-30 11:53:56 -04:00
parent dcf508a4ed
commit c7512a5f83
1 changed files with 111 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

119
cmd/vet/asmdecl.go
View File

@ -35,6 +35,8 @@ type asmArch struct {
intSize int intSize int
maxAlign int maxAlign int
bigEndian bool bigEndian bool
stack string
lr bool
} }
// An asmFunc describes the expected variables for a function on a given architecture. // An asmFunc describes the expected variables for a function on a given architecture.
@ -56,27 +58,33 @@ type asmVar struct {
} }
var ( var (
asmArch386 = asmArch{"386", 4, 4, 4, false} asmArch386 = asmArch{"386", 4, 4, 4, false, "SP", false}
asmArchArm = asmArch{"arm", 4, 4, 4, false} asmArchArm = asmArch{"arm", 4, 4, 4, false, "R13", true}
asmArchAmd64 = asmArch{"amd64", 8, 8, 8, false} asmArchAmd64 = asmArch{"amd64", 8, 8, 8, false, "SP", false}
asmArchAmd64p32 = asmArch{"amd64p32", 4, 4, 8, false} asmArchAmd64p32 = asmArch{"amd64p32", 4, 4, 8, false, "SP", false}
asmArchPower64 = asmArch{"power64", 8, 8, 8, true, "R1", true}
asmArchPower64LE = asmArch{"power64le", 8, 8, 8, false, "R1", true}
arches = []*asmArch{ arches = []*asmArch{
&asmArch386, &asmArch386,
&asmArchArm, &asmArchArm,
&asmArchAmd64, &asmArchAmd64,
&asmArchAmd64p32, &asmArchAmd64p32,
&asmArchPower64,
&asmArchPower64LE,
} }
) )
var ( var (
re = regexp.MustCompile re = regexp.MustCompile
asmPlusBuild = re(`//\s+\+build\s+([^\n]+)`) asmPlusBuild = re(`//\s+\+build\s+([^\n]+)`)
asmTEXT = re(`\bTEXT\b.*·([^\(]+)\(SB\)(?:\s*,\s*([0-9A-Z|+]+))?(?:\s*,\s*\$([0-9]+)(?:-([0-9]+))?)?`) asmTEXT = re(`\bTEXT\b.*·([^\(]+)\(SB\)(?:\s*,\s*([0-9A-Z|+]+))?(?:\s*,\s*\$(-?[0-9]+)(?:-([0-9]+))?)?`)
asmDATA = re(`\b(DATA|GLOBL)\b`) asmDATA = re(`\b(DATA|GLOBL)\b`)
asmNamedFP = re(`([a-zA-Z0-9_\xFF-\x{10FFFF}]+)(?:\+([0-9]+))\(FP\)`) asmNamedFP = re(`([a-zA-Z0-9_\xFF-\x{10FFFF}]+)(?:\+([0-9]+))\(FP\)`)
asmUnnamedFP = re(`[^+\-0-9]](([0-9]+)\(FP\))`) asmUnnamedFP = re(`[^+\-0-9]](([0-9]+)\(FP\))`)
asmSP = re(`[^+\-0-9](([0-9]+)\(([A-Z0-9]+)\))`)
asmOpcode = re(`^\s*(?:[A-Z0-9a-z_]+:)?\s*([A-Z]+)\s*([^,]*)(?:,\s*(.*))?`) asmOpcode = re(`^\s*(?:[A-Z0-9a-z_]+:)?\s*([A-Z]+)\s*([^,]*)(?:,\s*(.*))?`)
power64Suff = re(`([BHWD])(ZU|Z|U|BR)?$`)
) )
func asmCheck(pkg *Package) { func asmCheck(pkg *Package) {
@ -102,7 +110,6 @@ func asmCheck(pkg *Package) {
} }
} }
var fn *asmFunc
for _, f := range pkg.files { for _, f := range pkg.files {
if !strings.HasSuffix(f.name, ".s") { if !strings.HasSuffix(f.name, ".s") {
continue continue
@ -111,19 +118,39 @@ func asmCheck(pkg *Package) {
// Determine architecture from file name if possible. // Determine architecture from file name if possible.
var arch string var arch string
var archDef *asmArch
for _, a := range arches { for _, a := range arches {
if strings.HasSuffix(f.name, "_"+a.name+".s") { if strings.HasSuffix(f.name, "_"+a.name+".s") {
arch = a.name arch = a.name
archDef = a
break break
} }
} }
lines := strings.SplitAfter(string(f.content), "\n") lines := strings.SplitAfter(string(f.content), "\n")
var (
fn *asmFunc
fnName string
localSize, argSize int
wroteSP bool
haveRetArg bool
retLine []int
)
flushRet := func() {
if fn != nil && fn.vars["ret"] != nil && !haveRetArg && len(retLine) > 0 {
v := fn.vars["ret"]
for _, line := range retLine {
f.Badf(token.NoPos, "%s:%d: [%s] %s: RET without writing to %d-byte ret+%d(FP)", f.name, line, arch, fnName, v.size, v.off)
}
}
retLine = nil
}
for lineno, line := range lines { for lineno, line := range lines {
lineno++ lineno++
badf := func(format string, args ...interface{}) { badf := func(format string, args ...interface{}) {
f.Badf(token.NoPos, "%s:%d: [%s] %s", f.name, lineno, arch, fmt.Sprintf(format, args...)) f.Badf(token.NoPos, "%s:%d: [%s] %s: %s", f.name, lineno, arch, fnName, fmt.Sprintf(format, args...))
} }
if arch == "" { if arch == "" {
@ -134,6 +161,7 @@ func asmCheck(pkg *Package) {
for _, a := range arches { for _, a := range arches {
if a.name == fld { if a.name == fld {
arch = a.name arch = a.name
archDef = a
break Fields break Fields
} }
} }
@ -142,10 +170,12 @@ func asmCheck(pkg *Package) {
} }
if m := asmTEXT.FindStringSubmatch(line); m != nil { if m := asmTEXT.FindStringSubmatch(line); m != nil {
flushRet()
if arch == "" { if arch == "" {
f.Warnf(token.NoPos, "%s: cannot determine architecture for assembly file", f.name) f.Warnf(token.NoPos, "%s: cannot determine architecture for assembly file", f.name)
return return
} }
fnName = m[1]
fn = knownFunc[m[1]][arch] fn = knownFunc[m[1]][arch]
if fn != nil { if fn != nil {
size, _ := strconv.Atoi(m[4]) size, _ := strconv.Atoi(m[4])
@ -153,16 +183,69 @@ func asmCheck(pkg *Package) {
badf("wrong argument size %d; expected $...-%d", size, fn.size) badf("wrong argument size %d; expected $...-%d", size, fn.size)
} }
} }
localSize, _ = strconv.Atoi(m[3])
localSize += archDef.intSize
if archDef.lr {
// Account for caller's saved LR
localSize += archDef.intSize
}
argSize, _ = strconv.Atoi(m[4])
wroteSP = false
haveRetArg = false
continue continue
} else if strings.Contains(line, "TEXT") && strings.Contains(line, "SB") { } else if strings.Contains(line, "TEXT") && strings.Contains(line, "SB") {
// function, but not visible from Go (didn't match asmTEXT), so stop checking // function, but not visible from Go (didn't match asmTEXT), so stop checking
flushRet()
fn = nil fn = nil
fnName = ""
continue
}
if strings.Contains(line, "RET") {
retLine = append(retLine, lineno)
}
if fnName == "" {
continue continue
} }
if asmDATA.FindStringSubmatch(line) != nil { if asmDATA.FindStringSubmatch(line) != nil {
fn = nil fn = nil
} }
if archDef == nil {
continue
}
if strings.Contains(line, ", "+archDef.stack) || strings.Contains(line, ",\t"+archDef.stack) {
wroteSP = true
continue
}
for _, m := range asmSP.FindAllStringSubmatch(line, -1) {
if m[3] != archDef.stack || wroteSP {
continue
}
off := 0
if m[1] != "" {
off, _ = strconv.Atoi(m[2])
}
if off >= localSize {
if fn != nil {
v := fn.varByOffset[off-localSize]
if v != nil {
badf("%s should be %s+%d(FP)", m[1], v.name, off-localSize)
continue
}
}
if off >= localSize+argSize {
badf("use of %s points beyond argument frame", m[1])
continue
}
badf("use of %s to access argument frame", m[1])
}
}
if fn == nil { if fn == nil {
continue continue
} }
@ -177,6 +260,9 @@ func asmCheck(pkg *Package) {
if m[2] != "" { if m[2] != "" {
off, _ = strconv.Atoi(m[2]) off, _ = strconv.Atoi(m[2])
} }
if name == "ret" || strings.HasPrefix(name, "ret_") {
haveRetArg = true
}
v := fn.vars[name] v := fn.vars[name]
if v == nil { if v == nil {
// Allow argframe+0(FP). // Allow argframe+0(FP).
@ -194,6 +280,7 @@ func asmCheck(pkg *Package) {
asmCheckVar(badf, fn, line, m[0], off, v) asmCheckVar(badf, fn, line, m[0], off, v)
} }
} }
flushRet()
} }
} }
@ -453,7 +540,8 @@ func asmCheckVar(badf func(string, ...interface{}), fn *asmFunc, line, expr stri
case "amd64p32.LEAL": case "amd64p32.LEAL":
dst = 4 dst = 4
default: default:
if fn.arch.name == "386" || fn.arch.name == "amd64" { switch fn.arch.name {
case "386", "amd64":
if strings.HasPrefix(op, "F") && (strings.HasSuffix(op, "D") || strings.HasSuffix(op, "DP")) { if strings.HasPrefix(op, "F") && (strings.HasSuffix(op, "D") || strings.HasSuffix(op, "DP")) {
// FMOVDP, FXCHD, etc // FMOVDP, FXCHD, etc
src = 8 src = 8
@ -489,6 +577,21 @@ func asmCheckVar(badf func(string, ...interface{}), fn *asmFunc, line, expr stri
case 'D', 'Q': case 'D', 'Q':
src = 8 src = 8
} }
case "power64", "power64le":
// Strip standard suffixes to reveal size letter.
m := power64Suff.FindStringSubmatch(op)
if m != nil {
switch m[1][0] {
case 'B':
src = 1
case 'H':
src = 2
case 'W':
src = 4
case 'D':
src = 8
}
}
} }
} }
if dst == 0 { if dst == 0 {