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: stack overflow accessing large return value #14028

Open
Opposition opened this Issue Jan 20, 2016 · 8 comments

Comments

Projects
None yet
10 participants
@Opposition

Opposition commented Jan 20, 2016

Taken this toy example, that calculates prime numbers: http://play.golang.org/p/XrUCUvC7In

Building this with go build -gcflags -m shows following output

moved to heap: arr moved to heap: x

However, running the program I get following error:

runtime: goroutine stack exceeds 1000000000-byte limit
fatal error: stack overflow

runtime stack:
runtime.throw(0x470f80, 0xe)
c:/go/src/runtime/panic.go:527 +0x97
runtime.newstack()
c:/go/src/runtime/stack1.go:800 +0xb25
runtime.morestack()
c:/go/src/runtime/asm_amd64.s:330 +0x87

goroutine 1 [stack growth]:
main.main()
C:/Users/Daniel/Desktop/rensir.go:27 fp=0xc0c202be50 sp=0xc0c202be48
runtime.main()
c:/go/src/runtime/proc.go:111 +0x27e fp=0xc0c202bea0 sp=0xc0c202be50
runtime.goexit()
c:/go/src/runtime/asm_amd64.s:1721 +0x1 fp=0xc0c202bea8 sp=0xc0c202bea0

Is it supposed to get a stackoverflow, if it is a heap object?

Go version: go1.5.3 windows/amd64
OS: windows 7 64 bit

@dominikh

This comment has been minimized.

Member

dominikh commented Jan 20, 2016

Simpler example that reproduces the problem:

package main

func fn() (arr [1e9]bool) {
    for range arr {
    }
    return
}

func main() {
    fn()
}

Without the loop it's fine.

@bradfitz bradfitz added this to the Go1.7 milestone Jan 20, 2016

@bradfitz bradfitz changed the title from Stackoverflow on a heap array to cmd/compile: stack overflow accessing large return value Jan 20, 2016

@kostya-sh

This comment has been minimized.

Contributor

kostya-sh commented Jan 20, 2016

The following example:

package main

func fn() (arr [1e10]bool) {
    for range arr {
    }
    return
}

func main() {
    fn()
}

fails to compile with "stack frame too large (>2GB)" error. It looks like compiler and runtime disagree about the maximum stack frame size.

@kostya-sh

This comment has been minimized.

Contributor

kostya-sh commented Jan 20, 2016

incorrect example deleted

@bradfitz

This comment has been minimized.

Member

bradfitz commented Jan 20, 2016

cc @randall77 as fun tests to add to the ssa branch too

@kostya-sh

This comment has been minimized.

Contributor

kostya-sh commented Jan 20, 2016

Sorry, ignore my last comment. I didn't test it properly ;)

@minux

This comment has been minimized.

Member

minux commented Jan 20, 2016

Did you know that for range arr makes a copy of the array
(onto stack)?

Per the Go ABI, the return value will be passed on stack,
even if we have fixed the for range memory blowup, the
program will still overflow the stack because fn/sieve needs
almost 1GB on stack for their return values.

@kostya-sh

This comment has been minimized.

Contributor

kostya-sh commented Jan 20, 2016

@minux, for range arr (and for i := range arr) doesn't make a copy of the array onto stack. But for _, v := range arr does.

@rsc rsc modified the milestones: Go1.8, Go1.7 May 17, 2016

@quentinmit quentinmit added the NeedsFix label Oct 11, 2016

@rsc rsc modified the milestones: Go1.9, Go1.8 Oct 21, 2016

@randall77 randall77 modified the milestones: Go1.10, Go1.9 May 31, 2017

@mdempsky

This comment has been minimized.

Member

mdempsky commented Nov 29, 2017

Simpler repro:

package main

//go:noinline
func fn() (arr [1e9]bool) {
    return
}

func main() {
    fn()
}

Discussion about the for loop is a red herring. It just prevented inlining.

The issue seems to be that when we're warning about stack frames that are >1GB, we're failing to include the space reserved for calling other functions. In the example above, main needs to allocate 1GB of stack space for fn's return values, but we're not including that space in the test.

@mdempsky mdempsky modified the milestones: Go1.10, Go1.11 Nov 29, 2017

@gopherbot gopherbot modified the milestones: Go1.11, Unplanned May 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment