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

Roadmap: Disassembly View Feature #124163

Closed
yuehuang010 opened this issue May 19, 2021 · 59 comments · Fixed by #125737
Closed

Roadmap: Disassembly View Feature #124163

yuehuang010 opened this issue May 19, 2021 · 59 comments · Fixed by #125737
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan
Milestone

Comments

@yuehuang010
Copy link
Contributor

Hi Team, C++ Extension is interested in developing an Disassembly View to represent the added interface in DAP (https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Disassemble). Below is a quick bullet points of the set of features we would like to see.

To contrast, Visual Studio Disassembly Window is shown in the screenshot below:
image

The view should include:

  1. Address, Assembly Code in Byte/Hex, Assembly Instruction, Data of related memory and registers (All information is provided by DAP and debugger.)
  2. Functions Marker if available.
  3. Interleave with source code (if present)
  4. Breakpoints gutter to set and remove BP.

In additional to displaying the data, users could also interact with the view by:

  1. Setting Breakpoints at the per instruction and Breakpoints should appear in the breakpoint list.
  2. Stepping per instruction. Controls should flow nicely existing debug step in/out.
  3. Scrolling Up and Down will load instructions above and below.
  4. Need a way to toggle Hex view for values.

Thank you.

@weinand weinand changed the title Roadamp: Disassembly View Feature Roadmap: Disassembly View Feature May 19, 2021
@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label May 19, 2021
@weinand weinand added this to the On Deck milestone May 19, 2021
@weinand weinand added the feature-request Request for new features or functionality label May 19, 2021
@isidorn
Copy link
Contributor

isidorn commented May 19, 2021

@yuehuang010 thanks a lot for creating this issue

After some discussions with my fellow colleagues it feels like the best path forward would be to implement a custom editor internally. This custom editor would listen to DAP requests and based on that show the appropriate data.

A good example of a custom editor is the textAreaEditor implement by @bpasero as an example in his branch here. Though the memory editor would be read only so it would not require all the code for dealing with working copies. This custom memory editor would appear pinned while debugging and could maybe automatically go away once debugging is done.

If we were to introduce a custom memory editor I think a good place for it to live would be under src/vs/workbench/contrib/debug/browser/. It could depend on the IDebugService and from the service get all the needed data and pass breakpoints to it.

Maybe a Hex view could also be a good inspiration for a virtual editor. Though I know it is done from an extension. Maybe @lramos15 has a good pointer

Once we figure out how and where to do this, we can discuss how exactly it would look and behave. Maybe we do a 30 min meeting some time start of next week?

fyi @bpasero @mjbvz @lramos15 for custom editor thoughts

@bpasero
Copy link
Member

bpasero commented May 19, 2021

Maybe an easier example is to look at an existing readonly editor such as the "Runtime Extensions" editor:

export abstract class AbstractRuntimeExtensionsEditor extends EditorPane {

@isidorn
Copy link
Contributor

isidorn commented May 19, 2021

This RuntimeExtensionsEditor is used when a user does F1 > show running extensions if you want to check it out.

@yuehuang010
Copy link
Contributor Author

I was playing around with HTML onscroll event. Adding more "lines" to the body will extend the text document, which indirectly un-bottom's scroll bar. This would give it an infinite scrolling feel. Flipping the logic upwards scrolling would work too.

window.onscroll = function(ev) {
  if ((window.innerHeight + window.scrollY) >= document.body.offsetHeight) {
    document.body.innerHTML += `<p>${window.innerHeight} + ${window.scrollY} >= ${document.body.offsetHeight}</p><br>`;
  }
};

@xisui-MSFT
Copy link
Contributor

Simply using an onscroll event would likely cause performance issue at some point. Scroll itself had never been an issue for us. Our major issues are the problems associated with scrolling, including but not limited to:

  • How to create + destroy lines and margin overlays (rerender) dynamically.
  • How to keep users selections.
  • Corner cases related to font and foreign languages.

Using internal vs code stuff certainly helps us, as the main editor resolves these issues very well, but it does all the calculations based on line number. We need to settle a way to do layout without using line number. There are several possible ways, depending on how generic you would like this editor to be.

@mjbvz
Copy link
Collaborator

mjbvz commented May 19, 2021

@isidorn There are two routes you could go with a custom editor:

  • Create an internal custom editor (such as AbstractRuntimeExtensionsEditor). These are only possible to create internally in VS Code and cannot be created by extensions

  • Create a webview based custom editor. These are what the Custom Editor API lets extensions build. The hex editor extension uses these

I suggest looking into a non-webview based custom editor first since that would let you use all of our internal classes, such as the ListView. It is also easier to make them feel like part of VS Code compared to a webview

You can also check in with @lramos15 about some of the challenges he faced implementing the hex editor, specifically some of the work he did so that the hex editor can display large files and smoothly scroll through them

@isidorn
Copy link
Contributor

isidorn commented May 19, 2021

Yeah ListView might be a good way to go for the memory view, as it would give us virtualisation for free.
Interested to hear what @lramos15 faced when doing the hex editor

fyi @joaomoreno

@lramos15
Copy link
Member

Yeah I don't know how we feel about a one off solution that belongs to an external extension living within VS Code core. Although the implementation details are a lot easier when having access to VS Code internals.

Here are a few challenges I came across with the hex editor

  • Extension is responsible for all styling and implementation. Things like the find widget I had to restyle and remake and the experience doesn't match 100% to that of VS Code
  • Own implementation of virtualized scrolling. Using a JS framework might help in this case as they have third party modules for that, but I kept the hex editor without virtual scrolling and the scrolling was very difficult to implement and still not correct for things like trackpads.
  • Webview <-> Ext host communication bottlenecks meant proper packeting had to be implemented as sending too much data could hang the webview or crash the ext host.
  • Accessibility support is up to the extension author and therefore might not align with VS Code.
  • Extension is limited by available APIs (we could always expose more if necessary)

Overall making a custom editor extension isn't easy and those were my main challenges. I'm not sure what the challenges would be for an internal custom editor.

@xisui-MSFT
Copy link
Contributor

@mjbvz I'm not sure if we can use an custom editor. Two features are currently blocking us from using a custom editor:

  • Breakpoints that are not associated with line numbers.
  • It should be able to (act like) scrolling infinitely. The hex editor seems to depend on the file size for display. I'm not sure if that's possible for disassembly. Also the "file size" may change based users' config such as showing source code.

Any suggestions?

@isidorn
Copy link
Contributor

isidorn commented May 20, 2021

@xisui-MSFT custom editors do not have line numbers by themselves and can contain custom content. Just check out the "Runtime Extensions" editor. F1 > show running extensions.
That editor is interesting because it uses a List as an implementation detail and that will solve your scrolling infinitely problem. The list is virtualised and it will only render parts as you scroll into them.
Each list item would be a row of your content. The start of the row can contain a part which the user can click on and the breakpoint gets added.

Looking again at your content it seems like the Table widget might be a better fit, since you have both rows and columns.

If you think this approach makes sense or you would like to discuss it further with me feel free to schedule a 30min meeting with me. Just fyi I am in Europe, so something morning time Redmond would work best.

@yannickowow
Copy link
Contributor

yannickowow commented May 20, 2021

Hello,
Thanks for tackling this issue. I would be happy to try these features!
About Memory View, do you know how to deal with refresh ? In some cases, a debugger might want to send an event to tell the UI that a range of addresses has been modified, but I do not see any Memory-related events in DAP ?
(In the same way, would it be relevant to update DAP with a new writeMemoryRequest and an associated Capabilities (might be out of topic?))

Regards.

@weinand
Copy link
Contributor

weinand commented May 20, 2021

@yannickowow you've already filed a feature request for writeMemoryRequest in the DAP repo. I suggest that you file another request for a MemoryEvent...

@xisui-MSFT
Copy link
Contributor

@isidorn Is there an example using that table widget? I'm not sure how the selection works in it. Also, does it support tokenization & hover message for each token in it's content? We'll need to provide debugging info when hovering on some text like a register.

@isidorn
Copy link
Contributor

isidorn commented May 20, 2021

@xisui-MSFT for example of a list widget powered view you can checkout the Debug Console (for example how selection behaves). The biggest problem with selection and virtual views is that it can not be multi page and that is a limitation that the debug console still has.

You control the HTML rendering of each row, thus you can put any hover message you want.
Tokenization as editor language tokenization is not easily possible. Can you clarify what you would like to do regarding tokenisation?

The table widget is used in the Keybindings Editor. Preferences > keyboard shortcuts

Here's a code pointer https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts#L431

@xisui-MSFT
Copy link
Contributor

@isidorn Looks like selection in the output window works. How is it implemented?

And is there any plan to improve selection experience in the debug console?

Tokenization we need may not be as rich as the editor. The major scenario looks like this:
image

@isidorn
Copy link
Contributor

isidorn commented May 21, 2021

@xisui-MSFT output window uses the Editor as the implementation detail.
Debug console uses the list (tree). We should improve the selection in the debug console, however it is not on the immediate plan. All the details about this issue can be found here #2163

From the screenshot it is not 100% clear to me what is the tokenization scenario here. Is that a hover you are showing, or is it a small suggest box for the input?

@xisui-MSFT
Copy link
Contributor

@isidorn When hovering on token "esp", the evaluation result is shown in the hover message.

A list or table widget that looks similar to an editor sounds like a good idea. Can you schedule a meeting early next week for us to talk through this? You can invite me and I'll forward it to my colleagues.

@isidorn
Copy link
Contributor

isidorn commented May 21, 2021

@xisui-MSFT I see. Well if you know the content of the cell, each cell could have a custom hover widget which on show would evaluate.

I have sent out an invite for Tuesday, since Monday is a holiday in Switzerland.

@yannickowow
Copy link
Contributor

Good morning,
Sorry for the inconvenience, but do you have a target date for these features ?
Regards.

@isidorn
Copy link
Contributor

isidorn commented May 25, 2021

@yannickowow currently no. Once we do we will assign it to a milestone so you can check that.

@isidorn
Copy link
Contributor

isidorn commented May 25, 2021

Some code pointers:

Debug console registering commands https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/debug/browser/repl.ts#L782

Debug console revealing last element
https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/debug/browser/repl.ts#L74

Various templates that the debug console uses
https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/debug/browser/replViewer.ts#L325

I am checking with Table widget owner Joao for the reveal middle element potential problems and will keep you up to date here.

Let us know how it goes.

@isidorn
Copy link
Contributor

isidorn commented May 26, 2021

I discussed with the Table widget owner Joao and the revealing of the middle table element should work nicely for your use case.

So to recap you need a placeholder model with for example 500 elements, not all of them resolved. You should tell the tree to reveal the middle element 250, or the one you are interested in. Once the first or last element get revealed you need to tell the table to refresh and you need to provide an expanded model.

As for the tree reveal api it works like this:

  • reveal(250) will scroll just enough to get the element entirely visible in the viewport
  • reveal(250, 0) will scroll so the element is exactly aligned at the top of the viewport
  • reveal (250, 1) will scroll so the element is exactly aligned at the bottom of the viewport
  • reveal(250, x) will scroll so the element is exactly 1/x proportionally positioned between the last two states

So you probably need something like reveal(250, 0.5)

@xisui-MSFT
Copy link
Contributor

@isidorn Could you give some suggestions on the entry point? I could find examples for each feature we need but couldn't decide if there's a good way to combine them. Ideally, we need:

  1. Enable disassembly view for languages (modes) specified by package.json of a extension, for example:
    export const breakpointsExtPoint = extensionsRegistry.ExtensionsRegistry.registerExtensionPoint<IRawBreakpointContribution[]>({
    extensionPoint: 'breakpoints',
    jsonSchema: {
    description: nls.localize('vscode.extension.contributes.breakpoints', 'Contributes breakpoints.'),
    type: 'array',
    defaultSnippets: [{ body: [{ language: '' }] }],
    items: {
    type: 'object',
    additionalProperties: false,
    defaultSnippets: [{ body: { language: '' } }],
    properties: {
    language: {
    description: nls.localize('vscode.extension.contributes.breakpoints.language', "Allow breakpoints for this language."),
    type: 'string'
    },
    }
    }
    }
    });
  2. For enabled languages, add an item like "show disassembly" to right click menu for both editor and call stack window. And the handler needs to know where (source position or stack frame) the right click menu is triggered (similar to go to definition). One way to add items to the context menu I found is
    registerAction2(class extends Action2 {
    constructor() {
    super({
    id: 'notebook.format',
    title: { value: localize('format.title', "Format Notebook"), original: 'Format Notebook' },
    category: NOTEBOOK_ACTIONS_CATEGORY,
    precondition: ContextKeyExpr.and(NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_EDITOR_EDITABLE),
    keybinding: {
    when: EditorContextKeys.editorTextFocus.toNegated(),
    primary: KeyMod.Shift | KeyMod.Alt | KeyCode.KEY_F,
    linux: { primary: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.KEY_I },
    weight: KeybindingWeight.WorkbenchContrib
    },
    f1: true,
    menu: {
    id: MenuId.EditorContext,
    when: ContextKeyExpr.and(EditorContextKeys.inCompositeEditor, EditorContextKeys.hasDocumentFormattingProvider),
    group: '1_modification',
    order: 1.3
    }
    });
    }
    async run(accessor: ServicesAccessor): Promise<void> {
    const editorService = accessor.get(IEditorService);
    const textModelService = accessor.get(ITextModelService);
    const editorWorkerService = accessor.get(IEditorWorkerService);
    const bulkEditService = accessor.get(IBulkEditService);
    const editor = getNotebookEditorFromEditorPane(editorService.activeEditorPane);
    if (!editor || !editor.viewModel) {
    return;
    }
    const notebook = editor.viewModel.notebookDocument;
    const disposable = new DisposableStore();
    try {
    const edits: ResourceTextEdit[] = [];
    for (const cell of notebook.cells) {
    const ref = await textModelService.createModelReference(cell.uri);
    disposable.add(ref);
    const model = ref.object.textEditorModel;
    const formatEdits = await getDocumentFormattingEditsUntilResult(
    editorWorkerService, model,
    model.getOptions(), CancellationToken.None
    );
    if (formatEdits) {
    for (let edit of formatEdits) {
    edits.push(new ResourceTextEdit(model.uri, edit, model.getVersionId()));
    }
    }
    }
    await bulkEditService.apply(edits, { label: localize('label', "Format Notebook") });
    } finally {
    disposable.dispose();
    }
    }
    });
    , but I'm not sure how to do this for specific languages. Another way might be add it as a language feature. But I didn't find how the language features are added to the context menu...

@yuehuang010
Copy link
Contributor Author

@isidorn Ping.

@weinand
Copy link
Contributor

weinand commented Jun 17, 2021

@yuehuang010 Isidor is OOF and returns next Tuesday.

@isidorn
Copy link
Contributor

isidorn commented Jun 22, 2021

Sorry for the slow response I was on vacation.

@xisui-MSFT thanks for trying clearInput however this is not called whenever tab loses focus, but when it is no longer visible. If you want something to be called only when the editor gets closed then you should override dispose that method will only get called when the editor get explicitly closed.

@yuehuang010 if I understand you correctly you want a special step command that is only active in the Dissassembly view.
Here's what to do:

  1. Introduce a context key that is only active once the focus is in the Assembly view.
  2. Introduce your "special step command" that takes the same keybindings as the regular step command and is only registered when your context key from 1 is active using the when clause
  3. Your special step command will pass the required argumens so you stepping granularity is respected

As for the name of the view, I suggest to call it Assembly view. I did not hear any good arguments for the word dissasembly so far. Even the feature request in your repo first calls it assembly view microsoft/vscode-cpptools#206
So can you please address this? Thank you

@yannickowow
Copy link
Contributor

As for the name of the view, I suggest to call it Assembly view. I did not hear any good arguments for the word dissasembly so far. Even the feature request in your repo first calls it assembly view microsoft/vscode-cpptools#206

(my two cents)
For the name of the view, I would still prefer Disassembly over Assembly for two reasons

  • DAP request is labeled disassembleRequest
  • In C/C++ Debugging (and probably more language), disassembly makes more sense because you "disassemble" your binary to generate assembly code for your instruction set.

@isidorn
Copy link
Contributor

isidorn commented Jun 22, 2021

DAP request naming should not drive the UI and so far has not :)
The second argument makes sense. However I would still go for "assembly view" for clarity.

@yuehuang010
Copy link
Contributor Author

yuehuang010 commented Jun 22, 2021 via email

@yuehuang010
Copy link
Contributor Author

@isidorn, is there a unsetFocus()? Is there a way to clear the Context?

@isidorn
Copy link
Contributor

isidorn commented Jun 23, 2021

@yuehuang010 can you please clarify? What focus would you like to unset?
You should probably not programmatically manipulate the Context.

@yuehuang010
Copy link
Contributor Author

@isidorn, I am looking at EditorContextKeys.editorTextFocus as an example. Do you have another example of a Context?

@bobbrow
Copy link
Member

bobbrow commented Jun 23, 2021

@yuehuang010 I believe @isidorn is referring to context keys which are state variables that can be referenced in package.json using the when properties. It's often used to control when commands/UI/etc can appear in VS Code.

This is how we access it in extensions, but I would think that there is an API you can call directly when writing code within VS Code itself:

vscode.commands.executeCommand('setContext', key, value);

More info here: https://code.visualstudio.com/api/references/when-clause-contexts

@yuehuang010
Copy link
Contributor Author

@bobbrow, we are under the hood, creating a new context key for the Disassembly View.

@bobbrow
Copy link
Member

bobbrow commented Jun 23, 2021

Got it. I misunderstood.

@isidorn
Copy link
Contributor

isidorn commented Jun 24, 2021

@yuehuang010 context keys should just work based on focus, you should not need to manualy set and unset them if they are only related to focus.
Can you please explain what context you would like to have and I can provide suggestions on how to achieve that? One that is active only when the focus is in the Disassembly View? Or something else?

@yuehuang010
Copy link
Contributor Author

yuehuang010 commented Jul 7, 2021

Disassembly View Overview (Insiders) - PR #125737

Check if debugger supports "supportsDisassembleRequest", then while debugging, right click on a source code to see "Open Disassembly View".
image

A Disassembly View editor will appear with each line representing each instruction.
image

In the Disassembly View, you can:

Scroll up or down to see more instructions. Should be able to scroll without limit.
The yellow arrow marker represent current instruction. Use F10/F11 to step in or step over current instruction.
A red dot will appear while hovering over the left "gutter". Click to set or unset breakpoint. image
image

Thanks.

@Trass3r
Copy link

Trass3r commented Jul 7, 2021

Nice!
It'd be great if the 2 views were merged, showing the original source code inline with the asm like VS does iirc and objdump.

@yuehuang010
Copy link
Contributor Author

yuehuang010 commented Jul 7, 2021

Nice!
It'd be great if the 2 views were merged, showing the original source code inline with the asm like VS does iirc and objdump.

The initial insider is focused getting core functions up and running. Syntax color and Source code next on the todo list.

@weinand
Copy link
Contributor

weinand commented Jul 22, 2021

Please find additional discussions in the associated PR.

I've upgraded Mock Debug to support the "Disassembly View" feature:

2021-07-22_10-31-24 (1)

@learnpythontheew
Copy link

Could this extension add an address searcher like Visual Studio?

@github-actions github-actions bot locked and limited conversation to collaborators Sep 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan
Projects
None yet
Development

Successfully merging a pull request may close this issue.

12 participants