This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Fix gdb.mi internal_error on killing inferior
On Wednesday 20 May 2009 22:34:41, Jan Kratochvil wrote:
> On Wed, 20 May 2009 22:43:17 +0200, Pedro Alves wrote:
> > Weird, I don't see this happening.
>
> Another reproducer + a backtrace below:
> echo -e '#include <unistd.h>\nint main(void){pause();return 0;}'|gcc -Wall -o pauset -x c -pthread -;echo -e '-gdb-set target-async 1\n-exec-run &\n-target-attach\ny' >cmd;../gdb -nx -i=mi ./pauset <./cmd
>
Thanks. This one is tripping on 10 year old FIXMEs. You've started
the target in the background (-exec-run &), meaning that
GDB shouldn't try to set the inferior's terminal modes, yet,
attach->kill ended up calling target_terminal_ours, and
terminal_ours_1 claims that the target got the terminal.
This patches fixes your test above for me. I'm running
the full testsuite on {sync|async}/{remote|native}, after
having run a few smoke test .exp's and seeing no problem.
Does it work for you?
--
Pedro Alves
2009-05-21 Pedro Alves <pedro@codesourcery.com>
* linux-nat.c (linux_nat_terminal_inferior)
(linux_nat_terminal_ours): Don't check sync_execution.
* remote.c (remote_terminal_inferior, remote_terminal_ours):
Don't check sync_execution. Update comments.
* target.c (target_terminal_inferior): New.
* target.h (target_terminal_inferior): Delete macro, and declare
as function.
* event-top.c (async_disable_stdin): Make idempotent. Don't give
the target the terminal here.
* inflow.c (terminal_ours_1): Don't return early without setting
`terminal_is_ours'.
---
gdb/event-top.c | 13 +++++--------
gdb/inflow.c | 5 ++---
gdb/linux-nat.c | 10 +---------
gdb/remote.c | 22 ++++++----------------
gdb/target.c | 12 ++++++++++++
gdb/target.h | 3 +--
6 files changed, 27 insertions(+), 38 deletions(-)
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c 2009-05-21 00:56:59.000000000 +0100
+++ src/gdb/linux-nat.c 2009-05-21 01:18:04.000000000 +0100
@@ -4366,14 +4366,9 @@ linux_nat_terminal_inferior (void)
return;
}
- /* GDB should never give the terminal to the inferior, if the
- inferior is running in the background (run&, continue&, etc.).
- This check can be removed when the common code is fixed. */
- if (!sync_execution)
- return;
-
terminal_inferior ();
+ /* Calls to target_terminal_*() are meant to be idempotent. */
if (!async_terminal_is_ours)
return;
@@ -4399,9 +4394,6 @@ linux_nat_terminal_ours (void)
but claiming it sure should. */
terminal_ours ();
- if (!sync_execution)
- return;
-
if (async_terminal_is_ours)
return;
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c 2009-05-21 00:56:59.000000000 +0100
+++ src/gdb/remote.c 2009-05-21 01:16:00.000000000 +0100
@@ -4078,19 +4078,12 @@ remote_terminal_inferior (void)
/* Nothing to do. */
return;
- /* FIXME: cagney/1999-09-27: Shouldn't need to test for
- sync_execution here. This function should only be called when
- GDB is resuming the inferior in the forground. A background
- resume (``run&'') should leave GDB in control of the terminal and
- consequently should not call this code. */
- if (!sync_execution)
- return;
- /* FIXME: cagney/1999-09-27: Closely related to the above. Make
- calls target_terminal_*() idenpotent. The event-loop GDB talking
- to an asynchronous target with a synchronous command calls this
- function from both event-top.c and infrun.c/infcmd.c. Once GDB
- stops trying to transfer the terminal to the target when it
- shouldn't this guard can go away. */
+ /* FIXME: cagney/1999-09-27: Make calls to target_terminal_*()
+ idempotent. The event-loop GDB talking to an asynchronous target
+ with a synchronous command calls this function from both
+ event-top.c and infrun.c/infcmd.c. Once GDB stops trying to
+ transfer the terminal to the target when it shouldn't this guard
+ can go away. */
if (!remote_async_terminal_ours_p)
return;
delete_file_handler (input_fd);
@@ -4109,9 +4102,6 @@ remote_terminal_ours (void)
return;
/* See FIXME in remote_terminal_inferior. */
- if (!sync_execution)
- return;
- /* See FIXME in remote_terminal_inferior. */
if (remote_async_terminal_ours_p)
return;
cleanup_sigint_signal_handler (NULL);
Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c 2009-05-21 00:56:59.000000000 +0100
+++ src/gdb/target.c 2009-05-21 00:57:28.000000000 +0100
@@ -301,6 +301,18 @@ target_create_inferior (char *exec_file,
"could not find a target to create inferior");
}
+void
+target_terminal_inferior (void)
+{
+ /* A background resume (``run&'') should leave GDB in control of the
+ terminal. */
+ if (target_is_async_p () && !sync_execution)
+ return;
+
+ /* If GDB is resuming the inferior in the foreground, install
+ inferior's terminal modes. */
+ (*current_target.to_terminal_inferior) ();
+}
static int
nomemory (CORE_ADDR memaddr, char *myaddr, int len, int write,
Index: src/gdb/target.h
===================================================================
--- src.orig/gdb/target.h 2009-05-21 00:56:59.000000000 +0100
+++ src/gdb/target.h 2009-05-21 00:57:28.000000000 +0100
@@ -754,8 +754,7 @@ extern void print_section_info (struct t
/* Put the inferior's terminal settings into effect.
This is preparation for starting or resuming the inferior. */
-#define target_terminal_inferior() \
- (*current_target.to_terminal_inferior) ()
+extern void target_terminal_inferior (void);
/* Put some of our terminal settings into effect,
enough to get proper results from our output,
Index: src/gdb/event-top.c
===================================================================
--- src.orig/gdb/event-top.c 2009-05-21 00:56:59.000000000 +0100
+++ src/gdb/event-top.c 2009-05-21 00:57:28.000000000 +0100
@@ -458,14 +458,11 @@ async_enable_stdin (void)
void
async_disable_stdin (void)
{
- sync_execution = 1;
- push_prompt ("", "", "");
- /* FIXME: cagney/1999-09-27: At present this call is technically
- redundant since infcmd.c and infrun.c both already call
- target_terminal_inferior(). As the terminal handling (in
- sync/async mode) is refined, the duplicate calls can be
- eliminated (Here or in infcmd.c/infrun.c). */
- target_terminal_inferior ();
+ if (!sync_execution)
+ {
+ sync_execution = 1;
+ push_prompt ("", "", "");
+ }
}
Index: src/gdb/inflow.c
===================================================================
--- src.orig/gdb/inflow.c 2009-05-21 00:57:55.000000000 +0100
+++ src/gdb/inflow.c 2009-05-21 00:57:56.000000000 +0100
@@ -361,6 +361,8 @@ terminal_ours_1 (int output_only)
if (terminal_is_ours)
return;
+ terminal_is_ours = 1;
+
/* Checking inferior->run_terminal is necessary so that
if GDB is running in the background, it won't block trying
to do the ioctl()'s below. Checking gdb_has_a_terminal
@@ -371,7 +373,6 @@ terminal_ours_1 (int output_only)
if (inf->terminal_info->run_terminal != NULL || gdb_has_a_terminal () == 0)
return;
- if (!terminal_is_ours)
{
#ifdef SIGTTOU
/* Ignore this signal since it will happen when we try to set the
@@ -380,8 +381,6 @@ terminal_ours_1 (int output_only)
#endif
int result;
- terminal_is_ours = 1;
-
#ifdef SIGTTOU
if (job_control)
osigttou = (void (*)()) signal (SIGTTOU, SIG_IGN);