microsoft / MIEngine

The Visual Studio MI Debug Engine ("MIEngine") provides an open-source Visual Studio Debugger extension that works with MI-enabled debuggers such as gdb and lldb.
MIT License
818 stars 219 forks source link

Added extended-remote support #1260

Closed xisui-MSFT closed 2 years ago

xisui-MSFT commented 2 years ago

As discussed in https://github.com/microsoft/vscode-cpptools/issues/8497, this PR introduces a way to attach to a process after making remote connection.

A launch option field "useExtendedRemote" is added, which when being set to true, extended-remote mode will be used.

A known limitation is we can't guarantee the specified PID exists. We'll probably need to add a remote process picker in the future.

WardenGnaw commented 2 years ago

Is switching from remote to extended-remote going to break the scenarios where users already have gdbserver already attached to a PID and users are expecting GDB to disconnect from it?

xisui-MSFT commented 2 years ago

Is switching from remote to extended-remote going to break the scenarios where users already have gdbserver already attached to a PID and users are expecting GDB to disconnect from it?

I tried a scenario where only miDebuggerServerAddress needs to be specified, and didn't notice any behavior change.

Also checked the log, MIEngine sends target detach, gdb-exit, done and exit to gdb at the end, and those completely cuts off gdb connection. On the other hand, I think gdb would stay connected if the debugee exits, so the user will need to press 'stop debugging' button again. I'll double check.

chuckries commented 2 years ago

@xisui-MSFT it sounds like you did some testing against this already, which gdbserver scenarios did you test exactly? A few questions:

  1. is -target-select extended-remote supported in gdb-mi? I'm familliar with it as a CLI option, but not an MI option.
  2. Have you tested lldbserver? Does it support extended-remote?
  3. As I said in the code comments, extended-remote works when gdbserver is launched with --attach. What happens when we issues the -target-attach command if gdbserver is launched with --attach?
xisui-MSFT commented 2 years ago

@xisui-MSFT it sounds like you did some testing against this already, which gdbserver scenarios did you test exactly? A few questions:

  1. is -target-select extended-remote supported in gdb-mi? I'm familliar with it as a CLI option, but not an MI option.
  2. Have you tested lldbserver? Does it support extended-remote?
  3. As I said in the code comments, extended-remote works when gdbserver is launched with --attach. What happens when we issues the -target-attach command if gdbserver is launched with --attach?

I only tested with some basic gdb cases. Trying to answer your questions:

  1. What is gdb-mi? Is it what's used by MIEngine? If so, then extended-remote is supported since I've verified this change works for our scenario.
  2. Never tested with lldb.
  3. That's a good question. I'll test this as well.
chuckries commented 2 years ago

@xisui-MSFT MI is GDB's "Machine Interface" (this is where the MI in MIEngine comes from). For most versions of gdb, we invoke it with --interpreter=mi and we are in MI mode instead of the standard GDB CLI. For LLDB, the interpreter flag is not available and typically MI is only available through another executable called lldb-mi. @WardenGnaw can speak more to lldb-mi. In my experience, the MI frontends do not always support everything the CLI frontends do, so we just need to test the use of new commands/parameters.

WardenGnaw commented 2 years ago

Because lldb-mi is under Apache License v2.0, we can look at the source which shows it does not support extended-remote.

  if (rRemoteType != "remote") {
    SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_TYPE),
                                   m_cmdData.strMiCmd.c_str(),
                                   rRemoteType.c_str()));
    return MIstatus::failure;
  }

From the source

We would need to not change it to extended-remote when targeting lldb.

chuckries commented 2 years ago

@WardenGnaw are there any supported scenarios where we are not picking up the lldb-mi that we ship ourselves? one thing to check would be if

  1. lldb + lldbserver supports extended-remote
  2. if so, the mi frontend condition might just be superficial, and we could pass through extended-remote without issue.
gregg-miskelly commented 2 years ago

According to this the start of the documentation that was linked to (https://sourceware.org/gdb/onlinedocs/gdb/Connecting.html):

GDB supports two types of remote connections, target remote mode and target extended-remote mode. Note that many remote targets support only target remote mode

It sounds like we need a new option to indicate if extended-remote or remote should be used instead of always specifying extended-remote.

gregg-miskelly commented 2 years ago

I guess the other option is: only use extended-remote if we know we are in a scenario that requires it (like attach). But I think that is too magical.

xisui-MSFT commented 2 years ago

Looks like most discuss are around how compatible are remote and extended-remote, and lldb. While I didn't find a case where remote and extended-remote are incompatible, I agree that we should provide the option to use remote or extended remote. So I'll change this PR to add a useExtendedRemote option, and add some checks around lldb. Thanks for the advises!

xisui-MSFT commented 2 years ago

@WardenGnaw @chuckries @gregg-miskelly I've added an option "useExtendedRemote" to control when to use extended-remote mode, and modified the title and description of this PR. Can someone please re-review? Thanks!

fbs2016 commented 2 years ago

The option "useExtendedRemote" is supported in cpptools latest v1.10.7? I can't find it in cppdbg options.