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

Support storing and applying of memory content #96

Merged
merged 7 commits into from Mar 13, 2024
6 changes: 3 additions & 3 deletions src/common/debug-requests.ts
@@ -1,5 +1,5 @@
/********************************************************************************
* Copyright (C) 2018 Red Hat, Inc. and others.
* Copyright (C) 2024 EclipseSource.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
Expand Down Expand Up @@ -32,8 +32,8 @@ export interface DebugEvents {
'stopped': DebugProtocol.StoppedEvent
}

export type DebugRequest<COMMAND, ARGS> = Omit<DebugProtocol.Request, 'command' | 'arguments'> & { command: COMMAND, arguments: ARGS };
export type DebugResponse<COMMAND, BODY> = Omit<DebugProtocol.Response, 'command' | 'body'> & { command: COMMAND, body: BODY };
export type DebugRequest<C, A> = Omit<DebugProtocol.Request, 'command' | 'arguments'> & { command: C, arguments: A };
export type DebugResponse<C, B> = Omit<DebugProtocol.Response, 'command' | 'body'> & { command: C, body: B };
export type DebugEvent<T> = DebugProtocol.Event & { body: T };

export async function sendRequest<K extends keyof DebugRequestTypes>(session: DebugSession,
Expand Down
2 changes: 1 addition & 1 deletion src/common/messaging.ts
Expand Up @@ -29,7 +29,7 @@ export type MemoryOptions = Partial<DebugProtocol.ReadMemoryArguments>;
export type ReadMemoryArguments = DebugRequestTypes['readMemory'][0];
export type ReadMemoryResult = DebugRequestTypes['readMemory'][1];

export type WriteMemoryArguments = DebugRequestTypes['writeMemory'][0] & { count?: number };
export type WriteMemoryArguments = DebugRequestTypes['writeMemory'][0];
export type WriteMemoryResult = DebugRequestTypes['writeMemory'][1];

export type StoreMemoryArguments = MemoryOptions & { proposedOutputName?: string } | VariablesView.IVariablesContext | WebviewContext;
Expand Down
2 changes: 1 addition & 1 deletion src/plugin/adapter-registry/adapter-capabilities.ts
Expand Up @@ -112,7 +112,7 @@ export class AdapterVariableTracker implements vscode.DebugAdapterTracker {
throw new Error('To be implemented by derived classes!');
}

/** Resolves the address of a given variable in bytes withthe current context. */
/** Resolves the address of a given variable in bytes within the current context. */
getAddressOfVariable?(variableName: string, session: vscode.DebugSession): Promise<string | undefined>;

/** Resolves the size of a given variable in bytes within the current context. */
Expand Down
9 changes: 5 additions & 4 deletions src/plugin/adapter-registry/c-tracker.ts
Expand Up @@ -42,13 +42,14 @@ export class CTracker extends AdapterVariableTracker {
return undefined;
}
try {
const variableAddress = await this.getAddressOfVariable(variable.name, session);
const [variableAddress, variableSize] = await Promise.all([
variable.memoryReference && hexAddress.test(variable.memoryReference) ? variable.memoryReference : this.getAddressOfVariable(variable.name, session),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider different integer representations? Not sure if all debug adapters would always return a hex value for an address (although IMO the sensible thing to do).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point! But I'd rather defer this discussion into a separate issue. The previous behavior also used the hexAddress. I did add a commit that unifies that check but I still think it does not necessarily have to be part of this PR, do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me. TBF, I'd expect addresses to be returned as hex values anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this isn't standardized, I'd probably prefer a check for any (easily parsed) number format so that 16 is valid in addition to 0x10. That would follow the convention for things like the ReadMemoryResponse and DisassembledInstruction where at least hex and decimal are both considered acceptable formats.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thank you! I added #107 to track this. It shouldn't be too much work but it needs to be discussed first (all formats vs "only" hexadecimal/decimal because they are mentioned in the DAP).

this.getSizeOfVariable(variable.name, session)
]);
if (!variableAddress) { return undefined; }
const variableSize = await this.getSizeOfVariable(variable.name, session);
if (!variableSize) { return undefined; }

const startAddress = BigInt(variableAddress);
const endAddress = BigInt(variableAddress) + variableSize;
const endAddress = variableSize !== undefined ? startAddress + variableSize : undefined;
this.logger.debug('Resolved', variable.name, { start: variableAddress, size: variableSize });
return {
name: variable.name,
Expand Down
31 changes: 15 additions & 16 deletions src/plugin/memory-provider.ts
Expand Up @@ -37,7 +37,6 @@ export class MemoryProvider {
private _onDidWriteMemory = new vscode.EventEmitter<WrittenMemory>();
public readonly onDidWriteMemory = this._onDidWriteMemory.event;

private _sessionContext: SessionContext = { canRead: false, canWrite: false };
private _onDidChangeSessionContext = new vscode.EventEmitter<SessionContext>();
public readonly onDidChangeSessionContext = this._onDidChangeSessionContext.event;

Expand All @@ -47,10 +46,6 @@ export class MemoryProvider {
constructor(protected adapterRegistry: AdapterRegistry) {
}

get sessionContext(): SessionContext {
return this._sessionContext;
}

public activate(context: vscode.ExtensionContext): void {
const createDebugAdapterTracker = (session: vscode.DebugSession): Required<vscode.DebugAdapterTracker> => {
const handlerForSession = this.adapterRegistry.getHandlerForSession(session.type);
Expand Down Expand Up @@ -105,16 +100,21 @@ export class MemoryProvider {
this.sessionClientCapabilities.delete(session.id);
}

protected setContext(session?: vscode.DebugSession): void {
const capabilities = session && this.sessionDebugCapabilities.get(session.id);
this._sessionContext = {
sessionId: session?.id,
createContext(session = vscode.debug.activeDebugSession): SessionContext {
const sessionId = session?.id;
const capabilities = sessionId ? this.sessionDebugCapabilities.get(sessionId) : undefined;
return {
sessionId,
canRead: !!capabilities?.supportsReadMemoryRequest,
canWrite: !!capabilities?.supportsWriteMemoryRequest
};
vscode.commands.executeCommand('setContext', MemoryProvider.ReadKey, this.sessionContext.canRead);
vscode.commands.executeCommand('setContext', MemoryProvider.WriteKey, this.sessionContext.canWrite);
this._onDidChangeSessionContext.fire(this.sessionContext);
}

protected setContext(session?: vscode.DebugSession): void {
const newContext = this.createContext(session);
vscode.commands.executeCommand('setContext', MemoryProvider.ReadKey, newContext.canRead);
vscode.commands.executeCommand('setContext', MemoryProvider.WriteKey, newContext.canWrite);
this._onDidChangeSessionContext.fire(newContext);
}

/** Returns the session if the capability is present, otherwise throws. */
Expand Down Expand Up @@ -145,15 +145,14 @@ export class MemoryProvider {
return sendRequest(this.assertCapability('supportsReadMemoryRequest', 'read memory'), 'readMemory', args);
}

public async writeMemory(args: DebugProtocol.WriteMemoryArguments & { count?: number }): Promise<WriteMemoryResult> {
public async writeMemory(args: DebugProtocol.WriteMemoryArguments): Promise<WriteMemoryResult> {
const session = this.assertCapability('supportsWriteMemoryRequest', 'write memory');
return sendRequest(session, 'writeMemory', args).then(response => {
const offset = response?.offset ? (args.offset ?? 0) + response.offset : args.offset;
// we accept count as an additional argument so we can skip the memory length calculation
const count = response?.bytesWritten ?? args.count ?? stringToBytesMemory(args.data).length;
if (!this.hasClientCapabilitiy(session, 'supportsMemoryEvent')) {
// we only send out a custom event if we don't expect the client to handle the memory event
// since our client is VS Code we can assume that they will always support this but better to be safe
const offset = response?.offset ? (args.offset ?? 0) + response.offset : args.offset;
const count = response?.bytesWritten ?? stringToBytesMemory(args.data).length;
this._onDidWriteMemory.fire({ memoryReference: args.memoryReference, offset, count });
}
return response;
Expand Down
2 changes: 1 addition & 1 deletion src/plugin/memory-storage.ts
Expand Up @@ -169,7 +169,7 @@ export class MemoryStorage {
memoryReference = toHexStringWithRadixMarker(address);
count = memory.length;
const data = bytesToStringMemory(memory);
await this.memoryProvider.writeMemory({ memoryReference, data, count });
await this.memoryProvider.writeMemory({ memoryReference, data });
}
await vscode.window.showInformationMessage(`Memory from '${vscode.workspace.asRelativePath(options.uri)}' applied.`);
return { memoryReference, count, offset: 0 };
Expand Down
6 changes: 3 additions & 3 deletions src/plugin/memory-webview-main.ts
Expand Up @@ -206,7 +206,7 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider {
const disposables = [
this.messenger.onNotification(readyType, () => {
this.setInitialSettings(participant, panel.title);
this.setSessionContext(participant, this.memoryProvider.sessionContext);
this.setSessionContext(participant, this.memoryProvider.createContext());
this.refresh(participant, options);
}, { sender: participant }),
this.messenger.onRequest(setOptionsType, o => {
Expand Down Expand Up @@ -316,7 +316,7 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider {
protected async storeMemory(storeArguments: StoreMemoryArguments): Promise<void> {
// Even if we disable the command in VS Code through enablement or when condition, programmatic execution is still possible.
// However, we want to fail early in case the user tries to execute a disabled command
if (!this.memoryProvider.sessionContext.canRead) {
if (!this.memoryProvider.createContext().canRead) {
throw new Error('Cannot read memory, no valid debug session.');
}
return vscode.commands.executeCommand(StoreCommandType, storeArguments);
Expand All @@ -325,7 +325,7 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider {
protected async applyMemory(): Promise<MemoryOptions> {
// Even if we disable the command in VS Code through enablement or when condition, programmatic execution is still possible.
// However, we want to fail early in case the user tries to execute a disabled command
if (!this.memoryProvider.sessionContext.canWrite) {
if (!this.memoryProvider.createContext().canWrite) {
throw new Error('Cannot write memory, no valid debug session.');
}
return vscode.commands.executeCommand(ApplyCommandType);
Expand Down