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

Add additional data fields for breakpoints #445

Closed
planger opened this issue Nov 17, 2023 · 12 comments
Closed

Add additional data fields for breakpoints #445

planger opened this issue Nov 17, 2023 · 12 comments
Assignees
Labels
under-discussion Issue is under discussion for relevance, priority, approach

Comments

@planger
Copy link

planger commented Nov 17, 2023

The Debug Adapter Protocol (DAP) defines a rigid payload schema for breakpoints, without offering a standardized mechanism for transferring custom breakpoint data. This hampers implementing important features, such as breakpoint types (e.g., hardware/software breakpoints), multicore and multi thread features. Previously, more narrow-focused proposals didn't gain enough traction, so a general approach to custom breakpoint data could serve everyone better.

microsoft/vscode#152428
microsoft/vscode#181794
microsoft/vscode#187854

Proposal

We propose to add a new property named customData into the payload of the set breakpoint request and response (SourceBreakpoint type and Breakpoint type). The customData property should be of type any to support transferring arbitrary information.

To enable a generic editing UI in the development tool, we propose to add a supportedBreakpointData capability flag in the initialize request (client to adapter). In the Capabilities type, the adapter can then define in its response the supported custom breakpoint data properties using a format similar to the configuration property schema in VS Code. Based on this schema, the development tool can show a configuration widget on the breakpoint that allows the user to specify the supported breakpoint settings.

Additionally, we propose to introduce an explicit breakpointType property on SourceBreakpoint type and Breakpoint type and let the debug adapter specify which types it supports in the Capabilities response. The property schema for breakpoint data mentioned above could then be grouped by breakpoint type, so the development tool could offer a UI for the user to switch between supported breakpoint types and show the correct configuration widget depending on the selected breakpoint type. This could further be harmonized with the UI for switching between, e.g. conditional breakpoints and source breakpoints.

Context

This proposal originated from an ongoing initiative to improve embedded development in modern IDEs, such as VS Code and Eclipse Theia, in the CDT Cloud community, in particular

We are looking forward to your feedback and would happily support the implementation of this proposal.

@puremourning
Copy link
Contributor

I strongly oppose. Same reasons as usual. This is just an out of band inner-platform-effect protocol shoehorned into a well defined one. The correct approach is to define the full set of supported things in protocol, IMO. If we need more types of breakpoint with more properties, let's specify them and add them with well defined semantics.

@connor4312
Copy link
Member

connor4312 commented Nov 17, 2023

@puremourning worded it accurately. Such a generic customData property is impossible for any DA or client to implement independently. DAP describes the UI interactions between a user and their debugger, and customData doesn't describe anything.

The interest of improving embedded development in DAP and DAP-supporting clients is very welcome! However, it is usually more useful to start discussions from the perspective of, "what user actions or capabilities do we need to support that DAP can't currently?" Then from there we can determine the right enhancements to make to the protocol.

Additionally, we propose to introduce an explicit breakpointType property

I am not opposed to a breakpointType property, but we need a better definition of what it is, why it is, and how it would work.

@planger
Copy link
Author

planger commented Nov 17, 2023

Thank you for your prompt and insightful feedback!

I understand your concerns about straying from a well-defined protocol. Our goal is certainly not to undermine the protocol's integrity or hinder the development of entirely generic debug clients. Therefore, I want to clarify our reasoning behind the proposal:

  1. Vendor-Specific Configuration: For instance with hardware breakpoints, there is often vendor-specific settings involved that vary across devices. Our proposal aims to empower the debug adapter, tailored to specific device families or vendors, to define configuration options at the debug session's start, e.g. with a JSON schema. In my view, this approach indeed allows clients to stay entirely generic while still handling these configurations gracefully, either through a schema-driven JSON editor or even a nice UI similar to VS Code's generic handling of configuration property schemas.
  2. Well-Defined Within Session: While the configuration options may be undefined in the protocol, the protocol would foresee that they become well-defined within the debug session. The debug adapter and client determine the support for such configurations and the schema to use through the Capabilities exchange.
  3. Comparable approaches: This concept somewhat mirrors the Launch Request arguments, which are adapter-specific and yet passed through the protocol in the StartDebugging Request. Additionally, it's akin to SetExpressionArguments.expression, where the syntax and semantics are undefined from a protocol standpoint.

Anyway, I certainly understand your concerns, but would be very interested to get your feedback and insights after my attempt to clarify our goals above.

An alternative, as you suggested, would be to define additional breakpoint types, which list a number of optional properties. Whether or not those properties are then applicable in a concrete debug session for a specific hardware would then have to be enumerated in the capabilities' flags. I'd expect this to work for a good part of the use cases, even though it'd probably limit the flexibility across different device families and vendors. I'm sure others linked in the original proposal can give a more accurate judgement on that.

Thanks again for your speedy feedback!

@mfussenegger
Copy link
Contributor

For instance with hardware breakpoints, there is often vendor-specific settings involved that vary across devices

Do you have some concrete examples?

I also don't really understand how this relates to microsoft/vscode#152428 or microsoft/vscode#181794 - these are afaik both things a client could already implement, if they wanted to, using the functionality part of the existing protocol specification.

@connor4312
Copy link
Member

connor4312 commented Nov 20, 2023

Thanks for the detailed feedback. We do have precedent for some level of adapter-defined extensibility in, for example, the filters on extension breakpoints, and having 'types' for ordinary breakpoints is a bit analogous to that.

I do not want an 'unbounded' configuration scheme. Aside from theological concerns, building a UI that can express any JSON schema (and isn't just a JSON editor) is a huge undertaking for clients. VS Code's configuration editor that you reference is years old and still only handles relatively basic cases of JSON schema.

I'd be helpful to gather more information about the scenarios you're after, and where for example an adapter-enumerated list of 'breakpoint types' would not cover. I understand that there are an unbounded number of vendors with different hardware and configurations, but I'm sure there are some common cases that can cover the majority of uses :)

@planger
Copy link
Author

planger commented Nov 20, 2023

@mfussenegger Thanks for you feedback!

I also don't really understand how this relates to microsoft/vscode#152428 or microsoft/vscode#181794 - these are afaik both things a client could already implement, if they wanted to, using the functionality part of the existing protocol specification.

I mentioned those issues, as they would benefit from allowing users to specify additional data for breakpoints (e.g. dependent breakpoints, associated cores).

@planger
Copy link
Author

planger commented Nov 20, 2023

@connor4312 Thank you very much for your constructive feedback!

It certainly is a good idea to layout concrete examples and provide starting from the user perspective and work our way down to the protocol. I'll make sure to collect a few representative use cases and get back to you in the next few days.

Thanks again!

@markgoodchild
Copy link

markgoodchild commented Nov 27, 2023

Hi all,

As part of the CDT cloud - hardware breakpoint discussion we created a few slides to show the kind of control that is needed and why. Please see the attached slides from a Renesas point of view, if you have some questions, please let me know.

Best regards,
Mark.

renesashardwarebreakpointsv2.pptx

@connor4312
Copy link
Member

Thanks, Mark. Here are the things I see from the slide that are currently not possible in DAP:

  • Setting the hardware vs software data breakpoint type
  • The warning when the breakpoint limit is reached, but a DA can return an unverified breakpoint with a message when the limit is exceeded, so I think this scenario is okay.
  • The ability to set a data breakpoint at an address range. A DA could bolt this in with a special syntax of DataBreakpointInfoArguments.name expressions, but this would be adapter-specific and is not ideal.
  • The "Size" and "Bus Master" on the "data access settings" tab. The other properties can be expressed well in breakpoint condition and hitCondition.

@connor4312
Copy link
Member

I've opened a pair of proposals (linked above) to start tackling these requests. Trying to avoid having 'unbounded' behavior, these both add additional well-defined capabilities and type to the protocol. Please weigh in!

@connor4312 connor4312 self-assigned this Jan 19, 2024
@connor4312 connor4312 added the under-discussion Issue is under discussion for relevance, priority, approach label Jan 19, 2024
@samisalreadytaken
Copy link

Another extension would be function breakpoint sources. Multiple functions with identical names from different files can exist in most, if not all, languages. Specifying the source of the function to break on distinguishes them.

gdb does this with the syntax filename:funcname https://ftp.gnu.org/old-gnu/Manuals/gdb/html_node/gdb_28.html

Visual Studio with {funcname,filename,} https://learn.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2010/wztycb7f(v=vs.100)

I don't know how other debuggers handle this.

@connor4312
Copy link
Member

I've made a couple changes and additions here. Please, if there are more features you'd like in particular from DAP, open new issues for them so we can track them individually. Thanks!

@connor4312 connor4312 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

6 participants