phpdbg next command must step over function calls #6031
Conversation
|
This was broken in cf35dae. The fix is to try to better target the fix applied in that patch to only instances where we have left the current context and entered the previous context. It wasn't clear when exactly oplines should be printed. What I've done here is not print intermediate oplines while stepping over. |
ebernhardson
force-pushed
the
phpdbg-next-7.3
branch
from
ed7d788 to
f00a875
Compare
|
Thanks for the PR! Could you please open a new ticked on https://bugs.php.net for this? Mainly for reference; the entry in NEWS should be something like "Fixed bug #12345 (bug title). (developer)". |
ebernhardson
force-pushed
the
phpdbg-next-7.3
branch
from
f00a875 to
15bd0bd
Compare
|
Filed as 80006, updated NEWS |
|
Thanks, @ebernhardson! However, it is not recommendable to modify NEWS (or UPGRADING for that matter) directly in the pull request, because there quickly can be merge conflicts. Instead the person who merges the PR eventually, should add suitable entries to these files. Anyhow, @bwoebi, thoughts on this PR? |
sapi/phpdbg/phpdbg_prompt.c
Outdated
| /* perform seek operation */ | ||
| if ((PHPDBG_G(flags) & PHPDBG_SEEK_MASK) && !(PHPDBG_G(flags) & PHPDBG_IN_EVAL)) { | ||
| /* current address */ | ||
| zend_ulong address = (zend_ulong) execute_data->opline; | ||
|
|
||
| if (PHPDBG_G(seek_ex) != execute_data) { | ||
| if (PHPDBG_G(flags) & PHPDBG_IS_STEPPING) { | ||
| if (PHPDBG_G(seek_ex)->prev_execute_data == execute_data && PHPDBG_G(flags) & PHPDBG_IS_STEPPING) { |
There was a problem hiding this comment.
I think that needs to be phpdbg_user_execute_data(PHPDBG_G(seek_ex)->prev_execute_data)
There was a problem hiding this comment.
Any idea how to test this? I've tried swapping and this then fails the current test.and i'm not sure which new tests it would start passing.
There was a problem hiding this comment.
Yeah, this obviously fails when called on top of the stack (prev_execute_data is NULL then). My bad. Needs to be some check PHPDBG_G(seek_ex)->prev_execute_data && phpdbg_user_execute_data(PHPDBG_G(seek_ex)->prev_execute_data) == execute_data I suppose.
|
The intermediary opcodes handling doesn't seem correct though: see your changed test sapi/phpdbg/tests/next_001.phpt ; now you do not have an indication at all what opcode we're now at when hitting line 8. |
ebernhardson
force-pushed
the
phpdbg-next-7.3
branch
3 times, most recently
from
04811ed to
e4a46fe
Compare
|
Thanks for the review, updated patch should address all comments re: NEWS, phpdbg_user_execute_data, and intermediate opcode reporting |
|
I took a look over the CI failures but it's not clear they are related to the patch. I've tried to reproduce but not having any luck localy. Any suggestions? |
|
The CI failures are unrelated to this PR; they are caused by 2d4aa1e. |
|
@bwoebi or maybe @krakjoe, could you please review? @ebernhardson, the patch can no longer target PHP-7.3, since this is in security mode. Please rebase onto PHP-7.4 or maybe a later version. |
This was broken in cf35dae. The fix is to try to better target the fix applied in that patch to only instances where we have left the current context and entered the previous context.
ebernhardson
force-pushed
the
phpdbg-next-7.3
branch
from
e4a46fe to
4997492
Compare
|
Patch updated to base against PHP-7.4 branch. |
|
And now PHP-7.4 won't receive any regular bugfixes anymore. Could you please rebase again? And maybe write to the internals mailing list to get more attention? |
|
There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken. |
No description provided.