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

cmd/compile: compiler stores a stack pointer into a global object #61730

Open
wildoranges opened this issue Aug 3, 2023 · 3 comments
Open

cmd/compile: compiler stores a stack pointer into a global object #61730

wildoranges opened this issue Aug 3, 2023 · 3 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@wildoranges
Copy link

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

$ go version
go version go1.20.5 linux/amd64

Does this issue reproduce with the latest release?

Yes, I can reproduce in go1.20.7 linux/amd64

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

go env Output
$ go env
GO111MODULE="auto"
GOARCH="amd64"
GOBIN=""
GOCACHE="xxx/.cache/go-build"
GOENV="xxx/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="xxx/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="xxx/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,direct"
GOROOT="xxx/golang/go1.20.5"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="xxx/golang/go1.20.5/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.5"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2743047067=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I use go1.20.5 to compile the following code:

package main

func use(...interface{}) {
   
}

func main() {
    testCases := [...][][]int{
        {{42}},
        {{1, 2}},
        {{3, 4, 5}},
        {{}},
        {{1, 2}, {3, 4, 5}, {}, {7}},
    }
    for _, testCase := range testCases {
        use(testCase)
    }
}

In the generated SSA and assembly code, I notice that the Go compiler generates some instructions that store a stack pointer(point to the stack-allocated array) into a global slice header.

Just like the assembly code below, the MOV instruction at 0x4585bf stores a stack pointer into a global object:

  0x458589		48c744240800000000       MOVQ $0x0, 0x8(SP)	
  0x458592		48c74424082a000000	MOVQ $0x2a, 0x8(SP)	
	testCases := [...][][]int{
  0x45859b		48c705c28e060001000000	MOVQ $0x1, 0x68ec2(IP)			
  0x4585a6		48c705bf8e060001000000	MOVQ $0x1, 0x68ebf(IP)			
  0x4585b1		833d988d090000		CMPL $0x0, runtime.writeBarrier(SB)	
  0x4585b8		750e			JNE 0x4585c8				
  0x4585ba		488d442408		LEAQ 0x8(SP), AX			
  0x4585bf		4889059a8e0600		MOVQ AX, 0x68e9a(IP)			
  0x4585c6		eb11			JMP 0x4585d9				
  0x4585c8		488d3d918e0600		LEAQ 0x68e91(IP), DI			
  0x4585cf		488d442408		LEAQ 0x8(SP), AX			
  0x4585d4		e8e7cfffff		CALL runtime.gcWriteBarrier(SB)

I have read the comments in slicelit, but I didn't find any operations that can generate such stores. As far as I know, pointers to stack objects cannot be stored in global objects. So is this a compiler bug? Or the Go compiler does this on purpose to achieve some optimization I don't know yet?

note: this is originally posted on golang-nuts

Thanks

What did you expect to see?

The stack pointer is not stored into any global object.

What did you see instead?

The stack pointer is stored into a global object.

@wildoranges wildoranges changed the title affected/package: cmd/compile Maybe a Bug? The Go compiler stores a stack pointer into a global object Aug 3, 2023
@randall77 randall77 changed the title Maybe a Bug? The Go compiler stores a stack pointer into a global object cmd/compile: compiler stores a stack pointer into a global object Aug 3, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 3, 2023
@Lslightly
Copy link

Additional information:

in Go1.17.7, I notice that all arrays are allocated on heap, which follows the two invariants of escape analysis. The following assembly code is generated by Go1.17.7, allocating array {42} on heap.

  main.go:9		0x4554d0		e8eb58fbff			CALL runtime.newobject(SB)		
  main.go:9		0x4554d5		48c7002a000000			MOVQ $0x2a, 0(AX)			
  main.go:8		0x4554dc		48c705d14e060001000000		MOVQ $0x1, 0x64ed1(IP)			
  main.go:8		0x4554e7		48c705ce4e060001000000		MOVQ $0x1, 0x64ece(IP)			
  main.go:8		0x4554f2		833d675d090000			CMPL $0x0, runtime.writeBarrier(SB)	
  main.go:8		0x4554f9		7509				JNE 0x455504				
  main.go:8		0x4554fb		488905ae4e0600			MOVQ AX, 0x64eae(IP)			
  main.go:8		0x455502		eb0c				JMP 0x455510				
  main.go:8		0x455504		488d3da54e0600			LEAQ 0x64ea5(IP), DI			
  main.go:8		0x45550b		e8d0d0ffff			CALL runtime.gcWriteBarrier(SB)		

But in Go1.18, all arrays are allocated on the stack as the following assembly code.

  main.go:9		0x455212		48c74424082a000000		MOVQ $0x2a, 0x8(SP)			
  main.go:8		0x45521b		48c7050213060001000000		MOVQ $0x1, 0x61302(IP)			
  main.go:8		0x455226		48c705ff12060001000000		MOVQ $0x1, 0x612ff(IP)			
  main.go:8		0x455231		833dc821090000			CMPL $0x0, runtime.writeBarrier(SB)	
  main.go:8		0x455238		750e				JNE 0x455248				
  main.go:8		0x45523a		488d442408			LEAQ 0x8(SP), AX			
  main.go:8		0x45523f		488905da120600			MOVQ AX, 0x612da(IP)			
  main.go:8		0x455246		eb11				JMP 0x455259				
  main.go:8		0x455248		488d3dd1120600			LEAQ 0x612d1(IP), DI			
  main.go:8		0x45524f		488d442408			LEAQ 0x8(SP), AX			
  main.go:8		0x455254		e8c7cfffff			CALL runtime.gcWriteBarrier(SB)		

Go1.19.5 and Go1.20.6 share the above pattern, allocating all arrays on stack.

And I notice that in Go 1.18 Release Notes, "The garbage collector now includes non-heap sources of garbage collector work". Does the change of GC in Go1.18 affect the strategy of allocating small array?

@Jorropo
Copy link
Member

Jorropo commented Aug 3, 2023

This looks like:

I think the compiler is trying to cache the allocations of the static slice literals however it also for some reason it does not generate the code to do anything with the cached slices, it always recreates on each iteration even if you leak them on purpose.

That is if anything a performance bug, since this creates a bunch of writes into globals which add writebarriers for no reason.
The only read of theses globals is in the same place they are written to fill in the writebarrier's buffer.

I'm not sure why the compiler is somehow deciding it should generate caches but not use them.
Either the code that is supposed to reuse the cached statically initialized slices is broken and never generates the loads from cache. (if this worked you would see panics and memory corruptions)
Or the code that is creating theses stores into cache should not trigger in this case. (if this is spuriously triggering then you wouldn't have theses writes into globals)

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 4, 2023
@mknyszek mknyszek added this to the Backlog milestone Aug 9, 2023
@mdempsky
Copy link
Contributor

mdempsky commented Aug 9, 2023

Minimized:

package main

func main() {
        var x int
        _ = [...][]*int{         
                {&x},
                nil,
                nil,
                nil,
                nil,
        }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Status: In Progress
Development

No branches or pull requests

7 participants