go/go/analysis
Keith Randall 3288bc1ea1 go/analysis: add frame pointer check for vet
Our calling convention requires BP to be preserved (it is
callee-save).  But that only happened with the introduction of
framepointers in 1.5. Furthermore, nothing checks it, so there is
assembly from before and since which violates the calling
convention. The frame pointer is not used by Go, but only by kernel
traceback code (for sampled profiling), so no one noticed.  Also, the
frame pointer tends to "fix itself", since after it is clobbered by an
assembly function f, the calling function fixes the frame pointer up
when that function returns.

I've fixed the stdlib, CLs 248260, 248261, 248262.

This CL is a simple check, intended to be used in vet, to catch
assembly that violates the calling convention by overwriting the
frame pointer without saving/restoring it. It is not intended to
catch all violations, just ones that are easy to find.

We look for a write to the frame pointer before any use of the frame
pointer or any control flow instruction. In such a situation, there's
no way the code could restore the value of BP before returning. This
analysis is very conservative; it gives up in lots of cases. False
positive should be very rare, though.

Possible false positives:

 // looks like a write to BP, but isn't.
CMPQ  BP, BP

// BP actually isn't clobbered, just swapped with AX.
XORQ  AX, BP
XORQ  BP, AX
XORQ  AX, BP

The first is unlikely, as it is using the contents of an incoming
register, which is junk. The second is a general problem of op=
assembly operations that are indistiguishable in syntax from =
operations.  But anything other than the swap above also depends on
the incoming junk value in BP. The swap itself is pointless (XCHQ
works better).

Change-Id: Ie9d91ab3396409486f7022380ad46ac76c3fbed4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248686
Trust: Keith Randall <khr@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Keith Randall <khr@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2020-11-02 19:21:40 +00:00
..
analysistest go/analysis: add IgnoredFiles, check ignored files in buildtag check 2020-10-13 17:44:05 +00:00
doc all: fix more typos 2019-09-11 15:13:14 +00:00
internal go/analysis: add IgnoredFiles, check ignored files in buildtag check 2020-10-13 17:44:05 +00:00
multichecker Revert "go/packages: temporarily disable some tests running on go tip with -race" 2019-10-28 14:32:39 +00:00
passes go/analysis: add frame pointer check for vet 2020-11-02 19:21:40 +00:00
singlechecker go/analysis/singlechecker: fix whitespace in package documentation 2020-10-31 02:16:30 +00:00
unitchecker go/analysis: add IgnoredFiles, check ignored files in buildtag check 2020-10-13 17:44:05 +00:00
analysis.go go/analysis: add IgnoredFiles, check ignored files in buildtag check 2020-10-13 17:44:05 +00:00
diagnostic.go go/analysis, internal/lsp: add support for related information 2019-10-21 19:00:55 +00:00
doc.go go/analysis: add IgnoredFiles, check ignored files in buildtag check 2020-10-13 17:44:05 +00:00
validate.go report cycle when visiting a grey analyzer 2020-09-11 19:35:55 +00:00
validate_test.go report cycle when visiting a grey analyzer 2020-09-11 19:35:55 +00:00