Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/asm: R15 gets clobbered in plugin mode #43661

Closed
DankFC opened this issue Jan 13, 2021 · 7 comments
Closed

cmd/asm: R15 gets clobbered in plugin mode #43661

DankFC opened this issue Jan 13, 2021 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.17

Comments

@DankFC
Copy link

DankFC commented Jan 13, 2021

What version of Go are you using (go version)?

go version : 1.15.7 ( However go version 1.14 and 1.16beta1 also show same error behaviour.)

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

OS: linux x86_64

What did you do?

A bug which occurs only in plugins with assembly code.
The package consists of a bn256 crypto implementation which has amd64 assembly code and also a generic implementation.
The assembly implementation is default and generic implementation can be enabled using the "generic" tag.
The plugin when compiled as a binary runs OK with both assembly and go implementation.
The plugin when compiled as a plugin FAILS with assembly code but runs OK with generic implementation.

TESTING:

#Run plugin directly to see correct result.
go run plugin/plugin.go

#Compile as plugin.
go build -buildmode=plugin -o /tmp/plugin.so ../compiler_bug/plugin/

#Running plugin will trigger error
go run runplugin/main.go

However, if the plugin is compiled with generic tag (To disable asm code, the bug doesn't occur), Some register corruption occurs due to wrong code being generated.
go build -buildmode=plugin -tags generic -o /tmp/plugin.so ../compiler_bug/plugin/

Running plugin will not cause an error.
go run runplugin/main.go

Repo to reproduce

https://github.com/DankFC/compile-bug

What did you expect to see?

./demo.sh 
Actual a01f9bcc1208dee302769931ad378a4c0c4b2c21b0cfb3e752607e12d2b6fa6425 
Expected a01f9bcc1208dee302769931ad378a4c0c4b2c21b0cfb3e752607e12d2b6fa6425 
running plugin 
Actual a01f9bcc1208dee302769931ad378a4c0c4b2c21b0cfb3e752607e12d2b6fa6425 
Expected a01f9bcc1208dee302769931ad378a4c0c4b2c21b0cfb3e752607e12d2b6fa6425

What did you see instead?

./demo.sh 
Actual a01f9bcc1208dee302769931ad378a4c0c4b2c21b0cfb3e752607e12d2b6fa6425 
Expected a01f9bcc1208dee302769931ad378a4c0c4b2c21b0cfb3e752607e12d2b6fa6425 
running plugin 
panic: Decode point err bn256: malformed point err 
goroutine 1 [running]: 
_/tmp/compiler_bug/plugin.Export() 
/tmp/compiler_bug/plugin/plugin.go:23 +0x2d7 
main.main() 
/tmp/compiler_bug/runplugin/main.go:24 +0x107 
exit status 2

demo.sh is included in the repo to recreate the bug easily.
@randall77
Copy link
Contributor

Try adding NOSPLIT to your assembly code. For example,

TEXT ·gfpNeg(SB),0,$0-16

TEXT ·gfpNeg(SB),NOSPLIT,$0-16

You probably also need

#include "textflag.h"

What I suspect is happening is that depending on stack size (which is why plugin may change things) a stack growth is occurring on entry to your assembly. The stack gets copied (or possibly, a GC gets run), but the arguments to your assembly don't get adjusted (or, if it is the GC, its referents marked).

You need either NOSPLIT on your assembly, so a stack copy can't happen on entry, or provide a proper argument map for that assembly so the stack copier knows where the pointers are in its arguments. I believe the NO_LOCAL_POINTERS macro (see runtime/asm_amd64.s in the stdlib) will do that for you, but I haven't done that in a while. NOSPLIT is simpler.

@randall77
Copy link
Contributor

Acutally, maybe not. I think you should get pointer maps automatically added, see the docs for GO_ARGS in runtime/funcdata.h.
Worth trying NOSPLIT just to see if it helps, though.

@DankFC
Copy link
Author

DankFC commented Jan 13, 2021

Acutally, maybe not. I think you should get pointer maps automatically added, see the docs for GO_ARGS in runtime/funcdata.h.
Worth trying NOSPLIT just to see if it helps, though.

Thanks for the input, but there was no change using NOSPLIT.

@randall77
Copy link
Contributor

randall77 commented Jan 13, 2021 •
edited

New theory: you can't use R15 in plugin assembly.
It's used for accessing the GOT. See rewriteToUseGot in cmd/internal/obj/x86/obj6.go.
Or at least, you can't use R15 and access a global variable in the same assembly. Accessing a global variable clobbers R15.

Replacing R15 with CX in your assembly fixes it.

Let me know if you can get it to work as well.
We really need to either not clobber R15, or document it better, or have the toolchain complain when using R15 in GOT assembly, or something.

@randall77
Copy link
Contributor

Our compiler avoids R15 when compiling for dynamic linking (cmd/compile/internal/ssa/regalloc.go):

	if s.f.Config.ctxt.Flag_dynlink {
		switch s.f.Config.arch {
		case "amd64":
			s.allocatable &^= 1 << 15 // R15

@cagedmantis cagedmantis changed the title cmd/compile: Unexpected result in plugins with assembly code cmd/compile: unexpected result in plugins with assembly code Jan 13, 2021
@cagedmantis cagedmantis added this to the Backlog milestone Jan 13, 2021
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 13, 2021
@randall77
Copy link
Contributor

It's been like this for a while. See #18820.

I've got a CL that errors out in the assembler when it detects a bad case. I think waiting until 1.17 is the right plan.

@randall77 randall77 changed the title cmd/compile: unexpected result in plugins with assembly code cmd/asm: R15 gets clobbered in plugin mode Jan 13, 2021
@randall77 randall77 added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 13, 2021
@randall77 randall77 modified the milestones: Backlog, Go1.17 Jan 13, 2021
@randall77 randall77 self-assigned this Jan 13, 2021
@gopherbot
Copy link

Change https://golang.org/cl/283474 mentions this issue: cmd/asm: when dynamic linking, reject code that uses both R15 and globals

@rwynn rwynn mentioned this issue Aug 20, 2021
Open
@efabens efabens mentioned this issue Aug 31, 2021
Closed
@zishen zishen mentioned this issue Sep 19, 2021
Closed
@siddharthlatest siddharthlatest mentioned this issue Dec 15, 2021
Closed
@golang golang locked and limited conversation to collaborators Mar 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.17
Development

No branches or pull requests
4 participants
@cagedmantis @randall77 @gopherbot @DankFC