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

MIMode "gdb" wrongly assumes the type of Inferior; debugger pause does not work #844

Closed
haneefdm opened this issue Apr 3, 2019 · 12 comments
Assignees
Labels

Comments

@haneefdm
Copy link
Contributor

haneefdm commented Apr 3, 2019

Okay, here is the scenario. Sorry, long

I am using GDB on my Mac (via MIEngine/cpptools) to debug a program/image running on a board connected to my USB/serial port. Pretty much everything works except for one huge thing. I cannot pause a running target. I can pause it using any other tool (Eclipse, gdb command line, even another VSCode extension, etc.). This is the 99.9% use case for embedded/IoT applications where they can't actually afford to have an entire OS/gdb running on the board. When I try to pause, nothing happens. I pretty much have to kill the VSCode Debug session, find and kill the gdb-server and/or gdb itself.

I think I know why this is happening. In MICore/src/Debugger.cs, around line 707, you check to see if it is local gdb and assumes program is local too.

Then there is https://sourceware.org/bugzilla/show_bug.cgi?id=20035.

Well, that is only applicable to gdb executing native prorgrams. In our case, gdb is debugging a quasi-remote program in async mode. It is called "remote serial" or "extended remote serial". The local gdb talks to a gdb-server either using a serial port, local/remote IP address See -target-select doc.

Problem is that gdb-servers can create fake pids and thread-groups if they have to and gdb hasn't got a clue they are fake. Since MIEngine monitors and collects these pids, it tries to send a SIGINT to the debuggee which will never work. Might even kill an innocent process on local machine? :-).

Using the gdb command info target When you debug something with gdb, you can figure out what kind of target it is even though gdb and executable are local...

With gdb running on my Mac and board connected to a USB port, it shows.

Symbols from "/Users/hdm/tmp/cystuff/demo/main_final.elf".
Extended remote serial target in gdb-specific protocol:
Debugging a target over a serial line.
	While running this, GDB does not access memory from...
Local exec file:
	`/Users/hdm/tmp/cystuff/demo/main_final.elf', file type elf32-littlearm.
	Entry point: 0x100802ce
	0x10080000 - 0x100811e4 is .text
        ...

With gdb running a local process on Windows/Cygwin, it will say 'Native process'

Symbols from "/cygdrive/j/VSCode/adv-cppdbg/test/stdc-threads/bin/main.exe".
Native process:
	Using the running image of child Thread 27012.0x5d9c.
	While running this, GDB does not access memory from...
Local exec file:
	`/cygdrive/j/VSCode/adv-cppdbg/test/stdc-threads/bin/main.exe', file type pei-x86-64.
	Entry point: 0x100401000
	0x0000000100401000 - 0x0000000100403040 is .text

Applies to ALL platforms but just for gdb. If you don't see 'Native process', you are debugging something not local. And '-exec interrupt' will work. I have proof. Signaling the Inferior obviously wont work. Unfortunately, I don't have a MI equivalent of info target. I just did -exec info target You send an -exec-interrupt, gdb says, since I am running remote mode, I will pass it on to the gdb-server and they all report back to MIEngine/VSCode/User

You want to figure out the target type at the start of the session. There is another way, which is to look at the startup commands. Anyone who is doing this will have to execute '-target-select' command during startup, but there are 2-3 ways of doing it syntax wise. Logic for IsLocalGdb() and IsRemoteGdb() has to be reviewed and how they are used. May need isLocalGdbRemoteTarget() for instance. Lucky, these are internal/private all contained in that same file.

Let me know if you want my help fixing/testing this. I apologize, if I misread the code in MIEngine.

@haneefdm
Copy link
Contributor Author

haneefdm commented Apr 3, 2019

If my analysis is right (I hope I am wrong), it is a super serious problem. MIEngine might kill a random process Wanted to stress that.

@cynecx
Copy link

cynecx commented Apr 28, 2019

I think this is related to this issue: microsoft/vscode-cpptools#833.

@haneefdm
Copy link
Contributor Author

@cynecx Yes, it is. Thanks for the pointer. But there is more to it. When gdb is using a gdbserver (local or remote), pause/stop works everywhere now. I am testing my fix right now. Early next week, I hope to do a PR. If I can get cygwin/mingw to work reliably without a gdbserver, then we are golden. Lot of combinations to test :-)

The arm-none-eabi-gdb on Windows is actually a mingw compiled executable. This GDB was configured as "--host=i686-w64-mingw32 --target=arm-none-eabi"
`
There is another (separate) minor issue with 'stopAtEntry'. While it works, there is a better implementation. MIEngine should use a temporary breakpoint for this as breakpoints are very limited in embedded devices. My workaround is to not use this feature and do issue a temp. breakpoint in startup commands.

@gregg-miskelly
Copy link
Member

gregg-miskelly commented Apr 30, 2019

Is there a reason you are using:

            "setupCommands": [
...
                {
                    "text": "target remote localhost:4242"
                },

Instead of: "miDebuggerServerAddress": "localhost:4242",?

The reason this does the wrong thing is that MIEngine doesn't understand that it is doing remote debugging here.

@haneefdm
Copy link
Contributor Author

Hi @gregg-miskelly, not sure where you saw the 4242, but it is true that we use something like that
-target-select remote localhost:3333. We don't use the miDebuggerServerAddress because that requires us to launch the server separately (in another shell window). We typically launch both the server and gdb from the same button.... and they are kinda synchronized and far fewer manual steps.

A typical embedded setup looks something like

            "debugServerArgs": "-s ${workspaceRoot} -s /Applications/ModusToolbox_2.0/tools/openocd-2.2/scripts -s C:/Users/hdm/ModusToolbox_2.0/tools/openocd-2.2/scripts -f openocd.tcl",
            "serverStarted": "Info :.*Listening on port [0-9]+ for gdb connections",
            "setupCommands": [
                { "text": "-environment-cd ${workspaceRoot}" },
                { "text": "-gdb-set target-async on" },
                { "text": "-target-select extended-remote localhost:3333", "description": "connect to target", "ignoreFailures": false },
                { "text": "-file-exec-and-symbols main_final.elf", "description": "load file", "ignoreFailures": false},
                { "text": "-interpreter-exec console \"monitor reset init\""},
                { "text": "-target-download", "description": "flash target", "ignoreFailures": false },
                { "text": "-interpreter-exec console \"monitor reset run\""},
                { "text": "-interpreter-exec console \"monitor sleep 200\""},
                { "text": "-interpreter-exec console \"monitor psoc6 reset_halt sysresetreq\""},
                { "text": "-enable-pretty-printing" }
            ],
            "osx": {
                "MIMode": "gdb",
                "MIDebuggerPath": "arm-none-eabi-gdb",
                "debugServerPath": "/Applications/ModusToolbox_2.0/tools/openocd-2.2/bin/openocd"
            },

The first two lines in setupCommands are not needed because MIEngine already does this. And we need to select between 'remote' and 'extended-remote'. Doing it in setupCommands is more convenient and if we don't do it there, then we cannot erase/load the program into flash, do a proper reset/start and all that magic. gdbservers like OpenOCD/PyOCD, expect all that work to be done via gdb and don't provide any other way of doing it.

From what I understand, you either use the miDebuggerServerAddress or use the debugServerPath, not both. Btw, this same technique also works with cygwin and be able to pause using the normal debug buttons .. I know the workaround is to Control-C in the shell window. It only requires about 4 lines in the launch.json file, I don't recommend it but I am testing all the combinations.

When you use the '-target-select' command, I found out that gdb sends a pretty unique response and that is all we have to look for in MIEngine.

@haneefdm
Copy link
Contributor Author

Btw, @gregg-miskelly, a remote target can also be a serial device like COM port or a /dev/ttyX which I don't think you can do using miDebuggerServerAddress

I am finished testing but I'd be thrilled if you can review my changes here... I only had to touch one file, very few changes.

I did not have a Linux box, and I am in the process of installing one.

@gregg-miskelly
Copy link
Member

Using mode == "connected" sounds reasonable. I think you would want to change IsLocalGdb to return false for these scenarios rather than making the change you have on line 620.

@gregg-miskelly
Copy link
Member

I should add, your check is trying to determine exactly the same as string.IsNullOrWhiteSpace(localLaunchOptions.MIDebuggerServerAddress). So you probably want to scrutinize any place where we are checking that. Unfortunately some of the places we check for that are during initialization when we probably haven't received the 'connected' massage. But here are two more places that you could fix:

                // MinGW sends a stopped event on attach. gdb<->gdbserver also sends a stopped event when first attached.
                // If this is a gdb<->gdbserver connection, ignore this as the entryPoint
                if (this._launchOptions is LocalLaunchOptions &&
                    !String.IsNullOrWhiteSpace(((LocalLaunchOptions)this._launchOptions).MIDebuggerServerAddress))
        private bool IsRemoteGdb()
        {
            return this.MICommandFactory.Mode == MIMode.Gdb &&
               (this._launchOptions is PipeLaunchOptions || this._launchOptions is UnixShellPortLaunchOptions ||
               (this._launchOptions is LocalLaunchOptions
                    && !String.IsNullOrEmpty(((LocalLaunchOptions)this._launchOptions).MIDebuggerServerAddress)));
        }

@haneefdm
Copy link
Contributor Author

haneefdm commented May 1, 2019

@gregg-miskelly, I had initially changed the implementations of IsLocalGdb and IsRemoteGdb, but I got uncomfortable with the original names (imply) of those methods and backed the changes out. May I propose the following:

  • I will fold the IsTargetRemoteOrExtendedRemote uses into IsLocalGdb and IsRemoteGdb
  • Rename IsLocalGdb and IsRemoteGdb to IsLocalGdbTarget and IsRemoteGdbTarget, with your permission
  • Look for every place such checks are made to see what needs to be corrected - as you suggested
  • I will leave the IsTargetRemoteOrExtendedRemote impl. alone as lldb also uses this and it will be good to know in the future if any debugger has used a remote target independent of gdb
  • Remove the two Debug.Asserts() I added because I am less paranoid now.
  • Test all the scenarios I can think of

Boy, cygwin is a beast to deal with. I tried so many things to make it behave to no avail. I tried GenerateConsoleCtrlEvent, sending SIGINT to gdb itself and none of it works. However, if connected to a gdbserver, it behaves properly with a -exec-interrupt. For development, I always stayed away from cygwin and mingw has its share of bugs (lots of buffer overflows).

@pieandcakes pieandcakes self-assigned this May 7, 2019
@pieandcakes pieandcakes added the bug label May 7, 2019
gregg-miskelly pushed a commit that referenced this issue May 14, 2019
…ing gdbservers (#855)

* Support local gdb but remote target, testing ongoing: checkpoint

* Intermediate checkpoint, untested

* Intermediate checkpoint, untested

* Implement Pausing MIDebuggerServerAddress & DebugServer instead of 'connected' message

* Removed a debug stmt

* Be consistent with IsNullOrEmpty vs. the preferred IsNullOrWhiteSpace

* Broke up a long line

* Fided a defect in IsLocalGdbTarget() from code review. To make it mean same thing as before
@pieandcakes
Copy link
Collaborator

@haneefdm this is now in vscode-cpptools version 0.24.0-insiders

@haneefdm
Copy link
Contributor Author

Cool. Thanks.

@fbs2016
Copy link

fbs2016 commented Sep 1, 2021

I meet similar issue on latest extension microsoft/vscode-cpptools#7719 Unable to set breakpoint or pause on a running application with extended-remote connection debug.
@haneefdm Could you share the following operation? Thanks
I know the workaround is to Control-C in the shell window. It only requires about 4 lines in the launch.json file,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants