#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 be NULL. 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 Changed on 10/13/22 at 09:54:59 by 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).

So I'll trust your judgment on this!

Again, thanks for suggesting this.

comment:2 in reply to:  1 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: assignedclosed

In d574e08/rtems:

libdebugger: Add a target break call to suspend all running threads

  • Optionally wait if there is no remote debugger connected and break when the remote connects

Closes #4740

Note: See TracTickets for help on using tickets.