Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issue #1665: Segfault when overriding a function object parameter #473

Closed
wants to merge 31 commits into from

Conversation

Projects
None yet
2 participants
@SpyroTEQ
Copy link

commented May 15, 2019

XDebug might need to dump stack traces and such,
so we must hold a reference to the PHP variables
Otherwise, PHP might garbage collect these variables,
and XDebug will segfault when trying to dump them

Note that I've based this on the xdebug_2_7 branch, which conflicts with master, so I'm opening pull request on the xdebug_2_7 branch too. The "contributing" page is not clear about that IMO. if needed, I can merge with master (merge is fairly easy, the only conflict is about 2 clocks of added lines so just add both)

Note again: I've merged with the master because I'm used to do so and I didn't think twice, but I'm not 100% it was a right thing to do?! Tell me if you want me to "undo" the merge and I'll refork the 2.7 branch, recommit, and not merge with master

derickr and others added some commits Mar 19, 2019

Fixed issue #1388: Support 'resolved' flag for breakpoints
Refactor constant names for user-defined (userland) and built-in PHP functions

Added plumbing for resolved_breakpoints functionality

Refactor remote log feature into a helper function

Prefix BREAKPOINT_TYPE_* constants with XDEBUG_

Added 'breakpoint_resolved' notifications for lines that can be broken on

Scan lines before and after 'line' breakpoint location to try to resolve lineno

Remove check for '{main}' as it's irrelevant, and add tests

Added 'breakpoint_resolved' for main and included files

Don't use temporary 'TMP_RESOLVED', as we (annoyingly) always need to recheck

Refactor breakpoint types to not sure string comparisons

Swap order of pre-requisites in order to have the quick checks first

Change test suite to do better filename sanitation

Use spans to prevent loads of notifications and 'flipping'

Move logging to debug handlers, and rephrase log messages

Implement resolving and correct evaluation of breakpoints in eval statements

Refactor helper to a 'switch' to be able to resolve other types

Reset resolved values if they exactly match the original breakpoint info after a previous flip

Add support for resolving call/return breakpoints

Turn breakpoint type constants into bit values, so it can be used as a bitset

Add missing 'exception' attribute to breakpoint_list output

Refactor brk_info struct to contain the breakpoint ID as 'id' member

This is so that we don't always need an xdebug_brk_admin struct in many
operations.

Add support for resolving exception breakpoints

Fixed issue #1388: Restrict search to maximum 10 lines from requested breakpoint

Fix tests due to change in testing framework
Fixed issue #1665: Segfault when overriding a function object paramet…
…er + collect_params > 0

XDebug might need to dump stack traces and such,
so we must hold a reference to the PHP variables
Otherwise, PHP might garbage collect these variables,
and XDebug will segfault when trying to dump them
Merge branch 'master' of https://github.com/xdebug/xdebug into issue1…
…665-segfault

# Conflicts:
#	xdebug_stack.c
Fixed issue #1534: Segfault when exception thrown in a closure bound …
…to class scope

It's actually the same root cause than #1665 so fixing #1665 fixes this #1534 issue too
@derickr

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

This PR looks like it has the master branch merged in. Can you perhaps make a clean feature branch of xdebug_2_7 with only your own commits cherry picked in it?

@derickr

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

Sorry, I just saw that you actually asked about that. So yes, please refork and re-apply your commits. It's not a usual case, and GitHub is a bit silly here, which is why I hadn't put it in the contributing guidelines.

Also, I'd like to say thanks for looking into this. It's been on my list for a while, and I'm looking forwards to the clean PR so that I can review it.

cheers,
Derick

@SpyroTEQ

This comment has been minimized.

Copy link
Author

commented May 16, 2019

Alright, I'm gonna do so then. You're welcome tho, I feel proud and happy to have contributed for an open source PHP related project for the first time (and for a pretty challenging bug tho!) :)

@SpyroTEQ

This comment has been minimized.

Copy link
Author

commented May 16, 2019

PR reopened here #474 I'll close this one

@SpyroTEQ SpyroTEQ closed this May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.