diff --git a/go/analysis/passes/framepointer/framepointer.go b/go/analysis/passes/framepointer/framepointer.go new file mode 100644 index 0000000000..6c13eeb4ae --- /dev/null +++ b/go/analysis/passes/framepointer/framepointer.go @@ -0,0 +1,91 @@ +// Copyright 2020 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 framepointer defines an Analyzer that reports assembly code +// that clobbers the frame pointer before saving it. +package framepointer + +import ( + "go/build" + "regexp" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/internal/analysisutil" +) + +const Doc = "report assembly that clobbers the frame pointer before saving it" + +var Analyzer = &analysis.Analyzer{ + Name: "framepointer", + Doc: Doc, + 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)`) +) + +func run(pass *analysis.Pass) (interface{}, error) { + if build.Default.GOARCH != "amd64" { // TODO: arm64 also? + return nil, nil + } + if build.Default.GOOS != "linux" && build.Default.GOOS != "darwin" { + return nil, nil + } + + // Find assembly files to work on. + var sfiles []string + for _, fname := range pass.OtherFiles { + if strings.HasSuffix(fname, ".s") { + sfiles = append(sfiles, fname) + } + } + + for _, fname := range sfiles { + content, tf, err := analysisutil.ReadFile(pass.Fset, fname) + if err != nil { + return nil, err + } + + lines := strings.SplitAfter(string(content), "\n") + active := false + for lineno, line := range lines { + lineno++ + + // Ignore comments and commented-out code. + if i := strings.Index(line, "//"); i >= 0 { + line = line[:i] + } + line = strings.TrimSpace(line) + + // 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") { + active = true + continue + } + if !active { + continue + } + + if asmWriteBP.MatchString(line) { // clobber of BP, function is not OK + 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 + active = false + continue + } + } + } + return nil, nil +} diff --git a/go/analysis/passes/framepointer/framepointer_test.go b/go/analysis/passes/framepointer/framepointer_test.go new file mode 100644 index 0000000000..e849308a5d --- /dev/null +++ b/go/analysis/passes/framepointer/framepointer_test.go @@ -0,0 +1,26 @@ +// Copyright 2020 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 framepointer_test + +import ( + "go/build" + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/framepointer" +) + +func Test(t *testing.T) { + if build.Default.GOOS != "linux" && build.Default.GOOS != "darwin" { + // The test has an os-generic assembly file, testdata/a/asm_amd64.s. + // It should produce errors on linux or darwin, but not on other archs. + // Unfortunately, there's no way to say that in the "want" comments + // in that file. So we skip testing on other GOOSes. The framepointer + // analyzer should not report any errors on those GOOSes, so it's not + // really a hard test on those platforms. + t.Skipf("test for GOOS=%s is not implemented", build.Default.GOOS) + } + analysistest.Run(t, analysistest.TestData(), framepointer.Analyzer, "a") +} diff --git a/go/analysis/passes/framepointer/testdata/src/a/asm.go b/go/analysis/passes/framepointer/testdata/src/a/asm.go new file mode 100644 index 0000000000..20d6846a7a --- /dev/null +++ b/go/analysis/passes/framepointer/testdata/src/a/asm.go @@ -0,0 +1,5 @@ +// Copyright 2020 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 a diff --git a/go/analysis/passes/framepointer/testdata/src/a/asm_amd64.s b/go/analysis/passes/framepointer/testdata/src/a/asm_amd64.s new file mode 100644 index 0000000000..a7d1b1cce7 --- /dev/null +++ b/go/analysis/passes/framepointer/testdata/src/a/asm_amd64.s @@ -0,0 +1,36 @@ +// Copyright 2020 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 + MOVQ $0, BP // want `frame pointer is clobbered before saving` + RET +TEXT ·bad2(SB), 0, $0 + MOVQ AX, BP // want `frame pointer is clobbered before saving` + RET +TEXT ·bad3(SB), 0, $0 + MOVQ 6(AX), BP // want `frame pointer is clobbered before saving` + RET +TEXT ·good1(SB), 0, $0 + PUSHQ BP + MOVQ $0, BP // this is ok + POPQ BP + RET +TEXT ·good2(SB), 0, $0 + MOVQ BP, BX + MOVQ $0, BP // this is ok + MOVQ BX, BP + RET +TEXT ·good3(SB), 0, $0 + CMPQ AX, BX + JEQ skip + MOVQ $0, BP // this is ok +skip: + RET +TEXT ·good4(SB), 0, $0 + RET + MOVQ $0, BP // this is ok + RET +TEXT ·good5(SB), 0, $8 + MOVQ $0, BP // this is ok + RET diff --git a/go/analysis/passes/framepointer/testdata/src/a/asm_darwin_amd64.s b/go/analysis/passes/framepointer/testdata/src/a/asm_darwin_amd64.s new file mode 100644 index 0000000000..4a159ec813 --- /dev/null +++ b/go/analysis/passes/framepointer/testdata/src/a/asm_darwin_amd64.s @@ -0,0 +1,7 @@ +// Copyright 2020 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 ·z1(SB), 0, $0 + MOVQ $0, BP // want `frame pointer is clobbered before saving` + RET diff --git a/go/analysis/passes/framepointer/testdata/src/a/asm_linux_amd64.s b/go/analysis/passes/framepointer/testdata/src/a/asm_linux_amd64.s new file mode 100644 index 0000000000..4a159ec813 --- /dev/null +++ b/go/analysis/passes/framepointer/testdata/src/a/asm_linux_amd64.s @@ -0,0 +1,7 @@ +// Copyright 2020 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 ·z1(SB), 0, $0 + MOVQ $0, BP // want `frame pointer is clobbered before saving` + RET diff --git a/go/analysis/passes/framepointer/testdata/src/a/asm_windows_amd64.s b/go/analysis/passes/framepointer/testdata/src/a/asm_windows_amd64.s new file mode 100644 index 0000000000..9e81d6990b --- /dev/null +++ b/go/analysis/passes/framepointer/testdata/src/a/asm_windows_amd64.s @@ -0,0 +1,7 @@ +// Copyright 2020 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 ·z1(SB), 0, $0 + MOVQ $0, BP // not an error on windows + RET diff --git a/go/analysis/passes/framepointer/testdata/src/a/buildtag_amd64.s b/go/analysis/passes/framepointer/testdata/src/a/buildtag_amd64.s new file mode 100644 index 0000000000..6d2ea43c9a --- /dev/null +++ b/go/analysis/passes/framepointer/testdata/src/a/buildtag_amd64.s @@ -0,0 +1,9 @@ +// Copyright 2020 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. + +// +build nope + +TEXT ·bt1(SB), 0, $0 + MOVQ $0, BP // ok because of build tag + RET