Skip to content

Conversation

mlauter
Copy link

@mlauter mlauter commented Mar 26, 2019

Fixes segfault when attempting to run after setting a conditional breakpoint
without specifying the file path.

Additionally, adds functionality so that NUMERIC_PARAM breakpoints
are supported in addition to FILE type. Emits an error if
other conditional breakpoint formats are used as they are currently
unsupported.

Fixes segfault when attempting to run after setting a conditional breakpoint
without specifying the file path.

Additionally, adds functionality so that NUMERIC_PARAM breakpoints
are supported in addition to FILE type. Emits an error if
other conditional breakpoint formats are used as they are currently
unsupported.
@petk petk added the Bug label Mar 26, 2019
@krakjoe
Copy link
Member

krakjoe commented Mar 26, 2019

Fixed in 7df8e4f

I didn't add support for numeric param, it would seem ambiguous ... it's clear in a single script test file which line you want to break on, but in a realistic application it's not clear at all which file you want to break on.

@krakjoe krakjoe closed this Mar 26, 2019
@mlauter
Copy link
Author

mlauter commented Mar 26, 2019

That's fair, though it seems equally ambiguous for conditional and non-conditional bps. But I agree it doesn't seem very realistic that someone would want to do it.

That said I don't think 7df8e4f fully fixes supporting the different bp types for conditional breakpoints since PHPDBG_BREAK(at) doesn't do the work the various phpdbg_set_breakpoint* methods do. (And obviously neither did my patch, I can submit a separate bug report)

@petk
Copy link
Member

petk commented Mar 26, 2019

In such case if it is not fully fixed yet, we can also reopen the bug report...

@mlauter
Copy link
Author

mlauter commented Mar 26, 2019

@krakjoe what would you think about an approach such that conditional breakpoints shared more of the code path with regular breakpoints so that the handling would not have to be duplicated? (e.g. stuff in https://github.com/php/php-src/blob/master/sapi/phpdbg/phpdbg_bp.c#L419)

@krakjoe
Copy link
Member

krakjoe commented Mar 26, 2019

I've addressed that in 6d3a2b4

Oh I think I see the confusion ... conditional breakpoints are by nature a slow path, that's why, rather than using the same break tables as non-conditional breaks, they have a separate table ... the support for different types of conditional breakpoints is here, called in this path ...

They were lost on subsequent runs, which is what the commit above fixes ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants