On April 15, 2017 5:24:53 PM GMT+01:00, Nikita Popov nikita.ppv@gmail.com
wrote:

Hi Nikita,

Through Xdebug bug #1413 (https://bugs.xdebug.org/view.php?id=1413),
I
noticed
that behaviour with class constants changed between PHP 7.1.3 and PHP
7.1.4.
I believe it is because of the fix for issue #69676
(https://bugs.php.net/bug.php?id=69676) that got fixed in PHP 7.1.4.

With OPcache loaded, the INIT_ARRAY and further opcodes are relisted
as
being
on line 15 instead of line 17. Although, I would probably argue that
INIT_ARRAY should really be on line 13 (the opening [).

Without OPcache loaded, in PHP 7.1.3, FETCH_CLASS_CONSTANT is on line
15,
and
the INIT_ARRAY and further opcodes are on line 17. In PHP 7.1.4, they
are
all
on line 15 - with 17 no longer showing up.

I believe, the behaviour with and without OPcache should be the same,
and I
would argue that the correct result would need to be that
FETCH_CLASS_CONSTANT
(if still present, OPcache optimises it out) should be on line 15,
the
INIT_ARRAY on line 13, and the SEND_VAL_EX and further, are on line

Code

1 <?php
2
3 namespace ICanBoogie\Validate\Validator;
4
5 class Required2
6 {
7 const ALIAS = 'required';
8 const DEFAULT_MESSAGE = "is required";
9 const OPTION_STOP_ON_ERROR = 'stop_on_error';
10
11 public function normalize_params(array $params)
12 {
13 return array_merge([
14
15 self::OPTION_STOP_ON_ERROR => true
16
17 ], $params);
18 }
19 }

PHP 7.1.3 (no OPcache)

filename: /tmp/xdebug-bug-1/lib/Required2.php
function name: normalize_params
number of ops: 14
compiled vars: !0 = $params
line #* E I O op fetch ext
return
operands

---

11 0 E > EXT_NOP
1 RECV !0
13 2 EXT_STMT
3 INIT_NS_FCALL_BY_NAME
4 EXT_FCALL_BEGIN
15 5 FETCH_CLASS_CONSTANT ~1
'OPTION_STOP_ON_ERROR'
17 6 INIT_ARRAY ~2
<true>, ~1
7 SEND_VAL_EX
~2
8 SEND_VAR_EX
!0
9 DO_FCALL_BY_NAME
10 EXT_FCALL_END
11 > RETURN
$3
18 12* EXT_STMT
13* > RETURN
null

PHP 7.1.3 (OPcache)

filename: /tmp/xdebug-bug-1/lib/Required2.php
function name: normalize_params
number of ops: 11
compiled vars: !0 = $params
line #* E I O op fetch ext
return
operands

---

11 0 E > EXT_NOP
1 RECV !0
13 2 EXT_STMT
3 INIT_NS_FCALL_BY_NAME
4 EXT_FCALL_BEGIN
17 5 INIT_ARRAY ~1
<true>, 'stop_on_error'
6 SEND_VAL_EX
~1
7 SEND_VAR_EX
!0
8 DO_FCALL 0 $1
9 EXT_FCALL_END
10 > RETURN
$1

PHP 7.1.4 (no OPcache)

filename: /tmp/xdebug-bug-1/lib/Required2.php
function name: normalize_params
number of ops: 14
compiled vars: !0 = $params
line #* E I O op fetch ext
return
operands

---

11 0 E > EXT_NOP
1 RECV !0
13 2 EXT_STMT
3 INIT_NS_FCALL_BY_NAME
4 EXT_FCALL_BEGIN
15 5 FETCH_CLASS_CONSTANT ~1
'OPTION_STOP_ON_ERROR'
6 INIT_ARRAY ~2
<true>, ~1
7 SEND_VAL_EX
~2
8 SEND_VAR_EX
!0
9 DO_FCALL_BY_NAME
10 EXT_FCALL_END
11 > RETURN
$3
18 12* EXT_STMT
13* > RETURN
null

PHP 7.1.4 (OPcache)

filename: /tmp/xdebug-bug-1/lib/Required2.php
function name: normalize_params
number of ops: 11
compiled vars: !0 = $params
line #* E I O op fetch ext
return
operands

---

11 0 E > EXT_NOP
1 RECV !0
13 2 EXT_STMT
3 INIT_NS_FCALL_BY_NAME
4 EXT_FCALL_BEGIN
15 5 INIT_ARRAY ~1
<true>, 'stop_on_error'
6 SEND_VAL_EX
~1
7 SEND_VAR_EX
!0
8 DO_FCALL 0 $1
9 EXT_FCALL_END
10 > RETURN
$1

cheers,
Derick

This is due to
https://github.com/php/php-src/commit/183cd048f18fa4b04fb30448a84a54
cee80a2491,
which fixed lineno assignment for list AST nodes. This changed the
start
lineno of the array from line 17 (the end of the array) to line 15 (the
first array element). Of course this is still inaccurate (the actual
start
line is 13), but it's the closest we can get with the information we
currently have available.

The problem in this case is that the second SEND_VAR_EX did not bump
the
lineno. I've fixed this in
https://github.com/php/php-src/commit/e433c23b96e81340cd0e2d0b4b7f5f
ce7f72a931
.

This required a change to an existing test, which alerted me to some
weird
behavior I wasn't aware of: https://3v4l.org/6fSRb In backtraces the
call
is located on the line of it's last argument (in PHP 5.6 the line of
the
closing ")"). Wouldn't it make more sense to use the starting line of
the
call? That is, use the same lineno for INIT_FCALL and DO_FCALL?

Nikita

It makes sense to me to use the first line, as long as further opcodes
will have the right line number.

I agree the first line makes sense.

Selfishly, I'd prefer if that change was only made for master, due to the
increasingly complex tests I need to make for Xdebug's code coverage.

That means it will go into 7.2 and not into any 7.1 fix, correct?

Hi!

behavior I wasn't aware of: https://3v4l.org/6fSRb In backtraces the call
is located on the line of it's last argument (in PHP 5.6 the line of the
closing ")"). Wouldn't it make more sense to use the starting line of the
call? That is, use the same lineno for INIT_FCALL and DO_FCALL?

Since the call happens after evaluating the arguments, if you debug in
IDE and go through the arguments, it'll make IDE prompt jump back
sometimes.

Debuggers generally, and Xdebug specifically, only allow interruptions
on function entry or on an EXT_STMT call, so that shouldn't be a problem
here.

OTOH, this is what gdb most definitely does for optimised C code. It
jumps back and forwards through code if the compiler has reorganised
statements.

Not sure it's a good. Logically, it looks like first argument eval
happens, then call happens, so linking it to ")" makes sense (if you
read it left to right, that's kind of the point where the actual call
happens).

I do agree with that though.

cheers,
Derick

--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Like Xdebug? Consider a donation: https://xdebug.org/donate.php
twitter: @derickr and @xdebug