strconv: ParseFloat panics on very long, valid inputs

[remyoudompheng]

What steps will reproduce the problem?
1. Compile and run the following code:

package main

import (
    "fmt"
    "strconv"
)

func main() {
  var longnumber [4096]byte
  for i := range longnumber {
     longnumber[i] = '2'
  }
  longnumber[1] = '.'
  x, er := strconv.ParseFloat(string(longnumber[:]), 64)
  fmt.Println(x, er)
}

What is the expected output?

2.2222222222222222 <nil>

What do you see instead?

panic: runtime error: index out of range

goroutine 1 [running]:
strconv.(*decimal).set(0xf8400210d4, 0xf840024000, 0x1000, 0x0, 0xf840021100, ...)
    /opt/remy/go/src/pkg/strconv/atof.go:91 +0x315
----- stack segment boundary -----
strconv.atof64(0xf840024000, 0xf800001000, 0xf840021488, 0x0, 0x0, ...)
    /opt/remy/go/src/pkg/strconv/atof.go:383 +0xe0
strconv.ParseFloat(0xf840024000, 0x1000, 0xf800000040, 0x1000, 0x0, ...)
    /opt/remy/go/src/pkg/strconv/atof.go:432 +0x8d
main.main()
    /tmp/truc.go:14 +0xb9
----- stack segment boundary -----


Which compiler are you using (5g, 6g, 8g, gccgo)?

6g

Which operating system are you using?

linux

Which revision are you using?  (hg identify)

c956aa39a269+ (tip)

Please provide any additional information below.

[balasanjay]

Comment 1:

The comment in decimal.go states "TODO(rsc): Can make d[] a bit smaller and add
truncated bool;". I'm actually curious why an array was used at all, instead of a slice
initialized to a smaller value and autoexpanding as necessary.
If that would be a valid solution, I could try writing a patch although I'll freely
admit I remember practically nothing about the representation of floating point values,
and it'll be a learning experience for me. As far as I can tell, should be pretty simple
just converting to a slice representation and changing to appends where necessary.
Sanjay

[rsc]

Comment 2:

The reason for the fixed buffer is so that there is just one
allocation (of the decimal struct), and with the new escape
analysis that is done on the stack, so it incurs no garbage
collection overhead.  I am surprised that adding another
allocation makes it faster.

[remyoudompheng]

Comment 3:

Hum, i'm not sure what you are referring to. Is it CL
http://golang.org/cl/5503085/ ? 
In this CL I suggest using a *small* buffer in some cases, while still allowing for the
800-byte buffer. It is a bit ugly, but thanks to escape analysis, both are on the stack,
but only the one that comes into scope is zeroed (and zeroing 800 bytes is a significant
portion of the call).
However, escape analysis must be improved for this to work, which is proposed in CL
http://golang.org/cl/5489128/ . It would be great if you could have a look at
it, because I think it could have benefits in other places.

[remyoudompheng]

Comment 4:

By the way, is there a way dynamic length make()'s that do not escape can be optimized
by escape analysis? I was thinking of compiling them like
if length < SomeConstant {
  slice some fixed size buffer on the stack
} else {
  call runtime·malloc
}

[robpike]

Comment 5:

Labels changed: added priority-go1.

[robpike]

Comment 6:

Status changed to Accepted.

[robpike]

Comment 7:

Owner changed to builder@golang.org.

[rsc]

Comment 8:

Remy, are you still planning to work on this?
I think it can be done by reading at most the
first 800 characters and then remembering 
whether one of the discarded characters was nonzero,
to force round-up mode.

[rsc]

Comment 9:

[now cc'ing remy]
Remy, are you still planning to work on this?
I think it can be done by reading at most the
first 800 characters and then remembering 
whether one of the discarded characters was nonzero,
to force round-up mode.

[remyoudompheng]

Comment 10:

I may do so but not for the moment, I would have to get some time to get into it.

[rsc]

Comment 11:

Owner changed to @rsc.

Status changed to Started.

[rsc]

Comment 12:

This issue was closed by revision 3ee2085.

Status changed to Fixed.