Skip to content

Warn when coercing NAN to other types #19573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
+487 −115
Draft

Warn when coercing NAN to other types #19573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de78eb5
Zend: Add new test for coercions from NAN
Girgias Aug 24, 2025
656be82
Warn when coercing NAN to other types
Girgias Aug 24, 2025
12db8aa
try to fix JIT/Optimizations
Girgias Aug 24, 2025
7c29b7f
Address review
Girgias Aug 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 33 additions & 25 deletions Zend/Optimizer/block_pass.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,21 +436,14 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
Tsource[VAR_NUM(opline->op1.var)] = NULL;
break;
}
ZEND_FALLTHROUGH;

case ZEND_IS_EQUAL:
case ZEND_IS_NOT_EQUAL:
if (opline->op1_type == IS_CONST &&
opline->op2_type == IS_CONST) {
opline->op2_type == IS_CONST) {
goto optimize_constant_binary_op;
}
/* IS_EQ(TRUE, X) => BOOL(X)
* IS_EQ(FALSE, X) => BOOL_NOT(X)
* IS_NOT_EQ(TRUE, X) => BOOL_NOT(X)
* IS_NOT_EQ(FALSE, X) => BOOL(X)
* CASE(TRUE, X) => BOOL(X)
* CASE(FALSE, X) => BOOL_NOT(X)
*/
}
/*
* CASE(TRUE, X) => BOOL(X)
* CASE(FALSE, X) => BOOL_NOT(X)
*/
if (opline->op1_type == IS_CONST &&
(Z_TYPE(ZEND_OP1_LITERAL(opline)) == IS_FALSE ||
Z_TYPE(ZEND_OP1_LITERAL(opline)) == IS_TRUE)) {
Expand All @@ -464,19 +457,34 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
SET_UNUSED(opline->op2);
++(*opt_count);
goto optimize_bool;
} else if (opline->op2_type == IS_CONST &&
(Z_TYPE(ZEND_OP2_LITERAL(opline)) == IS_FALSE ||
Z_TYPE(ZEND_OP2_LITERAL(opline)) == IS_TRUE)) {
/* Optimization of comparison with "null" is not safe,
* because ("0" == null) is not equal to !("0")
*/
opline->opcode =
((opline->opcode != ZEND_IS_NOT_EQUAL) == ((Z_TYPE(ZEND_OP2_LITERAL(opline))) == IS_TRUE)) ?
ZEND_BOOL : ZEND_BOOL_NOT;
SET_UNUSED(opline->op2);
++(*opt_count);
goto optimize_bool;
} else if (opline->op2_type == IS_CONST &&
(Z_TYPE(ZEND_OP2_LITERAL(opline)) == IS_FALSE ||
Z_TYPE(ZEND_OP2_LITERAL(opline)) == IS_TRUE)) {
/* Optimization of comparison with "null" is not safe,
* because ("0" == null) is not equal to !("0")
*/
opline->opcode =
((opline->opcode != ZEND_IS_NOT_EQUAL) == ((Z_TYPE(ZEND_OP2_LITERAL(opline))) == IS_TRUE)) ?
ZEND_BOOL : ZEND_BOOL_NOT;
SET_UNUSED(opline->op2);
++(*opt_count);
goto optimize_bool;
}
break;

case ZEND_IS_EQUAL:
case ZEND_IS_NOT_EQUAL:
if (opline->op1_type == IS_CONST &&
opline->op2_type == IS_CONST) {
goto optimize_constant_binary_op;
}
/* IS_EQ(TRUE, X) => BOOL(X)
* IS_EQ(FALSE, X) => BOOL_NOT(X)
* IS_NOT_EQ(TRUE, X) => BOOL_NOT(X)
* IS_NOT_EQ(FALSE, X) => BOOL(X)
* Those optimizations are not safe if the other operand end up being NAN
* as BOOL/BOOL_NOT will warn which IS_EQUAL/IS_NOT_EQUAL do not.
Comment on lines +485 to +486
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Triggered a benchmark to check the effect of this: valgrind shows no regression (changes are under 0.0x%).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do wonder if this is not actually a problem for switch cases too now that I think of.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be worth it to check

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it seems I completely broke switch statements...

<?php

$nan = fdiv(0, 0);
switch ($nan) {
    case true:
        echo "true";
        break;
    case false:
        echo "false";
        break;
}
?>

Returns nothing now, even without opcache :|

*/
break;
case ZEND_IS_IDENTICAL:
if (opline->op1_type == IS_CONST &&
Expand Down
4 changes: 4 additions & 0 deletions Zend/Optimizer/sccp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,10 @@ static inline zend_result ct_eval_bool_cast(zval *result, zval *op) {
ZVAL_TRUE(result);
return SUCCESS;
}
/* NAN warns when casting */
if (Z_TYPE_P(op) == IS_DOUBLE && zend_isnan(Z_DVAL_P(op))) {
return FAILURE;
}

ZVAL_BOOL(result, zend_is_true(op));
return SUCCESS;
Expand Down
6 changes: 4 additions & 2 deletions Zend/Optimizer/zend_inference.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5105,14 +5105,16 @@ ZEND_API bool zend_may_throw_ex(const zend_op *opline, const zend_ssa_op *ssa_op
case ZEND_PRE_DEC:
case ZEND_POST_DEC:
return (t1 & (MAY_BE_NULL|MAY_BE_FALSE|MAY_BE_TRUE|MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE));
case ZEND_BOOL_NOT:
case ZEND_JMPZ:
case ZEND_JMPNZ:
case ZEND_JMPZ_EX:
case ZEND_JMPNZ_EX:
case ZEND_BOOL:
case ZEND_JMP_SET:
return (t1 & MAY_BE_OBJECT);
case ZEND_BOOL:
case ZEND_BOOL_NOT:
/* NAN Cast to bool will warn */
return (t1 & MAY_BE_OBJECT) || (t1 & MAY_BE_DOUBLE);
case ZEND_BOOL_XOR:
return (t1 & MAY_BE_OBJECT) || (t2 & MAY_BE_OBJECT);
case ZEND_IS_EQUAL:
Expand Down
3 changes: 3 additions & 0 deletions Zend/Optimizer/zend_optimizer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ zend_result zend_optimizer_eval_unary_op(zval *result, uint8_t opcode, zval *op1
}
return unary_op(result, op1);
} else { /* ZEND_BOOL */
if (Z_TYPE_P(op1) == IS_DOUBLE && zend_isnan(Z_DVAL_P(op1))) {
return FAILURE;
}
ZVAL_BOOL(result, zend_is_true(op1));
return SUCCESS;
}
Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/runtime_compile_time_binary_operands.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ if (getenv("SKIP_SLOW_TESTS")) die('skip slow test');
?>
--FILE--
<?php
error_reporting(E_ALL ^ E_DEPRECATED);
error_reporting(E_ALL ^ E_DEPRECATED ^ E_WARNING);

$binaryOperators = [
"==",
Expand Down
6 changes: 1 addition & 5 deletions Zend/tests/type_coercion/float_to_int/explicit_casts_should_not_warn.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
Explicit (int) cast must not warn
Explicit (int) cast must not warn (except for NAN)
--SKIPIF--
<?php
if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only");
Expand All @@ -12,12 +12,10 @@ $values =[
3.5,
10e120,
10e300,
fdiv(0, 0),
(string) 3.0,
(string) 3.5,
(string) 10e120,
(string) 10e300,
(string) fdiv(0, 0),
];

foreach($values as $value) {
Expand All @@ -30,9 +28,7 @@ int(3)
int(3)
int(0)
int(0)
int(0)
int(3)
int(3)
int(9223372036854775807)
int(9223372036854775807)
int(0)
6 changes: 1 addition & 5 deletions Zend/tests/type_coercion/float_to_int/explicit_casts_should_not_warn_32bit.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
Explicit (int) cast must not warn 32bit variation
Explicit (int) cast must not warn (except for NAN) 32bit variation
--SKIPIF--
<?php
if (PHP_INT_SIZE != 4) die("skip this test is for 32bit platform only");
Expand All @@ -12,12 +12,10 @@ $values =[
3.5,
10e120,
10e300,
fdiv(0, 0),
(string) 3.0,
(string) 3.5,
(string) 10e120,
(string) 10e300,
(string) fdiv(0, 0),
];

foreach($values as $value) {
Expand All @@ -30,9 +28,7 @@ int(3)
int(3)
int(0)
int(0)
int(0)
int(3)
int(3)
int(2147483647)
int(2147483647)
int(0)
2 changes: 2 additions & 0 deletions Zend/tests/type_coercion/float_to_int/union_int_string_type_arg.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ int(1)

Deprecated: Implicit conversion from float 1.5 to int loses precision in %s on line %d
int(1)

Warning: unexpected NAN value was coerced to string in %s on line %d
string(3) "NAN"
string(8) "1.0E+121"
string(3) "INF"
4 changes: 0 additions & 4 deletions Zend/tests/type_coercion/int_special_values.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ $values = [
-INF,
1 / INF,
-1 / INF, // Negative zero,
NAN
];

foreach($values as $value) {
Expand All @@ -32,6 +31,3 @@ int(0)

float(-0)
int(0)

float(NAN)
int(0)
123 changes: 123 additions & 0 deletions Zend/tests/type_coercion/nan_comp_op.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
--TEST--
NAN coerced to other types
--FILE--
<?php

$binaryOperators = [
"==",
"!=",
"===",
"!==",
"<",
"<=",
">",
">=",
"<=>",
];
$inputs = [
0,
null,
false,
true,
"",
[],
NAN,
];

$nan = fdiv(0, 0);
var_dump($nan);
foreach ($inputs as $right) {
echo 'Using ';
var_export($right);
echo ' as right op', PHP_EOL;
var_dump($nan == $right);
var_dump($nan != $right);
var_dump($nan === $right);
var_dump($nan !== $right);
var_dump($nan < $right);
var_dump($nan <= $right);
var_dump($nan > $right);
var_dump($nan >= $right);
var_dump($nan <=> $right);
}

?>
--CLEAN--
<?php
$fl = __DIR__ . DIRECTORY_SEPARATOR . 'compare_binary_operands_nan_temp.php';
@unlink($fl);
?>
--EXPECT--
float(NAN)
Using 0 as right op
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)
bool(false)
bool(false)
bool(false)
int(1)
Using NULL as right op
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)
bool(false)
bool(false)
bool(false)
int(1)
Using false as right op
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)
bool(false)
bool(false)
bool(false)
int(1)
Using true as right op
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)
bool(false)
bool(false)
bool(false)
int(1)
Using '' as right op
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)
bool(false)
bool(false)
bool(false)
int(1)
Using array (
) as right op
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)
bool(false)
bool(false)
bool(false)
int(1)
Using NAN as right op
bool(false)
bool(true)
bool(false)
bool(true)
bool(false)
bool(false)
bool(false)
bool(false)
int(1)

Loading