Merge pull request #11927 from ktopley-apple/dispatch-time-overflows

Fix overflow traps in DispatchTime/DispatchWallTime/DispatchTimeInterval

Author: ktopley-apple

stdlib/public/SDK/Dispatch/Time.swift

@@ -114,6 +114,41 @@ extension DispatchWallTime {
   }
 }
 
+
+// Returns m1 * m2, clamped to the range [Int64.min, Int64.max].
+// Because of the way this function is used, we can always assume
+// that m2 > 0.
+private func clampedInt64Product(_ m1: Int64, _ m2: Int64) -> Int64 {
+	assert(m2 > 0, "multiplier must be positive")
+	let (result, overflow) = m1.multipliedReportingOverflow(by: m2)
+	if overflow {
+		return m1 > 0 ? Int64.max : Int64.min
+	}
+	return result
+}
+
+// Returns its argument clamped to the range [Int64.min, Int64.max].
+private func toInt64Clamped(_ value: Double) -> Int64 {
+	if value.isNaN { return Int64.max }
+	if value >= Double(Int64.max) { return Int64.max }
+	if value <= Double(Int64.min) { return Int64.min }
+	return Int64(value)
+}
+
+/// Represents a time interval that can be used as an offset from a `DispatchTime`
+/// or `DispatchWallTime`.
+///
+/// For example:
+///		let inOneSecond = DispatchTime.now() + DispatchTimeInterval.seconds(1)
+///
+///	If the requested time interval is larger then the internal representation
+/// permits, the result of adding it to a `DispatchTime` or `DispatchWallTime`
+/// is `DispatchTime.distantFuture` and `DispatchWallTime.distantFuture`
+/// respectively. Such time intervals compare as equal:
+///
+///		let t1 = DispatchTimeInterval.seconds(Int.max)
+///		let t2 = DispatchTimeInterval.milliseconds(Int.max)
+///		let result = t1 == t2   // true
 public enum DispatchTimeInterval : Equatable {
 	case seconds(Int)
 	case milliseconds(Int)
@@ -124,9 +159,9 @@ public enum DispatchTimeInterval : Equatable {
 
 	internal var rawValue: Int64 {
 		switch self {
-		case .seconds(let s): return Int64(s) * Int64(NSEC_PER_SEC)
-		case .milliseconds(let ms): return Int64(ms) * Int64(NSEC_PER_MSEC)
-		case .microseconds(let us): return Int64(us) * Int64(NSEC_PER_USEC)
+		case .seconds(let s): return clampedInt64Product(Int64(s), Int64(NSEC_PER_SEC))
+		case .milliseconds(let ms): return clampedInt64Product(Int64(ms), Int64(NSEC_PER_MSEC))
+		case .microseconds(let us): return clampedInt64Product(Int64(us), Int64(NSEC_PER_USEC))
 		case .nanoseconds(let ns): return Int64(ns)
 		case .never: return Int64.max
 		}
@@ -153,16 +188,12 @@ public func -(time: DispatchTime, interval: DispatchTimeInterval) -> DispatchTim
 }
 
 public func +(time: DispatchTime, seconds: Double) -> DispatchTime {
-	let interval = seconds * Double(NSEC_PER_SEC)
-	let t = __dispatch_time(time.rawValue,
-		interval.isInfinite || interval.isNaN ? Int64.max : Int64(interval))
+	let t = __dispatch_time(time.rawValue, toInt64Clamped(seconds * Double(NSEC_PER_SEC)));
 	return DispatchTime(rawValue: t)
 }
 
 public func -(time: DispatchTime, seconds: Double) -> DispatchTime {
-	let interval = -seconds * Double(NSEC_PER_SEC)
-	let t = __dispatch_time(time.rawValue,
-		interval.isInfinite || interval.isNaN ? Int64.min : Int64(interval))
+	let t = __dispatch_time(time.rawValue, toInt64Clamped(-seconds * Double(NSEC_PER_SEC)));
 	return DispatchTime(rawValue: t)
 }
 
@@ -177,15 +208,11 @@ public func -(time: DispatchWallTime, interval: DispatchTimeInterval) -> Dispatc
 }
 
 public func +(time: DispatchWallTime, seconds: Double) -> DispatchWallTime {
-	let interval = seconds * Double(NSEC_PER_SEC)
-	let t = __dispatch_time(time.rawValue,
-		interval.isInfinite || interval.isNaN ? Int64.max : Int64(interval))
+	let t = __dispatch_time(time.rawValue, toInt64Clamped(seconds * Double(NSEC_PER_SEC)));
 	return DispatchWallTime(rawValue: t)
 }
 
 public func -(time: DispatchWallTime, seconds: Double) -> DispatchWallTime {
-	let interval = -seconds * Double(NSEC_PER_SEC)
-	let t = __dispatch_time(time.rawValue,
-		interval.isInfinite || interval.isNaN ? Int64.min : Int64(interval))
+	let t = __dispatch_time(time.rawValue, toInt64Clamped(-seconds * Double(NSEC_PER_SEC)));
 	return DispatchWallTime(rawValue: t)
 }

test/stdlib/Dispatch.swift

@@ -126,21 +126,47 @@ DispatchAPI.test("DispatchTime.addSubtract") {
 	expectEqual(DispatchTime(uptimeNanoseconds: 1), then)
 
 	then = DispatchTime.now() - Double.nan
+	expectEqual(DispatchTime.distantFuture, then)
+
+	then = DispatchTime.now() + Date.distantFuture.timeIntervalSinceNow
+	expectEqual(DispatchTime(uptimeNanoseconds: UInt64.max), then)
+
+	then = DispatchTime.now() + Date.distantPast.timeIntervalSinceNow
 	expectEqual(DispatchTime(uptimeNanoseconds: 1), then)
+
+	then = DispatchTime.now() - Date.distantFuture.timeIntervalSinceNow
+	expectEqual(DispatchTime(uptimeNanoseconds: 1), then)
+
+	then = DispatchTime.now() - Date.distantPast.timeIntervalSinceNow
+	expectEqual(DispatchTime(uptimeNanoseconds: UInt64.max), then)
 }
 
 DispatchAPI.test("DispatchWallTime.addSubtract") {
+	let distantPastRawValue = DispatchWallTime.distantFuture.rawValue - UInt64(1)
+
 	var then = DispatchWallTime.now() + Double.infinity
 	expectEqual(DispatchWallTime.distantFuture, then)
 
 	then = DispatchWallTime.now() + Double.nan
 	expectEqual(DispatchWallTime.distantFuture, then)
 
 	then = DispatchWallTime.now() - Double.infinity
-	expectEqual(DispatchWallTime.distantFuture.rawValue - UInt64(1), then.rawValue)
+	expectEqual(distantPastRawValue, then.rawValue)
 
 	then = DispatchWallTime.now() - Double.nan
-	expectEqual(DispatchWallTime.distantFuture.rawValue - UInt64(1), then.rawValue)
+	expectEqual(DispatchWallTime.distantFuture, then)
+
+	then = DispatchWallTime.now() + Date.distantFuture.timeIntervalSinceNow
+	expectEqual(DispatchWallTime.distantFuture, then)
+
+	then = DispatchWallTime.now() + Date.distantPast.timeIntervalSinceNow
+	expectEqual(distantPastRawValue, then.rawValue)
+
+	then = DispatchWallTime.now() - Date.distantFuture.timeIntervalSinceNow
+	expectEqual(distantPastRawValue, then.rawValue)
+
+	then = DispatchWallTime.now() - Date.distantPast.timeIntervalSinceNow
+	expectEqual(DispatchWallTime.distantFuture, then)
 }
 
 DispatchAPI.test("DispatchTime.uptimeNanos") {
@@ -516,6 +542,35 @@ if #available(OSX 10.13, iOS 11.0, watchOS 4.0, tvOS 11.0, *) {
 	}
 }
 
+DispatchAPI.test("DispatchTimeInterval") {
+	// Basic tests that the correct value is stored and the == method works
+	for i in stride(from:1, through: 100, by: 5) {
+		expectEqual(DispatchTimeInterval.seconds(i), DispatchTimeInterval.milliseconds(i * 1000))
+		expectEqual(DispatchTimeInterval.milliseconds(i), DispatchTimeInterval.microseconds(i * 1000))
+		expectEqual(DispatchTimeInterval.microseconds(i), DispatchTimeInterval.nanoseconds(i * 1000))
+	}
+
+
+	// Check some cases that used to cause arithmetic overflow when evaluating the rawValue for ==
+	var t = DispatchTimeInterval.seconds(Int.max)
+	expectTrue(t == t) // This would crash.
+
+	t = DispatchTimeInterval.seconds(-Int.max)
+	expectTrue(t == t) // This would crash.
+
+	t = DispatchTimeInterval.milliseconds(Int.max)
+	expectTrue(t == t) // This would crash.
+
+	t = DispatchTimeInterval.milliseconds(-Int.max)
+	expectTrue(t == t) // This would crash.
+
+	t = DispatchTimeInterval.microseconds(Int.max)
+	expectTrue(t == t) // This would crash.
+
+	t = DispatchTimeInterval.microseconds(-Int.max)
+	expectTrue(t == t) // This would crash.
+}
+
 #if swift(>=4.0)
 DispatchAPI.test("DispatchTimeInterval.never.equals") {
 	expectTrue(DispatchTimeInterval.never == DispatchTimeInterval.never)