I can confirm. It seems like the fast path in strconv/atof.go:627, particularly the AssignDecimal call, is to blame. If I turn the optimize flag off, the bug goes away.
This has been an issue since at least 1.11.
@griesemer

This behavior has existed since go 1, so it wasn't introduced in any recent release. Digging into extfloat now.

The ParseFloat doc says:

If s is well-formed and near a valid floating-point number, ParseFloat returns the nearest floating-point number rounded using IEEE754 unbiased rounding.

So it appears that it doesn't unconditionally guarantee to always return the closest IEEE754 number... only if the argument is "near a valid floating-point number" (whatever that means). I believe that your number does not qualify as "near a valid floating-point number".

Also, the CL associated with #15672 solves this.

This has been like this forever. Removing release-blocker label.

We didn’t cover much progress on the related CL unfortunately because folks were swamped. I shall move this to the Unplanned milestone given that the parallel issue is in the Unplanned milestone too.

@nigeltao Given your recent work, is this still an issue?

This is still an issue as of 1.15.3. I'm still losing precision. I have a float string of -0.63571428571428568 and it's getting truncated to -0.6357142857142857, losing 1428568.

The reproducer in the first post still prints

1.0905441441816094e+30 is not equal to 1.0905441441816093e+30
expected: 0x462b8779f2474dfb     7748782196739579p+47 1090544144181609419040633126912.000000000
actual  : 0x462b8779f2474dfa     7748782196739578p+47 1090544144181609278303144771584.000000000

at tip, so yes, this is still an issue.

go version devel +c3fe874f25 Fri Oct 23 18:02:16 2020 +0000 linux/amd64

Right because the input string has many more than 17 digits. I was being hopeful.

IIRC, CL 170078 (implement Ryū-like algorithm for atof) did address this issue, but it was abandoned in favor of CL 260858 (use the Eisel-Lemire ParseFloat algorithm).

I have basically ported the Go code more or less as-is to C++ from @nigeltao's own C port... and I don't have this issue. See https://github.com/lemire/fast_float/pull/6

The code follows the same logic, so it ought to be work. I mean... there is no big conceptual gap.

Ok. So the problem goes away entirely if you comment out this third fast path in atof.go...

		// Try another fast path.
		/*ext := new(extFloat)
		if ok := ext.AssignDecimal(mantissa, exp, neg, trunc, &float64info); ok {

			b, ovf := ext.floatBits(&float64info)
			f = math.Float64frombits(b)
			if ovf {
				err = rangeError(fnParseFloat, s)
			}
			return f, n, err
		}*/

Looking at ext.AssignDecimal, it says...

	// Errors (in the "numerical approximation" sense, not the "Go's error
	// type" sense) in this function are measured as multiples of 1/8 of a ULP,
	// so that "1/2 of a ULP" can be represented in integer arithmetic.
	//
	// The C++ double-conversion library also uses this 8x scaling factor:
	// https://github.com/google/double-conversion/blob/f4cb2384/double-conversion/strtod.cc#L291
	// but this Go implementation has a bug, where it forgets to scale other
	// calculations (further below in this function) by the same number. The
	// C++ implementation does not forget:
	// https://github.com/google/double-conversion/blob/f4cb2384/double-conversion/strtod.cc#L366
	//
	// Scaling the "errors" in the "is mant_extra in the range (halfway ±
	// errors)" check, but not scaling the other values, means that we return
	// ok=false (and fall back to a slower atof code path) more often than we
	// could. This affects performance but not correctness.
	

The part where it says we return ok=false (and fall back to a slower atof code path) more often than we could. This affects performance but not correctness might be incorrect, as illustrated by the example (1090544144181609348835077142190). In that case, it appears to me that double-conversion does reject the fast path but not the Go code?

Maybe it is helpful to have some perspective. Prior to the recent changes made by @nigeltao, this third fast path was the second fast path. I would argue that it was absolutely needed.

Now you have effectively three fast paths followed by a fallback.

The first and second fast paths will catch nearly everything in the real world. Like 99% of anything that has no more than 19 digits.

So what the third fallback does is protect against the fallback. But that may not be useful:

1. Nobody that cares about performance will generate floats with more than 19 digits of accuracy (mantissa). It is an awkward thing to do in the best of times.
2. People who do throw long mantissa at the parser, would probably prefer to get correct rounding given a choice. That is, they are probably not in the business of ingesting large CSV files filled with 100 digit numbers. That's really an edge case.
3. Having more layers can sometimes make things worse. It certainly increases the surface area for maintenance.
4. The fallback is not that slow. I have not benchmarked the Go version, but the direct C++ port runs at an acceptable speed. You may be rightfully concerned with denial of service attacks where someone throws numbers with very long strings at you... but that's not an effective attack... in fact, beyond a certain point, the parsing speeds goes up. My C++ port of the fallback runs at 2 GB/s for streams of long numbers. (Asymptotically, parsing is O(N).)

I think point 4 is maybe key. Assuming that the fallback is "fast enough", and assuming that it is correct (my port of it sure appears correct), then when not go to it directly?

Change https://golang.org/cl/264518 mentions this issue: strconv: remove extfloat.go atof code path

Ok. So the problem goes away entirely if you comment out this third fast path in atof.go...

Coincidentally, that's what https://golang.org/cl/264518 does, which I sent out yesterday.

Thank you for the change and foresight @nigeltao! Thank you @lemire for the rationale in #36657 (comment) and also for the algorithms!

@lemire, it would be a delight to have you as a Go contributor/reviewer, and Nigel's CL could use the rationale you gave for points 1 and 4 in his commit message, but would be nice if you could post those up and reflect in the record, and have you also approve that change :)
@nigeltao please take a look at @lemire's rationale in the various points and perhaps also include that reasoning that he raised in those points.

@lemire said:

Nobody that cares about performance will generate floats with more than 19 digits of accuracy (mantissa). It is an awkward thing to do in the best of times.

OTOH, separately on https://go-review.googlesource.com/c/go/+/264518/3#message-7fda4a76b2d909bb21794b84f9c3d30f4730adb5 @remyoudompheng said:

It's not for malicious inputs. It's for inputs which are too long. In my environment I have old apps which produce non-shortest floats and they are a pain.

In the same thread, @remyoudompheng also mentions that https://golang.org/cl/264677 should help with the more-than-19-digits case.

concerned with denial of service attacks where someone throws numbers with very long strings at you

In any case, if it's through a JSON API, there might be more worrisome DoS attacks where decoding lots of empty JSON arrays or JSON objects might be more expensive (due to mallocs and garbage collection) than decoding long JSON numbers.

Always happy to help.

@nigeltao I think https://golang.org/cl/264677 is what I had in mind in an independently sent private message. As I also privately communicated, we could tighten further the second fast path if we wanted to catch even more cases (e.g., subnormals and round-to-even cases) but it is only worth doing if too many fall through.