dap: implement readMemory for strings and expose memoryReference #4083

MistaTwista · 2025-08-08T21:53:42Z

Motivation

Large Go strings are still truncated in common debugging workflows (variables view uses ~512 chars; hover / “Copy Value” use ~4096 chars). That’s often not enough for JSON payloads, logs, etc., and users rely on awkward workarounds (manual slicing, multiple reads).

Adding DAP readMemory enables clients to page the underlying bytes on demand and render full values.

Fixes: #4084

Vscode-go feature: golang/vscode-go#3817

service/dap/server.go

aarzilli · 2025-08-09T05:52:48Z

vendor/github.com/google/go-dap/schematypes.go

 	SupportsVariablePaging              bool   `json:"supportsVariablePaging,omitempty"`
 	SupportsRunInTerminalRequest        bool   `json:"supportsRunInTerminalRequest,omitempty"`
 	SupportsMemoryReferences            bool   `json:"supportsMemoryReferences,omitempty"`
+	SupportsReadMemoryRequest           bool   `json:"supportsReadMemoryRequest,omitempty"`


You can't change vendored packages like this, you need to make a PR to google/go-dap first.

yeap, i know, i did google/go-dap#94

service/dap/server.go

aarzilli · 2025-08-13T07:06:16Z

LGTM but held because we need the go-dap change.
@h9jiang do you know who's maintaining google/go-dap now?

h9jiang · 2025-08-14T14:35:09Z

Hi @aarzilli

I think the go tools team is the maintainer of google/go-dap, I will discuss this within the team and review the PR and CL that @MistaTwista drafted.

MistaTwista · 2025-09-07T12:10:05Z

sorry for inconvenience with go-dap PR. I closed it because it looks like we don't need those changes here 😐

aarzilli · 2025-09-07T15:29:40Z

Many tests fail.

aarzilli · 2025-09-08T12:02:51Z

PS. the TODO on line service/dap/server.go:841 needs to be removed.

MistaTwista · 2025-09-13T10:00:16Z

Hey @aarzilli, I’ve fixed the previously failing tests and also added a new one for my changes to be sure everything works.

The main idea of this new test is to generate a large JSON payload, retrieve it from the Delve server via the read-memory request page-by-page, compute a hash, and compare it with the original hash to ensure there are no duplicates or missing data.

All tests pass locally, but the CI still fails for my test with:

=== RUN   TestReadMemory_StringPagination
Error compiling ../../_fixtures/readmem_json.go: wait6: no child processes

FAIL	github.com/go-delve/delve/service/dap	50.196s
?   	github.com/go-delve/delve/service/dap/daptest	[no test files]
?   	github.com/go-delve/delve/service/dap/daptest/gen	[no test files]

It seems like an environment difference between my setup and the CI runner, and I can’t reproduce it locally. Do you have any idea what might be wrong?

aarzilli · 2025-09-13T15:26:03Z

I think the reason the tests do not pass is just because your branch is fairly old and needs to be rebased.
However, I also tested this with vscode and I don't get the "open in new document" option that you have.

MistaTwista · 2025-09-13T22:48:56Z

I think the reason the tests do not pass is just because your branch is fairly old and needs to be rebased.

Ok, I'll rebase it and try again, thanks

However, I also tested this with vscode and I don't get the "open in new document" option that you have.

I'm not sure I understand you correctly. In the description of the original issue I showed what functionality I'm trying to archive in vscode and why.

And this comment is about why I’m proposing delve changes.

aarzilli · 2025-09-15T13:20:11Z

So, this would also require a change to vscode?

MistaTwista · 2025-09-15T18:25:28Z

not vscode itself, but vscode-go plugin for vscode golang/vscode-go#3818

aarzilli

I was looking over the DAP specification again and it occurs to me that memory references are used in one other place in the spec: for disassembly.

I think it would be prudent to put memory references for disassembly and the ones for the memory of variables in the same address space, in case clients expects them to be. That would mean that instead of using a sequential number to number memory references you just send the address of the variable formatted into a string.

service/dap/server_test.go

_fixtures/readmem_json.go

service/dap/server.go

MistaTwista · 2025-09-30T20:26:54Z

I think it would be prudent to put memory references for disassembly and the ones for the memory of variables in the same address space, in case clients expects them to be

done 👌 test is also updated for the new format, but still there are some issues

aarzilli · 2025-10-01T14:30:00Z

I don't know why your test is failing like that on freebsd, there is no obvious problem that would lead to that. I think it's fine to just add a skip for freebsd (if runtime.GOOS == "freebsd" { t.Skip(...) })

aarzilli

LGTM

MistaTwista · 2025-10-02T12:38:04Z

I'm thinking about support that kind of strings representation too. Make it possible in this PR or make the new one? Current version gives me error for them like

But it looks obvious for me that this is a good case to support. What do you think?

aarzilli · 2025-10-02T12:49:08Z

The []byte variable, sure, not the string() thing we return.

MistaTwista · 2025-10-04T19:18:02Z

I've added slices

and removed max limit to get memory address, because it can be useful to check even small variables in a hex editor

aarzilli

LGTM

MistaTwista · 2025-10-07T11:22:30Z

LGTM

Do I need to do something (like rebase to current master) or just wait until merge?

aarzilli · 2025-10-07T12:34:29Z

Just wait.

derekparker · 2025-10-08T22:03:12Z

@MistaTwista this patch does need a rebase now. After the rebase I will merge.

MistaTwista · 2025-10-09T07:00:18Z

Done, would you like me to squash the commits, or should I improve the commit messages for better documentation?

- enable setting SupportsReadMemoryRequest in Initialize response - variable listings now include MemoryReference for strings and slices - implement Session.onReadMemoryRequest handling for readMemory requests

MistaTwista · 2025-10-15T20:39:06Z

fixed conflict with master, squashed commits, made better description of a commit

aarzilli reviewed Aug 9, 2025

View reviewed changes

MistaTwista mentioned this pull request Aug 9, 2025

[Debugger] Feature request: “View string as document” for multiline strings golang/vscode-go#3817

Open

aarzilli reviewed Aug 11, 2025

View reviewed changes

service/dap/server.go Outdated Show resolved Hide resolved

MistaTwista marked this pull request as ready for review August 12, 2025 23:13

MistaTwista mentioned this pull request Sep 7, 2025

Expose memory-related capability booleans google/go-dap#94

Closed

MistaTwista force-pushed the feature/read_memory branch from c841fe0 to e04d4b8 Compare September 17, 2025 20:58

aarzilli reviewed Sep 18, 2025

View reviewed changes

service/dap/server_test.go Outdated Show resolved Hide resolved

_fixtures/readmem_json.go Outdated Show resolved Hide resolved

service/dap/server.go Show resolved Hide resolved

service/dap/server.go Outdated Show resolved Hide resolved

MistaTwista force-pushed the feature/read_memory branch from 06034a4 to efcb70a Compare September 25, 2025 21:55

aarzilli approved these changes Oct 2, 2025

View reviewed changes

aarzilli approved these changes Oct 6, 2025

View reviewed changes

firelizzard18 mentioned this pull request Oct 7, 2025

debug: bytes to string "Copy Value" truncates string if's too long golang/vscode-go#3893

Closed

MistaTwista force-pushed the feature/read_memory branch from 868991c to a6fb2e6 Compare October 9, 2025 06:54

add dap read memory request handler

0db7dce

- enable setting SupportsReadMemoryRequest in Initialize response - variable listings now include MemoryReference for strings and slices - implement Session.onReadMemoryRequest handling for readMemory requests

MistaTwista force-pushed the feature/read_memory branch from a6fb2e6 to 0db7dce Compare October 15, 2025 20:36

derekparker merged commit 9103a45 into go-delve:master Oct 15, 2025
7 of 8 checks passed

dap: implement readMemory for strings and expose memoryReference #4083

dap: implement readMemory for strings and expose memoryReference #4083

Uh oh!

Conversation

MistaTwista commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Uh oh!

Uh oh!

aarzilli Aug 9, 2025

Choose a reason for hiding this comment

Uh oh!

MistaTwista Aug 9, 2025

Choose a reason for hiding this comment

Uh oh!

aarzilli Aug 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aarzilli commented Aug 13, 2025

Uh oh!

h9jiang commented Aug 14, 2025

Uh oh!

MistaTwista commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aarzilli commented Sep 7, 2025

Uh oh!

aarzilli commented Sep 8, 2025

Uh oh!

MistaTwista commented Sep 13, 2025

Uh oh!

aarzilli commented Sep 13, 2025

Uh oh!

MistaTwista commented Sep 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aarzilli commented Sep 15, 2025

Uh oh!

MistaTwista commented Sep 15, 2025

Uh oh!

aarzilli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MistaTwista commented Sep 30, 2025

Uh oh!

aarzilli commented Oct 1, 2025

Uh oh!

aarzilli left a comment

Choose a reason for hiding this comment

Uh oh!

MistaTwista commented Oct 2, 2025

Uh oh!

aarzilli commented Oct 2, 2025

Uh oh!

MistaTwista commented Oct 4, 2025

Uh oh!

aarzilli left a comment

Choose a reason for hiding this comment

Uh oh!

MistaTwista commented Oct 7, 2025

Uh oh!

aarzilli commented Oct 7, 2025

Uh oh!

derekparker commented Oct 8, 2025

Uh oh!

MistaTwista commented Oct 9, 2025

Uh oh!

MistaTwista commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

MistaTwista commented Aug 8, 2025 •

edited

Loading

MistaTwista commented Sep 7, 2025 •

edited

Loading

MistaTwista commented Sep 13, 2025 •

edited

Loading

MistaTwista commented Oct 15, 2025 •

edited

Loading