Bug #76029 [Fbk]: Serious regression in foreach() looping
| From: | nikic@php.net | Date: | Wed, 28 Feb 2018 22:01:26 +0000 |
| Subject: | Bug #76029 [Fbk]: Serious regression in foreach() looping | ||
| References: | 1 | Groups: | php.bugs |
| Request: | Send a blank email to php-bugs+get-214153@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=76029&edit=1
ID: 76029
Updated by: nikic@php.net
Reported by: mark dot scherer at spryker dot com
Summary: Serious regression in foreach() looping
Status: Feedback
Type: Bug
Package: *General Issues
Operating System: Linux
PHP Version: 7.2.2
Block user comment: N
Private report: N
New Comment:
Looking at the code, my best guess is that we're contracting the assignment for
$shouldBeTextArea into IS_SMALLER and then something eats the NOP, resulting in a smart branch. But
I can't reproduce this. Would be nice to have a self-contained reproducing script for this.
Instead of disabling opcache entirely, it should be enough to set opcache.optimization_level=0 until
this is fixed.
Previous Comments:
------------------------------------------------------------------------
[2018-02-28 17:19:56] mark dot scherer at gmx dot de
OK, so disabled opcache and all is fine, so this at least limits itself to the opcode cache. We will
keep it disabled for now - but this will sure kill a lot of php applications once deployed.
------------------------------------------------------------------------
[2018-02-28 17:14:44] mark dot scherer at gmx dot de
opcode is on, I will disable and try again
$value = ...;
was a missing line I removed before the "$shouldBeTextArea = mb_strlen($value) > 255;",
but it does not change the result/report.
------------------------------------------------------------------------
[2018-02-28 16:54:34] peehaa@php.net
Could you also test it with opcache enabled and disabled please and post the results?
------------------------------------------------------------------------
[2018-02-28 16:50:52] peehaa@php.net
No repro: https://3v4l.org/fuGjJ
Please provide a simplified and working repro case.
------------------------------------------------------------------------
[2018-02-28 16:37:04] mark dot scherer at spryker dot com
Description:
------------
The recent changes in 7.2 must have introduced a major regression in foreach() looping and variable
assignment.
Test script:
---------------
// BROKEN NOW IN PHP7.2
foreach ($productAttributeKeys as $type) {
$isDefined = $this->attributeTransferCollection->has($type);
$shouldBeTextArea = mb_strlen($value) > 255;
if ($isDefined) {
continue;
}
if ($shouldBeTextArea) {
$inputType = self::TEXT_AREA_INPUT_TYPE;
}
...
}
// FIXED WITH: Moving continue statement up
foreach ($productAttributeKeys as $type) {
$isDefined = $this->attributeTransferCollection->has($type);
if ($isDefined) {
continue;
}
$shouldBeTextArea = (mb_strlen($value) > 255);
if ($shouldBeTextArea === true) {
$inputType = self::TEXT_AREA_INPUT_TYPE;
}
...
}
Expected result:
----------------
No notice/error on the most basic
$shouldBeTextArea = mb_strlen($value) > 255;
if ($isDefined) {
continue;
}
if ($shouldBeTextArea) {
$inputType = self::TEXT_AREA_INPUT_TYPE;
}
Actual result:
--------------
When using continue, variables that must be assigned and fine are suddenly now throwing
"Undefined variable: shouldBeTextArea" - this worked in all PHP versions until 7.1 incl.
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=76029&edit=1