golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.73k stars 17.5k forks source link

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

Open wildoranges opened 1 year ago

wildoranges commented 1 year ago

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.

Lslightly commented 1 year ago

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 commented 1 year ago

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)

mdempsky commented 1 year ago

Minimized:

package main

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