implement reverse execution for gdb

[Trass3r]

fixes microsoft/vscode-cpptools#2130
See microsoft/vscode-cpptools#2130 (comment)

[WardenGnaw]

@Trass3r Thank you for making this contribution!

We will want to check if gdb can do reverse commands. You will need to execute -list-target-features and check for reverse.

This will be done in src/MICore/CommandFactories/MICommandFactory.cs as an abstract method, and for cldbg/lldb it will be

return Task.FromResult(new Results(ResultClass.None));

and for gdb it will be:

Results results = await _debugger.CmdAsync("-list-target-features", ResultClass.done);	
return results;

You will want to use the results to see if we have reverse. This will be used for IDebugReversibleEngineProgram160.CanReverse

[Trass3r]

We will want to check if gdb can do reverse commands. You will need to execute -list-target-features and check for reverse.

The problem is we need to advertise the capability before the target is even launched.

[WardenGnaw]

There is a CapabilitiesEvent that can be sent if it has changed. We are not sure if VS Code supports changing the UI for reverse if the capabilities have changed.

[Trass3r]

Drafted something.

[Trass3r]

Done, it's a lot cleaner now.
I even implemented stepping granularity. microsoft/vscode#102236

[Trass3r]

Done.

[WardenGnaw]

@Trass3r For the launch scenario, do you have -interpreter-exec console \"target record-full\" in the setup command?

[Trass3r]

No. Even in gdb itself there are different methods to record and then there's the next level rr tool which is what I use.
You can't hardcode the method.

[WardenGnaw]

Do we need to worry that someone would enable reverse execution in the middle of debugging? We only check and enable Reverse functionality in Launch/Attach. Is there a set of commands we can capture to resend the capabilities?

[Trass3r]

That's a good question, is it possible with the gdb way?
Using rr it's a two stage procedure. First you record the entire run, then you run rr disguising as gdb with full forward and backward capability.

[WardenGnaw]

I've never used RR, but its good to know that it works in that scenario.
This is how I was testing with GDB.

-list-target-features
^done,features=[]
(gdb)
-exec-step
^running
*running,thread-id="all"
(gdb)
*stopped,reason="end-stepping-range",frame={addr="0x0000000008001224",func="main",args=[],file="main.cpp",fullname="/home/waan/cpp/main.cpp",line="7",arch="i386:x86-64"},thread-id="1",stopped-threads="all",core="0"
(gdb)
-exec-step --reverse
^error,msg="Target native does not support this command."
(gdb)
target record-full
&"target record-full\n"
=record-started,thread-group="i1",method="full"
^done
(gdb)
-list-target-features
^done,features=["reverse"]
(gdb)
-exec-step
^running
*running,thread-id="all"
(gdb)
*stopped,reason="end-stepping-range",frame={addr="0x000000000800139a",func="std::chrono::duration<long, std::ratio<1l, 1000l> >::duration<int, void>",args=[{name="this",value="0x80018bd <__libc_csu_init+77>"},{name="__rep",value="<error reading variable>"}],file="/usr/include/c++/9/chrono",fullname="/usr/include/c++/9/chrono",line="331",arch="i386:x86-64"},thread-id="1",stopped-threads="all",core="0"
(gdb)
-exec-step --reverse
^running
*running,thread-id="1"
(gdb)
*stopped,reason="end-stepping-range",frame={addr="0x000000000800122b",func="main",args=[],file="main.cpp",fullname="/home/waan/cpp/main.cpp",line="7",arch="i386:x86-64"},thread-id="1",stopped-threads="all",core="0"
(gdb)

We may want a check in HandleEvaluateRequestAsync and if its a -exec with record, check for target-features and update the capabilities.

See suggestion in #1037 (comment)

[Trass3r]

I see, makes sense.

[pirpyn]

Do we need to worry that someone would enable reverse execution in the middle of debugging? We only check and enable Reverse functionality in Launch/Attach. Is there a set of commands we can capture to resend the capabilities?

Hello, I'm using also UndoDB which record from the beginning, but with gdb I only record specific portion of code.
So yeah, as a user, that something i'm likely to do.

[WardenGnaw]

@Trass3r Ignore my suggestion above about capturing commands for users in HandleEvaluateAsync. We could detect recording when GDB sends the output
=record-started,thread-group="i1",method="full" then update the capabilities

Associated stop string:

(gdb)
=record-stopped,thread-group="i1"

[gregg-miskelly]

FYI from https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Async-Records.html:

=record-started,thread-group="id",method="method"[,format="format"]
=record-stopped,thread-group="id"

Execution log recording was either started or stopped on an inferior. The id is the GDB identifier of the thread group corresponding to the affected inferior.

The method field indicates the method used to record execution. If the method in use supports multiple recording formats, format will be present and contain the currently used format. See Process Record and Replay, for existing method and format values.

[Trass3r]

Something like this (see commit). The question is how to proceed from there.

[Trass3r]

@gregg-miskelly Could you also approve the workflow here? I don't know why it suddenly asks for this again for every PR.

[gregg-miskelly]

            Protocol.SendEvent(new InitializedEvent());

You want to call UpdateCapabilities here instead of after LaunchSuspended.

---

Refers to: src/OpenDebugAD7/AD7DebugSession.cs:3516 in d84213d. [](commit_id = d84213d, deletion_comment = False)

[gregg-miskelly]

@gregg-miskelly Could you also approve the workflow here? I don't know why it suddenly asks for this again for every PR.

It looks like someone did this for you already right? Or did I miss something?

[Trass3r]

@gregg-miskelly Looks like your review unlocked it. Now that I pushed the next commit it's 1 workflow awaiting approval again.

[WardenGnaw]

@gregg-miskelly Could you also approve the workflow here? I don't know why it suddenly asks for this again for every PR.

It looks like someone did this for you already right? Or did I miss something?

Sorry, that was me. I had a moment during my break to poke the "Approve button". I just approved it again.

[gregg-miskelly]

@Trass3r I see that your PR still has some TODO comments. Would you consider this ready for a full review? Or are you still working on it.?

[gregg-miskelly]

// TODO: notify OpenDebugAD7 to run AD7DebugSession.UpdateCapabilities

Do you still want to do this? Or are you happy with the way you have implemented this?

[Trass3r]

Wouldn't know how. Just an idea to make things more asynchronous rather than checking reversibility on every stop event.
But yeah, no major further development planned. Just want to get these old PRs merged.

[gregg-miskelly]

Got it. In that case I would probably suggest changing the comment to something like:

// NOTE: Currently OpenDebugAD7 polls to decide if we can reverse execute. It might make sense to push an event. If we ever create such an event, probably makes sense to use it here also.

[Trass3r]

Ok. Btw the events are RecordStartedEvent and RecordStoppedEvent for gdb. And we check it after starting up to support rr.

[Trass3r]

Btw, for testing:

int main()
{
	int i = 0;
	++i;
	++i;
	return i;
}

$ gdb --batch -ex 'b main' -ex r -ex n -ex 'p i' -ex 'record full' -ex n -ex 'p i' -ex n -ex 'p i' -ex n -ex 'p i' -ex 'reverse-next' -ex 'p i' -ex 'reverse-next' -ex 'p i' -ex 'reverse-next' -ex 'p i' test

3           int i = 0;
completed.
+p i
$1 = 17
+record full
+target record-full
+n
4           ++i;
completed.
+p i
$2 = 0
+n
5           ++i;
completed.
+p i
$3 = 1
+n
6               return i;
completed.
+p i
$4 = 2
+reverse-next
+next 
5           ++i;
completed.
+p i
$5 = 1
+reverse-next
+next 
4           ++i;
completed.
+p i
$6 = 0
+reverse-next
+next 

No more reverse-execution history.
main () at /tmp/test.cpp:3
3           int i = 0;
completed.
+p i
$7 = 17

[Trass3r]

Any ideas for improving the UX when reaching the boundaries of the recorded data:

https://github.com/microsoft/MIEngine/pull/1037/files#diff-e6a6b2741c810dcffe4bf13d4c697e2a326b9aa13752825ee0544977797273b0R1409-R1416

[gregg-miskelly]

Any ideas for improving the UX when reaching the boundaries of the recorded data:

First, I would suggest trying a quick hack where you patch up the properties in OpenDebugStoppedEvent to see what it will take to make VS Code display something good -- do we just need to fix the Description? The Description and then Reason? etc.

If you can get that to a place you want, then it will give me a better idea in the MIEngine side.

[Trass3r]

First, I would suggest trying a quick hack where you patch up the properties in OpenDebugStoppedEvent to see what it will take to make VS Code display something good -- do we just need to fix the Description?

Well several issues.
MIEngine is abusing the exception reason in that place. It's meant for exceptions in the target process (at least it's presented like that in the UI), not in the debug adapter. They do allow a generic | string reason:
https://microsoft.github.io/debug-adapter-protocol/specification#Events_Stopped
But enum Microsoft.VisualStudio.Shared.VSCodeDebugProtocol.Messages.StoppedEvent.ReasonValue is pretty fixed.

And VSCode ignores the description field for exceptions, violating the spec. Only text is shown, the title is always 'Exception has occurred'.

{"type":"event","event":"stopped","body":{"reason":"exception","description":"Unknown","threadId":25608,"text":"Unknown stopping event","allThreadsStopped":true,"source":{"name":"test.cpp","path":"/tmp/test.cpp","sources":[],"checksums":[]},"line":3,"column":1},"seq":960}

Maybe due to https://github.com/microsoft/vscode/blob/a7d2cfe5c56ac960584c9b8a7127fd5450792aa4/src/vs/workbench/contrib/debug/common/debugModel.ts#L606.

I also created a gdb ticket for the missing reason: https://sourceware.org/bugzilla/show_bug.cgi?id=29260

[Trass3r]

@gregg-miskelly but I guess that issue can be captured in a ticket and fixed separately?!

[Trass3r]

I also created a gdb ticket for the missing reason: https://sourceware.org/bugzilla/show_bug.cgi?id=29260

Finally fixed.
bminor/binutils-gdb@37f5406

[dslijepcevic]

Hi, is there any update on this issue? Thanks

[Trass3r]

Nope it got stuck in review hell.