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 full support for data breakpoints #1161

Conversation

puremourning
Copy link
Contributor

RFC - Opening this to see if you're interested in the contribution. I'm working on implementing data breakpoints in vimspector, and as always it's much easier to hack codelldb than anything else, so I implemented the extra features from DAP spec. Only lightly tested at this point.


Currently, we only provode support for data breakpoints where the request is for a child of some container using variables_reference. However, the spec includes support for:

  • an arbitrary expression, and
  • an arbitrary address, and
  • an optional size in bytes.

The later two are supported via a capability
"supportsDataBreakpointBytes". We add support for all of these options.

It's fairly trivial to evaluate an expression and do esentially what we do now for children (find the load address, check its size). But we can also accept a size, so long as it's valid watchpoint size. Similarly for addresses, if 'asAddress' is supplied we can just try to use it. It will probably fail if the address is actually invalid, but that's sort of the point.

@vadimcn
Copy link
Owner

vadimcn commented Oct 18, 2024

Would be great to have that. These extra options were added after I implemented data breakpoints.

@puremourning
Copy link
Contributor Author

puremourning commented Dec 12, 2024

Spent a bunch more time testing this and good news/bad news.

Good news: All the hackery that I added for calculating which watchpoints to actually add is completely unnecessary. LLDB is supposed to do that all by itself.

Bad news is that this clearly doesn't actually work in some versions on LLDB (at least on my platform, Mac arm64).

LLDB master works fine, but the bundled LLDB 19.1.0-custom seems to fail when watched sizes are > about 24 bytes.

Looks like this might have been added in llvm/llvm-project#79962

Either way, I'm thinking to remove all of the complexity from CodeLLDB and whenever it starts working in libLLDB, then we'll just start reaping the benefits, rather than implementing a half-baked version of the above PR here.

@puremourning puremourning force-pushed the data-breakpoints-info-addresses-and-expressions branch from 16396be to 0e759b4 Compare December 12, 2024 11:38
@puremourning
Copy link
Contributor Author

Pushed the simplified version of the change. The second commit message covers the situation:

Current released versions of lldb have seeming partial support for
"large" watchpoints. That is they break a user requested watchpoint and
size into specific minimal set of working hardware watchpoints supported
by the target.

Full support for this is in progress in master and works well, allowing
to watch any address that's part of some larger structure. In any case,
CodeLLDB rejecting a watch due to its size is swimming against the tide
and by removing the size restriction we can take advantage of whatever
support LLDB adds in the future versions without code changes.

The drawback is that we get a slightly less precise error
"Setting one of the watchpoint resources failed" in the case where the
size is not currently supported.

Let me know what you think.

Currently, we only provode support for data breakpoints where the
request is for a child of some container using variables_reference.
However, the spec includes support for:

- an arbitrary expression, and
- an arbitrary address, and
- an optional size in bytes.

The later two are supported via a capability
"supportsDataBreakpointBytes". We add support for all of these options.

It's fairly trivial to evaluate an expression and do esentially what we
do now for children (find the load address, check its size). But we can
also accept a size, so long as it's valid watchpoint size. Similarly for
addresses, if 'asAddress' is supplied we can just try to use it. It will
probably fail if the address is actually invalid, but that's sort of the
point.
Current released versions of lldb have seeming partial support for
"large" watchpoints. That is they break a user requested watchpoint and
size into specific minimal set of working hardware watchpoints supported
by the target.

Full support for this is in progress in master and works well, allowing
to watch any address that's part of some larger structure. In any case,
CodeLLDB rejecting a watch due to its size is swimming against the tide
and by removing the size restriction we can take advantage of whatever
support LLDB adds in the future versions without code changes.

The drawback is that we get a slightly less precise error
"Setting one of the watchpoint resources failed" in the case where the
size is not currently supported.
@puremourning puremourning force-pushed the data-breakpoints-info-addresses-and-expressions branch from 0e759b4 to 4562055 Compare December 12, 2024 11:44
@puremourning
Copy link
Contributor Author

Actually it's a bit more confusing. If I build lldb 19.1.0 branch from source it works fine for arbitrary sizes, just not with the bundled liblldb.dylib. Strange.

@puremourning
Copy link
Contributor Author

codelldb-data-breakpoints

Here's a demo in vscode.

You can see the slightly broken behaviour for the larger array (t.big is a large char array).

here's it with a custom build lldb 19.1.0:

codelldb-data-breakpoints-fixed

Either way demos show:

  • picking a variable to watch from he variables view
  • entering an address and size

@puremourning puremourning changed the title [RFC] Add full support for data breakpoints Add full support for data breakpoints Dec 12, 2024
@vadimcn
Copy link
Owner

vadimcn commented Dec 20, 2024

Looking... How does one enter an expression whose result to watch in VSCode?

@vadimcn vadimcn force-pushed the master branch 7 times, most recently from 84814be to ca59065 Compare January 13, 2025 01:30
@vadimcn
Copy link
Owner

vadimcn commented Jan 21, 2025

Pulled this in

@vadimcn vadimcn closed this Jan 21, 2025
@puremourning puremourning deleted the data-breakpoints-info-addresses-and-expressions branch January 21, 2025 19:03
@vadimcn
Copy link
Owner

vadimcn commented Jan 25, 2025

Hmm, looks like SBWatchpointOptions was added in lldb 18, so this change breaks compatibility with older liblldb's: #1221

@puremourning
Copy link
Contributor Author

Ugh. I didn't consider backward compat with lldb. Assumed you only supported 'at least as new' as the one in codelldb.

I'll need to find some time to look into it and I'm chocca this weekend.

If it's urgent fix I guess revert and we can go again?

@vadimcn
Copy link
Owner

vadimcn commented Jan 26, 2025

I think I'll change it back to WatchAddress and then fix this in LLDB itself.

@puremourning
Copy link
Contributor Author

I have a patch and test for lldb. I'll submit it today.

@puremourning
Copy link
Contributor Author

Here: llvm/llvm-project#124847

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

Successfully merging this pull request may close these issues.

2 participants