Fix circumvented type check with return by ref + finally #19172
Conversation
| @@ -5703,6 +5703,15 @@ static void zend_compile_return(zend_ast *ast) /* {{{ */ | |||
|
|
|||
| zend_handle_loops_and_finally((expr_node.op_type & (IS_TMP_VAR | IS_VAR)) ? &expr_node : NULL); | |||
|
|
|||
| /* Content of reference might have changed in finally, repeat type check. */ | |||
| if (by_ref | |||
| && !zend_stack_is_empty(&CG(loop_var_stack)) | |||
There was a problem hiding this comment.
Why is this check needed? The by_ref makes sense because that is how the type of the returned variable changes, and the is_generator and ZEND_ACC_HAS_RETURN_TYPE make sense because those are the conditions used above, but this condition (empty loop variable stack) isn't checked above - maybe add a comment about why it is needed here but not previously?
There was a problem hiding this comment.
It's just a fast way to check whether we're in a try. We could use zend_has_finally(), but technically even loop/switch var FREEs could change the content of the reference. It's very unlikely to matter in practice, but given return-by-ref with finally/loops should be rare enough I didn't bother optimizing this more.
|
Should this be targeting 8.3 rather than master? |
Fixes GH-18736