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

bytes+strings: Repeat can appear to succeed with an unreasonably large count #16237

Closed
pat42smith opened this issue Jul 1, 2016 · 5 comments
Closed
Milestone

Comments

@pat42smith
Copy link

@pat42smith pat42smith commented Jul 1, 2016

  1. What version of Go are you using (go version)?
    go version go1.6.2 linux/amd64
  2. What operating system and processor architecture are you using (go env)?
    GOARCH="amd64"
    GOBIN=""
    GOEXE=""
    GOHOSTARCH="amd64"
    GOHOSTOS="linux"
    GOOS="linux"
    GOPATH="/home/pat/go"
    GORACE=""
    GOROOT="/usr/lib/go"
    GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
    GO15VENDOREXPERIMENT="1"
    CC="gcc"
    GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
    CXX="g++"
    CGO_ENABLED="1"
  3. What did you do?
    https://play.golang.org/p/o-5UiG_cKc
  4. What did you expect to see?
    A panic because the program tries to allocate too much memory.
  5. What did you see instead?
    Both bytes.Repeat and strings.Repeat appear to succeed, but the result is shorter than the input string.

Looking at https://golang.org/src/bytes/bytes.go?s=9819:9858#L381, the cause seems to be that len(b)*count overflows, and happens to produce a result that is a small positive integer. Perhaps the routine should check for overflow and panic if one occurs.

@bradfitz bradfitz added this to the Go1.8 milestone Jul 1, 2016
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Aug 13, 2016

I was just looking into fixing this issue but am wondering if a panic might cause incompatibility with pre-go1.8 programs.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 13, 2016

It's better to panic than to return the wrong answer. Given that we have no way to return an error from these functions, I think panicing is our only option here.

@pat42smith
Copy link
Author

@pat42smith pat42smith commented Aug 13, 2016

I tried https://play.golang.org/p/qQ37uU4mGT to see what might happen in other cases that require more memory than is available. On the playground, that produces a fatal runtime error that seems not to be a panic:

2147483647
runtime: memory allocated by OS (0x7ef60000) not in usable range [0x10400000,0x90400000)
runtime: out of memory: cannot allocate 2147483648-byte block (1048576 in use)
fatal error: out of memory
...

On my desktop, with 64 bit ints, the same program produces a panic:
9223372036854775807
panic: runtime error: makeslice: len out of range

goroutine 1 [running]:
panic(0x4e1f20, 0xc82000a340)
/usr/lib/go/src/runtime/panic.go:481 +0x3e6
strings.Repeat(0x5058a8, 0x1, 0x7fffffffffffffff, 0x0, 0x0)
/usr/lib/go/src/strings/strings.go:427 +0x5f
main.main()
/home/pat/xx.go:9 +0xfc
exit status 2

So a panic for this case would be at least somewhat consistent.

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Aug 13, 2016

Thank you @ianlancetaylor and @pat42smith for the answers, sounds like a plan.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 28, 2016

CL https://golang.org/cl/29954 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants