From 438b127635173aaae9c69c9a29acd29a17b423e8 Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Mon, 27 Oct 2025 20:56:09 -0700 Subject: [PATCH 01/11] Fix SORT_REGULAR transitivity for mixed types Fixes GH-20262: SORT_REGULAR now uses transitive comparison for consistent sorting of mixed numeric/non-numeric strings and types. Added EG(transitive_compare_mode) flag with save/restore pattern to enforce ordering: numeric-types < numeric-strings < non-numeric. This fixes sort() and array_unique() inconsistencies with mixed types, nested arrays, and objects. Comparison operators unchanged. --- Zend/zend_globals.h | 4 + Zend/zend_operators.c | 25 +++++- ext/standard/array.c | 9 ++ .../tests/array/sort_regular_transitive.phpt | 88 +++++++++++++++++++ .../sort_regular_transitive_objects.phpt | 81 +++++++++++++++++ 5 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 ext/standard/tests/array/sort_regular_transitive.phpt create mode 100644 ext/standard/tests/array/sort_regular_transitive_objects.phpt diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h index ef81ae5faaf25..11646fa77d1c2 100644 --- a/Zend/zend_globals.h +++ b/Zend/zend_globals.h @@ -301,6 +301,10 @@ struct _zend_executor_globals { uint32_t num_errors; zend_error_info **errors; + /* If transitive_compare_mode is enabled, string comparisons in zendi_smart_strcmp + * will enforce transitivity by consistently ordering numeric vs non-numeric strings. */ + bool transitive_compare_mode; + /* Override filename or line number of thrown errors and exceptions */ zend_string *filename_override; zend_long lineno_override; diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 2550fcbeb1cde..4fe448c4d5d40 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2271,6 +2271,12 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */ return ZEND_THREEWAY_COMPARE((double) lval, str_dval); } + /* String is non-numeric. In transitive mode, enforce: numeric < non-numeric. + * Since lval is numeric and str is non-numeric, lval < str, so return -1. */ + if (UNEXPECTED(EG(transitive_compare_mode))) { + return -1; + } + zend_string *lval_as_str = zend_long_to_str(lval); int cmp_result = zend_binary_strcmp( ZSTR_VAL(lval_as_str), ZSTR_LEN(lval_as_str), ZSTR_VAL(str), ZSTR_LEN(str)); @@ -2295,6 +2301,12 @@ static int compare_double_to_string(double dval, zend_string *str) /* {{{ */ return ZEND_THREEWAY_COMPARE(dval, str_dval); } + /* String is non-numeric. In transitive mode, enforce: numeric < non-numeric. + * Since dval is numeric and str is non-numeric, dval < str, so return -1. */ + if (UNEXPECTED(EG(transitive_compare_mode))) { + return -1; + } + zend_string *dval_as_str = zend_double_to_str(dval); int cmp_result = zend_binary_strcmp( ZSTR_VAL(dval_as_str), ZSTR_LEN(dval_as_str), ZSTR_VAL(str), ZSTR_LEN(str)); @@ -3425,8 +3437,17 @@ ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2) zend_long lval1 = 0, lval2 = 0; double dval1 = 0.0, dval2 = 0.0; - if ((ret1 = is_numeric_string_ex(s1->val, s1->len, &lval1, &dval1, false, &oflow1, NULL)) && - (ret2 = is_numeric_string_ex(s2->val, s2->len, &lval2, &dval2, false, &oflow2, NULL))) { + ret1 = is_numeric_string_ex(s1->val, s1->len, &lval1, &dval1, false, &oflow1, NULL); + ret2 = is_numeric_string_ex(s2->val, s2->len, &lval2, &dval2, false, &oflow2, NULL); + + /* When in transitive comparison mode (used by SORT_REGULAR), enforce transitivity + * by consistently ordering numeric vs non-numeric strings. */ + if (UNEXPECTED(EG(transitive_compare_mode)) && (ret1 != 0) != (ret2 != 0)) { + /* One is numeric, one is not. Order: numeric < non-numeric (matches PHP 8+ rules) */ + return ret1 ? -1 : 1; + } + + if (ret1 && ret2) { #if ZEND_ULONG_MAX == 0xFFFFFFFF if (oflow1 != 0 && oflow1 == oflow2 && dval1 - dval2 == 0. && ((oflow1 == 1 && dval1 > 9007199254740991. /*0x1FFFFFFFFFFFFF*/) diff --git a/ext/standard/array.c b/ext/standard/array.c index 4097d71899011..881d21d649b50 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -285,7 +285,16 @@ static zend_always_inline int php_array_key_compare_string_locale_unstable_i(Buc static zend_always_inline int php_array_data_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { + /* Enable transitive comparison mode for this comparison tree. + * Save the previous state to handle reentrancy (e.g., usort with callback that calls sort). */ + bool old_transitive_mode = EG(transitive_compare_mode); + EG(transitive_compare_mode) = true; + int result = zend_compare(&f->val, &s->val); + + /* Restore previous state */ + EG(transitive_compare_mode) = old_transitive_mode; + /* Special enums handling for array_unique. We don't want to add this logic to zend_compare as * that would be observable via comparison operators. */ zval *rhs = &s->val; diff --git a/ext/standard/tests/array/sort_regular_transitive.phpt b/ext/standard/tests/array/sort_regular_transitive.phpt new file mode 100644 index 0000000000000..c8e419607324c --- /dev/null +++ b/ext/standard/tests/array/sort_regular_transitive.phpt @@ -0,0 +1,88 @@ +--TEST-- +SORT_REGULAR uses transitive comparison for consistent sorting +--FILE-- + '5'], + ['streetNumber' => '10'], + ['streetNumber' => '3A'], +]; +sort($arr3, SORT_REGULAR); +foreach ($arr3 as $item) { + echo $item['streetNumber']; + if ($item !== end($arr3)) echo " "; +} +echo "\n"; + +echo "\nTest 4: Same nested arrays, reversed (should match)\n"; +$arr4 = [ + ['streetNumber' => '3A'], + ['streetNumber' => '10'], + ['streetNumber' => '5'], +]; +sort($arr4, SORT_REGULAR); +foreach ($arr4 as $item) { + echo $item['streetNumber']; + if ($item !== end($arr4)) echo " "; +} +echo "\n"; + +echo "\nTest 5: array_unique with nested arrays\n"; +$arr5 = [ + ['number' => '5'], + ['number' => '10'], + ['number' => '5'], + ['number' => '3A'], + ['number' => '5'], +]; +$unique = array_unique($arr5, SORT_REGULAR); +echo "Unique count: " . count($unique) . "\n"; + +echo "\nTest 6: Comparison operators remain unchanged\n"; +echo '"5" <=> "3A": ' . ("5" <=> "3A") . "\n"; +echo '"10" <=> "3A": ' . ("10" <=> "3A") . "\n"; +?> +--EXPECT-- +Test 1: Scalar mixed numeric/non-numeric strings +array(3) { + [0]=> + string(1) "5" + [1]=> + string(2) "10" + [2]=> + string(2) "3A" +} + +Test 2: Same values, different order (should match) +array(3) { + [0]=> + string(1) "5" + [1]=> + string(2) "10" + [2]=> + string(2) "3A" +} + +Test 3: Nested arrays with mixed strings +5 10 3A + +Test 4: Same nested arrays, reversed (should match) +5 10 3A + +Test 5: array_unique with nested arrays +Unique count: 3 + +Test 6: Comparison operators remain unchanged +"5" <=> "3A": 1 +"10" <=> "3A": -1 diff --git a/ext/standard/tests/array/sort_regular_transitive_objects.phpt b/ext/standard/tests/array/sort_regular_transitive_objects.phpt new file mode 100644 index 0000000000000..503e0b486b19c --- /dev/null +++ b/ext/standard/tests/array/sort_regular_transitive_objects.phpt @@ -0,0 +1,81 @@ +--TEST-- +SORT_REGULAR with objects containing mixed numeric/non-numeric strings is transitive +--FILE-- +streetNumber; +} +echo "\n"; + +echo "\nTest 2: Same objects, different order (should match Test 1)\n"; +$addresses2 = [ + new Address('3A', 'Main St'), + new Address('10', 'Main St'), + new Address('5', 'Main St'), +]; +sort($addresses2, SORT_REGULAR); +echo "Result:"; +foreach ($addresses2 as $addr) { + echo " " . $addr->streetNumber; +} +echo "\n"; + +echo "\nTest 3: Another permutation (should match Test 1)\n"; +$addresses3 = [ + new Address('10', 'Main St'), + new Address('5', 'Main St'), + new Address('3A', 'Main St'), +]; +sort($addresses3, SORT_REGULAR); +echo "Result:"; +foreach ($addresses3 as $addr) { + echo " " . $addr->streetNumber; +} +echo "\n"; + +echo "\nTest 4: array_unique() with objects\n"; +$addresses_with_dupes = [ + new Address('5', 'Main St'), + new Address('10', 'Main St'), + new Address('10', 'Main St'), + new Address('3A', 'Main St'), + new Address('5', 'Main St'), +]; +$unique = array_unique($addresses_with_dupes, SORT_REGULAR); +echo "Input: 5 objects (with duplicates)\n"; +echo "Output: " . count($unique) . " unique objects\n"; +echo "Street numbers:"; +foreach ($unique as $addr) { + echo " " . $addr->streetNumber; +} +echo "\n"; +?> +--EXPECT-- +Test 1: Objects with mixed numeric/non-numeric string properties +Result: 5 10 3A + +Test 2: Same objects, different order (should match Test 1) +Result: 5 10 3A + +Test 3: Another permutation (should match Test 1) +Result: 5 10 3A + +Test 4: array_unique() with objects +Input: 5 objects (with duplicates) +Output: 3 unique objects +Street numbers: 5 10 3A From 3a300dc079e61a9fd5a31cf3829a50ba8a6bdbb0 Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Wed, 29 Oct 2025 13:25:46 -0700 Subject: [PATCH 02/11] Fix empty string ordering in transitive comparison Empty strings must sort before numbers to match PHP 8+ semantics where '' < 5 is true. Updated compare_long_to_string(), compare_double_to_string(), and zendi_smart_strcmp() to handle empty string as a special case in transitive mode. --- Zend/zend_operators.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 4fe448c4d5d40..d73423118a6fe 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2271,10 +2271,16 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */ return ZEND_THREEWAY_COMPARE((double) lval, str_dval); } - /* String is non-numeric. In transitive mode, enforce: numeric < non-numeric. - * Since lval is numeric and str is non-numeric, lval < str, so return -1. */ + /* String is non-numeric. In transitive mode, enforce consistent ordering. + * Empty string < numeric < non-numeric string. + * Since str is non-numeric, check if it's empty. */ if (UNEXPECTED(EG(transitive_compare_mode))) { - return -1; + /* Empty string comes before everything */ + if (ZSTR_LEN(str) == 0) { + return 1; /* lval > empty string */ + } + /* Non-empty, non-numeric string comes after numbers */ + return -1; /* lval < non-numeric string */ } zend_string *lval_as_str = zend_long_to_str(lval); @@ -2301,10 +2307,16 @@ static int compare_double_to_string(double dval, zend_string *str) /* {{{ */ return ZEND_THREEWAY_COMPARE(dval, str_dval); } - /* String is non-numeric. In transitive mode, enforce: numeric < non-numeric. - * Since dval is numeric and str is non-numeric, dval < str, so return -1. */ + /* String is non-numeric. In transitive mode, enforce consistent ordering. + * Empty string < numeric < non-numeric string. + * Since str is non-numeric, check if it's empty. */ if (UNEXPECTED(EG(transitive_compare_mode))) { - return -1; + /* Empty string comes before everything */ + if (ZSTR_LEN(str) == 0) { + return 1; /* dval > empty string */ + } + /* Non-empty, non-numeric string comes after numbers */ + return -1; /* dval < non-numeric string */ } zend_string *dval_as_str = zend_double_to_str(dval); @@ -3443,7 +3455,18 @@ ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2) /* When in transitive comparison mode (used by SORT_REGULAR), enforce transitivity * by consistently ordering numeric vs non-numeric strings. */ if (UNEXPECTED(EG(transitive_compare_mode)) && (ret1 != 0) != (ret2 != 0)) { - /* One is numeric, one is not. Order: numeric < non-numeric (matches PHP 8+ rules) */ + /* One is numeric, one is not. + * Special case: empty strings are non-numeric but sort BEFORE numeric strings. + * Order: empty < numeric < non-numeric (matches PHP 8+ comparison semantics) */ + bool is_empty1 = (s1->len == 0); + bool is_empty2 = (s2->len == 0); + + if (is_empty1 || is_empty2) { + /* If one is empty, empty comes first */ + return is_empty1 ? -1 : 1; + } + + /* Neither is empty: numeric < non-numeric */ return ret1 ? -1 : 1; } From 7f8261d69007995a7fc45af12a0dbfdfcaec1f08 Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Wed, 29 Oct 2025 14:13:26 -0700 Subject: [PATCH 03/11] Initialize transitive_compare_mode to fix Windows ZTS Without initialization, transitive_compare_mode had garbage values on Windows ZTS, causing the flag to leak into normal comparisons. Initialize to false in init_executor(). --- Zend/zend_execute_API.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index 660975f9bc1b5..a3333f4e89690 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -196,6 +196,8 @@ void init_executor(void) /* {{{ */ EG(num_errors) = 0; EG(errors) = NULL; + EG(transitive_compare_mode) = false; + EG(filename_override) = NULL; EG(lineno_override) = -1; From 768ec318802c64a57877be30eb0d8facd4aa6e80 Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Wed, 29 Oct 2025 17:46:02 -0700 Subject: [PATCH 04/11] Refactor for readability Suggested-by: Rob Landers --- Zend/zend_operators.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index d73423118a6fe..7def07d19096c 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -3454,7 +3454,9 @@ ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2) /* When in transitive comparison mode (used by SORT_REGULAR), enforce transitivity * by consistently ordering numeric vs non-numeric strings. */ - if (UNEXPECTED(EG(transitive_compare_mode)) && (ret1 != 0) != (ret2 != 0)) { + bool num1 = ret1 != 0; + bool num2 = ret2 != 0; + if (UNEXPECTED(EG(transitive_compare_mode)) && (num1 ^ num2)) { /* One is numeric, one is not. * Special case: empty strings are non-numeric but sort BEFORE numeric strings. * Order: empty < numeric < non-numeric (matches PHP 8+ comparison semantics) */ From f1f8d6b95e1152f9256bf6d9351b6bf6b287e35f Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Wed, 29 Oct 2025 17:51:42 -0700 Subject: [PATCH 05/11] Add tests for GH-20262 - Add gh20262.phpt: Bug reproduction test (replaces sort_regular_transitive*.phpt) - Add sort_variation_numeric_strings.phpt: Edge case tests for numeric strings Suggested-by: Rob Landers --- ext/standard/tests/array/gh20262.phpt | 93 +++++++ .../sort/sort_variation_numeric_strings.phpt | 241 ++++++++++++++++++ .../tests/array/sort_regular_transitive.phpt | 88 ------- .../sort_regular_transitive_objects.phpt | 81 ------ 4 files changed, 334 insertions(+), 169 deletions(-) create mode 100644 ext/standard/tests/array/gh20262.phpt create mode 100644 ext/standard/tests/array/sort/sort_variation_numeric_strings.phpt delete mode 100644 ext/standard/tests/array/sort_regular_transitive.phpt delete mode 100644 ext/standard/tests/array/sort_regular_transitive_objects.phpt diff --git a/ext/standard/tests/array/gh20262.phpt b/ext/standard/tests/array/gh20262.phpt new file mode 100644 index 0000000000000..4a64c8d8ce357 --- /dev/null +++ b/ext/standard/tests/array/gh20262.phpt @@ -0,0 +1,93 @@ +--TEST-- +GH-20262 (array_unique() with SORT_REGULAR returns duplicate values) +--FILE-- +streetNumber; +} +echo "\n"; + +echo "\nTest 4: Nested arrays\n"; +$addresses = [ + ['streetNumber' => '5', 'streetName' => 'Main St'], + ['streetNumber' => '10', 'streetName' => 'Main St'], + ['streetNumber' => '10', 'streetName' => 'Main St'], + ['streetNumber' => '3A', 'streetName' => 'Main St'], + ['streetNumber' => '5', 'streetName' => 'Main St'], +]; + +$unique = array_unique($addresses, SORT_REGULAR); +echo "Unique count: " . count($unique) . " (expected 3)\n"; +echo "Street numbers:"; +foreach ($unique as $addr) { + echo " " . $addr['streetNumber']; +} +echo "\n"; + +echo "\nTest 5: sort() consistency with SORT_REGULAR\n"; +$arr1 = ["5", "10", "3A"]; +$arr2 = ["3A", "10", "5"]; +sort($arr1, SORT_REGULAR); +sort($arr2, SORT_REGULAR); +echo "arr1 sorted: ['" . implode("', '", $arr1) . "']\n"; +echo "arr2 sorted: ['" . implode("', '", $arr2) . "']\n"; +echo "Results match: " . ($arr1 === $arr2 ? "yes" : "no") . "\n"; + +?> +--EXPECT-- +Test 1: Scalar array (original bug report) +Array +( + [0] => 5 + [1] => 10 + [3] => 3A +) + +Test 2: Same array with SORT_STRING +Array +( + [0] => 5 + [1] => 10 + [3] => 3A +) + +Test 3: Objects +Unique count: 3 (expected 3) +Street numbers: 5 10 3A + +Test 4: Nested arrays +Unique count: 3 (expected 3) +Street numbers: 5 10 3A + +Test 5: sort() consistency with SORT_REGULAR +arr1 sorted: ['5', '10', '3A'] +arr2 sorted: ['5', '10', '3A'] +Results match: yes diff --git a/ext/standard/tests/array/sort/sort_variation_numeric_strings.phpt b/ext/standard/tests/array/sort/sort_variation_numeric_strings.phpt new file mode 100644 index 0000000000000..4a3d5ef2a7878 --- /dev/null +++ b/ext/standard/tests/array/sort/sort_variation_numeric_strings.phpt @@ -0,0 +1,241 @@ +--TEST-- +Test sort() function: SORT_REGULAR with numeric string edge cases +--FILE-- + +--EXPECT-- +*** Testing sort() : SORT_REGULAR with numeric edge cases *** + +-- Test 1: Empty string and zero variations -- +array(4) { + [0]=> + string(0) "" + [1]=> + string(1) "0" + [2]=> + string(2) "00" + [3]=> + string(1) "A" +} + +-- Test 2: Numeric strings with whitespace and signs -- +array(5) { + [0]=> + string(2) "-0" + [1]=> + string(1) "0" + [2]=> + string(2) " 5" + [3]=> + string(2) "+5" + [4]=> + string(1) "A" +} + +-- Test 3: Scientific notation and special floats -- +array(5) { + [0]=> + string(3) "5e2" + [1]=> + string(3) "500" + [2]=> + string(4) "-INF" + [3]=> + string(3) "INF" + [4]=> + string(3) "NAN" +} + +-- Test 4: Hexadecimal, binary and decimal strings -- +array(3) { + [0]=> + string(2) "16" + [1]=> + string(7) "0b10000" + [2]=> + string(4) "0x10" +} + +-- Test 5: Mixed integers and numeric strings -- +array(5) { + [0]=> + string(0) "" + [1]=> + int(5) + [2]=> + int(10) + [3]=> + string(2) "10" + [4]=> + string(2) "3A" +} + +-- Test 6: LONG_MAX boundary -- +array(3) { + [0]=> + string(19) "9223372036854775807" + [1]=> + string(19) "9223372036854775808" + [2]=> + int(9223372036854775807) +} + +-- Test 7: Leading/trailing whitespace -- +array(5) { + [0]=> + string(1) "5" + [1]=> + string(2) " 5" + [2]=> + string(2) "5 " + [3]=> + string(3) " 5 " + [4]=> + string(1) "A" +} + +-- Test 8: Zero variations with signs -- +array(5) { + [0]=> + string(1) "0" + [1]=> + string(2) "-0" + [2]=> + string(2) "+0" + [3]=> + string(3) "0.0" + [4]=> + string(4) "-0.0" +} + +-- Test 9: Multiple plus/minus signs -- +array(5) { + [0]=> + string(1) "5" + [1]=> + string(3) "++5" + [2]=> + string(3) "+-5" + [3]=> + string(3) "-+5" + [4]=> + string(3) "--5" +} + +-- Test 10: Decimal point variations -- +array(5) { + [0]=> + string(2) "0." + [1]=> + string(2) ".0" + [2]=> + string(3) "0.0" + [3]=> + string(1) "." + [4]=> + string(1) "A" +} + +-- Test 11: Leading zeros with different values -- +array(5) { + [0]=> + string(2) "00" + [1]=> + string(1) "0" + [2]=> + string(2) "01" + [3]=> + string(3) "001" + [4]=> + string(1) "1" +} + +-- Test 12: Scientific notation variations -- +array(5) { + [0]=> + string(4) "1e-2" + [1]=> + string(3) "1e2" + [2]=> + string(3) "1E2" + [3]=> + string(4) "1e+2" + [4]=> + string(3) "100" +} + +-- Test 13: Consistency check -- +All runs produce same result: yes +Done diff --git a/ext/standard/tests/array/sort_regular_transitive.phpt b/ext/standard/tests/array/sort_regular_transitive.phpt deleted file mode 100644 index c8e419607324c..0000000000000 --- a/ext/standard/tests/array/sort_regular_transitive.phpt +++ /dev/null @@ -1,88 +0,0 @@ ---TEST-- -SORT_REGULAR uses transitive comparison for consistent sorting ---FILE-- - '5'], - ['streetNumber' => '10'], - ['streetNumber' => '3A'], -]; -sort($arr3, SORT_REGULAR); -foreach ($arr3 as $item) { - echo $item['streetNumber']; - if ($item !== end($arr3)) echo " "; -} -echo "\n"; - -echo "\nTest 4: Same nested arrays, reversed (should match)\n"; -$arr4 = [ - ['streetNumber' => '3A'], - ['streetNumber' => '10'], - ['streetNumber' => '5'], -]; -sort($arr4, SORT_REGULAR); -foreach ($arr4 as $item) { - echo $item['streetNumber']; - if ($item !== end($arr4)) echo " "; -} -echo "\n"; - -echo "\nTest 5: array_unique with nested arrays\n"; -$arr5 = [ - ['number' => '5'], - ['number' => '10'], - ['number' => '5'], - ['number' => '3A'], - ['number' => '5'], -]; -$unique = array_unique($arr5, SORT_REGULAR); -echo "Unique count: " . count($unique) . "\n"; - -echo "\nTest 6: Comparison operators remain unchanged\n"; -echo '"5" <=> "3A": ' . ("5" <=> "3A") . "\n"; -echo '"10" <=> "3A": ' . ("10" <=> "3A") . "\n"; -?> ---EXPECT-- -Test 1: Scalar mixed numeric/non-numeric strings -array(3) { - [0]=> - string(1) "5" - [1]=> - string(2) "10" - [2]=> - string(2) "3A" -} - -Test 2: Same values, different order (should match) -array(3) { - [0]=> - string(1) "5" - [1]=> - string(2) "10" - [2]=> - string(2) "3A" -} - -Test 3: Nested arrays with mixed strings -5 10 3A - -Test 4: Same nested arrays, reversed (should match) -5 10 3A - -Test 5: array_unique with nested arrays -Unique count: 3 - -Test 6: Comparison operators remain unchanged -"5" <=> "3A": 1 -"10" <=> "3A": -1 diff --git a/ext/standard/tests/array/sort_regular_transitive_objects.phpt b/ext/standard/tests/array/sort_regular_transitive_objects.phpt deleted file mode 100644 index 503e0b486b19c..0000000000000 --- a/ext/standard/tests/array/sort_regular_transitive_objects.phpt +++ /dev/null @@ -1,81 +0,0 @@ ---TEST-- -SORT_REGULAR with objects containing mixed numeric/non-numeric strings is transitive ---FILE-- -streetNumber; -} -echo "\n"; - -echo "\nTest 2: Same objects, different order (should match Test 1)\n"; -$addresses2 = [ - new Address('3A', 'Main St'), - new Address('10', 'Main St'), - new Address('5', 'Main St'), -]; -sort($addresses2, SORT_REGULAR); -echo "Result:"; -foreach ($addresses2 as $addr) { - echo " " . $addr->streetNumber; -} -echo "\n"; - -echo "\nTest 3: Another permutation (should match Test 1)\n"; -$addresses3 = [ - new Address('10', 'Main St'), - new Address('5', 'Main St'), - new Address('3A', 'Main St'), -]; -sort($addresses3, SORT_REGULAR); -echo "Result:"; -foreach ($addresses3 as $addr) { - echo " " . $addr->streetNumber; -} -echo "\n"; - -echo "\nTest 4: array_unique() with objects\n"; -$addresses_with_dupes = [ - new Address('5', 'Main St'), - new Address('10', 'Main St'), - new Address('10', 'Main St'), - new Address('3A', 'Main St'), - new Address('5', 'Main St'), -]; -$unique = array_unique($addresses_with_dupes, SORT_REGULAR); -echo "Input: 5 objects (with duplicates)\n"; -echo "Output: " . count($unique) . " unique objects\n"; -echo "Street numbers:"; -foreach ($unique as $addr) { - echo " " . $addr->streetNumber; -} -echo "\n"; -?> ---EXPECT-- -Test 1: Objects with mixed numeric/non-numeric string properties -Result: 5 10 3A - -Test 2: Same objects, different order (should match Test 1) -Result: 5 10 3A - -Test 3: Another permutation (should match Test 1) -Result: 5 10 3A - -Test 4: array_unique() with objects -Input: 5 objects (with duplicates) -Output: 3 unique objects -Street numbers: 5 10 3A From e3b9ab9ba844524cb500dd41862e2f3be262b805 Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Wed, 29 Oct 2025 18:42:13 -0700 Subject: [PATCH 06/11] Fix test for x32 platform compatibility On x32, 9223372036854775807 becomes float instead of int. --- .../tests/array/sort/sort_variation_numeric_strings.phpt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/standard/tests/array/sort/sort_variation_numeric_strings.phpt b/ext/standard/tests/array/sort/sort_variation_numeric_strings.phpt index 4a3d5ef2a7878..5cf9d555f37f3 100644 --- a/ext/standard/tests/array/sort/sort_variation_numeric_strings.phpt +++ b/ext/standard/tests/array/sort/sort_variation_numeric_strings.phpt @@ -75,7 +75,7 @@ echo "All runs produce same result: " . (count(array_unique($results)) === 1 ? " echo "Done\n"; ?> ---EXPECT-- +--EXPECTF-- *** Testing sort() : SORT_REGULAR with numeric edge cases *** -- Test 1: Empty string and zero variations -- @@ -149,7 +149,7 @@ array(3) { [1]=> string(19) "9223372036854775808" [2]=> - int(9223372036854775807) + %r(int\(9223372036854775807\)|float\(9\.22337203685477[0-9]E\+18\))%r } -- Test 7: Leading/trailing whitespace -- From 38b9e2eb9dad20219f9d92051a87309b43c0b7e3 Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Tue, 11 Nov 2025 16:52:45 -0800 Subject: [PATCH 07/11] Extend transitive comparison to key sorting and add bailout safety - Apply transitive comparison mode to ksort/krsort for consistency with sort(). - Reset transitive_compare_mode in _zend_bailout() to prevent mode leakage. - Add tests for ksort consistency and bailout scenarios. --- Zend/zend.c | 1 + ext/standard/array.c | 23 ++++- ext/standard/tests/array/gh20262_bailout.phpt | 34 +++++++ .../sort/ksort_variation_numeric_strings.phpt | 90 +++++++++++++++++++ 4 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 ext/standard/tests/array/gh20262_bailout.phpt create mode 100644 ext/standard/tests/array/sort/ksort_variation_numeric_strings.phpt diff --git a/Zend/zend.c b/Zend/zend.c index 4d024444a4be9..32efe3b22e864 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -1265,6 +1265,7 @@ ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32 CG(in_compilation) = 0; CG(memoize_mode) = 0; EG(current_execute_data) = NULL; + EG(transitive_compare_mode) = false; LONGJMP(*EG(bailout), FAILURE); } /* }}} */ diff --git a/ext/standard/array.c b/ext/standard/array.c index 881d21d649b50..2f3a6160e816f 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -127,7 +127,16 @@ static zend_always_inline int php_array_key_compare_unstable_i(Bucket *f, Bucket if (f->key == NULL && s->key == NULL) { return (zend_long)f->h > (zend_long)s->h ? 1 : -1; } else if (f->key && s->key) { - return zendi_smart_strcmp(f->key, s->key); + /* Enable transitive comparison mode for consistent key sorting. + * Save the previous state to handle reentrancy. */ + bool old_transitive_mode = EG(transitive_compare_mode); + EG(transitive_compare_mode) = true; + + int result = zendi_smart_strcmp(f->key, s->key); + + /* Restore previous state */ + EG(transitive_compare_mode) = old_transitive_mode; + return result; } if (f->key) { ZVAL_STR(&first, f->key); @@ -139,7 +148,17 @@ static zend_always_inline int php_array_key_compare_unstable_i(Bucket *f, Bucket } else { ZVAL_LONG(&second, s->h); } - return zend_compare(&first, &second); + + /* Enable transitive comparison mode for mixed key types. + * Save the previous state to handle reentrancy. */ + bool old_transitive_mode = EG(transitive_compare_mode); + EG(transitive_compare_mode) = true; + + int result = zend_compare(&first, &second); + + /* Restore previous state */ + EG(transitive_compare_mode) = old_transitive_mode; + return result; } /* }}} */ diff --git a/ext/standard/tests/array/gh20262_bailout.phpt b/ext/standard/tests/array/gh20262_bailout.phpt new file mode 100644 index 0000000000000..7260011ba03eb --- /dev/null +++ b/ext/standard/tests/array/gh20262_bailout.phpt @@ -0,0 +1,34 @@ +--TEST-- +GH-20262: Transitive comparison mode is reset on bailout +--FILE-- + +--EXPECTF-- +bool(false) + +Fatal error: Cannot use "self" as a class name as it is reserved in %s(%d) : eval()'d code on line %d +bool(false) diff --git a/ext/standard/tests/array/sort/ksort_variation_numeric_strings.phpt b/ext/standard/tests/array/sort/ksort_variation_numeric_strings.phpt new file mode 100644 index 0000000000000..fb35f953fc1c1 --- /dev/null +++ b/ext/standard/tests/array/sort/ksort_variation_numeric_strings.phpt @@ -0,0 +1,90 @@ +--TEST-- +Test ksort() function: SORT_REGULAR consistency with sort() for numeric string keys +--FILE-- + "a", "16" => "b", "0b10000" => "c"]; + +sort($values, SORT_REGULAR); +ksort($keyed, SORT_REGULAR); + +echo "sort() result: "; +var_dump($values); +echo "ksort() result: "; +var_dump(array_keys($keyed)); + +echo "\n-- Test 2: Mixed integers and numeric strings (from sort test) --\n"; +// Note: This uses actual integer keys mixed with string keys +$values = [10, "3A", 5, "10", ""]; +$keyed = [10 => "a", "3A" => "b", 5 => "c", "10" => "d", "" => "e"]; + +sort($values, SORT_REGULAR); +ksort($keyed, SORT_REGULAR); + +echo "sort() result: "; +var_dump($values); +echo "ksort() result: "; +var_dump(array_keys($keyed)); + +echo "\n-- Test 3: Consistency check (multiple runs) --\n"; +$results = []; +for ($i = 0; $i < 3; $i++) { + $keyed = ["5" => 1, "3A" => 2, "10" => 3]; + ksort($keyed, SORT_REGULAR); + $results[] = implode(",", array_keys($keyed)); +} +echo "All runs produce same result: " . (count(array_unique($results)) === 1 ? "yes" : "no") . "\n"; + +echo "Done\n"; +?> +--EXPECT-- +*** Testing ksort() : SORT_REGULAR consistency with sort() *** + +-- Test 1: Hexadecimal, binary and decimal strings -- +sort() result: array(3) { + [0]=> + string(2) "16" + [1]=> + string(7) "0b10000" + [2]=> + string(4) "0x10" +} +ksort() result: array(3) { + [0]=> + int(16) + [1]=> + string(7) "0b10000" + [2]=> + string(4) "0x10" +} + +-- Test 2: Mixed integers and numeric strings (from sort test) -- +sort() result: array(5) { + [0]=> + string(0) "" + [1]=> + int(5) + [2]=> + int(10) + [3]=> + string(2) "10" + [4]=> + string(2) "3A" +} +ksort() result: array(4) { + [0]=> + string(0) "" + [1]=> + int(5) + [2]=> + int(10) + [3]=> + string(2) "3A" +} + +-- Test 3: Consistency check (multiple runs) -- +All runs produce same result: yes +Done From 1bc08242a656211a763edfecc8f67db8dce0b0fc Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Tue, 11 Nov 2025 23:33:05 -0800 Subject: [PATCH 08/11] Add test for reentrancy handling in comparisons This test covers reentrancy cases with usort, uksort, sort, array_unique, and ksort. --- .../tests/array/gh20262_reentrancy.phpt | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 ext/standard/tests/array/gh20262_reentrancy.phpt diff --git a/ext/standard/tests/array/gh20262_reentrancy.phpt b/ext/standard/tests/array/gh20262_reentrancy.phpt new file mode 100644 index 0000000000000..37d98be5b67d3 --- /dev/null +++ b/ext/standard/tests/array/gh20262_reentrancy.phpt @@ -0,0 +1,106 @@ +--TEST-- +GH-20262: Reentrancy handling in transitive comparison mode +--FILE-- + $b; +}); + +echo "Result: " . implode(', ', $data) . "\n"; +echo "Inner sort calls: $inner_calls\n"; + +echo "\nTest 2: Nested sort() with SORT_REGULAR\n"; +function outer_sort() { + $arr = ['5', '10', '3A']; + sort($arr, SORT_REGULAR); + return $arr; +} + +function nested_sort() { + $outer = outer_sort(); + $inner = outer_sort(); + + // Both should produce identical results + if ($outer === $inner) { + echo "Nested sorts consistent\n"; + } else { + echo "FAIL: Nested sorts inconsistent\n"; + } + + return $outer; +} + +$result = nested_sort(); +echo "Result: " . implode(', ', $result) . "\n"; + +echo "\nTest 3: array_unique() during comparison\n"; +$test_data = ['a', 'b', 'c']; +$call_count = 0; + +usort($test_data, function($a, $b) use (&$call_count) { + $call_count++; + // Call array_unique with SORT_REGULAR during comparison + if ($call_count === 1) { + $temp = ['x', 'x', 'y']; + $unique = array_unique($temp, SORT_REGULAR); + if (count($unique) === 2) { + echo "Inner array_unique worked correctly\n"; + } + } + return strcmp($a, $b); +}); + +echo "Outer usort completed\n"; + +echo "\nTest 4: uksort() calling ksort() (key comparison reentrancy)\n"; +$arr = ['5' => 'a', '10' => 'b', '3A' => 'c']; +$inner_calls = 0; + +uksort($arr, function($a, $b) use (&$inner_calls) { + // Call ksort during key comparison to test reentrancy + if ($inner_calls < 2) { + $inner_calls++; + $inner = ['x' => 1, 'y' => 2, 'z' => 3]; + ksort($inner, SORT_REGULAR); + if (array_keys($inner) === ['x', 'y', 'z']) { + echo "Inner ksort in uksort worked correctly\n"; + } + } + return strcmp($a, $b); +}); + +echo "Outer uksort completed: " . implode(', ', array_keys($arr)) . "\n"; + +?> +--EXPECT-- +Test 1: usort() calling sort() internally (reentrancy) +Result: a, b, c +Inner sort calls: 3 + +Test 2: Nested sort() with SORT_REGULAR +Nested sorts consistent +Result: 5, 10, 3A + +Test 3: array_unique() during comparison +Inner array_unique worked correctly +Outer usort completed + +Test 4: uksort() calling ksort() (key comparison reentrancy) +Inner ksort in uksort worked correctly +Inner ksort in uksort worked correctly +Outer uksort completed: 10, 3A, 5 From 74cef2131e550f509b2d72b10b8ad3a085ea7c5c Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Tue, 11 Nov 2025 23:40:49 -0800 Subject: [PATCH 09/11] Use ZEND_THREEWAY_COMPARE for numeric comparisons This commit replaces manual comparisons with ZEND_THREEWAY_COMPARE to improve consistency. Additionally, it adds type-specific fast paths for integer, string, and double comparisons in array sorting. --- Zend/zend_operators.c | 40 +++++++++++++++++++-------------------- ext/standard/array.c | 44 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 7def07d19096c..984d1c48dc9e5 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2264,7 +2264,7 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */ uint8_t type = is_numeric_string(ZSTR_VAL(str), ZSTR_LEN(str), &str_lval, &str_dval, 0); if (type == IS_LONG) { - return lval > str_lval ? 1 : lval < str_lval ? -1 : 0; + return ZEND_THREEWAY_COMPARE(lval, str_lval); } if (type == IS_DOUBLE) { @@ -3454,22 +3454,23 @@ ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2) /* When in transitive comparison mode (used by SORT_REGULAR), enforce transitivity * by consistently ordering numeric vs non-numeric strings. */ - bool num1 = ret1 != 0; - bool num2 = ret2 != 0; - if (UNEXPECTED(EG(transitive_compare_mode)) && (num1 ^ num2)) { - /* One is numeric, one is not. - * Special case: empty strings are non-numeric but sort BEFORE numeric strings. - * Order: empty < numeric < non-numeric (matches PHP 8+ comparison semantics) */ - bool is_empty1 = (s1->len == 0); - bool is_empty2 = (s2->len == 0); - - if (is_empty1 || is_empty2) { - /* If one is empty, empty comes first */ - return is_empty1 ? -1 : 1; - } - - /* Neither is empty: numeric < non-numeric */ - return ret1 ? -1 : 1; + if (UNEXPECTED(EG(transitive_compare_mode))) { + int type_mismatch = (!!ret1) ^ (!!ret2); + if (UNEXPECTED(type_mismatch)) { + /* One is numeric, one is not. + * Special case: empty strings are non-numeric but sort BEFORE numeric strings. + * Order: empty < numeric < non-numeric (matches PHP 8+ comparison semantics) */ + bool is_empty1 = (s1->len == 0); + bool is_empty2 = (s2->len == 0); + + if (is_empty1 || is_empty2) { + /* If one is empty, empty comes first */ + return is_empty1 ? -1 : 1; + } + + /* Neither is empty: numeric < non-numeric */ + return ret1 ? -1 : 1; + } } if (ret1 && ret2) { @@ -3501,10 +3502,9 @@ ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2) * so a numeric comparison would be inaccurate */ goto string_cmp; } - dval1 = dval1 - dval2; - return ZEND_NORMALIZE_BOOL(dval1); + return ZEND_THREEWAY_COMPARE(dval1, dval2); } else { /* they both have to be long's */ - return lval1 > lval2 ? 1 : (lval1 < lval2 ? -1 : 0); + return ZEND_THREEWAY_COMPARE(lval1, lval2); } } else { int strcmp_ret; diff --git a/ext/standard/array.c b/ext/standard/array.c index 2f3a6160e816f..48590defe3103 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -304,16 +304,42 @@ static zend_always_inline int php_array_key_compare_string_locale_unstable_i(Buc static zend_always_inline int php_array_data_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { - /* Enable transitive comparison mode for this comparison tree. - * Save the previous state to handle reentrancy (e.g., usort with callback that calls sort). */ - bool old_transitive_mode = EG(transitive_compare_mode); - EG(transitive_compare_mode) = true; - - int result = zend_compare(&f->val, &s->val); - - /* Restore previous state */ - EG(transitive_compare_mode) = old_transitive_mode; + int result; + + if (EXPECTED(Z_TYPE(f->val) == IS_LONG && Z_TYPE(s->val) == IS_LONG)) { + zend_long l1 = Z_LVAL(f->val); + zend_long l2 = Z_LVAL(s->val); + return ZEND_THREEWAY_COMPARE(l1, l2); + } + + if (Z_TYPE(f->val) == IS_STRING && Z_TYPE(s->val) == IS_STRING) { + bool old_mode = EG(transitive_compare_mode); + if (EXPECTED(!old_mode)) { + EG(transitive_compare_mode) = true; + result = zendi_smart_strcmp(Z_STR(f->val), Z_STR(s->val)); + EG(transitive_compare_mode) = false; + } else { + result = zendi_smart_strcmp(Z_STR(f->val), Z_STR(s->val)); + } + goto check_enums; + } + + if (Z_TYPE(f->val) == IS_DOUBLE && Z_TYPE(s->val) == IS_DOUBLE) { + double d1 = Z_DVAL(f->val); + double d2 = Z_DVAL(s->val); + return ZEND_THREEWAY_COMPARE(d1, d2); + } + bool old_transitive_mode = EG(transitive_compare_mode); + if (EXPECTED(!old_transitive_mode)) { + EG(transitive_compare_mode) = true; + result = zend_compare(&f->val, &s->val); + EG(transitive_compare_mode) = false; + } else { + result = zend_compare(&f->val, &s->val); + } + +check_enums: /* Special enums handling for array_unique. We don't want to add this logic to zend_compare as * that would be observable via comparison operators. */ zval *rhs = &s->val; From 8dca012009a0914f399fc4e509b61525ea08037b Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Wed, 12 Nov 2025 00:24:24 -0800 Subject: [PATCH 10/11] Add null statement after label for C99 compatibility Fixes CI error: "label followed by a declaration is a C23 extension" --- ext/standard/array.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/standard/array.c b/ext/standard/array.c index 48590defe3103..35a318b1a3b75 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -340,6 +340,7 @@ static zend_always_inline int php_array_data_compare_unstable_i(Bucket *f, Bucke } check_enums: + ; /* Special enums handling for array_unique. We don't want to add this logic to zend_compare as * that would be observable via comparison operators. */ zval *rhs = &s->val; From 1333407d831d90c6d7510a1369d42a48213440e7 Mon Sep 17 00:00:00 2001 From: Jason Marble Date: Wed, 12 Nov 2025 21:03:56 -0800 Subject: [PATCH 11/11] Refactor comparison function for better performance Refactors php_array_data_compare_unstable_i() to dereference before type checking and simplify control flow with if/else-if chain. This eliminates redundant mode toggling and improves branch prediction. Pass transitive_mode parameter to compare_long_to_string() and compare_double_to_string() to reduce repeated EG() calls. --- Zend/zend_operators.c | 51 +++++++++++---------- ext/standard/array.c | 100 ++++++++++++++++++++++++------------------ 2 files changed, 86 insertions(+), 65 deletions(-) diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 984d1c48dc9e5..028249c76a56e 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2257,7 +2257,7 @@ ZEND_API zend_result ZEND_FASTCALL compare_function(zval *result, zval *op1, zva } /* }}} */ -static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */ +static int compare_long_to_string(zend_long lval, zend_string *str, bool transitive_mode) /* {{{ */ { zend_long str_lval; double str_dval; @@ -2274,7 +2274,7 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */ /* String is non-numeric. In transitive mode, enforce consistent ordering. * Empty string < numeric < non-numeric string. * Since str is non-numeric, check if it's empty. */ - if (UNEXPECTED(EG(transitive_compare_mode))) { + if (UNEXPECTED(transitive_mode)) { /* Empty string comes before everything */ if (ZSTR_LEN(str) == 0) { return 1; /* lval > empty string */ @@ -2291,7 +2291,7 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */ } /* }}} */ -static int compare_double_to_string(double dval, zend_string *str) /* {{{ */ +static int compare_double_to_string(double dval, zend_string *str, bool transitive_mode) /* {{{ */ { zend_long str_lval; double str_dval; @@ -2310,7 +2310,7 @@ static int compare_double_to_string(double dval, zend_string *str) /* {{{ */ /* String is non-numeric. In transitive mode, enforce consistent ordering. * Empty string < numeric < non-numeric string. * Since str is non-numeric, check if it's empty. */ - if (UNEXPECTED(EG(transitive_compare_mode))) { + if (UNEXPECTED(transitive_mode)) { /* Empty string comes before everything */ if (ZSTR_LEN(str) == 0) { return 1; /* dval > empty string */ @@ -2331,6 +2331,8 @@ ZEND_API int ZEND_FASTCALL zend_compare(zval *op1, zval *op2) /* {{{ */ { bool converted = false; zval op1_copy, op2_copy; + + bool transitive_mode = UNEXPECTED(EG(transitive_compare_mode)); while (1) { switch (TYPE_PAIR(Z_TYPE_P(op1), Z_TYPE_P(op2))) { @@ -2375,24 +2377,24 @@ ZEND_API int ZEND_FASTCALL zend_compare(zval *op1, zval *op2) /* {{{ */ return Z_STRLEN_P(op1) == 0 ? 0 : 1; case TYPE_PAIR(IS_LONG, IS_STRING): - return compare_long_to_string(Z_LVAL_P(op1), Z_STR_P(op2)); + return compare_long_to_string(Z_LVAL_P(op1), Z_STR_P(op2), transitive_mode); case TYPE_PAIR(IS_STRING, IS_LONG): - return -compare_long_to_string(Z_LVAL_P(op2), Z_STR_P(op1)); + return -compare_long_to_string(Z_LVAL_P(op2), Z_STR_P(op1), transitive_mode); case TYPE_PAIR(IS_DOUBLE, IS_STRING): if (zend_isnan(Z_DVAL_P(op1))) { return 1; } - return compare_double_to_string(Z_DVAL_P(op1), Z_STR_P(op2)); + return compare_double_to_string(Z_DVAL_P(op1), Z_STR_P(op2), transitive_mode); case TYPE_PAIR(IS_STRING, IS_DOUBLE): if (zend_isnan(Z_DVAL_P(op2))) { return 1; } - return -compare_double_to_string(Z_DVAL_P(op2), Z_STR_P(op1)); + return -compare_double_to_string(Z_DVAL_P(op2), Z_STR_P(op1), transitive_mode); case TYPE_PAIR(IS_OBJECT, IS_NULL): return 1; @@ -3449,26 +3451,31 @@ ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2) zend_long lval1 = 0, lval2 = 0; double dval1 = 0.0, dval2 = 0.0; + /* Handle empty strings */ + if (UNEXPECTED(s1->len == 0 || s2->len == 0)) { + if (UNEXPECTED(EG(transitive_compare_mode))) { + if (s1->len == 0 && s2->len == 0) return 0; + return s1->len == 0 ? -1 : 1; + } + goto string_cmp; + } + + /* Skip numeric parsing if both strings start with letters */ + unsigned char c1 = (unsigned char)s1->val[0]; + unsigned char c2 = (unsigned char)s2->val[0]; + + if (((c1 >= 'a' && c1 <= 'z') || (c1 >= 'A' && c1 <= 'Z')) && + ((c2 >= 'a' && c2 <= 'z') || (c2 >= 'A' && c2 <= 'Z'))) { + goto string_cmp; + } + ret1 = is_numeric_string_ex(s1->val, s1->len, &lval1, &dval1, false, &oflow1, NULL); ret2 = is_numeric_string_ex(s2->val, s2->len, &lval2, &dval2, false, &oflow2, NULL); - /* When in transitive comparison mode (used by SORT_REGULAR), enforce transitivity - * by consistently ordering numeric vs non-numeric strings. */ + /* In transitive mode, enforce numeric < non-numeric ordering */ if (UNEXPECTED(EG(transitive_compare_mode))) { int type_mismatch = (!!ret1) ^ (!!ret2); if (UNEXPECTED(type_mismatch)) { - /* One is numeric, one is not. - * Special case: empty strings are non-numeric but sort BEFORE numeric strings. - * Order: empty < numeric < non-numeric (matches PHP 8+ comparison semantics) */ - bool is_empty1 = (s1->len == 0); - bool is_empty2 = (s2->len == 0); - - if (is_empty1 || is_empty2) { - /* If one is empty, empty comes first */ - return is_empty1 ? -1 : 1; - } - - /* Neither is empty: numeric < non-numeric */ return ret1 ? -1 : 1; } } diff --git a/ext/standard/array.c b/ext/standard/array.c index 35a318b1a3b75..bbfc241d63d16 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -304,62 +304,76 @@ static zend_always_inline int php_array_key_compare_string_locale_unstable_i(Buc static zend_always_inline int php_array_data_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { - int result; - if (EXPECTED(Z_TYPE(f->val) == IS_LONG && Z_TYPE(s->val) == IS_LONG)) { - zend_long l1 = Z_LVAL(f->val); - zend_long l2 = Z_LVAL(s->val); - return ZEND_THREEWAY_COMPARE(l1, l2); + return ZEND_THREEWAY_COMPARE(Z_LVAL(f->val), Z_LVAL(s->val)); } - if (Z_TYPE(f->val) == IS_STRING && Z_TYPE(s->val) == IS_STRING) { - bool old_mode = EG(transitive_compare_mode); - if (EXPECTED(!old_mode)) { - EG(transitive_compare_mode) = true; - result = zendi_smart_strcmp(Z_STR(f->val), Z_STR(s->val)); - EG(transitive_compare_mode) = false; - } else { - result = zendi_smart_strcmp(Z_STR(f->val), Z_STR(s->val)); - } - goto check_enums; - } - - if (Z_TYPE(f->val) == IS_DOUBLE && Z_TYPE(s->val) == IS_DOUBLE) { - double d1 = Z_DVAL(f->val); - double d2 = Z_DVAL(s->val); - return ZEND_THREEWAY_COMPARE(d1, d2); + if (EXPECTED(Z_TYPE(f->val) == IS_DOUBLE && Z_TYPE(s->val) == IS_DOUBLE)) { + return ZEND_THREEWAY_COMPARE(Z_DVAL(f->val), Z_DVAL(s->val)); } bool old_transitive_mode = EG(transitive_compare_mode); if (EXPECTED(!old_transitive_mode)) { EG(transitive_compare_mode) = true; - result = zend_compare(&f->val, &s->val); - EG(transitive_compare_mode) = false; - } else { - result = zend_compare(&f->val, &s->val); } -check_enums: - ; - /* Special enums handling for array_unique. We don't want to add this logic to zend_compare as - * that would be observable via comparison operators. */ - zval *rhs = &s->val; - ZVAL_DEREF(rhs); - if (UNEXPECTED(Z_TYPE_P(rhs) == IS_OBJECT) - && result == ZEND_UNCOMPARABLE - && (Z_OBJCE_P(rhs)->ce_flags & ZEND_ACC_ENUM)) { - zval *lhs = &f->val; - ZVAL_DEREF(lhs); - if (Z_TYPE_P(lhs) == IS_OBJECT && (Z_OBJCE_P(lhs)->ce_flags & ZEND_ACC_ENUM)) { - // Order doesn't matter, we just need to group the same enum values - uintptr_t lhs_uintptr = (uintptr_t)Z_OBJ_P(lhs); - uintptr_t rhs_uintptr = (uintptr_t)Z_OBJ_P(rhs); - return lhs_uintptr == rhs_uintptr ? 0 : (lhs_uintptr < rhs_uintptr ? -1 : 1); + int result; + + /* Dereference before type checking */ + zval *op1 = &f->val; + zval *op2 = &s->val; + ZVAL_DEREF(op1); + ZVAL_DEREF(op2); + + if (Z_TYPE_P(op1) == IS_STRING && Z_TYPE_P(op2) == IS_STRING) { + result = zendi_smart_strcmp(Z_STR_P(op1), Z_STR_P(op2)); + } else if (Z_TYPE_P(op1) == IS_OBJECT && Z_TYPE_P(op2) == IS_OBJECT) { + if (Z_OBJ_P(op1) == Z_OBJ_P(op2)) { + result = 0; + } else if (Z_OBJCE_P(op1) != Z_OBJCE_P(op2)) { + result = ZEND_UNCOMPARABLE; + + /* Enum ordering for array_unique */ + if (UNEXPECTED((Z_OBJCE_P(op1)->ce_flags & ZEND_ACC_ENUM) || + (Z_OBJCE_P(op2)->ce_flags & ZEND_ACC_ENUM))) { + if ((Z_OBJCE_P(op1)->ce_flags & ZEND_ACC_ENUM) && + !(Z_OBJCE_P(op2)->ce_flags & ZEND_ACC_ENUM)) { + result = 1; + } else if (!(Z_OBJCE_P(op1)->ce_flags & ZEND_ACC_ENUM) && + (Z_OBJCE_P(op2)->ce_flags & ZEND_ACC_ENUM)) { + result = -1; + } else { + result = ZEND_THREEWAY_COMPARE((uintptr_t)Z_OBJ_P(op1), (uintptr_t)Z_OBJ_P(op2)); + } + } + } else if ((Z_OBJCE_P(op1)->ce_flags & ZEND_ACC_ENUM)) { + result = ZEND_THREEWAY_COMPARE((uintptr_t)Z_OBJ_P(op1), (uintptr_t)Z_OBJ_P(op2)); } else { - // Shift enums to the end of the array - return -1; + result = zend_compare(op1, op2); + } + } else if (Z_TYPE_P(op1) == IS_ARRAY && Z_TYPE_P(op2) == IS_ARRAY) { + if (Z_ARR_P(op1) == Z_ARR_P(op2)) { + result = 0; + } else { + uint32_t n1 = zend_hash_num_elements(Z_ARRVAL_P(op1)); + uint32_t n2 = zend_hash_num_elements(Z_ARRVAL_P(op2)); + + if (n1 != n2) { + /* Different sizes - order by size */ + result = ZEND_THREEWAY_COMPARE(n1, n2); + } else { + /* Same size - deep comparison */ + result = zend_compare(op1, op2); + } } + } else { + result = zend_compare(op1, op2); + } + + if (EXPECTED(!old_transitive_mode)) { + EG(transitive_compare_mode) = false; } + return result; } /* }}} */