time: allow minimum int64 in ParseDuration

ParseDuration should handle minimum int64 (-1<<63) nanosecond
since type Duration is alias of int64

name           old time/op  new time/op  delta
ParseDuration  91.4ns ± 0%  86.4ns ± 1%  -5.49%  (p=0.000 n=9+8)

Fixes: #48629
Change-Id: I81b7035b25cefb4c1e5b7801c20f2d335e29358a
Reviewed-on: https://go-review.googlesource.com/c/go/+/352269
Trust: Meng Zhuo <mzh@golangcn.org>
Run-TryBot: Meng Zhuo <mzh@golangcn.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/time/format.go b/src/time/format.go
index 464effdb438885a20f3d09378c8006919623b4a3..5fb9cdc969395a0ef040de8d877cdbece5c60520 100644 (file)
--- a/src/time/format.go
+++ b/src/time/format.go
@@ -1402,10 +1402,7 @@ func parseSignedOffset(value string) int {
        if err != nil || value[1:] == rem {
                return 0
        }
-       if sign == '-' {
-               x = -x
-       }
-       if x < -23 || 23 < x {
+       if x > 23 {
                return 0
        }
        return len(value) - len(rem)
@@ -1443,19 +1440,19 @@ func parseNanoseconds(value string, nbytes int) (ns int, rangeErrString string,
 var errLeadingInt = errors.New("time: bad [0-9]*") // never printed
 
 // leadingInt consumes the leading [0-9]* from s.
-func leadingInt(s string) (x int64, rem string, err error) {
+func leadingInt(s string) (x uint64, rem string, err error) {
        i := 0
        for ; i < len(s); i++ {
                c := s[i]
                if c < '0' || c > '9' {
                        break
                }
-               if x > (1<<63-1)/10 {
+               if x > 1<<63/10 {
                        // overflow
                        return 0, "", errLeadingInt
                }
-               x = x*10 + int64(c) - '0'
-               if x < 0 {
+               x = x*10 + uint64(c) - '0'
+               if x > 1<<63 {
                        // overflow
                        return 0, "", errLeadingInt
                }
@@ -1466,7 +1463,7 @@ func leadingInt(s string) (x int64, rem string, err error) {
 // leadingFraction consumes the leading [0-9]* from s.
 // It is used only for fractions, so does not return an error on overflow,
 // it just stops accumulating precision.
-func leadingFraction(s string) (x int64, scale float64, rem string) {
+func leadingFraction(s string) (x uint64, scale float64, rem string) {
        i := 0
        scale = 1
        overflow := false
@@ -1483,8 +1480,8 @@ func leadingFraction(s string) (x int64, scale float64, rem string) {
                        overflow = true
                        continue
                }
-               y := x*10 + int64(c) - '0'
-               if y < 0 {
+               y := x*10 + uint64(c) - '0'
+               if y > 1<<63 {
                        overflow = true
                        continue
                }
@@ -1494,15 +1491,15 @@ func leadingFraction(s string) (x int64, scale float64, rem string) {
        return x, scale, s[i:]
 }
 
-var unitMap = map[string]int64{
-       "ns": int64(Nanosecond),
-       "us": int64(Microsecond),
-       "µs": int64(Microsecond), // U+00B5 = micro symbol
-       "μs": int64(Microsecond), // U+03BC = Greek letter mu
-       "ms": int64(Millisecond),
-       "s":  int64(Second),
-       "m":  int64(Minute),
-       "h":  int64(Hour),
+var unitMap = map[string]uint64{
+       "ns": uint64(Nanosecond),
+       "us": uint64(Microsecond),
+       "µs": uint64(Microsecond), // U+00B5 = micro symbol
+       "μs": uint64(Microsecond), // U+03BC = Greek letter mu
+       "ms": uint64(Millisecond),
+       "s":  uint64(Second),
+       "m":  uint64(Minute),
+       "h":  uint64(Hour),
 }
 
 // ParseDuration parses a duration string.
@@ -1513,7 +1510,7 @@ var unitMap = map[string]int64{
 func ParseDuration(s string) (Duration, error) {
        // [-+]?([0-9]*(\.[0-9]*)?[a-z]+)+
        orig := s
-       var d int64
+       var d uint64
        neg := false
 
        // Consume [-+]?
@@ -1533,7 +1530,7 @@ func ParseDuration(s string) (Duration, error) {
        }
        for s != "" {
                var (
-                       v, f  int64       // integers before, after decimal point
+                       v, f  uint64      // integers before, after decimal point
                        scale float64 = 1 // value = v + f/scale
                )
 
@@ -1581,7 +1578,7 @@ func ParseDuration(s string) (Duration, error) {
                if !ok {
                        return 0, errors.New("time: unknown unit " + quote(u) + " in duration " + quote(orig))
                }
-               if v > (1<<63-1)/unit {
+               if v > 1<<63/unit {
                        // overflow
                        return 0, errors.New("time: invalid duration " + quote(orig))
                }
@@ -1589,21 +1586,22 @@ func ParseDuration(s string) (Duration, error) {
                if f > 0 {
                        // float64 is needed to be nanosecond accurate for fractions of hours.
                        // v >= 0 && (f*unit/scale) <= 3.6e+12 (ns/h, h is the largest unit)
-                       v += int64(float64(f) * (float64(unit) / scale))
-                       if v < 0 {
+                       v += uint64(float64(f) * (float64(unit) / scale))
+                       if v > 1<<63 {
                                // overflow
                                return 0, errors.New("time: invalid duration " + quote(orig))
                        }
                }
                d += v
-               if d < 0 {
-                       // overflow
+               if d > 1<<63 {
                        return 0, errors.New("time: invalid duration " + quote(orig))
                }
        }
-
        if neg {
-               d = -d
+               return -Duration(d), nil
+       }
+       if d > 1<<63-1 {
+               return 0, errors.New("time: invalid duration " + quote(orig))
        }
        return Duration(d), nil
 }

diff --git a/src/time/time_test.go b/src/time/time_test.go
index e2fb897b6d6edc72170155e6e39e95a08a89b006..5007b6e723128a0827afb8af853e0a6e9086f10c 100644 (file)
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -9,6 +9,7 @@ import (
        "encoding/gob"
        "encoding/json"
        "fmt"
+       "math"
        "math/big"
        "math/rand"
        "os"
@@ -885,8 +886,13 @@ var parseDurationTests = []struct {
        {"9223372036854775807ns", (1<<63 - 1) * Nanosecond},
        {"9223372036854775.807us", (1<<63 - 1) * Nanosecond},
        {"9223372036s854ms775us807ns", (1<<63 - 1) * Nanosecond},
-       // large negative value
-       {"-9223372036854775807ns", -1<<63 + 1*Nanosecond},
+       {"-9223372036854775808ns", -1 << 63 * Nanosecond},
+       {"-9223372036854775.808us", -1 << 63 * Nanosecond},
+       {"-9223372036s854ms775us808ns", -1 << 63 * Nanosecond},
+       // largest negative value
+       {"-9223372036854775808ns", -1 << 63 * Nanosecond},
+       // largest negative round trip value, see https://golang.org/issue/48629
+       {"-2562047h47m16.854775808s", -1 << 63 * Nanosecond},
        // huge string; issue 15011.
        {"0.100000000000000000000h", 6 * Minute},
        // This value tests the first overflow check in leadingFraction.
@@ -924,9 +930,7 @@ var parseDurationErrorTests = []struct {
        // overflow
        {"9223372036854775810ns", `"9223372036854775810ns"`},
        {"9223372036854775808ns", `"9223372036854775808ns"`},
-       // largest negative value of type int64 in nanoseconds should fail
-       // see https://go-review.googlesource.com/#/c/2461/
-       {"-9223372036854775808ns", `"-9223372036854775808ns"`},
+       {"-9223372036854775809ns", `"-9223372036854775809ns"`},
        {"9223372036854776us", `"9223372036854776us"`},
        {"3000000h", `"3000000h"`},
        {"9223372036854775.808us", `"9223372036854775.808us"`},
@@ -945,6 +949,19 @@ func TestParseDurationErrors(t *testing.T) {
 }
 
 func TestParseDurationRoundTrip(t *testing.T) {
+       // https://golang.org/issue/48629
+       max0 := Duration(math.MaxInt64)
+       max1, err := ParseDuration(max0.String())
+       if err != nil || max0 != max1 {
+               t.Errorf("round-trip failed: %d => %q => %d, %v", max0, max0.String(), max1, err)
+       }
+
+       min0 := Duration(math.MinInt64)
+       min1, err := ParseDuration(min0.String())
+       if err != nil || min0 != min1 {
+               t.Errorf("round-trip failed: %d => %q => %d, %v", min0, min0.String(), min1, err)
+       }
+
        for i := 0; i < 100; i++ {
                // Resolutions finer than milliseconds will result in
                // imprecise round-trips.