Support for a new WriteMemoryRequest if debuggee permits to

[yannickowow]

Hi,
In order to implement a full set of debug tools from DAP, I would like if it would be ok to implement a request such as WriteMemoryRequest, the same way it works for ReadMemoryRequest

interface WriteMemoryArguments {
  /**
   * Memory reference to the base location from which data should be written.
   */
  memoryReference: string;

  /**
   * Bytes to write, [encoded using base64 like ReadMemoryRequest ?]
   */
  content: string;

  /**
   * Optional offset (in bytes) to be applied to the reference location before
   * writing data. Can be negative.
   */
  offset?: number;
}

For the response, I don't know if it can be the same way as ReadMemoryResponse. It can also be a new Memory event with a flag 'modified' and a content/address

interface WriteMemoryResponse {
  // body is not mandatory if write has worked correctly
  body?: {
    /**
     * The address of the first byte of data returned.
     * Treated as a hex value if prefixed with '0x', or as a decimal value
     * otherwise. (It would match previous memoryReference+offset)
     */
    address?: string;

    /**
     * The bytes read from memory, encoded using base64.
     */
    data?: string;
  };
}

Thanks in advance

[connor4312]

The "write" request makes sense. I'm not sure about the response--is the use case to return that if the write only partially succeeded?

---

We also should have readMemory return information about whether the memory is writable, and whether it can be resized. In languages where the binary data is in a well-defined container, like JS or Python, we can grow or shrink it by assigning a new value. However, in statically compiled languages, changing the size of data is likely to be illegal.

[yannickowow]

Exactly. My first proposition was to ensure that content has been updated correctly.
If data is empty, we should consider that data has been correctly written. Else, we use returned data to update current view based on returned address.

For your second point, I was considering C/C++ debugging in a view which should not grow nor shrink memory size, but rather edit it in place.

[connor4312]

I'm not very familiar with C/++ debugging. Are there cases where a write to memory would fail? From the little C work I've done, wouldn't it either always succeed or (if the write was in an unallocated/freed area) segfault?

[gregg-miskelly]

On some devices/operating systems it could partially succeed if the address range crossed a memory page boundary such that some of the addresses were valid and writable while others were not.

If we want to support partial failures, I wonder if we should have this return a 'bytesWritten' count instead of returning data.

[yannickowow]

Good point, bytesWritten looks more adequate and lighter than a whole data content

[weinand]

Thanks for the proposal and the subsequent discussion.

Yes, I prefer the bytesWritten count over returning the written data.

@gregg-miskelly you said:

[ a WriteMemoryRequest ] could partially succeed if the address range crossed a memory page boundary such that some of the addresses were valid and writable while others were not

This means that a WriteMemoryRequest with a large address range could cross more than one page each with different write permissions. Do we need a way to return multiple address and bytesWritten properties? And would a client actually want to show this in the UI?

Or is it enough to have a single address and bytesWritten property, and bytesWritten would have the total number of bytes written?

[weinand]

@connor4312 I have created a new feature request for your comment from above:

We also should have readMemory return information about whether the memory is writable, and whether it can be resized. In languages where the binary data is in a well-defined container, like JS or Python, we can grow or shrink it by assigning a new value. However, in statically compiled languages, changing the size of data is likely to be illegal.

[gregg-miskelly]

Do we need a way to return multiple address and bytesWritten properties? And would a client actually want to show this in the UI?

There are two use cases I can think of for WriteMemoryRequest:

1. A plugin that actually understands the structure of the memory it is probing (ex: it has some sort of symbols or memory schema that it is following). In this case, I don't think allowing partial writes is useful - if the write fails that probably means something very bad happened.
2. A memory window that is just showing the raw bytes of memory and allows the use to edit it. In this case I think partial writes would be useful, but it probably isn't important if the DA attempts to write as much of the request as it possibly can -- giving up when it hits the first failure should be good enough, and the memory window can just restore the bytes back to the way they were after that point.

To support these two cases I would suggest:

1. WriteMemoryArguments should also have an allowPartial boolean field with a description of something like: "If true, the debug adapter should attempt to write memory even if the entire memory region is not writable. In such a case the debug adapter should stop after hitting the first byte of memory that cannot be written and returning the number of bytes written in the response. If false, a debug adapter should attempt to verify the region is writable before writing, and fail the response if it is not."
2. As I suggested earlier, WriteMemoryResponse should have a bytesWritten optional integer with a description something like: "Optional field that should be returned when allowPartial is true to indicate the number of bytes starting from address that were successfully written."

[weinand]

Thanks everyone for the input.
Here is the updated proposal:

A new capability supportsWriteMemoryRequest indicates that the debug adapter implements the writeMemory request.

interface Capabilities {

  /**
   * The debug adapter supports the 'writeMemory' request.
   */
  supportsWriteMemoryRequest?: boolean;

}

For the writeMemory request arguments:

• content has been renamed to data to better align with the readMemory request,
• an optional allowPartial property has been added

interface WriteMemoryArguments {
  /**
   * Memory reference to the base location to which data should be written.
   */
  memoryReference: string;

  /**
   * Optional offset (in bytes) to be applied to the reference location before
   * writing data. Can be negative.
   */
  offset?: number;

  /**
   * Optional property to control partial writes. If true, the debug adapter should attempt to write memory
   * even if the entire memory region is not writable. In such a case the debug adapter should stop after 
   * hitting the first byte of memory that cannot be written and return the number of bytes written in the
   * response via the 'address' and 'bytesWritten' properties.
   * If false or missing, a debug adapter should attempt to verify the region is writable before writing,
   * and fail the response if it is not.
   */
  allowPartial?: boolean;

  /**
   * Bytes to write, encoded using base64.
   */
  data: string;
}

For the writeMemory request response:

• the data property has been replaced by a bytesWritten property.

interface WriteMemoryResponse {
  body?: {
    /**
     * Optional property that should be returned when 'allowPartial' is true to indicate the address
     * of the first byte of data successfully written.
     * Treated as a hex value if prefixed with '0x', or as a decimal value otherwise. 
     */
    address?: string;

    /**
     * Optional property that should be returned when 'allowPartial' is true to indicate the number
     * of bytes starting from address that were successfully written.
     */
    bytesWritten?: number;
  };
}

@gregg-miskelly @yannickowow @connor4312
If there are no objections, I'll proceed updating the DAP schema.

[weinand]

I forgot to include the supportsWriteMemoryRequest capability. This is fixed.

[puremourning]

what's the semantics of the returned address field ? Is it really a memoryReference of its own in some way? Asking because presumably the 'memoryReference' abstraction exists for some reason (as opposed to client handling raw addresses) ?

[weinand]

@puremourning your question about the address field raises a good point: what's the purpose of address in the context of the writeMemory request?

In the readMemory request the address is returned to provide a UI label for the memory data returned. A client's hex viewer could show that label next to the data block (without having to interpret the string value).

In the writeMemory request I do not see a need for a client to render the returned address in the UI. I think it would make more sense to return an offset (of type number) instead. With this we could get rid of one of the ugly type specifiers: "Treated as a hex value if prefixed with '0x', or as a decimal value otherwise."

@gregg-miskelly do you agree?

[weinand]

Updated proposal:

A new capability supportsWriteMemoryRequest indicates that the debug adapter implements the writeMemory request.

interface Capabilities {

  /**
   * The debug adapter supports the 'writeMemory' request.
   */
  supportsWriteMemoryRequest?: boolean;

}

For the writeMemory request arguments:

• content has been renamed to data to better align with the readMemory request,
• an optional allowPartial property has been added

interface WriteMemoryArguments {
  /**
   * Memory reference to the base location to which data should be written.
   */
  memoryReference: string;

  /**
   * Optional offset (in bytes) to be applied to the reference location before
   * writing data. Can be negative.
   */
  offset?: number;

  /**
   * Optional property to control partial writes. If true, the debug adapter
   * should attempt to write memory even if the entire memory region is not
   * writable. In such a case the debug adapter should stop after hitting the
   * first byte of memory that cannot be written and return the number of bytes
   * written in the response via the 'offset' and 'bytesWritten' properties.
   * If false or missing, a debug adapter should attempt to verify the region is
   * writable before writing, and fail the response if it is not.
   */
  allowPartial?: boolean;

  /**
   * Bytes to write, encoded using base64.
   */
  data: string;
}

For the writeMemory request response:

• the string property address has been replaced by a number property offset.
• the data property has been replaced by a bytesWritten property.

interface WriteMemoryResponse extends Response {
  body?: {
    /**
     * Optional property that should be returned when 'allowPartial' is true to
     * indicate the offset of the first byte of data successfully written. Can
     * be negative.
     */
    offset?: number;

    /**
     * Optional property that should be returned when 'allowPartial' is true to
     * indicate the number of bytes starting from address that were successfully
     * written.
     */
    bytesWritten?: number;
  };
}

@gregg-miskelly @yannickowow @connor4312 @puremourning
If there are no objections, I'll proceed updating the DAP schema.