inc/dec with undefined vars #11850
inc/dec with undefined vars #11850
Conversation
| $x++; | ||
| } catch (\Exception $e) { | ||
| echo $e->getMessage(), PHP_EOL; | ||
| if (!isset($x)) { echo("UNDEF\n"); } else { var_dump($x); } |
There was a problem hiding this comment.
I just realized that isset() doesn't make distinct between UNDEF and NULL, so the test prints UNDEF insted of NULL. It's possible to fix this using the following terrible code.
if (!isset($x) && !@is_null($x)) { echo("UNDEF\n"); } else { var_dump($x); }There was a problem hiding this comment.
@dstogov I don't think that works, as $x is coerced to NULL, no? It could be possible to just access $x and catch the warning if promoted to an exception with an error handler.
Edit: Ah, we already have the error handler...
Zend/zend_vm_def.h
Outdated
| @@ -1487,6 +1487,13 @@ ZEND_VM_HELPER(zend_pre_inc_helper, VAR|CV, ANY) | |||
| SAVE_OPLINE(); | |||
| if (OP1_TYPE == IS_CV && UNEXPECTED(Z_TYPE_P(var_ptr) == IS_UNDEF)) { | |||
| ZVAL_UNDEFINED_OP1(); | |||
| if (UNEXPECTED(EG(exception))) { | |||
| /* Smart branch expects result to be set with exceptions */ | |||
There was a problem hiding this comment.
I didn't understand this comment.
PRE/POST_INC/DEC are never combined with the following JMPZ/NZ into smart branches.
There was a problem hiding this comment.
It's actually about this line:
Line 8099 in 66b359e
The problem is not related to smart branches. Instead, it's every op but smart branches, that are expected to initialize the result (apart from some others that don't store zvals). If the result is not initialized, we'll free uninitialized value. With opcache, we can get a use-after-free if the zval still contains some old value due to shared VAR slots.
9a53794 to
3940fca
Compare
| ?> | ||
| --EXPECT-- | ||
| Undefined variable $x | ||
| NULL |
There was a problem hiding this comment.
Should it actually be NULL or should $x remain undefined?
There was a problem hiding this comment.
Should it actually be NULL or should
$xremain undefined?
If you didn't define the new behaviour in your RFC it should be the same as in PHP-8.2.
3940fca to
0ac447d
Compare
I split the first commit from #11759 into its own PR as it is not an OSS-fuzz fix, and like said it is a pre-existing issue so maybe should be backported.
The rationale is that, considering undefined variables will become Errors at one point, the behaviour of this and what happens now when the warning is converted to an exception should be, IMHO, identical.
However, it seems that a lot of the VM handlers that use
ZVAL_UNDEFINED_OP1()do not explicitly check if an exception is thrown from an undefined operand.