Opened on 10/13/22 at 09:51:53
Closed on 10/18/22 at 22:36:41
#4740 closed enhancement (fixed)
Teach libdebugger to wait for GDB to connect and ask to continue
Reported by: | Kévin Le Gouguec | Owned by: | Chris Johns |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | lib/debugger | Version: | |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: |
Description
In #4739, Chris noted this snippet in the reproducer:
/* Debugger lock, there is no way to ask the libdebugger to wait for a * connection, that would block the kernel. Instead use the following global * variable and 'set release_debugger = 1' in GDB when ready. */ volatile int release_debugger = 0; static void start_gdb_server(void) { rtems_printer printer; int r; rtems_print_printer_fprintf(&printer, stdout); r = rtems_debugger_register_tcp_remote(); if (r < 0) { perror("TCP remote register"); abort(); } r = rtems_debugger_start("tcp", "1122", 3, 1, &printer); if (r < 0) { perror("can't start debugger"); abort(); } /* Definitely not nice, but it doesn't seem like there is any other choice * for libdebugger based debugging. GDB just has to setup and only then * set release_debugger variable. */ while (!release_debugger) { sleep(0.1); } }
Chris suggests adding an option to make libdebugger halt execution until GDB connects and sends a "continue", which would make the debugging experience similar to targeting QEMU with the -S (uppercase S) switch:
https://www.qemu.org/docs/master/system/gdb.html#gdb-usage
We would appreciate that option very much, as it would make our testsuite support code for RTEMS more akin to other platforms by getting rid of that release_debugger variable.
The API has yet to be defined:
Don't know how that would translate in terms of API (adding an argument to rtems_debugger_start, if breaking the API is allowed; adding a function to request the wait after rtems_debugger_start has been called; …);
I think breaking the API may be worth the short term pain. I tend to prefer a
const char* option
type argument that can beNULL
. That argument can then be passed around and parsed by backends, transports etc and options can come and go without breaking the API.
I know some programmers feel strongly against so-called "stringly-typed" APIs (e.g. they subvert type-checking; they require "
Change History (5)
comment:1 follow-up: 2 Changed on 10/13/22 at 09:54:59 by Kévin Le Gouguec
comment:2 Changed on 10/14/22 at 02:12:44 by Chris Johns
Replying to Kévin Le Gouguec:
(Woops, looks like part of my OP got bitten off. Here it is, without the weird Unicode characters that presumably caused it to be truncated?)
I know some programmers feel strongly against so-called "stringly-typed" APIs (e.g. they subvert type-checking; they require "parsing"), but I've seen reasonable cases for them; FWIW I do not have a clear preference for or against them in this context (C type-checking is a bit of a joke so not much is lost; parsing can be fraught but libdebugger probably does not belong in a critical stack anyway).
The statements are true and valid however given the context I see more value in a set of string options. The link you provide is great. I will add to the list of examples the NFS options to a kernel such as FreeBSD.
Libdebugger will expand and grow in ways we might not appreciate today and I do not see value in maintaining C code to do this. The first example is continue
. I would make waiting the default as GDB does when you connect to a running process. A continue option would be:
"cont"
Now a setting for a transports such as serial (which does not exist):
cont;serial=tty0:115000,8,1,e
and in time some adds support to load XML files so registers for a target SoC can be seen by GDB:
"cont;serial=tty0:115000,8,1,e;xml=/xx/soc-abc.xml"
The options can be easily and cheaply passed around and then used as needed by different subsystems in libdebugger
.
So I'll trust your judgment on this!
It is also the lazy alternative so maybe you should not ;)
Again, thanks for suggesting this.
No problem.
comment:3 Changed on 10/17/22 at 20:30:38 by Chris Johns
I took a closer look at the debug server code and saw a simpler solution. I have have added a new call:
/** * Suspend all running threads including the caller if not * excluded. Returns when the debugger has connected and continued. * * If wait is true and there is no remote connected wait then break. */ extern int rtems_debugger_break(bool wait);
I think this is simpler and cleaner to implement and use and it also lets users make the call anytime to break the target.
The patch can be found here https://lists.rtems.org/pipermail/devel/2022-October/073581.html
comment:4 Changed on 10/18/22 at 09:23:52 by Kévin Le Gouguec
I gave the patch a whirl, removing the release_debugger
variable in our code and replacing the while
loop with a call to rtems_debugger_break(true)
, and as far as I can tell, it works pretty well.
Thanks for implementing this!
comment:5 Changed on 10/18/22 at 22:36:41 by Chris Johns <chrisj@…>
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
In d574e08/rtems:
(Woops, looks like part of my OP got bitten off. Here it is, without the weird Unicode characters that presumably caused it to be truncated?)
I know some programmers feel strongly against so-called "stringly-typed" APIs (e.g. they subvert type-checking; they require "parsing"), but I've seen reasonable cases for them; FWIW I do not have a clear preference for or against them in this context (C type-checking is a bit of a joke so not much is lost; parsing can be fraught but libdebugger probably does not belong in a critical stack anyway).
So I'll trust your judgment on this!
Again, thanks for suggesting this.