x/tools/go/analysis: framepointer doesn't report BP clobbering on Windows

[egonelbre]

https://go-review.googlesource.com/c/tools/+/248686 introduced frame pointer check for clobbering BP register, however the check is only enabled for linux and darwin.

// func mixBlocksSSE2(out, a, b, c *block)
TEXT ·mixBlocksSSE2(SB), 4, $0-32
	MOVQ out+0(FP), DX
	MOVQ a+8(FP), AX
	MOVQ b+16(FP), BX
	MOVQ c+24(FP), CX
	MOVQ $128, BP
	RET

As an example doesn't report the clobbering on windows. https://github.com/golang/crypto/blob/5ff15b29337e062d850872081bcd9f4d784f4c25/argon2/blamka_amd64.s#L204

It's not clear why other OS-s were excluded from this analysis.

I would expect the clobbering to be reported on files that are compiled for linux/darwin on windows as well.

[dmitshur]

CC @randall77, @timothy-king.

[randall77]

I don't remember why I excluded other OSes. It's worth trying to enable them and see what happens.
Does Windows ever try to traceback user code? Does it use BP to do so? If the answer is no to either, then it may just not matter on Windows.

[egonelbre]

If the answer is no to either, then it may just not matter on Windows.

I'll have to check the BP usage on Windows, I suspect no. But, even if BP usage is valid on Windows it might not be on Linux/Darwin -- so, I would still expect the warning for assembly that is for all OS-s when running go vet on Windows. If the assembly file is tagged only for Windows, then not reporting BP clobbering is understandable.

[pilinux]

Even the latest crypto package v0.12.0 does not provide option to use secret for argon2. Hence yesterday I started developing my own argon2 package on top of the latest crypto package from Go. Since I need to access the deriveKey function to achieve the gaol easily, I decided to clone argon2 from official Go repo and everything went fine. On mac (arm), I faced no issue. But then I ran static analysis on other machines and encountered these problems.

On Ubuntu 22.04 (amd64):

/blamka_amd64.s:203:1: [amd64] mixBlocksSSE2: invalid offset a+24(FP); expected a+8(FP)
./blamka_amd64.s:226:1: [amd64] xorBlocksSSE2: invalid offset a+24(FP); expected a+8(FP)
./blamka_amd64.s:204:1: frame pointer is clobbered before saving
./blamka_amd64.s:227:1: frame pointer is clobbered before saving

On Windows 11 (intel/amd64):

/blamka_amd64.s:203:1: [amd64] mixBlocksSSE2: invalid offset a+24(FP); expected a+8(FP)
./blamka_amd64.s:226:1: [amd64] xorBlocksSSE2: invalid offset a+24(FP); expected a+8(FP)

Then I tried to understand the assembly codes for Go assembler and I got confused here:

https://cs.opensource.google/go/x/crypto/+/refs/tags/v0.12.0:argon2/blamka_amd64.s

• line 201: MOVQ a+8(FP), AX
• line 203: MOVQ a+24(FP), CX
• line 224: MOVQ a+8(FP), AX
• line 226: MOVQ a+24(FP), CX

In my understanding, line 203 should be MOVQ c+24(FP), CX and line 226 should be MOVQ c+24(FP), CX.

Also, the original BP was not restored properly, hence the error on Ubuntu. Later I did modify the code this way:
pilinux/argon2@e374b3d

And all those errors were solved. I did static analysis for arm64 and amd64 and for win, mac and linux using Github actions (you can check the results in the repo). Today I used this package in an app and can confirm that everything worked as they should.

I wanted to create an issue but then found this one, hence I am writing here.

Perhaps someone from the official Go dev team will look into this.

Thank you.

[gopherbot]

Change https://go.dev/cl/332289 mentions this issue: argon2: avoid clobbering BP