Bug #76466 [Ana]: Loop variable confusion

Edit report at https://bugs.php.net/bug.php?id=76466&edit=1 ID: 76466 Updated by: dmitry@php.net Reported by: cmb@php.net Summary: Loop variable confusion Status: Analyzed Type: Bug Package: opcache Operating System: Linux PHP Version: 7.3.0alpha1 Assigned To: laruence Block user comment: N Private report: N New Comment: @laruence @nikic I propose another approach - make BOT all the chain of ADD_ARRAY_ELEMENT opcodes, when some variable changes. Please review https://gist.github.com/dstogov/142223b0c42c49a523f72a814db8b4f0 Previous Comments: ------------------------------------------------------------------------ [2018-06-14 14:47:08] laruence@php.net @nikic probably you saw the old patch？ the result parts are already changed :) about the array size limit, yeah, it's good idea... as after this change the array numbers will increased visibly. I've asked dmitry to also have a look.. let's see what's his opinion . ------------------------------------------------------------------------ [2018-06-14 09:46:55] nikic@php.net @laruence: Yes, this is clearly wrong. Without the partial array propagation this would still be fine if we made sure to propagate a BOT op1/op2 before the check (which we didn't do either...), as we know that these are the only possible lowerings and both result in a BOT. However, with partial arrays this is no longer possible, so I'd say we should remove this (you'll also have to modify the if (result) code below) and instead add a check for the array size and return BOT if the array is larger than say 16 elements, to avoid quadratic blowup. ------------------------------------------------------------------------ [2018-06-14 09:26:54] laruence@php.net the problem is that we may visit one instruction multiply times, depends on new infos comes... let me try to explain: let say we have two ops #1 INIT_ARRAY #2 ADD_ARRAY_ELEMENT when we first visit INIT_ARRAY, we have op1 is an const value, then we set INIT_ARRAY’s result as a const array. then we visist #2, we get the result value from #1, and set the #1 result to NULL, then make #2 result a const array. but later, INIT_ARRAY op1’s value become BOT… and #1 is added into work instr again… but this time, we visist #1, we get INIT_ARRAY->Result.def is IS_NULL, then we simply return, then we leave #2’s result as an incorrect value type…. (it should be BOT, but it’s array) fix could be something like https://gist.github.com/laruence/4068c564e38e8e0f84148cc1b3f977cf ------------------------------------------------------------------------ [2018-06-13 13:17:41] cmb@php.net Looks like the SCCP is too aggressive as of commit ce3dbb5[1]. For what it's worth, removing the if statement from the test script gives expected results. [1] <https://github.com/php/php-src/commit/ce3dbb53a7dcef314bba1d355f049ddb2252fad1> ------------------------------------------------------------------------ [2018-06-13 10:38:20] cmb@php.net Related To: Bug #76446 ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=76466 -- Edit this bug report at https://bugs.php.net/bug.php?id=76466&edit=1