Proposal: Support memory reference breakpoints

[auott]

Memory reference breakpoints allow setting a breakpoint based off of memory reference previously provided by an adapter.

export interface Capabilities {
    // ...

    /** The debug adapter supports setting breakpoints based on instruction references. */
    supportsInstructionBreakpoints?: boolean;
}

/** 
 * Replaces all existing instruction breakpoints. Typically, instruction breakpoints would be set from a disassembly window. 
 * To clear all instruction breakpoints, specify an empty array.
 * When a memory reference breakpoint is hit, a 'stopped' event (with reason 'instruction breakpoint') is generated.
 * Clients should only call this request if the capability 'supportsInstructionBreakpoints' is true.
 */
export interface SetInstructionBreakpointsRequest extends Request {
    // command: 'setInstructionBreakpoints'
    arguments: SetInstructionBreakpointsArguments;
}

/** Arguments for 'setInstructionBreakpoints' request */
export interface SetInstructionBreakpointsArguments {
    /** The instruction references of the breakpoints */
    breakpoints: InstructionBreakpoint[];
}

/** Response to 'setInstructionBreakpoints' request */
export interface SetInstructionBreakpointsResponse extends Response {
    body: {
        /**
         * Information about the breakpoints. The array elements correspond to the elements of the 'breakpoints' array.
         */
        breakpoints: Breakpoint[];
    }
}

/** Properties of a breakpoint passed to the setInstructionBreakpoints request */
export interface InstructionBreakpoint {
    /** 
    * The instruction reference of the breakpoint.
    * This should be a memory or instruction pointer reference from an EvaluateResponse, Variable, 
    * StackFrame, GotoTarget, or Breakpoint.
    */
    instructionReference: string;

    /** 
     * An optional offset from the instruction reference.
     * This can be negative.
     */
    offset?: int;

    /**
     * An optional expression for conditional breakpoints.
     * It is only honored by a debug adapter if the capability 'supportsConditionalBreakpoints' is true.
     */
    condition?: string;

    /**
     * An optional expression that controls how many hits of the breakpoint are ignored.
     * The backend is expected to interpret the expression as needed.
     * The attribute is only honored by a debug adapter if the capability 
     * 'supportsHitConditionalBreakpoints' is true.
     */
    hitCondition?: string;
}

export interface Breakpoint {
    // ...

    /** 
     * An optional memory reference where the breakpoint is located.
     */
    instructionReference?: string;

    /**
     * An optional offset from the instruction reference.
     */
     offset?: int;
}

CC: @andrewcrawley, @weinand

[int19h]

For some high-level languages, data breakpoints can be done, but they cannot operate on memory addresses - only on variables and children. It would be nice to have that accommodated.

[auott]

@int19h These memory reference breakpoints are not intended as Data Breakpoints, but execution breakpoints (i.e. setting a breakpoint on a specific instruction). I can update the description to indicate that this is for when code corresponding to the memory location is executed. Or would something else make this clearer?

I believe that databreakpoints in the protocol already support being based off of variables? So supporting memory reference based databreakpoints would be a separate proposal to allow adding a databp based directly on a memory reference/user supplied address/

[int19h]

My apologies - I misunderstood the meaning of "memory reference" here. I think confusion stemmed from me mentally conflating it with "variable reference".

And yes, you're right, data breakpoints are already in the spec.

[weinand]

@auott thanks for the proposals.

General feedback: please never combine multiple requests into one. Discussions jumping between the different features are hard to follow and it is impossible to drive the requests through the different phases in a single issue (and state).

I suggest that you move the "Stepping Granularities" feature into a separate issue (because the existing discussion here targets "Memory Reference Breakpoints"). I will move my comments/questions to the new issue.

[auott]

@weinand Thanks for the feedback - I've created the second proposal here: Issue 110

I'll also prepare some responses to your questions so that we can continue the discussion on that issue.

[weinand]

The proposal looks good to me, however it seems to be very specific to s small number of debuggers. So I do not see big pressure to rush this into DAP.

Has the proposal already proved its soundness in a real implementation? How mature is it?

Some comments/questions:

• is my assumption correct that the only source of memory references used in MemoryReferenceBreakpoints are the requests listed under "Memory references" in this comment?
• a new reason memory reference breakpoint needs to be added to the reason value set of the stopped event.
• "memory reference" is a bit misleading because it alludes to "data references" and "data breakpoints" (as already indicated by a comment above). Could we find a better name that makes more clear that "memory reference breakpoints" are still about execution, and not "data access"?
• more to come...

[auott]

Has the proposal already proved its soundness in a real implementation? How mature is it?

Yep! I have a working end to end implementation in VS.

• is my assumption correct that the only source of memory references used in MemoryReferenceBreakpoints are the requests listed under "Memory references" in this comment?

Yes, the memory references used here are intended to be ones that were previously supplied by the DA.

• a new reason memory reference breakpoint needs to be added to the reason value set of the stopped event.

Is there anything else I should add directly into the proposal around this?

• "memory reference" is a bit misleading because it alludes to "data references" and "data breakpoints" (as already indicated by a comment above). Could we find a better name that makes more clear that "memory reference breakpoints" are still about execution, and not "data access"?

How about InstructionBreakpoint? For an adapter to support this, I think it is fair to say that they would have to have some concept of an 'instruction' that would exist at the given memory reference.

[int19h]

The current DAP spec is a bit confusing on what memory references are for, in general. For variables, it says:

Optional memory reference for the variable if the variable represents executable code, such as a function pointer.

This implies that it's strictly for referencing code. But then for "evaluate":

Optional memory reference to a location appropriate for this result. For pointer type eval results, this is generally a reference to the memory address contained in the pointer.

So no longer just function pointers, but any pointers. And "readMemory" doesn't say anything about what the bytes might be at all.

[auott]

@int19h To be clear - I don't mean that requesting a BP based on any memory reference provided by the DA should succeed, but you could always try to request it.
More specifically - assuming that this renamed to InstructionBreakpoint:
If the supplied memory reference doesn't correspond to something the DA recognizes as being a (potential) instruction, then I would expect the request to fail. And if that's the final name then I will add that as a comment around the InstructionBreakpointRequest.

[gregg-miskelly]

I wonder if it would be more clear what these are expected to be used for if we added text like "typically these breakpoints would be set from a disassembly window" to the description.

[weinand]

@auott thanks for your comments.

• Name "memory reference breakpoint": I like your proposed InstructionBreakpoint a lot. The question is: what does that name change mean for "Memory reference" and the memoryReference property. Should it then become instructionReference?
• The stoppedevent reason memory reference breakpoint should be added here. It will show up as a comment here or here.
• The description of the MemoryReferenceBreakpoint should explain where the "memory references" come from, e.g. the variables etc. requests.

[auott]

I think instructionReference makes sense to go with InstructionBreakpoint, though I've kept some 'memory reference' phrasing since it appears elsewhere in the protocol.

[weinand]

@auott since you are using the proposed feature already I assume that you have an updated JSON schema for it. Could you please submit a PR for that?

[auott]

PR submitted: #111