Skip to content

Commit 3db8c0c

Browse files
nyamsprodnicolas-grekas
authored andcommitted
Make Time\Duration exact on 32 bit platforms
multiplyBy() and divideBy() are the two operations whose intermediate values do not fit in an integer when PHP_INT_SIZE is 4: a duration holds up to 2**61 nanoseconds. They used to throw Time\TimeException there, while the native implementation computes the result with 64 bit arithmetic. * multiplyBy() now multiplies by doubling and adding (seconds, nanoseconds) pairs, which is exact whatever the integer width, and no longer needs the nanoseconds overflow guard * divideBy() falls back to a digit-wise division when the dividend does not fit in an integer; every intermediate value stays below 2**53, where floats represent integers exactly The digit-wise division is unreachable on 64 bit platforms, so DurationTest checks it against intdiv() there, since no CI job runs on 32 bit.
1 parent c2db99f commit 3db8c0c

3 files changed

Lines changed: 115 additions & 27 deletions

File tree

src/Time/README.md

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,10 @@ added to PHP 8.6 core, for PHP >= 8.1:
77
- `Time\TimeException`, `Time\Duration`
88

99
The phpt tests of the native implementation, borrowed from php-src, run against
10-
the polyfill as part of the test suite. Two divergences remain:
11-
12-
- `Duration` is a plain final class with readonly properties instead of a
13-
`readonly` class, so `ReflectionClass::newInstanceWithoutConstructor()`
14-
succeeds where the native class rejects it; dynamic properties are rejected
15-
as they are natively.
16-
- On 32-bit platforms, operations whose intermediate values do not fit in an
17-
integer throw `Time\TimeException` instead of returning the in-range result
18-
the native implementation computes with 64-bit arithmetic.
10+
the polyfill as part of the test suite. One divergence remains: `Duration` is a
11+
plain final class with readonly properties instead of a `readonly` class, so
12+
`ReflectionClass::newInstanceWithoutConstructor()` succeeds where the native
13+
class rejects it. Dynamic properties are rejected as they are natively.
1914

2015
More information can be found in the
2116
[main Polyfill README](https://github.com/symfony/polyfill/blob/main/README.md).

src/Time/Resources/stubs/Time/Duration.php

Lines changed: 82 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -310,23 +310,26 @@ public function multiplyBy(int $factor): self
310310
return $this;
311311
}
312312

313-
if (0 < $this->seconds && $factor > intdiv(self::MAX_SECONDS, $this->seconds)) {
314-
throw new TimeException(self::RANGE_ERROR);
315-
}
316-
317-
if (0 < $this->nanoseconds && $factor > intdiv(\PHP_INT_MAX, $this->nanoseconds)) {
318-
throw new TimeException(self::RANGE_ERROR);
319-
}
313+
// double-and-add: multiplying the nanoseconds directly would overflow
314+
$seconds = 0;
315+
$nanoseconds = 0;
316+
$addSeconds = $this->seconds;
317+
$addNanoseconds = $this->nanoseconds;
318+
319+
while (true) {
320+
if (1 & $factor) {
321+
[$seconds, $nanoseconds] = self::addMagnitudes($seconds, $nanoseconds, $addSeconds, $addNanoseconds);
322+
}
320323

321-
$seconds = $this->seconds * $factor;
322-
$nanoseconds = $this->nanoseconds * $factor;
323-
$carry = intdiv($nanoseconds, self::NANOS_PER_SECOND);
324+
if (0 === $factor >>= 1) {
325+
break;
326+
}
324327

325-
if ($carry > self::MAX_SECONDS - $seconds) {
326-
throw new TimeException(self::RANGE_ERROR);
328+
// the remaining factor is not zero, so an out of range double is out of range for the result too
329+
[$addSeconds, $addNanoseconds] = self::addMagnitudes($addSeconds, $addNanoseconds, $addSeconds, $addNanoseconds);
327330
}
328331

329-
return self::create($seconds + $carry, $nanoseconds % self::NANOS_PER_SECOND, $this->negative);
332+
return self::create($seconds, $nanoseconds, $this->negative);
330333
}
331334

332335
/**
@@ -354,11 +357,9 @@ public function divideBy(int $divisor): self
354357

355358
$remainder = $this->seconds % $divisor;
356359

357-
if ($remainder > intdiv(\PHP_INT_MAX - $this->nanoseconds, self::NANOS_PER_SECOND)) {
358-
throw new TimeException(self::RANGE_ERROR);
359-
}
360-
361-
$nanoseconds = intdiv($this->nanoseconds + $remainder * self::NANOS_PER_SECOND, $divisor);
360+
$nanoseconds = $remainder > intdiv(\PHP_INT_MAX - $this->nanoseconds, self::NANOS_PER_SECOND)
361+
? self::divideNanoseconds($remainder, $this->nanoseconds, $divisor)
362+
: intdiv($this->nanoseconds + $remainder * self::NANOS_PER_SECOND, $divisor);
362363

363364
return self::create(intdiv($this->seconds, $divisor), $nanoseconds, $this->negative);
364365
}
@@ -385,6 +386,69 @@ public function __set(string $name, mixed $value): void
385386
throw new \Error(\sprintf('Cannot create dynamic property %s::$%s', self::class, $name));
386387
}
387388

389+
/**
390+
* Adds two non-negative (seconds, nanoseconds) pairs.
391+
*
392+
* @return array{0: int, 1: int}
393+
*
394+
* @throws TimeException when the sum is out of the representable range
395+
*/
396+
private static function addMagnitudes(int $seconds, int $nanoseconds, int $addSeconds, int $addNanoseconds): array
397+
{
398+
if ($seconds > self::MAX_SECONDS - $addSeconds) {
399+
throw new TimeException(self::RANGE_ERROR);
400+
}
401+
402+
$seconds += $addSeconds;
403+
// the sum of two nanoseconds components is always below 2**31
404+
$nanoseconds += $addNanoseconds;
405+
406+
if ($nanoseconds >= self::NANOS_PER_SECOND) {
407+
if (self::MAX_SECONDS === $seconds) {
408+
throw new TimeException(self::RANGE_ERROR);
409+
}
410+
411+
++$seconds;
412+
$nanoseconds -= self::NANOS_PER_SECOND;
413+
}
414+
415+
return [$seconds, $nanoseconds];
416+
}
417+
418+
/**
419+
* Returns intdiv($remainder * 1_000_000_000 + $nanoseconds, $divisor) for
420+
* a $remainder that is lower than $divisor.
421+
*
422+
* The dividend does not fit in an integer on 32 bit platforms, so the
423+
* division is done one decimal digit at a time. Every intermediate value
424+
* stays below 2**53, where floats represent integers exactly.
425+
*/
426+
private static function divideNanoseconds(int $remainder, int $nanoseconds, int $divisor): int
427+
{
428+
$quotient = 0;
429+
$rest = (float) $remainder;
430+
431+
foreach (str_split(str_pad((string) $nanoseconds, 9, '0', \STR_PAD_LEFT)) as $digit) {
432+
$rest = $rest * 10 + (int) $digit;
433+
$digit = (int) ($rest / $divisor);
434+
$rest -= $digit * $divisor;
435+
436+
// the float division may be off by one in either direction
437+
while (0 > $rest) {
438+
--$digit;
439+
$rest += $divisor;
440+
}
441+
while ($rest >= $divisor) {
442+
++$digit;
443+
$rest -= $divisor;
444+
}
445+
446+
$quotient = $quotient * 10 + $digit;
447+
}
448+
449+
return $quotient;
450+
}
451+
388452
/**
389453
* @throws TimeException when the duration is out of the representable range
390454
*/

tests/Time/DurationTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,35 @@ public static function provideOverflowingOperations(): array
348348
];
349349
}
350350

351+
/**
352+
* The digit-wise division is only reached on 32 bit platforms, where the dividend
353+
* does not fit in an integer. Check it here against the integer arithmetic of a
354+
* 64 bit platform, since no CI job runs on 32 bit.
355+
*/
356+
public function testDivideNanosecondsMatchesIntegerArithmetic()
357+
{
358+
if (\PHP_VERSION_ID >= 80600) {
359+
$this->markTestSkipped('This checks an implementation detail of the polyfill.');
360+
}
361+
if (8 > \PHP_INT_SIZE) {
362+
$this->markTestSkipped('The expected values are computed with 64 bit integers.');
363+
}
364+
365+
$divideNanoseconds = new \ReflectionMethod(Duration::class, 'divideNanoseconds');
366+
367+
foreach ([2, 3, 7, 10, 999999999, 1000000000, 1073741823, 2147483647] as $divisor) {
368+
foreach ([0, 1, 2, 3, intdiv($divisor, 2), $divisor - 1] as $remainder) {
369+
foreach ([0, 1, 5, 999999998, 999999999] as $nanoseconds) {
370+
$this->assertSame(
371+
intdiv($remainder * 1000000000 + $nanoseconds, $divisor),
372+
$divideNanoseconds->invoke(null, $remainder, $nanoseconds, $divisor),
373+
"intdiv($remainder * 1e9 + $nanoseconds, $divisor)"
374+
);
375+
}
376+
}
377+
}
378+
}
379+
351380
public function testCompare()
352381
{
353382
$this->assertSame(0, Duration::compare(Duration::fromSeconds(1), Duration::fromSeconds(1)));

0 commit comments

Comments
 (0)