From 83bd6d124ffad7f7d7e0b0cb6a7e1ec21c7ee764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Sat, 8 Aug 2026 12:59:48 +0200 Subject: [PATCH] Fix Int overflow in maxTotalSubFrames on 32-bit platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `maxTotalSubFrames(in:base:)` computed `maxTotalFrames(in: extent) * base.rawValue` directly in `Int`. With `extent == .max100Days` that product exceeds `Int32.max` for every frame rate — the smallest case, 23.976 fps at 80 subframes, is already 2_073_600 * 100 * 80 = 16_588_800_000 against an `Int.max` of 2_147_483_647 — so the multiplication traps on any 32-bit platform (wasm32, watchOS armv7k/arm64_32). Because the bound is recomputed inside every wrapping add (`sfcNew.clamped(to: 0 ... maxSubFrameCountExpressible)`), all arithmetic on a `.max100Days` timecode trapped on those platforms however small the operands. Compute the product in `Int64` and saturate on return. No behaviour change on 64-bit: the product peaks at ~82.9e9, far below `Int64.max`, so the clamp never engages. Correct on 32-bit: the value is only ever used as an upper bound, and a `subFrameCount` that large is itself unrepresentable there, so `Int.max` still bounds the whole representable domain. Signature unchanged. Adds two regression tests covering every frame rate and subframe base, and a `.max100Days` wrapping add. --- .../TimecodeFrameRate Properties.swift | 19 +++++++- .../TimecodeFrameRate Properties Tests.swift | 46 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftTimecodeCore/TimecodeFrameRate/TimecodeFrameRate Properties.swift b/Sources/SwiftTimecodeCore/TimecodeFrameRate/TimecodeFrameRate Properties.swift index dccb0e21..50d94fb0 100644 --- a/Sources/SwiftTimecodeCore/TimecodeFrameRate/TimecodeFrameRate Properties.swift +++ b/Sources/SwiftTimecodeCore/TimecodeFrameRate/TimecodeFrameRate Properties.swift @@ -304,7 +304,24 @@ extension TimecodeFrameRate { in extent: Timecode.UpperLimit, base: Timecode.SubFramesBase ) -> Int { - maxTotalFrames(in: extent) * base.rawValue + // Computed in `Int64` and saturated on the way back. + // + // On a 32-bit platform (wasm32, watchOS armv7k/arm64_32) this product + // exceeds `Int.max` for every frame rate when `extent` is + // `.max100Days`. The smallest case, 23.976 fps at 80 subframes, is + // already `2_073_600 * 100 * 80 = 16_588_800_000` against an `Int.max` + // of `2_147_483_647`, so the multiplication traps on overflow. + // + // Saturating is safe because this value is only ever used as an upper + // BOUND — a `clamped(to:)` range, or a `>` comparison against a + // `subFrameCount`. A `subFrameCount` that large is itself + // unrepresentable in `Int` on those platforms, so `Int.max` still + // bounds the entire representable domain. + // + // No behaviour change on 64-bit: the product is at most ~82.9e9, far + // below a 64-bit `Int.max`, so the clamp never engages. + let product = Int64(maxTotalFrames(in: extent)) * Int64(base.rawValue) + return Int(clamping: product) } /// Returns max elapsed subframes possible before rolling over to 0. diff --git a/Tests/SwiftTimecodeCoreTests/TimecodeFrameRate/TimecodeFrameRate Properties Tests.swift b/Tests/SwiftTimecodeCoreTests/TimecodeFrameRate/TimecodeFrameRate Properties Tests.swift index dcb90e02..a8db2132 100644 --- a/Tests/SwiftTimecodeCoreTests/TimecodeFrameRate/TimecodeFrameRate Properties Tests.swift +++ b/Tests/SwiftTimecodeCoreTests/TimecodeFrameRate/TimecodeFrameRate Properties Tests.swift @@ -89,6 +89,52 @@ struct TimecodeFrameRate_Properties_Tests { #expect(frameRate.framesDroppedPerMinute == 0.0) } + /// `.max100Days` subframe bounds must not overflow `Int` on a 32-bit platform. + /// + /// `maxTotalSubFrames` used to compute `maxTotalFrames(in:) * base.rawValue` + /// directly in `Int`. At `.max100Days` that product exceeds `Int32.max` for + /// EVERY frame rate — the smallest, 23.976 fps at 80 subframes, is already + /// `2_073_600 * 100 * 80 = 16_588_800_000` — so it trapped on wasm32 and on + /// watchOS armv7k/arm64_32. Since the bound is recomputed inside every + /// wrapping add, that made all arithmetic on a `.max100Days` timecode trap + /// on those platforms regardless of how small the operands were. + @Test + func maxTotalSubFramesDoesNotOverflowOn32Bit() { + for frameRate in TimecodeFrameRate.allCases { + for base in Timecode.SubFramesBase.allCases { + // Must not trap. On 32-bit the result saturates at `Int.max`; + // on 64-bit it is the exact product. + let total = frameRate.maxTotalSubFrames(in: .max100Days, base: base) + #expect(total > 0) + #expect(frameRate.maxSubFrameCountExpressible(in: .max100Days, base: base) == total - 1) + + if Int.bitWidth >= 64 { + let expected = frameRate.maxTotalFrames(in: .max100Days) * base.rawValue + #expect(total == expected) + } else { + #expect(total == Int.max) + } + } + } + } + + /// Arithmetic on a `.max100Days` timecode must work on every platform. + /// + /// The regression this guards is not about large values — these operands are + /// tiny. It is the upper BOUND, recomputed on each wrapping add, that used + /// to overflow. + @Test + func max100DaysArithmeticDoesNotTrap() throws { + var lhs = try Timecode(.realTime(seconds: 1.0), at: .fps59_94) + var rhs = try Timecode(.realTime(seconds: 192.0), at: .fps59_94) + lhs.properties.upperLimit = .max100Days + rhs.properties.upperLimit = .max100Days + + let sum = try lhs.adding(rhs, by: .wrapping) + #expect(sum.components.seconds == 12) + #expect(sum.components.minutes == 3) + } + @Test func initStringValue() { #expect(TimecodeFrameRate(stringValue: "23.976") == .fps23_976)