[GIT-PULLS] [php-src] PR #23505: Fix inconsistent min()/max() tie-breaking on signed zero via the frameless 2-arg fast path

[email protected] (2akouwu)
Newsgroups php.git-pulls
Message-ID <[email protected]>
Pull Request: https://github.com/php/php-src/pull/23505
Author: 2akouwu

## Root cause

`min($a, $b)` and `max($a, $b)` calls with exactly two positional arguments are compiled by the engine to the `ZEND_FRAMELESS_FUNCTION(min, 2)` / `ZEND_FRAMELESS_FUNCTION(max, 2)` fast paths in `ext/standard/array.c` (wired up in `Zend/zend_compile.c` for any call site whose argument count matches a registered frameless variant), bypassing the normal argument-array based implementation.

`ZEND_FRAMELESS_FUNCTION(max, 2)` already resolves ties with `>=`, so when the two operands compare equal it keeps `lhs` (the first-seen operand). That matches `php_array_data_minmax()` (used for `min(array)`/`max(array)`) and the variadic `PHP_FUNCTION(min)`/`PHP_FUNCTION(max)` implementations, both of which only replace the running result on a *strict* improvement and therefore also keep the first-seen operand on a tie.

`ZEND_FRAMELESS_FUNCTION(min, 2)` was the one path that didn't follow this convention: every comparison used strict `<`, so on a tie it returned `rhs` (the second operand) instead of `lhs`. This is invisible for most values, but `-0.0` and `0` compare equal under ordinary IEEE-754 double comparison while still being distinguishable via `var_dump()` (`float(-0)` vs `int(0)`). That's exactly what `round(-0.01 / 2, 0)` produces (`-0.0`), so:

```php
var_dump(min(round(-0.01 / 2, 0), 0)); // returned int(0)  -- wrong operand picked on tie
var_dump(max(round(-0.01 / 2, 0), 0)); // returned float(-0) -- correct, first operand kept
```

min() and max() disagreeing on which operand "wins" a tie, given the exact same two operands in the exact same order, is the actual bug -- not merely a cosmetic `-0` formatting issue.

## Why this fix

I considered special-casing signed zero directly (e.g. using `signbit()` so min() always prefers the negative-zero representation and max() always prefers positive zero, regardless of argument order, similar to `Math.min`/`Math.max` in some other languages). I rejected that: PHP's comparison operators and `zend_compare()` have no such total-ordering concept for `-0.0` anywhere else in the engine (array `min`/`max`, the variadic `min`/`max`, `<`, `>`, sort callbacks, etc. all treat `-0.0 == 0.0`), so introducing it only for the 2-arg frameless path would create a *new*, narrower inconsistency instead of fixing the existing one.

Instead, the minimal, targeted fix is to make the frameless `min()` tie-break the same way as every other min/max code path already does: keep the first-seen operand on a tie. Concretely, this changes the three `<` comparisons (long/long, double/double, double/long) and the `zend_compare(...) < 0` generic fallback in `ZEND_FRAMELESS_FUNCTION(min, 2)` to `<=`, mirroring the `>=` already used in `ZEND_FRAMELESS_FUNCTION(max, 2)`. No behavior changes for non-tied values; only the tie-break operand selection changes, and only for the frameless 2-arg path.

## Testing

Added `ext/standard/tests/array/gh20221.phpt`, following this project's `gh<issue>.phpt` naming convention (see e.g. `gh18480.phpt`, `gh17977.phpt` in the same directory). It reproduces the issue's exact scenario (`-0.0` vs `0`) through the 2-argument frameless calls in both operand orders for both `min()` and `max()`, and cross-checks the same values through the 3-argument variadic path and the array-argument path (`min([...])`/`max([...])`), asserting that the frameless results now agree with those already-correct paths.

I could not build the PHP interpreter in this sandboxed source-only bundle (no toolchain/build artifacts are present), so the `.phpt` was not executed through `run-tests.php`. I traced the expected output by hand against the modified control flow in `ext/standard/array.c` for every branch the test exercises (long/long, double/double, double/long, and both operand orders), which is why the test includes the redundant array/variadic cross-checks -- so a reviewer running it in CI has an internally-consistent oracle, not just my hand-derived expectations.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.