feat(protocol): complete the core DBGp command set - #19
Merged
Conversation
Closes the five commands the spec requires and zdebug still answered error 4 for, adds the two Xdebug breakpoint types it was missing, and gives object properties the visibility an IDE draws them with. stack_depth and typemap_get are the small ones: a frame count, and a table whose `type` column is the exact spelling PropertySerializer puts on its <property> elements, so what the map promises and what the properties carry cannot drift. source hands the IDE the code the debuggee is running - remote debugging, a container path, sources never checked out locally. It reads through the SAME FileFilter the instrumentation uses, which is the whole security story: a DBGp connection is a file-read primitive pointed at whatever the debuggee can open, so it is scoped to the code configured as debuggable rather than to the disk. breakpoint_update changes a breakpoint in place, keeping its id and its accumulated hit count. The line goes through the registry rather than the field: it is a key of the index the statement hook reads, and assigning it alone would leave the breakpoint firing on its old line forever. property_set is the first write path into the engine. It resolves the base variable to its live CV slot and pushes the rebuilt root back through setNativeValue() - rebuilding, because PHP arrays are value types and the copy a read handed over is not what the debuggee will look at. Every step is a refusal by default: the slot must exist, intermediate steps must already exist (error 300, never an invented array key), and a write the engine rejects - a readonly property, a typed property the value does not fit - is success="0" rather than a broken debuggee. The integration test asserts the DEBUGGEE's own output after it resumes, because a success="1" for a write into a materialized copy would look exactly the same on the wire. Call and return breakpoints ride different machinery. A return breakpoint is a handler on the RETURN opline, where the leaving frame is still current and its locals still readable. A call breakpoint has no function-entry event to hook, so it uses the one statement guaranteed to run once per call: the op_array's first EXT_STMT. That scan is memoized per op_array and only ever reached when a call breakpoint exists, so nothing else pays for it. facet comes free from the mangled property-table key ObjectProperties already demangles - it was computing the visibility and discarding it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCNB8eDgS6NEnfCFw7tPjU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the five commands the spec requires and zdebug still answered error 4 for, adds the two Xdebug breakpoint types it was missing, and gives object properties the visibility an IDE draws them with.
Commands
stack_depthstack_getwill report; zero rather than an error while nothing is suspended.typemap_gettypecolumn is the exact spellingPropertySerializerputs on its<property>elements, so what the map promises and what the properties carry cannot drift. Declares thexsi/xsdnamespaces its<map>elements need.source-fdefaults to the frame's own file,-b/-eselect an inclusive range.breakpoint_update-s/-n/-h/-oin place, keeping the id and the accumulated hit count.property_setsourceis a file-read primitive on an open socketIt reads through the same
FileFilterthe instrumentation uses, so it is scoped to the code configured as debuggable rather than to whatever the debuggee process can open. An integration test asserts that a file outsideZDEBUG_PATH_FILTERcomes back as error 100. With no filter configured it observes everything — the same trade the instrumentation already makes, now made once and visibly.breakpoint_updatemoves the breakpoint, not just the fieldlineis a key of the index the statement hook reads on its hot path. Assigning it alone would leave the breakpoint firing on its old line forever, so-ngoes throughBreakpointRegistry::relocate().property_set— the first write path into the engineIt resolves the base variable to its live CV slot (
ExecutionData::getLocalVariable()) and pushes the rebuilt root back throughsetNativeValue(). Rebuilding rather than mutating: PHP arrays are value types, so the array a read handed over is a copy, and writing into it would change nothing the debuggee can observe. Objects are handles, so aReflectionPropertywrite lands in the real object and assigning the root back is a harmless no-op that keeps one code path for both.Every step is a refusal by default:
$thisis deliberately not writable;success="0", not a broken debuggee;-tnames the incoming type, and without it the type already at that path is kept, so editing an int in the variables panel does not silently store"42".A declared-but-unset local is exempt from the "must exist" rule on purpose: it has a slot to write and no value to resolve, and giving it one is a legitimate edit.
The test asserts the debuggee's own output after it resumes —
MUTATED=42|after|true|2.5|99|77|5|PV|locked|given— because asuccess="1"for a write into a materialized copy would look exactly the same on the wire.Call / return breakpoints
They ride different machinery:
-t returnis a handler on theRETURNopline, where the leaving frame is still current and its locals still readable. Every function body ends in one, including a body that falls off the end.RETURN_BY_REFandGENERATOR_RETURNare not this opcode and are documented as out of reach.-t callhas no function-entry event to hook, so it uses the one statement guaranteed to run once per call: the op_array's firstEXT_STMT. Control always enters a frame at the top of its op_array, so that statement is exact. Consecutive statements on the same line are deduped; the residual gap (a one-statement function called twice inside one expression,f(f()), reports one entry) is documented in the code.The opcode scan is memoized per op_array and only reached when a call breakpoint exists, so a debuggee without one pays nothing.
-macceptsfn,Class::fnorClass->fn; the class part is checked against the bound object when there is one, and ignored for a static method rather than never matching — a breakpoint that silently never fires is worse to debug than one that fires once too often.facetObjectPropertieswas already demangling the property-table key to recover the declaring class and throwing the visibility away. It now travels with the value, so<property facet="private">is what the IDE draws its padlock from. Array elements and frame locals carry no facet — they are not declared under a visibility.Tests
PropertyMutationSessionTest— writes to scalars, nested array elements, private/protected/public object properties, a private array one step deeper, a null local; a refused readonly write; four unwritable paths as error 300; and the debuggee's own output as proof.FunctionBreakpointSessionTest— call fires once per call with the body not yet run, return stops with the frame still readable, both on a single-statement function, a hit condition skipping the first call, and-m-less rejection.ProtocolCommandsSessionTest—stack_depthagreeing withstack_get, the typemap,sourceinside and outside the path filter,breakpoint_updatedisabling without losing the id or hit count, and the facets of$this.SourceReaderTest— inclusive one-based ranges, empty-versus-null, and the filter refusal.CommandDispatcherTestfor every new command's error paths, and the line-index move.All four gates pass locally:
phpunit(281 tests),phpunit --group integration --fail-on-skipped(37 tests),phpstan analyseat level max,php-cs-fixer --dry-run.Still not implemented
break(async pause) was postponed by request.stdout/stderrremain acknowledged-as-unsupported, andwatchbreakpoints,spawnpoint_*,interact,exec/exprandstdinare unimplemented — Xdebug declines most of those too.feature_getcontinues to report every one of them as unsupported.Generated by Claude Code