Implement disassembly debugging #1041

Trass3r · 2020-08-28T15:23:42Z

Proof of concept of disassembly debugging.
Will fix part of #816 and microsoft/vscode-cpptools#206.

Working:

show disassembly if source is not available
step through assembly code

Not working (in order of importance):

support for -data-disassemble -a to get the entire current function: https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Data-Manipulation.html
setting breakpoints
goto
show disassembly with source code when it is available
support for different scopes

gregg-miskelly · 2020-08-28T18:05:56Z

src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs

@@ -2252,6 +2258,8 @@ private Task<string> ResetConsole()

        public bool IsChildProcessDebugging => _childProcessHandler != null;

+        public string[] Features { get; private set; }


nit: instead of exposing an array, use a IReadOnlyCollection<string>. You probably also want to initialize it to an empty collection since we will only have it for GDB.

lldb doesn't implement that part of the MI?

You put your code that populates under an 'if (gdb)' path. I don't know for sure if LLDB supports -list-features or not, but your code will not assign to it.

Yeah it's probably better as a virtual method in the factory like in the other PR.

gregg-miskelly · 2020-08-28T18:14:33Z

src/OpenDebugAD7/AD7DebugSession.cs

+            BeforeContinue();
+            ErrorBuilder builder = new ErrorBuilder(() => errorMessage);
+            m_isStepping = true;
+            if (!m_currentFrameHasSourceCode)


m_currentFrameHasSourceCode [](start = 17, length = 27)

We need to add a client check here -- we don't want to do this in full Visual Studio (or other IDEs that have full disassembly support)

We should probably check if the top frame of the specified thread has source, since the user can switch threads.

This is used in VS?

gregg-miskelly · 2020-08-28T18:18:27Z

src/OpenDebugAD7/AD7DebugSession.cs

-                        response.StackFrames.Add(new ProtocolMessages.StackFrame()
+                        // remember the state for the lowermost frame
+                        if (startFrame == 0 && i == 0)
+                            m_currentFrameHasSourceCode = textPosition.Source?.Path != null;


m_currentFrameHasSourceCode [](start = 28, length = 27)

You will wind up setting this to true if the top frame of any thread doesn't have source. It needs to be changed to a Dictionary/HashSet. I would probably also suggest changing the name from 'current' to 'top' since 'current frame' has another meaning that can be confusing.

Depends on the UI I guess. VSCode probably fetches only the current thread so it works.

No, VS Code fetches the stack of all threads very aggressively.

Ok. Haven't noticed any fallout yet though.
While without the if I have. All frames of a thread were marked.

gregg-miskelly · 2020-08-28T18:21:21Z

src/OpenDebugAD7/AD7DebugSession.cs

+            {
+                int sourceRef = (int)source.SourceReference;
+                string content = TextPositionTuple.GetSourceForRef(sourceRef);
+                response.MimeType = "text/x-lldb.disassembly";


x-lldb [](start = 42, length = 6)

Does something in VS Code already understand this type?

It's borrowed from the CodeLLDB extension.
Asm CodeLens doesn't support this kind of disassembly yet.

I don't know what VS Code uses the MimeType for. Does it matter what we put? Does it matter if CodeLLDB isn't installed?

If it isn't installed you won't get syntax highlighting. If it is you will.

gregg-miskelly · 2020-08-28T18:24:25Z

src/OpenDebugAD7/TextPositionTuple.cs

@@ -24,30 +36,132 @@ private TextPositionTuple(Source source, int line, int column)
        public static TextPositionTuple GetTextPositionOfFrame(PathConverter converter, IDebugStackFrame2 frame)


GetTextPositionOfFrame [](start = 40, length = 22)

Similar to my suggestion in the other review - you don't want to do this here, but rather in HandleStackTraceRequestAsync because there is no need to do this stuff for stopping event processing.

Well here it's deliberate, sort of. VSCode didn't use the stopped event source but maybe other clients do?

The source in the stopped event was actually an extension added long ago to support testing. It should be fine to just use the original path.

Ha, something to add to the spec.

gregg-miskelly · 2020-08-28T18:25:29Z

src/OpenDebugAD7/TextPositionTuple.cs

            }

+            // this frame is lacking source code
+            source.Name = Path.ChangeExtension(source.Name, ".s") ?? "???";


Name [](start = 54, length = 4)

source.Name may be null. If your goal is to make a document of the disassembly of this function, maybe something like: Disassembly: <Func-Name>

Which is fine according to the ChangeExtension docs.
The goal is to make file extension based syntax highlighting work.

gregg-miskelly · 2020-08-28T18:28:52Z

src/OpenDebugAD7/TextPositionTuple.cs


-                return new TextPositionTuple(source, line, column);
+                    return new TextPositionTuple(source, srcline, srccolumn);
+                }
            }


} [](start = 12, length = 1)

A few comments for the rest of this:

We need a client check - we don't want to do this stuff with an IDE that has real disassembly support

I don't think you want to do the disassembly up front. Instead we should keep a collection of sourceReferences and do the disassembly when the IDE requests source.

Makes sense.
The thing is there seems to be no way to find the start of the current function in gdb.
So different address requests belonging to the same function without actually disassembling seem to be impossible to relate.

Looks like VS uses DisassembleRequest though?
b2a5c7a

Yes, in the VS case, we want to return whatever source path we have in the symbols. VS is fine with source paths that don't exist and will request disassembly if that is what the user chooses to do.

But how to handle this? I guess there'd need to be a client support flag to do it properly.

Do you mean how to check for VS? If yes, Gabrielle added support for this to AD7Session recently: https://github.com/gregg-miskelly/MIEngine/blob/896c72d633b39a0571b9599d15c5a8fd0f1fcf58/src/OpenDebugAD7/AD7DebugSession.cs#L632-L638

…ence implemented in TextPositionTuple so both StackTrace and StoppedEvent get it even though the stopped event source is currently not used by vscode for now we leverage CodeLLDB's disasm language definition

display stack frames without source differently in the UI

gregg-miskelly · 2021-07-15T19:40:13Z

@Trass3r The VS Code team is currently working on a real disassembly window: microsoft/vscode#124163 so I don't think we will need this anymore.

Trass3r · 2021-07-15T20:33:56Z

Yeah I know. Closing this.

Trass3r mentioned this pull request Aug 28, 2020

avoid "Unable to open file" popups caused by missing source code #1038

Merged

gregg-miskelly reviewed Aug 28, 2020

View reviewed changes

microsoft deleted a comment from Trass3r Aug 28, 2020

gregg-miskelly reviewed Aug 28, 2020

View reviewed changes

WardenGnaw changed the base branch from master to main September 11, 2020 21:29

Trass3r force-pushed the disassembly branch from 4999042 to 75601e6 Compare December 22, 2020 21:59

Trass3r added 5 commits July 15, 2021 20:42

sealed

d405947

add Loyc for their Dictionary with FindUpperBound method

2b60f33

show disassembly when source code is missing by sending a SourceRefer…

e13ebf1

…ence implemented in TextPositionTuple so both StackTrace and StoppedEvent get it even though the stopped event source is currently not used by vscode for now we leverage CodeLLDB's disasm language definition

enable stepping in disassembly

1516418

display stack frames without source differently in the UI

when an exception is thrown in the target negative offsets can occur

26d026b

Trass3r force-pushed the disassembly branch from 75601e6 to 26d026b Compare July 15, 2021 18:46

Trass3r closed this Jul 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement disassembly debugging #1041

Implement disassembly debugging #1041

Trass3r commented Aug 28, 2020

gregg-miskelly Aug 28, 2020

Trass3r Aug 28, 2020

gregg-miskelly Aug 28, 2020

Trass3r Aug 28, 2020

gregg-miskelly Aug 28, 2020

Trass3r Aug 29, 2020

gregg-miskelly Aug 31, 2020

gregg-miskelly Aug 28, 2020

Trass3r Aug 28, 2020

gregg-miskelly Aug 28, 2020

Trass3r Aug 28, 2020 •

edited

Loading

gregg-miskelly Aug 28, 2020

Trass3r Aug 28, 2020

gregg-miskelly Aug 28, 2020

Trass3r Aug 28, 2020

gregg-miskelly Aug 28, 2020

Trass3r Aug 28, 2020

gregg-miskelly Aug 28, 2020

Trass3r Aug 28, 2020

gregg-miskelly Aug 28, 2020 •

edited

Loading

Trass3r Aug 28, 2020 •

edited

Loading

gregg-miskelly Aug 28, 2020 •

edited

Loading

Trass3r Aug 28, 2020

Trass3r Dec 22, 2020

gregg-miskelly Jan 8, 2021

Trass3r Jan 8, 2021

gregg-miskelly Jan 8, 2021

gregg-miskelly commented Jul 15, 2021

Trass3r commented Jul 15, 2021

		@@ -2252,6 +2258,8 @@ private Task<string> ResetConsole()

		public bool IsChildProcessDebugging => _childProcessHandler != null;

		public string[] Features { get; private set; }

		@@ -24,30 +36,132 @@ private TextPositionTuple(Source source, int line, int column)
		public static TextPositionTuple GetTextPositionOfFrame(PathConverter converter, IDebugStackFrame2 frame)

Implement disassembly debugging #1041

Implement disassembly debugging #1041

Conversation

Trass3r commented Aug 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trass3r Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregg-miskelly Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

Trass3r Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

gregg-miskelly Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregg-miskelly commented Jul 15, 2021

Trass3r commented Jul 15, 2021

Trass3r Aug 28, 2020 •

edited

Loading

gregg-miskelly Aug 28, 2020 •

edited

Loading

Trass3r Aug 28, 2020 •

edited

Loading

gregg-miskelly Aug 28, 2020 •

edited

Loading