* [RFC v5 4/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode.
2014-03-04 18:33 [RFC v5 0/8] enable target-async by default Tom Tromey
@ 2014-03-04 18:33 ` Tom Tromey
2014-03-18 19:01 ` [RFC v6 " Pedro Alves
2014-03-04 18:33 ` [RFC v5 7/8] separate MI and target notions of async Tom Tromey
` (6 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Tom Tromey @ 2014-03-04 18:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
From: Pedro Alves <palves@redhat.com>
The other part of PR gdb/13860 is about console execution commands
in MI getting their output half lost. E.g., take the finish command,
executed on a frontend's GDB console:
sync:
finish
&"finish\n"
~"Run till exit from #0 usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
^running
*running,thread-id="1"
(gdb)
~"0x00000000004004d7 in foo () at stepinf.c:6\n"
~"6\t usleep (10);\n"
~"Value returned is $1 = 0\n"
*stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},thread-id="1",stopped-threads="all",core="1"
async:
finish
&"finish\n"
~"Run till exit from #0 usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
^running
*running,thread-id="1"
(gdb)
*stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},gdb-result-var="$1",return-value="0",thread-id="1",stopped-threads="all",core="0"
Note how all the "Value returned" etc. output is missing in async mode.
The same happens with e.g., catchpoints:
=breakpoint-modified,bkpt={number="1",type="catchpoint",disp="keep",enabled="y",what="22016",times="1"}
~"\nCatchpoint "
~"1 (forked process 22016), 0x0000003791cbd8a6 in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:131\n"
~"131\t pid = ARCH_FORK ();\n"
*stopped,reason="fork",disp="keep",bkptno="1",newpid="22016",frame={addr="0x0000003791cbd8a6",func="__libc_fork",args=[],file="../nptl/sysdeps/unix/sysv/linux/fork.c",fullname="/usr/src/debug/glibc-2.14-394-g8f3b1ff/nptl/sysdeps/unix/sysv/linux/fork.c",line="131"},thread-id="1",stopped-threads="all",core="0"
where all those ~ lines are missing in async mode, or just the "step"
current line indication:
s
&"s\n"
^running
*running,thread-id="all"
(gdb)
~"13\t foo ();\n"
*stopped,frame={addr="0x00000000004004ef",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdd78"}],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="13"},thread-id="1",stopped-threads="all",core="3"
(gdb)
Or in the case of the PRs example, the "Stopped due to shared library
event" note:
start
&"start\n"
~"Temporary breakpoint 1 at 0x400608: file ../../../src/gdb/testsuite/gdb.mi/solib-main.c, line 21.\n"
=breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000000000400608",func="main",file="../../../src/gdb/testsuite/gdb.mi/solib-main.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/solib-main.c",line="21",times="0",original-location="main"}
~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
=thread-group-started,id="i1",pid="21990"
=thread-created,id="1",group-id="i1"
^running
*running,thread-id="all"
(gdb)
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
~"Stopped due to shared library event (no libraries added or removed)\n"
*stopped,reason="solib-event",thread-id="1",stopped-threads="all",core="3"
(gdb)
IMO, if you're typing execution commands in a frontend's console, you
expect to see their output. Indeed it's what you get in sync mode. I
think async mode should do the same.
That's what this patch does.
Notes:
- mi->out is the same as gdb_stdout when MI is the current
interpreter. I think that referring to that directly is cleaner.
An earlier revision of this patch made the changes that are now
done in mi_on_normal_stop directly in infrun.c:normal_stop, and so
not having an obvious place to put the new uiout by then, and not
wanting to abuse CLI's uiout, I made a temporary uiout when
necessary.
- Hopefuly the rest of the patch is more or less obvious given the
comments I added.
Tested on x86_64 Fedora 16, no regressions.
2013-10-30 Pedro Alves <palves@redhat.com>
PR gdb/13860
* gdbthread.h (struct thread_control_state): New field
`command_interp'.
* infrun.c (follow_fork): Copy the new thread control field to the
child fork thread.
(clear_proceed_status_thread): Clear the new thread control field.
(proceed): Set the new thread control field.
* interps.h (command_interp): Declare.
* interps.c (command_interpreter): New global.
(command_interp): New function.
(interp_exec): Set `command_interpreter' while here.
* cli-out.c (cli_uiout_dtor): New function.
(cli_ui_out_impl): Install it.
* mi/mi-interp.c: Include cli-out.h.
(mi_cmd_interpreter_exec): Add comment.
(restore_current_uiout_cleanup): New function.
(ui_out_free_cleanup): New function.
(mi_on_normal_stop): In async mode, if finishing an execution
command started by a CLI command, or any kind of breakpoint-like
event triggered, print the stop event to the output (CLI) stream.
* mi/mi-out.c (mi_ui_out_impl): Install NULL `dtor' handler.
2013-10-30 Pedro Alves <palves@redhat.com>
* gdb.mi/mi-cli.exp: Also expect the new source line to be output
after a "next", in async mode. Make it a pass/fail test.
* gdb.mi/mi-solib.exp: Test that the CLI solib event note is
output.
---
gdb/ChangeLog | 24 ++++++++++++++
gdb/cli-out.c | 13 +++++++-
gdb/gdbthread.h | 5 +++
gdb/infrun.c | 14 +++++++++
gdb/interps.c | 36 ++++++++++++++++++++-
gdb/interps.h | 2 ++
gdb/mi/mi-interp.c | 66 +++++++++++++++++++++++++++++++++++++++
gdb/testsuite/ChangeLog | 7 +++++
gdb/testsuite/gdb.mi/mi-cli.exp | 13 +++++---
gdb/testsuite/gdb.mi/mi-solib.exp | 11 +++++++
10 files changed, 185 insertions(+), 6 deletions(-)
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 5943fa7..eedbd2c 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -40,6 +40,17 @@ static void out_field_fmt (struct ui_out *uiout, int fldno,
const char *fldname,
const char *format,...) ATTRIBUTE_PRINTF (4, 5);
+/* The destructor. */
+
+static void
+cli_uiout_dtor (struct ui_out *ui_out)
+{
+ cli_out_data *data = ui_out_data (ui_out);
+
+ VEC_free (ui_filep, data->streams);
+ xfree (data);
+}
+
/* These are the CLI output functions */
/* Mark beginning of a table */
@@ -367,7 +378,7 @@ const struct ui_out_impl cli_ui_out_impl =
cli_wrap_hint,
cli_flush,
cli_redirect,
- 0,
+ cli_uiout_dtor,
0, /* Does not need MI hacks (i.e. needs CLI hacks). */
};
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 9b43a68..9f5dee6 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -122,6 +122,11 @@ struct thread_control_state
/* Chain containing status of breakpoint(s) the thread stopped
at. */
bpstat stop_bpstat;
+
+ /* The interpreter that issued the execution command. NULL if the
+ thread was resumed as a result of a command applied to some other
+ thread (e.g., "next" with scheduler-locking off). */
+ struct interp *command_interp;
};
/* Inferior thread specific part of `struct infcall_suspend_state'.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e46f5d4..188a6e8 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -433,6 +433,7 @@ follow_fork (void)
CORE_ADDR step_range_start = 0;
CORE_ADDR step_range_end = 0;
struct frame_id step_frame_id = { 0 };
+ struct interp *command_interp = NULL;
if (!non_stop)
{
@@ -484,6 +485,7 @@ follow_fork (void)
step_frame_id = tp->control.step_frame_id;
exception_resume_breakpoint
= clone_momentary_breakpoint (tp->control.exception_resume_breakpoint);
+ command_interp = tp->control.command_interp;
/* For now, delete the parent's sr breakpoint, otherwise,
parent/child sr breakpoints are considered duplicates,
@@ -495,6 +497,7 @@ follow_fork (void)
tp->control.step_range_end = 0;
tp->control.step_frame_id = null_frame_id;
delete_exception_resume_breakpoint (tp);
+ tp->control.command_interp = NULL;
}
parent = inferior_ptid;
@@ -539,6 +542,7 @@ follow_fork (void)
tp->control.step_frame_id = step_frame_id;
tp->control.exception_resume_breakpoint
= exception_resume_breakpoint;
+ tp->control.command_interp = command_interp;
}
else
{
@@ -1999,6 +2003,8 @@ clear_proceed_status_thread (struct thread_info *tp)
tp->control.proceed_to_finish = 0;
+ tp->control.command_interp = NULL;
+
/* Discard any remaining commands or status from previous stop. */
bpstat_clear (&tp->control.stop_bpstat);
}
@@ -2200,6 +2206,14 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
regcache_write_pc (regcache, addr);
}
+ /* Record the interpreter that issued the execution command that
+ caused this thread to resume. If the top level interpreter is
+ MI/async, and the execution command was a CLI command
+ (next/step/etc.), we'll want to print stop event output to the MI
+ console channel (the stepped-to line, etc.), as if the user
+ entered the execution command on a real GDB console. */
+ inferior_thread ()->control.command_interp = command_interp ();
+
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: proceed (addr=%s, signal=%s, step=%d)\n",
diff --git a/gdb/interps.c b/gdb/interps.c
index e446747..f6b941c 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -318,6 +318,29 @@ current_interp_display_prompt_p (void)
data);
}
+/* The interpreter that is active while `interp_exec' is active, NULL
+ at all other times. */
+static struct interp *command_interpreter;
+
+/* The interpreter that was active when a command was executed.
+ Normally that'd always be CURRENT_INTERPRETER, except that MI's
+ -interpreter-exec command doesn't actually flip the current
+ interpreter when running its sub-command. The
+ `command_interpreter' global tracks when interp_exec is called
+ (IOW, when -interpreter-exec is called). If that is set, it is
+ INTERP in '-interpreter-exec INTERP "CMD"' or in 'interpreter-exec
+ INTERP "CMD". Otherwise, interp_exec isn't active, and so the
+ interpreter running the command is the current interpreter. */
+
+struct interp *
+command_interp (void)
+{
+ if (command_interpreter != NULL)
+ return command_interpreter;
+ else
+ return current_interpreter;
+}
+
/* Run the current command interpreter's main loop. */
void
current_interp_command_loop (void)
@@ -351,9 +374,20 @@ interp_set_quiet (struct interp *interp, int quiet)
struct gdb_exception
interp_exec (struct interp *interp, const char *command_str)
{
+ struct gdb_exception ex;
+ struct interp *save_command_interp;
+
gdb_assert (interp->procs->exec_proc != NULL);
- return interp->procs->exec_proc (interp->data, command_str);
+ /* See `command_interp' for why we do this. */
+ save_command_interp = command_interpreter;
+ command_interpreter = interp;
+
+ ex = interp->procs->exec_proc (interp->data, command_str);
+
+ command_interpreter = save_command_interp;
+
+ return ex;
}
/* A convenience routine that nulls out all the common command hooks.
diff --git a/gdb/interps.h b/gdb/interps.h
index 568b5df..13edf42 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -96,6 +96,8 @@ extern int current_interp_set_logging (int start_log, struct ui_file *out,
extern void *top_level_interpreter_data (void);
extern struct interp *top_level_interpreter (void);
+extern struct interp *command_interp (void);
+
/* True if the current interpreter is in async mode, false if in sync
mode. If in sync mode, running a synchronous execution command
(with execute_command, e.g, "next") will not return until the
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 862beaf..833e7f1 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -37,6 +37,7 @@
#include "gdb.h"
#include "objfiles.h"
#include "tracepoint.h"
+#include "cli-out.h"
/* These are the interpreter setup, etc. functions for the MI
interpreter. */
@@ -231,6 +232,10 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc)
error (_("-interpreter-exec: could not find interpreter \"%s\""),
argv[0]);
+ /* Note that unlike the CLI version of this command, we don't
+ actually set INTERP_TO_USE as the current interpreter, as we
+ still want gdb_stdout, etc. to point at MI streams. */
+
/* Insert the MI out hooks, making sure to also call the
interpreter's hooks if it has any. */
/* KRS: We shouldn't need this... Events should be installed and
@@ -415,6 +420,26 @@ mi_inferior_removed (struct inferior *inf)
gdb_flush (mi->event_channel);
}
+/* Cleanup that restores a previous current uiout. */
+
+static void
+restore_current_uiout_cleanup (void *arg)
+{
+ struct ui_out *saved_uiout = arg;
+
+ current_uiout = saved_uiout;
+}
+
+/* Cleanup that destroys the a ui_out object. */
+
+static void
+ui_out_free_cleanup (void *arg)
+{
+ struct ui_out *uiout = arg;
+
+ ui_out_destroy (uiout);
+}
+
static void
mi_on_normal_stop (struct bpstats *bs, int print_frame)
{
@@ -445,6 +470,47 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
current_uiout = saved_uiout;
}
+ /* Otherwise, frame information has already been printed by
+ normal_stop. */
+ else if (target_can_async_p ())
+ {
+ /* However, CLI execution commands (-interpreter-exec
+ console "next", for example) in async mode have the
+ opposite issue. normal_stop has already printed frame
+ information to MI uiout, but nothing has printed the same
+ information to the CLI channel. We should print the
+ source line to the console when stepping or other similar
+ commands, iff the step was started by a console command
+ (but not if it was started with -exec-step or similar).
+ Breakpoint hits should always be mirrored to the
+ console. */
+ struct thread_info *tp = inferior_thread ();
+
+ if ((tp->control.command_interp != NULL
+ && tp->control.command_interp != top_level_interpreter ())
+ || (!tp->control.stop_step
+ && !tp->control.proceed_to_finish))
+ {
+ struct mi_interp *mi = top_level_interpreter_data ();
+ struct target_waitstatus last;
+ ptid_t last_ptid;
+ struct ui_out *cli_uiout;
+ struct cleanup *old_chain;
+
+ /* Sets the current uiout to a new temporary CLI uiout
+ assigned to STREAM. */
+ cli_uiout = cli_out_new (mi->out);
+ old_chain = make_cleanup (ui_out_free_cleanup, cli_uiout);
+
+ make_cleanup (restore_current_uiout_cleanup, current_uiout);
+ current_uiout = cli_uiout;
+
+ get_last_target_status (&last_ptid, &last);
+ print_stop_event (&last);
+
+ do_cleanups (old_chain);
+ }
+ }
ui_out_field_int (mi_uiout, "thread-id",
pid_to_thread_id (inferior_ptid));
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 016f54f..e158b7e 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -166,10 +166,15 @@ mi_gdb_test "34 next" \
".*34\\\^running.*\\*running,thread-id=\"all\"" \
"34 next: run"
-if {!$async} {
- gdb_expect {
- -re "~\[^\r\n\]+\r\n" {
- }
+# Test that the new current source line is output, given we executed
+# the console 'next' command, not -exec-next.
+set test "34 next: CLI output"
+gdb_expect {
+ -re "~\"67\[^\r\n\]+\r\n" {
+ pass $test
+ }
+ timeout {
+ fail "$test (timeout)"
}
}
diff --git a/gdb/testsuite/gdb.mi/mi-solib.exp b/gdb/testsuite/gdb.mi/mi-solib.exp
index 608d2b7..a684a3d 100644
--- a/gdb/testsuite/gdb.mi/mi-solib.exp
+++ b/gdb/testsuite/gdb.mi/mi-solib.exp
@@ -60,4 +60,15 @@ mi_gdb_test "777-gdb-set stop-on-solib-events 1" "777\\^done" \
# commands still cause the correct MI output to be generated.
mi_run_with_cli
+# Also test that the CLI solib event note is output.
+set test "CLI prints solib event"
+gdb_expect {
+ -re "~\"Stopped due to shared library event \\(no libraries added or removed\\)\\\\n" {
+ pass "$test"
+ }
+ timeout {
+ fail "$test (timeout)"
+ }
+}
+
mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
--
1.8.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread* [RFC v6 4/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode.
2014-03-04 18:33 ` [RFC v5 4/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode Tom Tromey
@ 2014-03-18 19:01 ` Pedro Alves
2014-05-21 22:37 ` Pedro Alves
0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-03-18 19:01 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 03/04/2014 06:32 PM, Tom Tromey wrote:
> From: Pedro Alves <palves@redhat.com>
>
> The other part of PR gdb/13860 is about console execution commands
> in MI getting their output half lost. E.g., take the finish command,
> executed on a frontend's GDB console:
>
...
> +
> static void
> mi_on_normal_stop (struct bpstats *bs, int print_frame)
> {
> @@ -445,6 +470,47 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
>
> current_uiout = saved_uiout;
> }
> + /* Otherwise, frame information has already been printed by
> + normal_stop. */
> + else if (target_can_async_p ())
> + {
> + /* However, CLI execution commands (-interpreter-exec
> + console "next", for example) in async mode have the
> + opposite issue. normal_stop has already printed frame
> + information to MI uiout, but nothing has printed the same
> + information to the CLI channel. We should print the
> + source line to the console when stepping or other similar
> + commands, iff the step was started by a console command
> + (but not if it was started with -exec-step or similar).
> + Breakpoint hits should always be mirrored to the
> + console. */
I thought about this mirroring of breakpoints to the console
only done in async, and given we're trying to make the CLI vs MI
output match, it seemed a little silly to not do the same in
sync mode. So I removed the target_can_async_p() check above,
and extended the comments to explain why we do this. I then
extended the tests to make sure the CLI output appears at
the right times, and that it doesn't appear when it shouldn't.
See below. Let me know what you think.
> + struct thread_info *tp = inferior_thread ();
> +
> + if ((tp->control.command_interp != NULL
> + && tp->control.command_interp != top_level_interpreter ())
> + || (!tp->control.stop_step
> + && !tp->control.proceed_to_finish))
> + {
> + struct mi_interp *mi = top_level_interpreter_data ();
> + struct target_waitstatus last;
> + ptid_t last_ptid;
> + struct ui_out *cli_uiout;
> + struct cleanup *old_chain;
> +
> + /* Sets the current uiout to a new temporary CLI uiout
> + assigned to STREAM. */
> + cli_uiout = cli_out_new (mi->out);
> + old_chain = make_cleanup (ui_out_free_cleanup, cli_uiout);
> +
> + make_cleanup (restore_current_uiout_cleanup, current_uiout);
> + current_uiout = cli_uiout;
> +
> + get_last_target_status (&last_ptid, &last);
> + print_stop_event (&last);
> +
> + do_cleanups (old_chain);
> + }
> + }
------------------
[PATCH] PR gdb/13860: don't lose '-interpreter-exec console
EXECUTION_COMMAND''s output in async mode.
The other part of PR gdb/13860 is about console execution commands in
MI getting their output half lost. E.g., take the finish command,
executed on a frontend's GDB console:
sync:
finish
&"finish\n"
~"Run till exit from #0 usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
^running
*running,thread-id="1"
(gdb)
~"0x00000000004004d7 in foo () at stepinf.c:6\n"
~"6\t usleep (10);\n"
~"Value returned is $1 = 0\n"
*stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},thread-id="1",stopped-threads="all",core="1"
async:
finish
&"finish\n"
~"Run till exit from #0 usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
^running
*running,thread-id="1"
(gdb)
*stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},gdb-result-var="$1",return-value="0",thread-id="1",stopped-threads="all",core="0"
Note how all the "Value returned" etc. output is missing in async mode.
The same happens with e.g., catchpoints:
=breakpoint-modified,bkpt={number="1",type="catchpoint",disp="keep",enabled="y",what="22016",times="1"}
~"\nCatchpoint "
~"1 (forked process 22016), 0x0000003791cbd8a6 in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:131\n"
~"131\t pid = ARCH_FORK ();\n"
*stopped,reason="fork",disp="keep",bkptno="1",newpid="22016",frame={addr="0x0000003791cbd8a6",func="__libc_fork",args=[],file="../nptl/sysdeps/unix/sysv/linux/fork.c",fullname="/usr/src/debug/glibc-2.14-394-g8f3b1ff/nptl/sysdeps/unix/sysv/linux/fork.c",line="131"},thread-id="1",stopped-threads="all",core="0"
where all those ~ lines are missing in async mode, or just the "step"
current line indication:
s
&"s\n"
^running
*running,thread-id="all"
(gdb)
~"13\t foo ();\n"
*stopped,frame={addr="0x00000000004004ef",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdd78"}],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="13"},thread-id="1",stopped-threads="all",core="3"
(gdb)
Or in the case of the PRs example, the "Stopped due to shared library
event" note:
start
&"start\n"
~"Temporary breakpoint 1 at 0x400608: file ../../../src/gdb/testsuite/gdb.mi/solib-main.c, line 21.\n"
=breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000000000400608",func="main",file="../../../src/gdb/testsuite/gdb.mi/solib-main.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/solib-main.c",line="21",times="0",original-location="main"}
~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
=thread-group-started,id="i1",pid="21990"
=thread-created,id="1",group-id="i1"
^running
*running,thread-id="all"
(gdb)
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
~"Stopped due to shared library event (no libraries added or removed)\n"
*stopped,reason="solib-event",thread-id="1",stopped-threads="all",core="3"
(gdb)
IMO, if you're typing execution commands in a frontend's console, you
expect to see their output. Indeed it's what you get in sync mode. I
think async mode should do the same. Deciding what to mirror to the
console wrt to breakpoints and random stops gets messy real fast.
E.g., say "s" trips on a breakpoint. We'd clearly want to mirror the
event to the console in this case. But what about more complicated
cases like "s&; thread n; s&", and one of those steps spawning a new
thread, and that thread hitting a breakpoint? It's impossible in
general to track whether the thread had any relation to the commands
that had been executed. So I think we should just simplify and always
mirror breakpoints and random events to the console.
Notes:
- mi->out is the same as gdb_stdout when MI is the current
interpreter. I think that referring to that directly is cleaner.
An earlier revision of this patch made the changes that are now
done in mi_on_normal_stop directly in infrun.c:normal_stop, and so
not having an obvious place to put the new uiout by then, and not
wanting to abuse CLI's uiout, I made a temporary uiout when
necessary.
- Hopefuly the rest of the patch is more or less obvious given the
comments added.
Tested on x86_64 Fedora 17, no regressions.
2014-03-18 Pedro Alves <palves@redhat.com>
PR gdb/13860
* gdbthread.h (struct thread_control_state): New field
`command_interp'.
* infrun.c (follow_fork): Copy the new thread control field to the
child fork thread.
(clear_proceed_status_thread): Clear the new thread control field.
(proceed): Set the new thread control field.
* interps.h (command_interp): Declare.
* interps.c (command_interpreter): New global.
(command_interp): New function.
(interp_exec): Set `command_interpreter' while here.
* cli-out.c (cli_uiout_dtor): New function.
(cli_ui_out_impl): Install it.
* mi/mi-interp.c: Include cli-out.h.
(mi_cmd_interpreter_exec): Add comment.
(restore_current_uiout_cleanup): New function.
(ui_out_free_cleanup): New function.
(mi_on_normal_stop): If finishing an execution command started by
a CLI command, or any kind of breakpoint-like event triggered,
print the stop event to the output (CLI) stream.
* mi/mi-out.c (mi_ui_out_impl): Install NULL `dtor' handler.
2014-03-18 Pedro Alves <palves@redhat.com>
PR gdb/13860
* gdb.mi/mi-cli.exp (line_callee4_next_step): New global.
(top level): Test that output related to execution commands is
sent to the console with CLI commands, but not with MI commands.
Test that breakpoint events are always mirrored to the console.
Also expect the new source line to be output after a "next" in
async mode too. Make it a pass/fail test.
* gdb.mi/mi-solib.exp: Test that the CLI solib event note is
output.
* lib/mi-support.exp (mi_gdb_expect_cli_output): New procedure.
---
gdb/cli-out.c | 13 ++++++-
gdb/gdbthread.h | 5 +++
gdb/infrun.c | 14 +++++++
gdb/interps.c | 36 +++++++++++++++++-
gdb/interps.h | 2 +
gdb/mi/mi-interp.c | 77 +++++++++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.mi/mi-cli.exp | 52 ++++++++++++++++++++++----
gdb/testsuite/gdb.mi/mi-solib.exp | 11 ++++++
gdb/testsuite/lib/mi-support.exp | 24 ++++++++++++
9 files changed, 225 insertions(+), 9 deletions(-)
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 5943fa7..eedbd2c 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -40,6 +40,17 @@ static void out_field_fmt (struct ui_out *uiout, int fldno,
const char *fldname,
const char *format,...) ATTRIBUTE_PRINTF (4, 5);
+/* The destructor. */
+
+static void
+cli_uiout_dtor (struct ui_out *ui_out)
+{
+ cli_out_data *data = ui_out_data (ui_out);
+
+ VEC_free (ui_filep, data->streams);
+ xfree (data);
+}
+
/* These are the CLI output functions */
/* Mark beginning of a table */
@@ -367,7 +378,7 @@ const struct ui_out_impl cli_ui_out_impl =
cli_wrap_hint,
cli_flush,
cli_redirect,
- 0,
+ cli_uiout_dtor,
0, /* Does not need MI hacks (i.e. needs CLI hacks). */
};
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 9b43a68..9f5dee6 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -122,6 +122,11 @@ struct thread_control_state
/* Chain containing status of breakpoint(s) the thread stopped
at. */
bpstat stop_bpstat;
+
+ /* The interpreter that issued the execution command. NULL if the
+ thread was resumed as a result of a command applied to some other
+ thread (e.g., "next" with scheduler-locking off). */
+ struct interp *command_interp;
};
/* Inferior thread specific part of `struct infcall_suspend_state'.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index f783260..fed4a27 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -433,6 +433,7 @@ follow_fork (void)
CORE_ADDR step_range_start = 0;
CORE_ADDR step_range_end = 0;
struct frame_id step_frame_id = { 0 };
+ struct interp *command_interp = NULL;
if (!non_stop)
{
@@ -484,6 +485,7 @@ follow_fork (void)
step_frame_id = tp->control.step_frame_id;
exception_resume_breakpoint
= clone_momentary_breakpoint (tp->control.exception_resume_breakpoint);
+ command_interp = tp->control.command_interp;
/* For now, delete the parent's sr breakpoint, otherwise,
parent/child sr breakpoints are considered duplicates,
@@ -495,6 +497,7 @@ follow_fork (void)
tp->control.step_range_end = 0;
tp->control.step_frame_id = null_frame_id;
delete_exception_resume_breakpoint (tp);
+ tp->control.command_interp = NULL;
}
parent = inferior_ptid;
@@ -539,6 +542,7 @@ follow_fork (void)
tp->control.step_frame_id = step_frame_id;
tp->control.exception_resume_breakpoint
= exception_resume_breakpoint;
+ tp->control.command_interp = command_interp;
}
else
{
@@ -1999,6 +2003,8 @@ clear_proceed_status_thread (struct thread_info *tp)
tp->control.proceed_to_finish = 0;
+ tp->control.command_interp = NULL;
+
/* Discard any remaining commands or status from previous stop. */
bpstat_clear (&tp->control.stop_bpstat);
}
@@ -2200,6 +2206,14 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
regcache_write_pc (regcache, addr);
}
+ /* Record the interpreter that issued the execution command that
+ caused this thread to resume. If the top level interpreter is
+ MI/async, and the execution command was a CLI command
+ (next/step/etc.), we'll want to print stop event output to the MI
+ console channel (the stepped-to line, etc.), as if the user
+ entered the execution command on a real GDB console. */
+ inferior_thread ()->control.command_interp = command_interp ();
+
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: proceed (addr=%s, signal=%s, step=%d)\n",
diff --git a/gdb/interps.c b/gdb/interps.c
index e446747..f6b941c 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -318,6 +318,29 @@ current_interp_display_prompt_p (void)
data);
}
+/* The interpreter that is active while `interp_exec' is active, NULL
+ at all other times. */
+static struct interp *command_interpreter;
+
+/* The interpreter that was active when a command was executed.
+ Normally that'd always be CURRENT_INTERPRETER, except that MI's
+ -interpreter-exec command doesn't actually flip the current
+ interpreter when running its sub-command. The
+ `command_interpreter' global tracks when interp_exec is called
+ (IOW, when -interpreter-exec is called). If that is set, it is
+ INTERP in '-interpreter-exec INTERP "CMD"' or in 'interpreter-exec
+ INTERP "CMD". Otherwise, interp_exec isn't active, and so the
+ interpreter running the command is the current interpreter. */
+
+struct interp *
+command_interp (void)
+{
+ if (command_interpreter != NULL)
+ return command_interpreter;
+ else
+ return current_interpreter;
+}
+
/* Run the current command interpreter's main loop. */
void
current_interp_command_loop (void)
@@ -351,9 +374,20 @@ interp_set_quiet (struct interp *interp, int quiet)
struct gdb_exception
interp_exec (struct interp *interp, const char *command_str)
{
+ struct gdb_exception ex;
+ struct interp *save_command_interp;
+
gdb_assert (interp->procs->exec_proc != NULL);
- return interp->procs->exec_proc (interp->data, command_str);
+ /* See `command_interp' for why we do this. */
+ save_command_interp = command_interpreter;
+ command_interpreter = interp;
+
+ ex = interp->procs->exec_proc (interp->data, command_str);
+
+ command_interpreter = save_command_interp;
+
+ return ex;
}
/* A convenience routine that nulls out all the common command hooks.
diff --git a/gdb/interps.h b/gdb/interps.h
index 568b5df..13edf42 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -96,6 +96,8 @@ extern int current_interp_set_logging (int start_log, struct ui_file *out,
extern void *top_level_interpreter_data (void);
extern struct interp *top_level_interpreter (void);
+extern struct interp *command_interp (void);
+
/* True if the current interpreter is in async mode, false if in sync
mode. If in sync mode, running a synchronous execution command
(with execute_command, e.g, "next") will not return until the
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 862beaf..80fffa3 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -37,6 +37,7 @@
#include "gdb.h"
#include "objfiles.h"
#include "tracepoint.h"
+#include "cli-out.h"
/* These are the interpreter setup, etc. functions for the MI
interpreter. */
@@ -231,6 +232,10 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc)
error (_("-interpreter-exec: could not find interpreter \"%s\""),
argv[0]);
+ /* Note that unlike the CLI version of this command, we don't
+ actually set INTERP_TO_USE as the current interpreter, as we
+ still want gdb_stdout, etc. to point at MI streams. */
+
/* Insert the MI out hooks, making sure to also call the
interpreter's hooks if it has any. */
/* KRS: We shouldn't need this... Events should be installed and
@@ -415,6 +420,26 @@ mi_inferior_removed (struct inferior *inf)
gdb_flush (mi->event_channel);
}
+/* Cleanup that restores a previous current uiout. */
+
+static void
+restore_current_uiout_cleanup (void *arg)
+{
+ struct ui_out *saved_uiout = arg;
+
+ current_uiout = saved_uiout;
+}
+
+/* Cleanup that destroys the a ui_out object. */
+
+static void
+ui_out_free_cleanup (void *arg)
+{
+ struct ui_out *uiout = arg;
+
+ ui_out_destroy (uiout);
+}
+
static void
mi_on_normal_stop (struct bpstats *bs, int print_frame)
{
@@ -445,6 +470,58 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
current_uiout = saved_uiout;
}
+ /* Otherwise, frame information has already been printed by
+ normal_stop. */
+ else
+ {
+ /* Breakpoint hits should always be mirrored to the console.
+ Deciding what to mirror to the console wrt to breakpoints
+ and random stops gets messy real fast. E.g., say "s"
+ trips on a breakpoint. We'd clearly want to mirror the
+ event to the console in this case. But what about more
+ complicated cases like "s&; thread n; s&", and one of
+ those steps spawning a new thread, and that thread
+ hitting a breakpoint? It's impossible in general to
+ track whether the thread had any relation to the commands
+ that had been executed. So we just simplify and always
+ mirror breakpoints and random events to the console.
+
+ Also, CLI execution commands (-interpreter-exec console
+ "next", for example) in async mode have the opposite
+ issue as described in the "then" branch above --
+ normal_stop has already printed frame information to MI
+ uiout, but nothing has printed the same information to
+ the CLI channel. We should print the source line to the
+ console when stepping or other similar commands, iff the
+ step was started by a console command (but not if it was
+ started with -exec-step or similar). */
+ struct thread_info *tp = inferior_thread ();
+
+ if ((!tp->control.stop_step
+ && !tp->control.proceed_to_finish)
+ || (tp->control.command_interp != NULL
+ && tp->control.command_interp != top_level_interpreter ()))
+ {
+ struct mi_interp *mi = top_level_interpreter_data ();
+ struct target_waitstatus last;
+ ptid_t last_ptid;
+ struct ui_out *cli_uiout;
+ struct cleanup *old_chain;
+
+ /* Sets the current uiout to a new temporary CLI uiout
+ assigned to STREAM. */
+ cli_uiout = cli_out_new (mi->out);
+ old_chain = make_cleanup (ui_out_free_cleanup, cli_uiout);
+
+ make_cleanup (restore_current_uiout_cleanup, current_uiout);
+ current_uiout = cli_uiout;
+
+ get_last_target_status (&last_ptid, &last);
+ print_stop_event (&last);
+
+ do_cleanups (old_chain);
+ }
+ }
ui_out_field_int (mi_uiout, "thread-id",
pid_to_thread_id (inferior_ptid));
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index cb8d3af..50313ae 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -69,6 +69,7 @@ set line_main_callme_2 [expr $line_main_return + 1]
set line_callee4_head [gdb_get_line_number "callee4 ("]
set line_callee4_body [expr $line_callee4_head + 2]
set line_callee4_next [expr $line_callee4_body + 1]
+set line_callee4_next_step [expr $line_callee4_next + 3]
mi_gdb_test "-interpreter-exec console \"set args foobar\"" \
".*=cmd-param-changed,param=\"args\",value=\"foobar\".*\\^done" \
@@ -153,13 +154,44 @@ if {$async} {
mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \
"" "check *stopped from CLI command"
+mi_send_resuming_command "exec-step" "-exec-step to line \$line_callee4_next_step"
+
+# Test that the new current source line is _not_ output, given we
+# executed MI's -exec-next, not CLI's 'next' command.
+
+set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output for -exec-step"]
+
+set test "-exec-step does not produce CLI step output"
+if {[regexp "A + B" "$output"]} {
+ fail $test
+} else {
+ pass $test
+}
+
+mi_expect_stop "end-stepping-range" "callee4" "" ".*basics.c" $line_callee4_next_step \
+ "" "check *stopped from CLI command 2"
+
mi_gdb_test "600-break-insert -t basics.c:$line_main_hello" \
{600\^done,bkpt=.number="3",type="breakpoint".*\}} \
"-break-insert -t basics.c:\$line_main_hello"
-mi_execute_to "exec-continue" "breakpoint-hit" "main" "" ".*basics.c" \
- $line_main_hello { "" "disp=\"del\"" } \
- "-exec-continue to line \$line_main_hello"
+# Test that breakpoint events are always mirrored to the CLI output
+# stream (both sync and async modes).
+mi_send_resuming_command "exec-continue" "-exec-continue to line \$line_main_hello"
+
+set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output for breakpoint hit"]
+set test "breakpoint hit produces CLI output"
+set pattern "\\\\nTemporary breakpoint 3, main \\(\\) at \[^\n\]+basics.c:$line_main_hello\\\\n\[^\n\]+Hello"
+
+if {[regexp $pattern $output]} {
+ pass $test
+} else {
+ fail $test
+}
+
+# Test the MI output.
+mi_expect_stop "breakpoint-hit" "main" "" ".*basics.c" \
+ $line_main_hello { "" "disp=\"del\"" } "Temporary breakpoint output hit in MI"
# Test that the token is output even for CLI commands
# Also test that *stopped includes frame information.
@@ -167,10 +199,16 @@ mi_gdb_test "34 next" \
".*34\\\^running.*\\*running,thread-id=\"all\"" \
"34 next: run"
-if {!$async} {
- gdb_expect {
- -re "~\[^\r\n\]+\r\n" {
- }
+# Test that the new current source line is output to the console
+# stream, given we executed the console 'next' command, not
+# -exec-next.
+set test "34 next: CLI output"
+gdb_expect {
+ -re "~\"$line_main_return\[^\r\n\]+\r\n" {
+ pass $test
+ }
+ timeout {
+ fail "$test (timeout)"
}
}
diff --git a/gdb/testsuite/gdb.mi/mi-solib.exp b/gdb/testsuite/gdb.mi/mi-solib.exp
index 608d2b7..a684a3d 100644
--- a/gdb/testsuite/gdb.mi/mi-solib.exp
+++ b/gdb/testsuite/gdb.mi/mi-solib.exp
@@ -60,4 +60,15 @@ mi_gdb_test "777-gdb-set stop-on-solib-events 1" "777\\^done" \
# commands still cause the correct MI output to be generated.
mi_run_with_cli
+# Also test that the CLI solib event note is output.
+set test "CLI prints solib event"
+gdb_expect {
+ -re "~\"Stopped due to shared library event \\(no libraries added or removed\\)\\\\n" {
+ pass "$test"
+ }
+ timeout {
+ fail "$test (timeout)"
+ }
+}
+
mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 213073a..1f6dfa7 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -799,6 +799,30 @@ proc mi_gdb_test { args } {
return $result
}
+# Collect output sent to the console output stream until UNTIL is
+# seen. UNTIL is a regular expression. MESSAGE is the message to be
+# printed in case of timeout.
+
+proc mi_gdb_expect_cli_output {until message} {
+
+ set output ""
+ gdb_expect {
+ -re "~\"(\[^\r\n\]+)\"\r\n" {
+ append output $expect_out(1,string)
+ exp_continue
+ }
+ -notransfer -re "$until" {
+ # Done
+ }
+ timeout {
+ fail "$message (timeout)"
+ return ""
+ }
+ }
+
+ return $output
+}
+
#
# MI run command. (A modified version of gdb_run_cmd)
#
--
1.7.11.7
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v6 4/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode.
2014-03-18 19:01 ` [RFC v6 " Pedro Alves
@ 2014-05-21 22:37 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-05-21 22:37 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
On 03/18/2014 07:01 PM, Pedro Alves wrote:
> On 03/04/2014 06:32 PM, Tom Tromey wrote:
>> From: Pedro Alves <palves@redhat.com>
>>
>> The other part of PR gdb/13860 is about console execution commands
>> in MI getting their output half lost. E.g., take the finish command,
>> executed on a frontend's GDB console:
>>
> I thought about this mirroring of breakpoints to the console
> only done in async, and given we're trying to make the CLI vs MI
> output match, it seemed a little silly to not do the same in
> sync mode. So I removed the target_can_async_p() check above,
> and extended the comments to explain why we do this. I then
> extended the tests to make sure the CLI output appears at
> the right times, and that it doesn't appear when it shouldn't.
> See below. Let me know what you think.
Tom told me off list he's OK with this version.
I've pushed it in. Getting closer...
Pedro Alves
>
>> + struct thread_info *tp = inferior_thread ();
>> +
>> + if ((tp->control.command_interp != NULL
>> + && tp->control.command_interp != top_level_interpreter ())
>> + || (!tp->control.stop_step
>> + && !tp->control.proceed_to_finish))
>> + {
>> + struct mi_interp *mi = top_level_interpreter_data ();
>> + struct target_waitstatus last;
>> + ptid_t last_ptid;
>> + struct ui_out *cli_uiout;
>> + struct cleanup *old_chain;
>> +
>> + /* Sets the current uiout to a new temporary CLI uiout
>> + assigned to STREAM. */
>> + cli_uiout = cli_out_new (mi->out);
>> + old_chain = make_cleanup (ui_out_free_cleanup, cli_uiout);
>> +
>> + make_cleanup (restore_current_uiout_cleanup, current_uiout);
>> + current_uiout = cli_uiout;
>> +
>> + get_last_target_status (&last_ptid, &last);
>> + print_stop_event (&last);
>> +
>> + do_cleanups (old_chain);
>> + }
>> + }
>
> ------------------
> [PATCH] PR gdb/13860: don't lose '-interpreter-exec console
> EXECUTION_COMMAND''s output in async mode.
>
> The other part of PR gdb/13860 is about console execution commands in
> MI getting their output half lost. E.g., take the finish command,
> executed on a frontend's GDB console:
>
> sync:
>
> finish
> &"finish\n"
> ~"Run till exit from #0 usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
> ^running
> *running,thread-id="1"
> (gdb)
> ~"0x00000000004004d7 in foo () at stepinf.c:6\n"
> ~"6\t usleep (10);\n"
> ~"Value returned is $1 = 0\n"
> *stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},thread-id="1",stopped-threads="all",core="1"
>
> async:
>
> finish
> &"finish\n"
> ~"Run till exit from #0 usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
> ^running
> *running,thread-id="1"
> (gdb)
> *stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},gdb-result-var="$1",return-value="0",thread-id="1",stopped-threads="all",core="0"
>
> Note how all the "Value returned" etc. output is missing in async mode.
>
> The same happens with e.g., catchpoints:
>
> =breakpoint-modified,bkpt={number="1",type="catchpoint",disp="keep",enabled="y",what="22016",times="1"}
> ~"\nCatchpoint "
> ~"1 (forked process 22016), 0x0000003791cbd8a6 in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:131\n"
> ~"131\t pid = ARCH_FORK ();\n"
> *stopped,reason="fork",disp="keep",bkptno="1",newpid="22016",frame={addr="0x0000003791cbd8a6",func="__libc_fork",args=[],file="../nptl/sysdeps/unix/sysv/linux/fork.c",fullname="/usr/src/debug/glibc-2.14-394-g8f3b1ff/nptl/sysdeps/unix/sysv/linux/fork.c",line="131"},thread-id="1",stopped-threads="all",core="0"
>
> where all those ~ lines are missing in async mode, or just the "step"
> current line indication:
>
> s
> &"s\n"
> ^running
> *running,thread-id="all"
> (gdb)
> ~"13\t foo ();\n"
> *stopped,frame={addr="0x00000000004004ef",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdd78"}],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="13"},thread-id="1",stopped-threads="all",core="3"
> (gdb)
>
> Or in the case of the PRs example, the "Stopped due to shared library
> event" note:
>
> start
> &"start\n"
> ~"Temporary breakpoint 1 at 0x400608: file ../../../src/gdb/testsuite/gdb.mi/solib-main.c, line 21.\n"
> =breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000000000400608",func="main",file="../../../src/gdb/testsuite/gdb.mi/solib-main.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/solib-main.c",line="21",times="0",original-location="main"}
> ~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
> =thread-group-started,id="i1",pid="21990"
> =thread-created,id="1",group-id="i1"
> ^running
> *running,thread-id="all"
> (gdb)
> =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
> ~"Stopped due to shared library event (no libraries added or removed)\n"
> *stopped,reason="solib-event",thread-id="1",stopped-threads="all",core="3"
> (gdb)
>
> IMO, if you're typing execution commands in a frontend's console, you
> expect to see their output. Indeed it's what you get in sync mode. I
> think async mode should do the same. Deciding what to mirror to the
> console wrt to breakpoints and random stops gets messy real fast.
> E.g., say "s" trips on a breakpoint. We'd clearly want to mirror the
> event to the console in this case. But what about more complicated
> cases like "s&; thread n; s&", and one of those steps spawning a new
> thread, and that thread hitting a breakpoint? It's impossible in
> general to track whether the thread had any relation to the commands
> that had been executed. So I think we should just simplify and always
> mirror breakpoints and random events to the console.
>
> Notes:
>
> - mi->out is the same as gdb_stdout when MI is the current
> interpreter. I think that referring to that directly is cleaner.
> An earlier revision of this patch made the changes that are now
> done in mi_on_normal_stop directly in infrun.c:normal_stop, and so
> not having an obvious place to put the new uiout by then, and not
> wanting to abuse CLI's uiout, I made a temporary uiout when
> necessary.
>
> - Hopefuly the rest of the patch is more or less obvious given the
> comments added.
>
> Tested on x86_64 Fedora 17, no regressions.
>
> 2014-03-18 Pedro Alves <palves@redhat.com>
>
> PR gdb/13860
> * gdbthread.h (struct thread_control_state): New field
> `command_interp'.
> * infrun.c (follow_fork): Copy the new thread control field to the
> child fork thread.
> (clear_proceed_status_thread): Clear the new thread control field.
> (proceed): Set the new thread control field.
> * interps.h (command_interp): Declare.
> * interps.c (command_interpreter): New global.
> (command_interp): New function.
> (interp_exec): Set `command_interpreter' while here.
> * cli-out.c (cli_uiout_dtor): New function.
> (cli_ui_out_impl): Install it.
> * mi/mi-interp.c: Include cli-out.h.
> (mi_cmd_interpreter_exec): Add comment.
> (restore_current_uiout_cleanup): New function.
> (ui_out_free_cleanup): New function.
> (mi_on_normal_stop): If finishing an execution command started by
> a CLI command, or any kind of breakpoint-like event triggered,
> print the stop event to the output (CLI) stream.
> * mi/mi-out.c (mi_ui_out_impl): Install NULL `dtor' handler.
>
> 2014-03-18 Pedro Alves <palves@redhat.com>
>
> PR gdb/13860
> * gdb.mi/mi-cli.exp (line_callee4_next_step): New global.
> (top level): Test that output related to execution commands is
> sent to the console with CLI commands, but not with MI commands.
> Test that breakpoint events are always mirrored to the console.
> Also expect the new source line to be output after a "next" in
> async mode too. Make it a pass/fail test.
> * gdb.mi/mi-solib.exp: Test that the CLI solib event note is
> output.
> * lib/mi-support.exp (mi_gdb_expect_cli_output): New procedure.
> ---
> gdb/cli-out.c | 13 ++++++-
> gdb/gdbthread.h | 5 +++
> gdb/infrun.c | 14 +++++++
> gdb/interps.c | 36 +++++++++++++++++-
> gdb/interps.h | 2 +
> gdb/mi/mi-interp.c | 77 +++++++++++++++++++++++++++++++++++++++
> gdb/testsuite/gdb.mi/mi-cli.exp | 52 ++++++++++++++++++++++----
> gdb/testsuite/gdb.mi/mi-solib.exp | 11 ++++++
> gdb/testsuite/lib/mi-support.exp | 24 ++++++++++++
> 9 files changed, 225 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> index 5943fa7..eedbd2c 100644
> --- a/gdb/cli-out.c
> +++ b/gdb/cli-out.c
> @@ -40,6 +40,17 @@ static void out_field_fmt (struct ui_out *uiout, int fldno,
> const char *fldname,
> const char *format,...) ATTRIBUTE_PRINTF (4, 5);
>
> +/* The destructor. */
> +
> +static void
> +cli_uiout_dtor (struct ui_out *ui_out)
> +{
> + cli_out_data *data = ui_out_data (ui_out);
> +
> + VEC_free (ui_filep, data->streams);
> + xfree (data);
> +}
> +
> /* These are the CLI output functions */
>
> /* Mark beginning of a table */
> @@ -367,7 +378,7 @@ const struct ui_out_impl cli_ui_out_impl =
> cli_wrap_hint,
> cli_flush,
> cli_redirect,
> - 0,
> + cli_uiout_dtor,
> 0, /* Does not need MI hacks (i.e. needs CLI hacks). */
> };
>
> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index 9b43a68..9f5dee6 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -122,6 +122,11 @@ struct thread_control_state
> /* Chain containing status of breakpoint(s) the thread stopped
> at. */
> bpstat stop_bpstat;
> +
> + /* The interpreter that issued the execution command. NULL if the
> + thread was resumed as a result of a command applied to some other
> + thread (e.g., "next" with scheduler-locking off). */
> + struct interp *command_interp;
> };
>
> /* Inferior thread specific part of `struct infcall_suspend_state'.
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index f783260..fed4a27 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -433,6 +433,7 @@ follow_fork (void)
> CORE_ADDR step_range_start = 0;
> CORE_ADDR step_range_end = 0;
> struct frame_id step_frame_id = { 0 };
> + struct interp *command_interp = NULL;
>
> if (!non_stop)
> {
> @@ -484,6 +485,7 @@ follow_fork (void)
> step_frame_id = tp->control.step_frame_id;
> exception_resume_breakpoint
> = clone_momentary_breakpoint (tp->control.exception_resume_breakpoint);
> + command_interp = tp->control.command_interp;
>
> /* For now, delete the parent's sr breakpoint, otherwise,
> parent/child sr breakpoints are considered duplicates,
> @@ -495,6 +497,7 @@ follow_fork (void)
> tp->control.step_range_end = 0;
> tp->control.step_frame_id = null_frame_id;
> delete_exception_resume_breakpoint (tp);
> + tp->control.command_interp = NULL;
> }
>
> parent = inferior_ptid;
> @@ -539,6 +542,7 @@ follow_fork (void)
> tp->control.step_frame_id = step_frame_id;
> tp->control.exception_resume_breakpoint
> = exception_resume_breakpoint;
> + tp->control.command_interp = command_interp;
> }
> else
> {
> @@ -1999,6 +2003,8 @@ clear_proceed_status_thread (struct thread_info *tp)
>
> tp->control.proceed_to_finish = 0;
>
> + tp->control.command_interp = NULL;
> +
> /* Discard any remaining commands or status from previous stop. */
> bpstat_clear (&tp->control.stop_bpstat);
> }
> @@ -2200,6 +2206,14 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
> regcache_write_pc (regcache, addr);
> }
>
> + /* Record the interpreter that issued the execution command that
> + caused this thread to resume. If the top level interpreter is
> + MI/async, and the execution command was a CLI command
> + (next/step/etc.), we'll want to print stop event output to the MI
> + console channel (the stepped-to line, etc.), as if the user
> + entered the execution command on a real GDB console. */
> + inferior_thread ()->control.command_interp = command_interp ();
> +
> if (debug_infrun)
> fprintf_unfiltered (gdb_stdlog,
> "infrun: proceed (addr=%s, signal=%s, step=%d)\n",
> diff --git a/gdb/interps.c b/gdb/interps.c
> index e446747..f6b941c 100644
> --- a/gdb/interps.c
> +++ b/gdb/interps.c
> @@ -318,6 +318,29 @@ current_interp_display_prompt_p (void)
> data);
> }
>
> +/* The interpreter that is active while `interp_exec' is active, NULL
> + at all other times. */
> +static struct interp *command_interpreter;
> +
> +/* The interpreter that was active when a command was executed.
> + Normally that'd always be CURRENT_INTERPRETER, except that MI's
> + -interpreter-exec command doesn't actually flip the current
> + interpreter when running its sub-command. The
> + `command_interpreter' global tracks when interp_exec is called
> + (IOW, when -interpreter-exec is called). If that is set, it is
> + INTERP in '-interpreter-exec INTERP "CMD"' or in 'interpreter-exec
> + INTERP "CMD". Otherwise, interp_exec isn't active, and so the
> + interpreter running the command is the current interpreter. */
> +
> +struct interp *
> +command_interp (void)
> +{
> + if (command_interpreter != NULL)
> + return command_interpreter;
> + else
> + return current_interpreter;
> +}
> +
> /* Run the current command interpreter's main loop. */
> void
> current_interp_command_loop (void)
> @@ -351,9 +374,20 @@ interp_set_quiet (struct interp *interp, int quiet)
> struct gdb_exception
> interp_exec (struct interp *interp, const char *command_str)
> {
> + struct gdb_exception ex;
> + struct interp *save_command_interp;
> +
> gdb_assert (interp->procs->exec_proc != NULL);
>
> - return interp->procs->exec_proc (interp->data, command_str);
> + /* See `command_interp' for why we do this. */
> + save_command_interp = command_interpreter;
> + command_interpreter = interp;
> +
> + ex = interp->procs->exec_proc (interp->data, command_str);
> +
> + command_interpreter = save_command_interp;
> +
> + return ex;
> }
>
> /* A convenience routine that nulls out all the common command hooks.
> diff --git a/gdb/interps.h b/gdb/interps.h
> index 568b5df..13edf42 100644
> --- a/gdb/interps.h
> +++ b/gdb/interps.h
> @@ -96,6 +96,8 @@ extern int current_interp_set_logging (int start_log, struct ui_file *out,
> extern void *top_level_interpreter_data (void);
> extern struct interp *top_level_interpreter (void);
>
> +extern struct interp *command_interp (void);
> +
> /* True if the current interpreter is in async mode, false if in sync
> mode. If in sync mode, running a synchronous execution command
> (with execute_command, e.g, "next") will not return until the
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 862beaf..80fffa3 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -37,6 +37,7 @@
> #include "gdb.h"
> #include "objfiles.h"
> #include "tracepoint.h"
> +#include "cli-out.h"
>
> /* These are the interpreter setup, etc. functions for the MI
> interpreter. */
> @@ -231,6 +232,10 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc)
> error (_("-interpreter-exec: could not find interpreter \"%s\""),
> argv[0]);
>
> + /* Note that unlike the CLI version of this command, we don't
> + actually set INTERP_TO_USE as the current interpreter, as we
> + still want gdb_stdout, etc. to point at MI streams. */
> +
> /* Insert the MI out hooks, making sure to also call the
> interpreter's hooks if it has any. */
> /* KRS: We shouldn't need this... Events should be installed and
> @@ -415,6 +420,26 @@ mi_inferior_removed (struct inferior *inf)
> gdb_flush (mi->event_channel);
> }
>
> +/* Cleanup that restores a previous current uiout. */
> +
> +static void
> +restore_current_uiout_cleanup (void *arg)
> +{
> + struct ui_out *saved_uiout = arg;
> +
> + current_uiout = saved_uiout;
> +}
> +
> +/* Cleanup that destroys the a ui_out object. */
> +
> +static void
> +ui_out_free_cleanup (void *arg)
> +{
> + struct ui_out *uiout = arg;
> +
> + ui_out_destroy (uiout);
> +}
> +
> static void
> mi_on_normal_stop (struct bpstats *bs, int print_frame)
> {
> @@ -445,6 +470,58 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
>
> current_uiout = saved_uiout;
> }
> + /* Otherwise, frame information has already been printed by
> + normal_stop. */
> + else
> + {
> + /* Breakpoint hits should always be mirrored to the console.
> + Deciding what to mirror to the console wrt to breakpoints
> + and random stops gets messy real fast. E.g., say "s"
> + trips on a breakpoint. We'd clearly want to mirror the
> + event to the console in this case. But what about more
> + complicated cases like "s&; thread n; s&", and one of
> + those steps spawning a new thread, and that thread
> + hitting a breakpoint? It's impossible in general to
> + track whether the thread had any relation to the commands
> + that had been executed. So we just simplify and always
> + mirror breakpoints and random events to the console.
> +
> + Also, CLI execution commands (-interpreter-exec console
> + "next", for example) in async mode have the opposite
> + issue as described in the "then" branch above --
> + normal_stop has already printed frame information to MI
> + uiout, but nothing has printed the same information to
> + the CLI channel. We should print the source line to the
> + console when stepping or other similar commands, iff the
> + step was started by a console command (but not if it was
> + started with -exec-step or similar). */
> + struct thread_info *tp = inferior_thread ();
> +
> + if ((!tp->control.stop_step
> + && !tp->control.proceed_to_finish)
> + || (tp->control.command_interp != NULL
> + && tp->control.command_interp != top_level_interpreter ()))
> + {
> + struct mi_interp *mi = top_level_interpreter_data ();
> + struct target_waitstatus last;
> + ptid_t last_ptid;
> + struct ui_out *cli_uiout;
> + struct cleanup *old_chain;
> +
> + /* Sets the current uiout to a new temporary CLI uiout
> + assigned to STREAM. */
> + cli_uiout = cli_out_new (mi->out);
> + old_chain = make_cleanup (ui_out_free_cleanup, cli_uiout);
> +
> + make_cleanup (restore_current_uiout_cleanup, current_uiout);
> + current_uiout = cli_uiout;
> +
> + get_last_target_status (&last_ptid, &last);
> + print_stop_event (&last);
> +
> + do_cleanups (old_chain);
> + }
> + }
>
> ui_out_field_int (mi_uiout, "thread-id",
> pid_to_thread_id (inferior_ptid));
> diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
> index cb8d3af..50313ae 100644
> --- a/gdb/testsuite/gdb.mi/mi-cli.exp
> +++ b/gdb/testsuite/gdb.mi/mi-cli.exp
> @@ -69,6 +69,7 @@ set line_main_callme_2 [expr $line_main_return + 1]
> set line_callee4_head [gdb_get_line_number "callee4 ("]
> set line_callee4_body [expr $line_callee4_head + 2]
> set line_callee4_next [expr $line_callee4_body + 1]
> +set line_callee4_next_step [expr $line_callee4_next + 3]
>
> mi_gdb_test "-interpreter-exec console \"set args foobar\"" \
> ".*=cmd-param-changed,param=\"args\",value=\"foobar\".*\\^done" \
> @@ -153,13 +154,44 @@ if {$async} {
> mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \
> "" "check *stopped from CLI command"
>
> +mi_send_resuming_command "exec-step" "-exec-step to line \$line_callee4_next_step"
> +
> +# Test that the new current source line is _not_ output, given we
> +# executed MI's -exec-next, not CLI's 'next' command.
> +
> +set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output for -exec-step"]
> +
> +set test "-exec-step does not produce CLI step output"
> +if {[regexp "A + B" "$output"]} {
> + fail $test
> +} else {
> + pass $test
> +}
> +
> +mi_expect_stop "end-stepping-range" "callee4" "" ".*basics.c" $line_callee4_next_step \
> + "" "check *stopped from CLI command 2"
> +
> mi_gdb_test "600-break-insert -t basics.c:$line_main_hello" \
> {600\^done,bkpt=.number="3",type="breakpoint".*\}} \
> "-break-insert -t basics.c:\$line_main_hello"
>
> -mi_execute_to "exec-continue" "breakpoint-hit" "main" "" ".*basics.c" \
> - $line_main_hello { "" "disp=\"del\"" } \
> - "-exec-continue to line \$line_main_hello"
> +# Test that breakpoint events are always mirrored to the CLI output
> +# stream (both sync and async modes).
> +mi_send_resuming_command "exec-continue" "-exec-continue to line \$line_main_hello"
> +
> +set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output for breakpoint hit"]
> +set test "breakpoint hit produces CLI output"
> +set pattern "\\\\nTemporary breakpoint 3, main \\(\\) at \[^\n\]+basics.c:$line_main_hello\\\\n\[^\n\]+Hello"
> +
> +if {[regexp $pattern $output]} {
> + pass $test
> +} else {
> + fail $test
> +}
> +
> +# Test the MI output.
> +mi_expect_stop "breakpoint-hit" "main" "" ".*basics.c" \
> + $line_main_hello { "" "disp=\"del\"" } "Temporary breakpoint output hit in MI"
>
> # Test that the token is output even for CLI commands
> # Also test that *stopped includes frame information.
> @@ -167,10 +199,16 @@ mi_gdb_test "34 next" \
> ".*34\\\^running.*\\*running,thread-id=\"all\"" \
> "34 next: run"
>
> -if {!$async} {
> - gdb_expect {
> - -re "~\[^\r\n\]+\r\n" {
> - }
> +# Test that the new current source line is output to the console
> +# stream, given we executed the console 'next' command, not
> +# -exec-next.
> +set test "34 next: CLI output"
> +gdb_expect {
> + -re "~\"$line_main_return\[^\r\n\]+\r\n" {
> + pass $test
> + }
> + timeout {
> + fail "$test (timeout)"
> }
> }
>
> diff --git a/gdb/testsuite/gdb.mi/mi-solib.exp b/gdb/testsuite/gdb.mi/mi-solib.exp
> index 608d2b7..a684a3d 100644
> --- a/gdb/testsuite/gdb.mi/mi-solib.exp
> +++ b/gdb/testsuite/gdb.mi/mi-solib.exp
> @@ -60,4 +60,15 @@ mi_gdb_test "777-gdb-set stop-on-solib-events 1" "777\\^done" \
> # commands still cause the correct MI output to be generated.
> mi_run_with_cli
>
> +# Also test that the CLI solib event note is output.
> +set test "CLI prints solib event"
> +gdb_expect {
> + -re "~\"Stopped due to shared library event \\(no libraries added or removed\\)\\\\n" {
> + pass "$test"
> + }
> + timeout {
> + fail "$test (timeout)"
> + }
> +}
> +
> mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index 213073a..1f6dfa7 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -799,6 +799,30 @@ proc mi_gdb_test { args } {
> return $result
> }
>
> +# Collect output sent to the console output stream until UNTIL is
> +# seen. UNTIL is a regular expression. MESSAGE is the message to be
> +# printed in case of timeout.
> +
> +proc mi_gdb_expect_cli_output {until message} {
> +
> + set output ""
> + gdb_expect {
> + -re "~\"(\[^\r\n\]+)\"\r\n" {
> + append output $expect_out(1,string)
> + exp_continue
> + }
> + -notransfer -re "$until" {
> + # Done
> + }
> + timeout {
> + fail "$message (timeout)"
> + return ""
> + }
> + }
> +
> + return $output
> +}
> +
> #
> # MI run command. (A modified version of gdb_run_cmd)
> #
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v5 7/8] separate MI and target notions of async
2014-03-04 18:33 [RFC v5 0/8] enable target-async by default Tom Tromey
2014-03-04 18:33 ` [RFC v5 4/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode Tom Tromey
@ 2014-03-04 18:33 ` Tom Tromey
2014-03-04 18:40 ` Eli Zaretskii
2014-05-23 20:45 ` Pedro Alves
2014-03-04 18:33 ` [RFC v5 5/8] make dprintf.exp pass in always-async mode Tom Tromey
` (5 subsequent siblings)
7 siblings, 2 replies; 19+ messages in thread
From: Tom Tromey @ 2014-03-04 18:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This separates the MI and target notions of "async".
Unlike the CLI, MI chose to treat target-async specially -- setting it
changes the default behavior of commands. So, we can't get rid of the
option. Instead we have to make it MI-only.
Rather than make the targets always async, this patch introduces a new
"maint" parameter so that the gdb developer can control whether the
target is async. This makes it simpler to debug issues arising only
in the synchronous mode; important because it sync mode seems unlikely
to go away.
Note that "set target-async" also affects this new maint parameter.
The rationale for this is that then one can easily run the test suite
in the "maint set target-async off" mode and have tests which enable
target-async continue to work properly. This is unusual but, but it
is a maint command after all, and this behavior is useful.
The hardest part of this patch, to my surprise, was getting the MI
prompt to work properly. It was reasonably easy, and clean, to get
close to what the test suite expects; but to fix the last remaining
failure (mi-async.exp), I had to resort to a hack.
It seems to me that the MI grammar was never updated to account for
changes implied by async.
Perhaps some future MI can dispense with the prompt entirely.
2014-02-26 Tom Tromey <tromey@redhat.com>
* NEWS: Update.
* infrun.c (set_observer_mode): Use maint_async_permitted.
* linux-nat.c (linux_nat_is_async_p): Use maint_async_permitted.
(linux_nat_can_async_p): Likewise.
* mi/mi-interp.c (thread_command_not_mi): New function.
(mi_interpreter_prompt_p): Maybe print the MI prompt.
(mi_execute_command_input_handler): Conditionally print prompt.
(mi_on_resume): Check sync_execution before printing prompt.
* mi/mi-main.c (mi_target_can_async_p): New function.
(exec_continue): Maybe call async_disable_stdin.
(run_one_inferior, mi_cmd_exec_run, mi_cmd_list_target_features)
(mi_execute_async_cli_command): Use mi_target_can_async_p.
* remote.c (remote_open_1, remote_terminal_inferior)
(remote_terminal_ours, remote_can_async_p, remote_is_async_p):
Use maint_async_permitted.
* target.c (target_async_permitted): Update comment.
(maint_async_permitted, maint_async_permitted_1): New globals.
(set_maint_async_command, show_maint_async_command): New
functions.
(initialize_targets): Register new maint target-async commands.
* target.h (target_async_permitted): Update comment.
(maint_async_permitted): Declare.
2014-02-26 Tom Tromey <tromey@redhat.com>
* gdb.texinfo (Non-Stop Mode): Remove "set target-async 1"
from example.
(Background Execution): Move target-async docs...
(Asynchronous and non-stop modes): ... here. Rewrite to
MI form.
(Maintenance Commands): Document new "maint" commands.
2013-10-30 Tom Tromey <tromey@redhat.com>
* gdb.mi/mi-cli.exp: Don't check "$async".
* lib/mi-support.exp (mi_expect_stop): Special case for when
$reason is ".*".
---
gdb/ChangeLog | 25 +++++++++++++++++++++
gdb/NEWS | 10 +++++++++
gdb/doc/ChangeLog | 9 ++++++++
gdb/doc/gdb.texinfo | 37 ++++++++++++++++---------------
gdb/infrun.c | 2 +-
gdb/linux-nat.c | 10 ++-------
gdb/mi/mi-interp.c | 39 +++++++++++++++++++++++++++-----
gdb/mi/mi-main.c | 26 +++++++++++++++++-----
gdb/remote.c | 16 +++++++-------
gdb/target.c | 48 +++++++++++++++++++++++++++++++++++++++-
gdb/target.h | 7 ++++--
gdb/testsuite/ChangeLog | 6 +++++
gdb/testsuite/gdb.mi/mi-cli.exp | 17 +++-----------
gdb/testsuite/lib/mi-support.exp | 4 +++-
14 files changed, 191 insertions(+), 65 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 2a384ba..6d8dca5 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -43,6 +43,12 @@ maint ada show ignore-descriptive-types
the user manual for more details on descriptive types and the intended
usage of this option.
+maint set target-async (on|off)
+maint show target-async
+ This controls whether gdb targets operate in sync or async mode.
+ Normally the default is async, if it is available; but this can be
+ changed to more easily debug problems occurring only in sync mode.
+
* New features in the GDB remote stub, GDBserver
** New option --debug-format=option1[,option2,...] allows one to add
@@ -76,6 +82,10 @@ maint ada show ignore-descriptive-types
* The "catch syscall" command now works on s390*-linux* targets.
+* "set target-async" is deprecated as a CLI command and now only puts
+ MI into async mode. Targets which are capable of it use the async
+ mode by default.
+
* New remote packets
qXfer:btrace:read's annex
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index de5ac63..3f7d99e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5725,9 +5725,6 @@ To enter non-stop mode, use this sequence of commands before you run
or attach to your program:
@smallexample
-# Enable the async interface.
-set target-async 1
-
# If using the CLI, pagination breaks non-stop.
set pagination off
@@ -5797,21 +5794,6 @@ the program to report that some thread has stopped before prompting for
another command. In background execution, @value{GDBN} immediately gives
a command prompt so that you can issue other commands while your program runs.
-You need to explicitly enable asynchronous mode before you can use
-background execution commands. You can use these commands to
-manipulate the asynchronous mode setting:
-
-@table @code
-@kindex set target-async
-@item set target-async on
-Enable asynchronous mode.
-@item set target-async off
-Disable asynchronous mode.
-@kindex show target-async
-@item show target-async
-Show the current target-async setting.
-@end table
-
If the target doesn't support async mode, @value{GDBN} issues an error
message if you attempt to use the background execution commands.
@@ -24713,6 +24695,17 @@ frontend has started the executable or attached to the target, it can
find if asynchronous execution is enabled using the
@code{-list-target-features} command.
+@table @code
+@kindex -gdb-set target-async
+@item -gdb-set target-async on
+Enable asynchronous mode. This mode only affects MI commands.
+@item -gdb-set target-async off
+Disable asynchronous mode.
+@kindex -gdb-show target-async
+@item -gdb-show target-async
+Show the current target-async setting.
+@end table
+
Even if @value{GDBN} can accept a command while target is running,
many commands that access the target do not work when the target is
running. Therefore, asynchronous command execution is most useful
@@ -33421,6 +33414,14 @@ Control whether to show all non zero areas within a 1k block starting
at thread local base, when using the @samp{info w32 thread-information-block}
command.
+@kindex maint set target-async
+@kindex maint show target-async
+@item maint set target-async
+@itemx maint show target-async
+This controls whether gdb targets operate in sync or async mode.
+Normally the default is async, if it is available; but this can be
+changed to more easily debug problems occurring only in sync mode.
+
@kindex maint set per-command
@kindex maint show per-command
@item maint set per-command
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 188a6e8..05bca8b 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -247,7 +247,7 @@ set_observer_mode (char *args, int from_tty,
going out we leave it that way. */
if (observer_mode)
{
- target_async_permitted = 1;
+ maint_async_permitted = 1;
pagination_enabled = 0;
non_stop = non_stop_1 = 1;
}
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 5535462..844006d 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4514,10 +4514,7 @@ linux_trad_target (CORE_ADDR (*register_u_offset)(struct gdbarch *, int, int))
static int
linux_nat_is_async_p (struct target_ops *ops)
{
- /* NOTE: palves 2008-03-21: We're only async when the user requests
- it explicitly with the "set target-async" command.
- Someday, linux will always be async. */
- return target_async_permitted;
+ return maint_async_permitted;
}
/* target_can_async_p implementation. */
@@ -4525,10 +4522,7 @@ linux_nat_is_async_p (struct target_ops *ops)
static int
linux_nat_can_async_p (struct target_ops *ops)
{
- /* NOTE: palves 2008-03-21: We're only async when the user requests
- it explicitly with the "set target-async" command.
- Someday, linux will always be async. */
- return target_async_permitted;
+ return maint_async_permitted;
}
static int
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 833e7f1..ef19af6 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -212,6 +212,24 @@ mi_interpreter_exec (void *data, const char *command)
static int
mi_interpreter_prompt_p (void *data)
{
+ if (!interp_quiet_p (NULL))
+ {
+ /* This is a hack. In order to replicate gdb's behavior from
+ when target-async was not enabled by default, we need to
+ print the MI prompt. We can't return 1 here because that
+ will invoke the CLI prompt-printing machinery, which is not
+ what we want. So, we print the prompt directly.
+
+ We don't print the prompt when MI is in its special async
+ mode and when the target is async-capable. In this case the
+ printing is handled elsewhere. */
+ if (!target_async_permitted || !target_is_async_p ())
+ {
+ fputs_unfiltered ("(gdb) \n", raw_stdout);
+ gdb_flush (raw_stdout);
+ }
+ }
+
return 0;
}
@@ -303,8 +321,17 @@ mi_execute_command_input_handler (char *cmd)
{
mi_execute_command_wrapper (cmd);
- fputs_unfiltered ("(gdb) \n", raw_stdout);
- gdb_flush (raw_stdout);
+ /* MI generally prints a prompt after a command. However, if target
+ is async, and a synchronous command was issued, then we will
+ print the prompt elsewhere, after printing "*running".
+ target_is_async_p checks whether the target is async;
+ sync_execution checks whether a synchronous command was
+ issued. */
+ if (!target_is_async_p () || !sync_execution)
+ {
+ fputs_unfiltered ("(gdb) \n", raw_stdout);
+ gdb_flush (raw_stdout);
+ }
}
static void
@@ -816,10 +843,10 @@ mi_on_resume (ptid_t ptid)
running_result_record_printed = 1;
/* This is what gdb used to do historically -- printing prompt even if
it cannot actually accept any input. This will be surely removed
- for MI3, and may be removed even earler. */
- /* FIXME: review the use of target_is_async_p here -- is that
- what we want? */
- if (!target_is_async_p ())
+ for MI3, and may be removed even earler. SYNC_EXECUTION is
+ checked here because we only need to emit a prompt if a
+ synchronous command was issued when the target is async. */
+ if (!target_is_async_p () || sync_execution)
fputs_unfiltered ("(gdb) \n", raw_stdout);
}
gdb_flush (raw_stdout);
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 161307a..8b061d5 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -104,6 +104,15 @@ static int register_changed_p (int regnum, struct regcache *,
static void output_register (struct frame_info *, int regnum, int format,
int skip_unavailable);
+/* A wrapper for target_can_async_p that takes the MI setting into
+ account. */
+
+static int
+mi_target_can_async_p (void)
+{
+ return target_async_permitted && target_can_async_p ();
+}
+
/* Command implementations. FIXME: Is this libgdb? No. This is the MI
layer that calls libgdb. Any operation used in the below should be
formalized. */
@@ -260,6 +269,11 @@ exec_continue (char **argv, int argc)
{
struct cleanup *back_to = make_cleanup_restore_integer (&sched_multi);
+ /* If MI is in sync mode but the target is async, then
+ normal_stop enabled stdin. We undo the change here. */
+ if (!target_async_permitted && target_can_async_p ())
+ async_disable_stdin ();
+
if (current_context->all)
{
sched_multi = 1;
@@ -394,8 +408,8 @@ run_one_inferior (struct inferior *inf, void *arg)
switch_to_thread (null_ptid);
set_current_program_space (inf->pspace);
}
- mi_execute_cli_command (run_cmd, target_can_async_p (),
- target_can_async_p () ? "&" : NULL);
+ mi_execute_cli_command (run_cmd, mi_target_can_async_p (),
+ mi_target_can_async_p () ? "&" : NULL);
return 0;
}
@@ -449,8 +463,8 @@ mi_cmd_exec_run (char *command, char **argv, int argc)
{
const char *run_cmd = start_p ? "start" : "run";
- mi_execute_cli_command (run_cmd, target_can_async_p (),
- target_can_async_p () ? "&" : NULL);
+ mi_execute_cli_command (run_cmd, mi_target_can_async_p (),
+ mi_target_can_async_p () ? "&" : NULL);
}
}
@@ -1837,7 +1851,7 @@ mi_cmd_list_target_features (char *command, char **argv, int argc)
struct ui_out *uiout = current_uiout;
cleanup = make_cleanup_ui_out_list_begin_end (uiout, "features");
- if (target_can_async_p ())
+ if (mi_target_can_async_p ())
ui_out_field_string (uiout, NULL, "async");
if (target_can_execute_reverse)
ui_out_field_string (uiout, NULL, "reverse");
@@ -2268,7 +2282,7 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc)
struct cleanup *old_cleanups;
char *run;
- if (target_can_async_p ())
+ if (mi_target_can_async_p ())
run = xstrprintf ("%s %s&", cli_command, argc ? *argv : "");
else
run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
diff --git a/gdb/remote.c b/gdb/remote.c
index e03d3bf..94c7165 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4263,7 +4263,7 @@ remote_open_1 (char *name, int from_tty,
"(e.g. /dev/ttyS0, /dev/ttya, COM1, etc.)."));
/* See FIXME above. */
- if (!target_async_permitted)
+ if (!maint_async_permitted)
wait_forever_enabled_p = 1;
/* If we're connected to a running target, target_preopen will kill it.
@@ -4350,7 +4350,7 @@ remote_open_1 (char *name, int from_tty,
rs->use_threadinfo_query = 1;
rs->use_threadextra_query = 1;
- if (target_async_permitted)
+ if (maint_async_permitted)
{
/* With this target we start out by owning the terminal. */
remote_async_terminal_ours_p = 1;
@@ -4399,13 +4399,13 @@ remote_open_1 (char *name, int from_tty,
already before throwing the exception. */
if (rs->remote_desc != NULL)
remote_unpush_target ();
- if (target_async_permitted)
+ if (maint_async_permitted)
wait_forever_enabled_p = 1;
throw_exception (ex);
}
}
- if (target_async_permitted)
+ if (maint_async_permitted)
wait_forever_enabled_p = 1;
}
@@ -5135,7 +5135,7 @@ Give up (and stop debugging it)? ")))
static void
remote_terminal_inferior (struct target_ops *self)
{
- if (!target_async_permitted)
+ if (!maint_async_permitted)
/* Nothing to do. */
return;
@@ -5158,7 +5158,7 @@ remote_terminal_inferior (struct target_ops *self)
static void
remote_terminal_ours (struct target_ops *self)
{
- if (!target_async_permitted)
+ if (!maint_async_permitted)
/* Nothing to do. */
return;
@@ -11536,7 +11536,7 @@ remote_can_async_p (struct target_ops *ops)
{
struct remote_state *rs = get_remote_state ();
- if (!target_async_permitted)
+ if (!maint_async_permitted)
/* We only enable async when the user specifically asks for it. */
return 0;
@@ -11549,7 +11549,7 @@ remote_is_async_p (struct target_ops *ops)
{
struct remote_state *rs = get_remote_state ();
- if (!target_async_permitted)
+ if (!maint_async_permitted)
/* We only enable async when the user specifically asks for it. */
return 0;
diff --git a/gdb/target.c b/gdb/target.c
index 40a2a39..6711bd7 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -4199,7 +4199,38 @@ maintenance_print_target_stack (char *cmd, int from_tty)
}
}
-/* Controls if async mode is permitted. */
+/* Controls if targets can report that they are async. This is just
+ for maintainers to use when debugging gdb. */
+int maint_async_permitted = 0;
+
+/* The set command writes to this variable. If the inferior is
+ executing, maint_async_permitted is *not* updated. */
+static int maint_async_permitted_1 = 0;
+
+static void
+set_maint_async_command (char *args, int from_tty,
+ struct cmd_list_element *c)
+{
+ if (have_live_inferiors ())
+ {
+ maint_async_permitted_1 = maint_async_permitted;
+ error (_("Cannot change this setting while the inferior is running."));
+ }
+
+ maint_async_permitted = maint_async_permitted_1;
+}
+
+static void
+show_maint_async_command (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c,
+ const char *value)
+{
+ fprintf_filtered (file,
+ _("Controlling the inferior in "
+ "asynchronous mode is %s.\n"), value);
+}
+
+/* Controls if async mode is permitted. This is used only by MI. */
int target_async_permitted = 0;
/* The set command writes to this variable. If the inferior is
@@ -4217,6 +4248,12 @@ set_target_async_command (char *args, int from_tty,
}
target_async_permitted = target_async_permitted_1;
+ /* Sometimes it is handy to run the test suite with "maint set
+ target-async off". But, in this case, if the test suite uses
+ "set target-async on" we want to replicate the old behavior of
+ turning on target-async. Hence we set these variables here. */
+ maint_async_permitted = target_async_permitted_1;
+ maint_async_permitted_1 = target_async_permitted_1;
}
static void
@@ -4331,6 +4368,15 @@ Tells gdb whether to control the inferior in asynchronous mode."),
show_target_async_command,
&setlist,
&showlist);
+ add_setshow_boolean_cmd ("target-async", no_class,
+ &maint_async_permitted_1, _("\
+Set whether gdb controls the inferior in asynchronous mode."), _("\
+Show whether gdb controls the inferior in asynchronous mode."), _("\
+Tells gdb whether to control the inferior in asynchronous mode."),
+ set_maint_async_command,
+ show_maint_async_command,
+ &maintenance_set_cmdlist,
+ &maintenance_show_cmdlist);
add_setshow_boolean_cmd ("may-write-registers", class_support,
&may_write_registers_1, _("\
diff --git a/gdb/target.h b/gdb/target.h
index 871cc95..703af3a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1612,10 +1612,13 @@ extern int default_child_has_execution (struct target_ops *ops,
#define target_can_lock_scheduler \
(current_target.to_has_thread_control & tc_schedlock)
-/* Should the target enable async mode if it is supported? Temporary
- cludge until async mode is a strict superset of sync mode. */
+/* Should MI operate in async mode? */
extern int target_async_permitted;
+/* A maint setting so async-capable targets can enter sync mode for
+ debugging. */
+extern int maint_async_permitted;
+
/* Can the target support asynchronous execution? */
#define target_can_async_p() (current_target.to_can_async_p (¤t_target))
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index e158b7e..8cfab3b 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -134,20 +134,9 @@ mi_gdb_test "500-stack-select-frame 0" \
{500\^done} \
"-stack-select-frame 0"
-# When a CLI command is entered in MI session, the respose is different in
-# sync and async modes. In sync mode normal_stop is called when current
-# interpreter is CLI. So:
-# - print_stop_reason prints stop reason in CLI uiout, and we don't show it
-# in MI
-# - The stop position is printed, and appears in MI 'console' channel.
-#
-# In async mode the stop event is processed when we're back to MI interpreter,
-# so the stop reason is printed into MI uiout an.
-if {$async} {
- set reason "end-stepping-range"
-} else {
- set reason ""
-}
+# Allow a reason, or not. Which we get depends at least on whether
+# the target being tested supports async.
+set reason ".*"
mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \
"" "check *stopped from CLI command"
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 213073a..16c7951 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1096,7 +1096,9 @@ proc mi_expect_stop { reason func args file line extra test } {
}
set r ""
- if { $reason != "" } {
+ if { $reason == ".*" } {
+ set r "(?:reason=\".*\",)?"
+ } elseif { $reason != "" } {
set r "reason=\"$reason\","
}
--
1.8.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v5 7/8] separate MI and target notions of async
2014-03-04 18:33 ` [RFC v5 7/8] separate MI and target notions of async Tom Tromey
@ 2014-03-04 18:40 ` Eli Zaretskii
2014-03-04 19:11 ` Tom Tromey
2014-05-23 20:45 ` Pedro Alves
1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2014-03-04 18:40 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches, tromey
> From: Tom Tromey <tromey@redhat.com>
> Cc: Tom Tromey <tromey@redhat.com>
> Date: Tue, 4 Mar 2014 11:32:53 -0700
>
> +@kindex maint set target-async
> +@kindex maint show target-async
> +@item maint set target-async
> +@itemx maint show target-async
> +This controls whether gdb targets operate in sync or async mode.
^^^
@value{GDBN}
The documentation parts are OK with this fixed.
Thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v5 7/8] separate MI and target notions of async
2014-03-04 18:33 ` [RFC v5 7/8] separate MI and target notions of async Tom Tromey
2014-03-04 18:40 ` Eli Zaretskii
@ 2014-05-23 20:45 ` Pedro Alves
1 sibling, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-05-23 20:45 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 03/04/2014 06:32 PM, Tom Tromey wrote:
> This separates the MI and target notions of "async".
>
> Unlike the CLI, MI chose to treat target-async specially -- setting it
> changes the default behavior of commands. So, we can't get rid of the
> option. Instead we have to make it MI-only.
>
> Rather than make the targets always async, this patch introduces a new
> "maint" parameter so that the gdb developer can control whether the
> target is async. This makes it simpler to debug issues arising only
> in the synchronous mode; important because it sync mode seems unlikely
> to go away.
>
> Note that "set target-async" also affects this new maint parameter.
> The rationale for this is that then one can easily run the test suite
> in the "maint set target-async off" mode and have tests which enable
> target-async continue to work properly. This is unusual but, but it
> is a maint command after all, and this behavior is useful.
As discussed, v6 will not have this coupling. the idea is that
"maint set target-async off" emulates a non-async-capable target,
so "maint set target-async off" + "set target-async on" + "c&"
should result in failure to run a background command, just like
e.g., if one tries "set target-async on" + "c&" on Windows.
In v6, I'm proposing adding an "set mi-async" option, and making
"set target-async" a deprecated alias for "set mi-async" going
forward, to avoid confusion.
>
> The hardest part of this patch, to my surprise, was getting the MI
> prompt to work properly. It was reasonably easy, and clean, to get
> close to what the test suite expects; but to fix the last remaining
> failure (mi-async.exp), I had to resort to a hack.
I think I found a cleaner way to do this. v6 will include it.
> /* Command implementations. FIXME: Is this libgdb? No. This is the MI
> layer that calls libgdb. Any operation used in the below should be
> formalized. */
> @@ -260,6 +269,11 @@ exec_continue (char **argv, int argc)
> {
> struct cleanup *back_to = make_cleanup_restore_integer (&sched_multi);
>
> + /* If MI is in sync mode but the target is async, then
> + normal_stop enabled stdin. We undo the change here. */
This comment threw me off for a while. It doesn't matter what
normal_stop did in a previous command. The code below is actually
correct, in as much as we need to do that before all execution
commands. We can now just call prepare_execution_command to
do it for us.
> + if (!target_async_permitted && target_can_async_p ())
> + async_disable_stdin ();
> diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
> index e158b7e..8cfab3b 100644
> --- a/gdb/testsuite/gdb.mi/mi-cli.exp
> +++ b/gdb/testsuite/gdb.mi/mi-cli.exp
> @@ -134,20 +134,9 @@ mi_gdb_test "500-stack-select-frame 0" \
> {500\^done} \
> "-stack-select-frame 0"
>
> -# When a CLI command is entered in MI session, the respose is different in
> -# sync and async modes. In sync mode normal_stop is called when current
> -# interpreter is CLI. So:
> -# - print_stop_reason prints stop reason in CLI uiout, and we don't show it
> -# in MI
> -# - The stop position is printed, and appears in MI 'console' channel.
> -#
> -# In async mode the stop event is processed when we're back to MI interpreter,
> -# so the stop reason is printed into MI uiout an.
> -if {$async} {
> - set reason "end-stepping-range"
> -} else {
> - set reason ""
> -}
> +# Allow a reason, or not. Which we get depends at least on whether
> +# the target being tested supports async.
> +set reason ".*"
This patch gets rid of this in the opposite direction:
https://sourceware.org/ml/gdb-patches/2014-05/msg00566.html
> mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \
> "" "check *stopped from CLI command"
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index 213073a..16c7951 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -1096,7 +1096,9 @@ proc mi_expect_stop { reason func args file line extra test } {
> }
>
> set r ""
> - if { $reason != "" } {
> + if { $reason == ".*" } {
> + set r "(?:reason=\".*\",)?"
> + } elseif { $reason != "" } {
> set r "reason=\"$reason\","
> }
>
And ends up no longer necessary.
v6 coming up... Getting close. :-)
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v5 5/8] make dprintf.exp pass in always-async mode
2014-03-04 18:33 [RFC v5 0/8] enable target-async by default Tom Tromey
2014-03-04 18:33 ` [RFC v5 4/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode Tom Tromey
2014-03-04 18:33 ` [RFC v5 7/8] separate MI and target notions of async Tom Tromey
@ 2014-03-04 18:33 ` Tom Tromey
2014-03-20 18:04 ` Pedro Alves
2014-03-04 18:33 ` [RFC v5 1/8] fix latent bugs in ui-out.c Tom Tromey
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Tom Tromey @ 2014-03-04 18:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
When target-async is enabled, dprintf.exp fails.
This happens because run_inferior_call causes gdb to forget that it is
running in sync_execution mode, so something like a breakpoint
condition that makes an inferior call causes gdb to enter fully async
mode.
This patch fixes the problem by noticing when gdb was in
sync_execution mode in run_inferior_call, and taking care to restore
this state afterward.
This patch also adds a new test case, so that regression testing isn't
dependent on the implementation of dprintf.
2013-10-30 Tom Tromey <tromey@redhat.com>
PR cli/15718:
* infcall.c: Include event-top.h.
(run_inferior_call): Call async_disable_stdin if needed.
2014-03-04 Tom Tromey <tromey@redhat.com>
* gdb.base/async-cond.c: New file.
* gdb.base/async-cond.exp: New file.
---
gdb/ChangeLog | 6 ++++++
gdb/infcall.c | 10 ++++++++++
gdb/testsuite/ChangeLog | 5 +++++
gdb/testsuite/gdb.base/async-cond.c | 32 ++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/async-cond.exp | 25 +++++++++++++++++++++++++
5 files changed, 78 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/async-cond.c
create mode 100644 gdb/testsuite/gdb.base/async-cond.exp
diff --git a/gdb/infcall.c b/gdb/infcall.c
index ad18ff1..c917f92 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -36,6 +36,7 @@
#include "ada-lang.h"
#include "gdbthread.h"
#include "exceptions.h"
+#include "event-top.h"
/* If we can't find a function's name from its address,
we print this instead. */
@@ -398,6 +399,8 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
TRY_CATCH (e, RETURN_MASK_ALL)
{
+ int was_sync = sync_execution;
+
proceed (real_pc, GDB_SIGNAL_0, 0);
/* Inferior function calls are always synchronous, even if the
@@ -407,6 +410,13 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
{
wait_for_inferior ();
normal_stop ();
+ /* If gdb was previously in sync execution mode, then ensure
+ that it remains so. normal_stop calls
+ async_enable_stdin, so reset it again here. In other
+ cases, stdin will be re-enabled by
+ inferior_event_handler, when an exception is thrown. */
+ if (was_sync)
+ async_disable_stdin ();
}
}
diff --git a/gdb/testsuite/gdb.base/async-cond.c b/gdb/testsuite/gdb.base/async-cond.c
new file mode 100644
index 0000000..469fcc2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/async-cond.c
@@ -0,0 +1,32 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int
+zero (void)
+{
+ return 0;
+}
+
+int
+g (void)
+{
+ return 23;
+}
+
+int
+main (void)
+{
+ g();
+}
diff --git a/gdb/testsuite/gdb.base/async-cond.exp b/gdb/testsuite/gdb.base/async-cond.exp
new file mode 100644
index 0000000..8d857eb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/async-cond.exp
@@ -0,0 +1,25 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+ return -1
+}
+
+gdb_test_no_output "set target-async on"
+gdb_test "break g if zero()" "Breakpoint .*"
+gdb_test "run" "Inferior.*exited.*"
--
1.8.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v5 5/8] make dprintf.exp pass in always-async mode
2014-03-04 18:33 ` [RFC v5 5/8] make dprintf.exp pass in always-async mode Tom Tromey
@ 2014-03-20 18:04 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-03-20 18:04 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 03/04/2014 06:32 PM, Tom Tromey wrote:
> When target-async is enabled, dprintf.exp fails.
>
> This happens because run_inferior_call causes gdb to forget that it is
> running in sync_execution mode, so something like a breakpoint
> condition that makes an inferior call causes gdb to enter fully async
> mode.
>
> This patch fixes the problem by noticing when gdb was in
> sync_execution mode in run_inferior_call, and taking care to restore
> this state afterward.
>
> This patch also adds a new test case, so that regression testing isn't
> dependent on the implementation of dprintf.
...
> diff --git a/gdb/testsuite/gdb.base/async-cond.exp b/gdb/testsuite/gdb.base/async-cond.exp
> new file mode 100644
...
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> + return -1
> +}
> +
> +gdb_test_no_output "set target-async on"
As discussed, I don't think we should really have these knobs. The
test should pass in sync targets just as well. I think it's best
to not force async, and test the whole testsuite with a board
that forces async mode (or hack gdb) to exercise this in async,
just doing that caught the issue in dprintf.exp.
Given that reasoning, I've renamed the test to condbreak-$foo.exp,
for similarity with condbreak.exp. This was after noticing
the existence of condbreak.exp, and trying a little to exercise this
problem in that test, and giving
up -- break.c/break1.c/condbreak.exp/break.exp are just too ugly and
messy.
> +gdb_test "break g if zero()" "Breakpoint .*"
> +gdb_test "run" "Inferior.*exited.*"
Note that gdb_test "run" doesn't work with "target remote".
Running ../../../src/gdb/testsuite/gdb.base/async-cond.exp ...
ERROR: tcl error sourcing ../../../src/gdb/testsuite/gdb.base/async-cond.exp.
ERROR: gdbserver does not support run without extended-remote
while executing
"error "gdbserver does not support $command without extended-remote""
(procedure "gdb_test_multiple" line 23)
invoked from within
"gdb_test_multiple $command $message {
-re "\[\r\n\]*($pattern)\[\r\n\]+$gdb_prompt $" {
if ![string match "" $message] then {
pass "$message"
..."
I also extended the commit log description some.
Here's what I pushed.
-------
Subject: [PATCH] make dprintf.exp pass in target async mode
When target-async is enabled, dprintf.exp fails:
Running ../../../src/gdb/testsuite/gdb.base/dprintf.exp ...
FAIL: gdb.base/dprintf.exp: 1st dprintf, call
FAIL: gdb.base/dprintf.exp: 2nd dprintf, call
FAIL: gdb.base/dprintf.exp: Set dprintf function
FAIL: gdb.base/dprintf.exp: 1st dprintf, fprintf
FAIL: gdb.base/dprintf.exp: 2nd dprintf, fprintf
Breakpoint 2, main (argc=1, argv=0x7fffffffd3f8) at ../../../src/gdb/testsuite/gdb.base/dprintf.c:33
33 int loc = 1234;
(gdb) continue
Continuing.
kickoff 1234
also to stderr 1234
At foo entry
(gdb) FAIL: gdb.base/dprintf.exp: 1st dprintf, call
The problem is that GDB gave the prompt back to the user too early.
This happens when calling functions while handling an event that
doesn't cause a user visible stop. dprintf with "set dprintf-style
gdb" is one such case. This patch adds a test case that has a
breakpoint with a condition that calls a function that returns false,
so that regression testing isn't dependent on the implementation of
dprintf.
The problem happens because run_inferior_call causes GDB to forget
that it is running in sync_execution mode, so any event that runs an
inferior call causes fetch_inferior_event to display the prompt, even
if the event should not result in a user visible stop (that is, gdb
resumes the inferior and waits for the next event).
This patch fixes the issue by noticing when GDB was in sync_execution
mode in run_inferior_call, and taking care to restore this state
afterward.
gdb/
2014-03-20 Tom Tromey <tromey@redhat.com>
PR cli/15718
* infcall.c: Include event-top.h.
(run_inferior_call): Call async_disable_stdin if needed.
gdb/testsuite/
2014-03-20 Tom Tromey <tromey@redhat.com>
Pedro Alves <palves@redhat.com>
PR cli/15718
* gdb.base/condbreak-call-false.c: New file.
* gdb.base/condbreak-call-false.exp: New file.
---
gdb/ChangeLog | 6 ++++
gdb/infcall.c | 10 +++++++
gdb/testsuite/ChangeLog | 7 +++++
gdb/testsuite/gdb.base/condbreak-call-false.c | 39 +++++++++++++++++++++++++
gdb/testsuite/gdb.base/condbreak-call-false.exp | 33 +++++++++++++++++++++
5 files changed, 95 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/condbreak-call-false.c
create mode 100644 gdb/testsuite/gdb.base/condbreak-call-false.exp
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 814be2c..acb25f8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-03-20 Tom Tromey <tromey@redhat.com>
+
+ PR cli/15718
+ * infcall.c: Include event-top.h.
+ (run_inferior_call): Call async_disable_stdin if needed.
+
2014-03-20 Pedro Alves <palves@redhat.com>
* infrun.c (prepare_to_proceed): Delete.
diff --git a/gdb/infcall.c b/gdb/infcall.c
index ad18ff1..9907263 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -36,6 +36,7 @@
#include "ada-lang.h"
#include "gdbthread.h"
#include "exceptions.h"
+#include "event-top.h"
/* If we can't find a function's name from its address,
we print this instead. */
@@ -398,6 +399,8 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
TRY_CATCH (e, RETURN_MASK_ALL)
{
+ int was_sync = sync_execution;
+
proceed (real_pc, GDB_SIGNAL_0, 0);
/* Inferior function calls are always synchronous, even if the
@@ -407,6 +410,13 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
{
wait_for_inferior ();
normal_stop ();
+ /* If GDB was previously in sync execution mode, then ensure
+ that it remains so. normal_stop calls
+ async_enable_stdin, so reset it again here. In other
+ cases, stdin will be re-enabled by
+ inferior_event_handler, when an exception is thrown. */
+ if (was_sync)
+ async_disable_stdin ();
}
}
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 31e0e5d..d6fb1dc 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2014-03-20 Tom Tromey <tromey@redhat.com>
+ Pedro Alves <palves@redhat.com>
+
+ PR cli/15718
+ * gdb.base/condbreak-call-false.c: New file.
+ * gdb.base/condbreak-call-false.exp: New file.
+
2014-03-20 Pedro Alves <palves@redhat.com>
* gdb.threads/signal-while-stepping-over-bp-other-thread.c (pid):
diff --git a/gdb/testsuite/gdb.base/condbreak-call-false.c b/gdb/testsuite/gdb.base/condbreak-call-false.c
new file mode 100644
index 0000000..ed9a3f4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-call-false.c
@@ -0,0 +1,39 @@
+/* Copyright 2013-2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int
+zero (void)
+{
+ return 0;
+}
+
+int
+foo (void)
+{
+ return 23;
+}
+
+void
+bar (void)
+{
+}
+
+
+int
+main (void)
+{
+ foo ();
+ bar ();
+}
diff --git a/gdb/testsuite/gdb.base/condbreak-call-false.exp b/gdb/testsuite/gdb.base/condbreak-call-false.exp
new file mode 100644
index 0000000..d1cb2bb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-call-false.exp
@@ -0,0 +1,33 @@
+# Copyright 2013-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test that a breakpoint condition with a function call that returns
+# false works as expected.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+ return -1
+}
+
+if ![runto_main] then {
+ fail "Can't run to main"
+ return 0
+}
+
+gdb_test "break foo if zero()" "Breakpoint .*"
+gdb_test "break bar" "Breakpoint .*"
+
+gdb_test "c" "Breakpoint .* bar .*"
--
1.7.11.7
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v5 1/8] fix latent bugs in ui-out.c
2014-03-04 18:33 [RFC v5 0/8] enable target-async by default Tom Tromey
` (2 preceding siblings ...)
2014-03-04 18:33 ` [RFC v5 5/8] make dprintf.exp pass in always-async mode Tom Tromey
@ 2014-03-04 18:33 ` Tom Tromey
2014-03-17 19:16 ` Pedro Alves
2014-03-04 18:33 ` [RFC v5 2/8] PR gdb/13860: make -interpreter-exec console "list" behave more like "list" Tom Tromey
` (3 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Tom Tromey @ 2014-03-04 18:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
The destructor code in ui-out.c has a latent bug, which is hidden by
the fact that nothing uses this right now. This patch fixes the
problem. The bug is that we don't always clear a pointer in the
ui-out object, leading to a bad free.
2013-10-30 Tom Tromey <tromey@redhat.com>
* ui-out.c (clear_table, ui_out_new): Clear uiout->table.id.
---
gdb/ChangeLog | 4 ++++
gdb/ui-out.c | 5 +++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 2edb140..63cbc6e 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -807,8 +807,8 @@ uo_table_header (struct ui_out *uiout, int width, enum ui_align align,
static void
clear_table (struct ui_out *uiout)
{
- if (uiout->table.id)
- xfree (uiout->table.id);
+ xfree (uiout->table.id);
+ uiout->table.id = NULL;
clear_header_list (uiout);
}
@@ -1114,6 +1114,7 @@ ui_out_new (const struct ui_out_impl *impl, void *data,
current->field_count = 0;
VEC_safe_push (ui_out_level_p, uiout->levels, current);
+ uiout->table.id = NULL;
uiout->table.header_first = NULL;
uiout->table.header_last = NULL;
uiout->table.header_next = NULL;
--
1.8.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v5 1/8] fix latent bugs in ui-out.c
2014-03-04 18:33 ` [RFC v5 1/8] fix latent bugs in ui-out.c Tom Tromey
@ 2014-03-17 19:16 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-03-17 19:16 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi,
On 03/04/2014 06:32 PM, Tom Tromey wrote:
> The destructor code in ui-out.c has a latent bug, which is hidden by
> the fact that nothing uses this right now. This patch fixes the
> problem. The bug is that we don't always clear a pointer in the
> ui-out object, leading to a bad free.
>
> 2013-10-30 Tom Tromey <tromey@redhat.com>
>
> * ui-out.c (clear_table, ui_out_new): Clear uiout->table.id.
This one is obviously correct. There's really no need
to hold it further, so I went ahead and pushed it in.
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v5 2/8] PR gdb/13860: make -interpreter-exec console "list" behave more like "list".
2014-03-04 18:33 [RFC v5 0/8] enable target-async by default Tom Tromey
` (3 preceding siblings ...)
2014-03-04 18:33 ` [RFC v5 1/8] fix latent bugs in ui-out.c Tom Tromey
@ 2014-03-04 18:33 ` Tom Tromey
2014-03-18 16:04 ` [RFC v6] " Pedro Alves
2014-03-04 18:33 ` [RFC v5 3/8] PR gdb/13860: make "-exec-foo"'s MI output equal to "foo"'s MI output Tom Tromey
` (2 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Tom Tromey @ 2014-03-04 18:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
From: Pedro Alves <palves@redhat.com>
Patch 3 in this series made me notice that "list" behaves differently
in CLI vs MI. Particularly:
>./gdb -nx -q ./testsuite/gdb.mi/mi-cli
Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli...done.
(gdb) start
Temporary breakpoint 1 at 0x40054d: file ../../../src/gdb/testsuite/gdb.mi/basics.c, line 62.
Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli
Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62
62 callee1 (2, "A string argument.", 3.5);
(gdb) list
57 {
58 }
59
60 main ()
61 {
62 callee1 (2, "A string argument.", 3.5);
63 callee1 (2, "A string argument.", 3.5);
64
65 do_nothing (); /* Hello, World! */
66
(gdb)
Note the list started at line 57. IOW, the program stopped at line
62, and GDB centered the list on that.
compare with:
>./gdb -nx -q ./testsuite/gdb.mi/mi-cli -i=mi
=thread-group-added,id="i1"
~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli..."
~"done.\n"
(gdb)
start
&"start\n"
~"Temporary breakpoint 1 at 0x40054d: file ../../../src/gdb/testsuite/gdb.mi/basics.c, line 62.\n"
=breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x000000000040054d",func="main",file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62",times="0",original-location="main"}
~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli \n"
=thread-group-started,id="i1",pid="14221"
=thread-created,id="1",group-id="i1"
^running
*running,thread-id="all"
(gdb)
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
=library-loaded,id="/lib64/libm.so.6",target-name="/lib64/libm.so.6",host-name="/lib64/libm.so.6",symbols-loaded="0",thread-group="i1"
=library-loaded,id="/lib64/libc.so.6",target-name="/lib64/libc.so.6",host-name="/lib64/libc.so.6",symbols-loaded="0",thread-group="i1"
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x000000000040054d",func="main",file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62",times="1",original-location="main"}
~"\nTemporary breakpoint "
~"1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62\n"
~"62\t callee1 (2, \"A string argument.\", 3.5);\n"
*stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x000000000040054d",func="main",args=[],file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62"},thread-id="1",stopped-threads="all",core="0"
=breakpoint-deleted,id="1"
(gdb)
-interpreter-exec console list
~"62\t callee1 (2, \"A string argument.\", 3.5);\n"
~"63\t callee1 (2, \"A string argument.\", 3.5);\n"
~"64\t\n"
~"65\t do_nothing (); /* Hello, World! */\n"
~"66\t\n"
~"67\t callme (1);\n"
~"68\t callme (2);\n"
~"69\t\n"
~"70\t return 0;\n"
~"71\t}\n"
^done
(gdb)
Here the list starts at line 62, where the program was stopped.
This happens because print_stack_frame, called from both normal_stop
and mi_on_normal_stop, is the function responsible for setting the
current sal from the selected frame, overrides the PRINT_WHAT
argument, and only after that does it decide whether to center the
current sal line or not, based on the overriden value, and it will
always decide false.
(The print_stack_frame call in mi_on_normal_stop is a little different
from the call in normal_stop, in that it is an unconditional
SRC_AND_LOC call. The next patch will make those uniform.)
Tested on x86_64 Fedora 16, no regressions.
2013-10-30 Pedro Alves <palves@redhat.com>
* stack.c (print_stack_frame): Compute CENTER before overriding
PRINT_WHAT.
2013-10-30 Pedro Alves <palves@redhat.com>
* gdb.mi/mi-cli.exp: Adjust expected output of "list".
---
gdb/ChangeLog | 5 +++++
gdb/stack.c | 4 ++--
gdb/testsuite/ChangeLog | 4 ++++
gdb/testsuite/gdb.mi/mi-cli.exp | 2 +-
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/gdb/stack.c b/gdb/stack.c
index da7d977..6fd2be9 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -160,14 +160,14 @@ print_stack_frame (struct frame_info *frame, int print_level,
{
volatile struct gdb_exception e;
+ int center = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
+
/* For mi, alway print location and address. */
if (ui_out_is_mi_like_p (current_uiout))
print_what = LOC_AND_ADDRESS;
TRY_CATCH (e, RETURN_MASK_ERROR)
{
- int center = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
-
print_frame_info (frame, print_level, print_what, 1 /* print_args */,
set_current_sal);
if (set_current_sal)
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 5657be6..016f54f 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -91,7 +91,7 @@ mi_gdb_test "-interpreter-exec console \"set listsize 1\"" \
# {.*\~"32[ \t(\\t)]*callee1.*\\n".*\^done }
mi_gdb_test "-interpreter-exec console \"list\"" \
- ".*\~\"$line_main_body\[\\\\t \]*callee1.*;\\\\n\".*\\^done" \
+ ".*\~\"57\\\\t\{\\\\n\".*\\^done" \
"-interpreter-exec console \"list\""
mi_execute_to "exec-continue" "breakpoint-hit" "callee4" "" ".*basics.c" $line_callee4_body \
--
1.8.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread* [RFC v6] PR gdb/13860: make -interpreter-exec console "list" behave more like "list".
2014-03-04 18:33 ` [RFC v5 2/8] PR gdb/13860: make -interpreter-exec console "list" behave more like "list" Tom Tromey
@ 2014-03-18 16:04 ` Pedro Alves
2014-05-21 22:16 ` Pedro Alves
0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-03-18 16:04 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi!
I'm sending out a different fix for this one, below.
Quoting previous patch for context.
On 03/04/2014 06:32 PM, Tom Tromey wrote:
> From: Pedro Alves <palves@redhat.com>
>
> Patch 3 in this series made me notice that "list" behaves differently
> in CLI vs MI. Particularly:
>
> >./gdb -nx -q ./testsuite/gdb.mi/mi-cli
> Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli...done.
> (gdb) start
> Temporary breakpoint 1 at 0x40054d: file ../../../src/gdb/testsuite/gdb.mi/basics.c, line 62.
> Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli
>
> Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62
> 62 callee1 (2, "A string argument.", 3.5);
> (gdb) list
> 57 {
> 58 }
> 59
> 60 main ()
> 61 {
> 62 callee1 (2, "A string argument.", 3.5);
> 63 callee1 (2, "A string argument.", 3.5);
> 64
> 65 do_nothing (); /* Hello, World! */
> 66
> (gdb)
>
> Note the list started at line 57. IOW, the program stopped at line
> 62, and GDB centered the list on that.
>
> compare with:
>
> >./gdb -nx -q ./testsuite/gdb.mi/mi-cli -i=mi
> =thread-group-added,id="i1"
> ~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli..."
> ~"done.\n"
> (gdb)
> start
> &"start\n"
> ~"Temporary breakpoint 1 at 0x40054d: file ../../../src/gdb/testsuite/gdb.mi/basics.c, line 62.\n"
> =breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x000000000040054d",func="main",file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62",times="0",original-location="main"}
> ~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli \n"
> =thread-group-started,id="i1",pid="14221"
> =thread-created,id="1",group-id="i1"
> ^running
> *running,thread-id="all"
> (gdb)
> =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
> =library-loaded,id="/lib64/libm.so.6",target-name="/lib64/libm.so.6",host-name="/lib64/libm.so.6",symbols-loaded="0",thread-group="i1"
> =library-loaded,id="/lib64/libc.so.6",target-name="/lib64/libc.so.6",host-name="/lib64/libc.so.6",symbols-loaded="0",thread-group="i1"
> =breakpoint-modified,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x000000000040054d",func="main",file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62",times="1",original-location="main"}
> ~"\nTemporary breakpoint "
> ~"1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62\n"
> ~"62\t callee1 (2, \"A string argument.\", 3.5);\n"
> *stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x000000000040054d",func="main",args=[],file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62"},thread-id="1",stopped-threads="all",core="0"
> =breakpoint-deleted,id="1"
> (gdb)
> -interpreter-exec console list
> ~"62\t callee1 (2, \"A string argument.\", 3.5);\n"
> ~"63\t callee1 (2, \"A string argument.\", 3.5);\n"
> ~"64\t\n"
> ~"65\t do_nothing (); /* Hello, World! */\n"
> ~"66\t\n"
> ~"67\t callme (1);\n"
> ~"68\t callme (2);\n"
> ~"69\t\n"
> ~"70\t return 0;\n"
> ~"71\t}\n"
> ^done
> (gdb)
>
> Here the list starts at line 62, where the program was stopped.
>
> This happens because print_stack_frame, called from both normal_stop
> and mi_on_normal_stop, is the function responsible for setting the
> current sal from the selected frame, overrides the PRINT_WHAT
> argument, and only after that does it decide whether to center the
> current sal line or not, based on the overriden value, and it will
> always decide false.
>
> (The print_stack_frame call in mi_on_normal_stop is a little different
> from the call in normal_stop, in that it is an unconditional
> SRC_AND_LOC call. The next patch will make those uniform.)
>
> Tested on x86_64 Fedora 16, no regressions.
>
> 2013-10-30 Pedro Alves <palves@redhat.com>
>
> * stack.c (print_stack_frame): Compute CENTER before overriding
> PRINT_WHAT.
>
> 2013-10-30 Pedro Alves <palves@redhat.com>
>
> * gdb.mi/mi-cli.exp: Adjust expected output of "list".
> ---
> gdb/ChangeLog | 5 +++++
> gdb/stack.c | 4 ++--
> gdb/testsuite/ChangeLog | 4 ++++
> gdb/testsuite/gdb.mi/mi-cli.exp | 2 +-
> 4 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/stack.c b/gdb/stack.c
> index da7d977..6fd2be9 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -160,14 +160,14 @@ print_stack_frame (struct frame_info *frame, int print_level,
> {
> volatile struct gdb_exception e;
>
> + int center = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
> +
> /* For mi, alway print location and address. */
> if (ui_out_is_mi_like_p (current_uiout))
> print_what = LOC_AND_ADDRESS;
>
> TRY_CATCH (e, RETURN_MASK_ERROR)
> {
> - int center = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
> -
> print_frame_info (frame, print_level, print_what, 1 /* print_args */,
> set_current_sal);
> if (set_current_sal)
> diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
> index 5657be6..016f54f 100644
> --- a/gdb/testsuite/gdb.mi/mi-cli.exp
> +++ b/gdb/testsuite/gdb.mi/mi-cli.exp
> @@ -91,7 +91,7 @@ mi_gdb_test "-interpreter-exec console \"set listsize 1\"" \
>
> # {.*\~"32[ \t(\\t)]*callee1.*\\n".*\^done }
> mi_gdb_test "-interpreter-exec console \"list\"" \
> - ".*\~\"$line_main_body\[\\\\t \]*callee1.*;\\\\n\".*\\^done" \
> + ".*\~\"57\\\\t\{\\\\n\".*\\^done" \
> "-interpreter-exec console \"list\""
Looking again at this, I think there's a deeper issue here. This "list"
is the first source listing after a breakpoint stop, and "set listsize 1".
The breakpoint is at line 62. Why would we list line 57 (and only
that line) ? The issue is with the line centering done too soon. We should
be a little more lazy here, like in the patch below.
Let me know what you think.
---------
[PATCH] PR gdb/13860: make -interpreter-exec console "list" behave
more like "list".
I noticed that "list" behaves differently in CLI vs MI. Particularly:
$ ./gdb -nx -q ./testsuite/gdb.mi/mi-cli
Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli...done.
(gdb) start
Temporary breakpoint 1 at 0x40054d: file ../../../src/gdb/testsuite/gdb.mi/basics.c, line 62.
Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli
Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62
62 callee1 (2, "A string argument.", 3.5);
(gdb) list
57 {
58 }
59
60 main ()
61 {
62 callee1 (2, "A string argument.", 3.5);
63 callee1 (2, "A string argument.", 3.5);
64
65 do_nothing (); /* Hello, World! */
66
(gdb)
Note the list started at line 57. IOW, the program stopped at line
62, and GDB centered the list on that.
compare with:
$ ./gdb -nx -q ./testsuite/gdb.mi/mi-cli -i=mi
=thread-group-added,id="i1"
~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli..."
~"done.\n"
(gdb)
start
&"start\n"
...
~"\nTemporary breakpoint "
~"1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62\n"
~"62\t callee1 (2, \"A string argument.\", 3.5);\n"
*stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x000000000040054d",func="main",args=[],file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62"},thread-id="1",stopped-threads="all",core="0"
=breakpoint-deleted,id="1"
(gdb)
-interpreter-exec console list
~"62\t callee1 (2, \"A string argument.\", 3.5);\n"
~"63\t callee1 (2, \"A string argument.\", 3.5);\n"
~"64\t\n"
~"65\t do_nothing (); /* Hello, World! */\n"
~"66\t\n"
~"67\t callme (1);\n"
~"68\t callme (2);\n"
~"69\t\n"
~"70\t return 0;\n"
~"71\t}\n"
^done
(gdb)
Here the list starts at line 62, where the program was stopped.
This happens because print_stack_frame, called from both normal_stop
and mi_on_normal_stop, is the function responsible for setting the
current sal from the selected frame, overrides the PRINT_WHAT
argument, and only after that does it decide whether to center the
current sal line or not, based on the overriden value, and it will
always decide false.
(The print_stack_frame call in mi_on_normal_stop is a little different
from the call in normal_stop, in that it is an unconditional
SRC_AND_LOC call. A future patch will make those uniform.)
A previous version of this patch made MI uniform with CLI here, by
making print_stack_frame also center when MI is active. That changed
the output of a "list" command in mi-cli.exp, to expect line 57
instead of 62, as per the example above.
However, looking deeper, that list in question is the first "list"
after the program stops, and right after the stop, before the "list",
the test did "set listsize 1". Let's try the same thing with the CLI:
(gdb) start
62 callee1 (2, "A string argument.", 3.5);
(gdb) set listsize 1
(gdb) list
57 {
Huh, that's unexpected. Why the 57? It's because print_stack_frame,
called in reaction to the breakpoint stop, expecting the next "list"
to show 10 lines (the listsize at the time) around line 62, sets the
lines listed range to 57-67 (62 +/- 5). If the user changes the
listsize before "list", why would we still show that range? Looks
bogus to me.
So the fix for this whole issue should be delay trying to center the
listing to until actually listing, so that the correct listsize can be
taken into account. This makes MI and CLI uniform too, as it deletes
the center code from print_stack_frame.
A series of tests are added to list.exp to cover this. mi-cli.exp was
after all correct all along, but it now gains an additional test that
lists lines with listsize 10, to ensure the centering is consistent
with CLI's.
One related Python test changed related output -- it's a test that
prints the line number after stopping for a breakpoint, similar to the
new list.exp tests. Previously we'd print the stop line minus 5 (due
to the premature centering), now we print the stop line. I think
that's a good change.
Tested on x86_64 Fedora 17.
gdb/
2014-03-18 Pedro Alves <palves@redhat.com>
* cli/cli-cmds.c (list_command): Handle the first "list" after the
current source line having changed.
* frame.h (set_current_sal_from_frame): Remove 'center' parameter.
* infrun.c (normal_stop): Adjust call to
set_current_sal_from_frame.
* source.c (clear_lines_listed_range): New function.
(set_current_source_symtab_and_line, identify_source_line): Clear
the lines listed range.
(line_info): Handle the first "info line" after the current source
line having changed.
* stack.c (print_stack_frame): Remove center handling.
(set_current_sal_from_frame): Remove 'center' parameter. Don't
center sal.line.
gdb/testsuite/
2014-03-18 Pedro Alves <palves@redhat.com>
* gdb.base/list.exp (build_pattern, test_list): New procedures.
Use them to test variations of "list" after reaching a breakpoint.
* gdb.mi/mi-cli.exp (line_main_callme_2): New global.
Test "list" with listsize 10 after reaching a breakpoint.
* gdb.python/python.exp (decode_line current location line
number): Adjust expected line number.
---
gdb/cli/cli-cmds.c | 21 +++++++++
gdb/frame.h | 5 +-
gdb/infrun.c | 2 +-
gdb/source.c | 26 ++++++++--
gdb/source.h | 7 +--
gdb/stack.c | 14 ++----
gdb/testsuite/gdb.base/list.exp | 94 +++++++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.mi/mi-cli.exp | 20 ++++++++
gdb/testsuite/gdb.python/python.exp | 5 +-
9 files changed, 171 insertions(+), 23 deletions(-)
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index bfcd975..5c6129d 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -874,6 +874,27 @@ list_command (char *arg, int from_tty)
{
set_default_source_symtab_and_line ();
cursal = get_current_source_symtab_and_line ();
+
+ /* If this is the first "list" since we've set the current
+ source line, center the listing around that line. */
+ if (get_first_line_listed () == 0)
+ {
+ int first;
+
+ first = max (cursal.line - get_lines_to_list () / 2, 1);
+
+ /* A small special case --- if listing backwards, and we
+ should list only one line, list the preceding line,
+ instead of the exact line we've just shown after e.g.,
+ stopping for a breakpoint. */
+ if (arg != NULL && arg[0] == '-'
+ && get_lines_to_list () == 1 && first > 1)
+ first -= 1;
+
+ print_source_lines (cursal.symtab, first,
+ first + get_lines_to_list (), 0);
+ return;
+ }
}
/* "l" or "l +" lists next ten lines. */
diff --git a/gdb/frame.h b/gdb/frame.h
index e451a93..6a75f0d 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -388,10 +388,9 @@ extern void find_frame_sal (struct frame_info *frame,
struct symtab_and_line *sal);
/* Set the current source and line to the location given by frame
- FRAME, if possible. When CENTER is true, adjust so the relevant
- line is in the center of the next 'list'. */
+ FRAME, if possible. */
-void set_current_sal_from_frame (struct frame_info *, int);
+void set_current_sal_from_frame (struct frame_info *);
/* Return the frame base (what ever that is) (DEPRECATED).
diff --git a/gdb/infrun.c b/gdb/infrun.c
index b7c0969..3d3ba55 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6091,7 +6091,7 @@ normal_stop (void)
display the frame below, but the current SAL will be incorrect
during a user hook-stop function. */
if (has_stack_frames () && !stop_stack_dummy)
- set_current_sal_from_frame (get_current_frame (), 1);
+ set_current_sal_from_frame (get_current_frame ());
/* Let the user/frontend see the threads as stopped. */
do_cleanups (old_chain);
diff --git a/gdb/source.c b/gdb/source.c
index c112765..c77a4f4 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -133,7 +133,9 @@ show_filename_display_string (struct ui_file *file, int from_tty,
static int last_line_listed;
-/* First line number listed by last listing command. */
+/* First line number listed by last listing command. If 0, then no
+ source lines have yet been listed since the last time the current
+ source line was changed. */
static int first_line_listed;
@@ -153,6 +155,16 @@ get_first_line_listed (void)
return first_line_listed;
}
+/* Clear line listed range. This makes the next "list" center the
+ printed source lines around the current source line. */
+
+static void
+clear_lines_listed_range (void)
+{
+ first_line_listed = 0;
+ last_line_listed = 0;
+}
+
/* Return the default number of lines to print with commands like the
cli "list". The caller of print_source_lines must use this to
calculate the end line and use it in the call to print_source_lines
@@ -220,6 +232,9 @@ set_current_source_symtab_and_line (const struct symtab_and_line *sal)
current_source_symtab = sal->symtab;
current_source_line = sal->line;
+ /* Force the next "list" to center around the current line. */
+ clear_lines_listed_range ();
+
return cursal;
}
@@ -1294,9 +1309,8 @@ identify_source_line (struct symtab *s, int line, int mid_statement,
mid_statement, get_objfile_arch (s->objfile), pc);
current_source_line = line;
- first_line_listed = line;
- last_line_listed = line;
current_source_symtab = s;
+ clear_lines_listed_range ();
return 1;
}
\f
@@ -1488,7 +1502,11 @@ line_info (char *arg, int from_tty)
{
sal.symtab = current_source_symtab;
sal.pspace = current_program_space;
- sal.line = last_line_listed;
+ if (last_line_listed != 0)
+ sal.line = last_line_listed;
+ else
+ sal.line = current_source_line;
+
sals.nelts = 1;
sals.sals = (struct symtab_and_line *)
xmalloc (sizeof (struct symtab_and_line));
diff --git a/gdb/source.h b/gdb/source.h
index 61826ea..ad74df9 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -62,9 +62,10 @@ extern const char *symtab_to_filename_for_display (struct symtab *symtab);
lines. */
extern void find_source_lines (struct symtab *s, int desc);
-/* Return the first line listed by print_source_lines.
- Used by command interpreters to request listing from
- a previous point. */
+/* Return the first line listed by print_source_lines. Used by
+ command interpreters to request listing from a previous point. If
+ 0, then no source lines have yet been listed since the last time
+ the current source line was changed. */
extern int get_first_line_listed (void);
/* Return the default number of lines to print with commands like the
diff --git a/gdb/stack.c b/gdb/stack.c
index da7d977..0c6923d 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -166,12 +166,10 @@ print_stack_frame (struct frame_info *frame, int print_level,
TRY_CATCH (e, RETURN_MASK_ERROR)
{
- int center = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
-
print_frame_info (frame, print_level, print_what, 1 /* print_args */,
set_current_sal);
if (set_current_sal)
- set_current_sal_from_frame (frame, center);
+ set_current_sal_from_frame (frame);
}
}
@@ -716,17 +714,13 @@ print_frame_args (struct symbol *func, struct frame_info *frame,
line is in the center of the next 'list'. */
void
-set_current_sal_from_frame (struct frame_info *frame, int center)
+set_current_sal_from_frame (struct frame_info *frame)
{
struct symtab_and_line sal;
find_frame_sal (frame, &sal);
- if (sal.symtab)
- {
- if (center)
- sal.line = max (sal.line - get_lines_to_list () / 2, 1);
- set_current_source_symtab_and_line (&sal);
- }
+ if (sal.symtab != NULL)
+ set_current_source_symtab_and_line (&sal);
}
/* If ON, GDB will display disassembly of the next source line when
diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp
index 12b2c94..67eef12 100644
--- a/gdb/testsuite/gdb.base/list.exp
+++ b/gdb/testsuite/gdb.base/list.exp
@@ -529,4 +529,98 @@ if [ set_listsize 10 ] then {
test_only_end
}
+# Follows tests that require execution.
+
+# Build source listing pattern based on a line range spec string. The
+# range can be specificed as "START-END" indicating all lines in range
+# (inclusive); or just "LINE", indicating just that line.
+
+proc build_pattern { range_spec } {
+ global line_re
+
+ set range_list [split $range_spec -]
+ set range_list_len [llength $range_list]
+
+ set range_start [lindex $range_list 0]
+ if { $range_list_len > 2 || $range_list_len < 1} {
+ error "invalid range spec string: $range_spec"
+ } elseif { $range_list_len == 2 } {
+ set range_end [lindex $range_list 1]
+ } else {
+ set range_end $range_start
+ }
+
+ for {set i $range_start} {$i <= $range_end} {incr i} {
+ append pattern "\r\n$i\[ \t\]\[^\r\n\]*"
+ }
+
+ verbose -log "pattern $pattern"
+ return $pattern
+}
+
+# Test "list" command invocations right after stopping for an event.
+# COMMAND is the actual list command, including arguments. LISTSIZE1
+# and LISTSIZE2 are the listsizes set just before and after running
+# the program to the stop point. COMMAND is issued twice. The first
+# time, the lines specificed by LINERANGE1 are expected; the second
+# time, the lines specified by LINERANGE2 are expected.
+
+proc test_list {command listsize1 listsize2 linerange1 linerange2} {
+ with_test_prefix "$command after stop: $listsize1, $listsize2" {
+ global binfile
+
+ clean_restart $binfile
+ if ![runto_main] then {
+ fail "Can't run to main"
+ return
+ }
+
+ # Test changing the listsize both before nexting, and after
+ # stopping, but before listing. Only the second listsize
+ # change should affect which lines are listed.
+ gdb_test_no_output "set listsize $listsize1"
+ gdb_test "next" "foo \\(.*"
+ gdb_test_no_output "set listsize $listsize2"
+
+ set pattern1 [build_pattern $linerange1]
+ set pattern2 [build_pattern $linerange2]
+ gdb_test "$command" "${pattern1}" "$command #1"
+ gdb_test "$command" "${pattern2}" "$command #2"
+ }
+}
+
+
+# The first "list" should center the listing around line 8, the stop
+# line.
+test_list "list" 1 10 "3-12" "13-22"
+
+# Likewise.
+test_list "list" 10 10 "3-12" "13-22"
+
+# Likewise, but show only one line. IOW, the first list should show
+# line 8. Note how the listsize is 10 at the time of the stop, but
+# before any listing had been requested. That should not affect the
+# line range that is first listed.
+test_list "list" 10 1 "8" "9"
+
+# Likewise, but show two lines.
+test_list "list" 10 2 "7-8" "9-10"
+
+# Three lines.
+test_list "list" 10 3 "7-9" "10-12"
+
+# Now test backwards. Just like "list", the first "list -" should
+# center the listing around the stop line.
+test_list "list -" 10 10 "3-12" "2"
+
+# Likewise, but test showing 3 lines at a time.
+test_list "list -" 10 3 "7-9" "4-6"
+
+# 2 lines at a time.
+test_list "list -" 10 2 "7-8" "5-6"
+
+# Test listing one line one. This case is a little special, and
+# starts showing the previous line immediately.
+test_list "list -" 10 1 "7" "6"
+
remote_exec build "rm -f list0.h"
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 5657be6..566d64d 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -65,6 +65,7 @@ set line_main_head [gdb_get_line_number "main ("]
set line_main_body [expr $line_main_head + 2]
set line_main_hello [gdb_get_line_number "Hello, World!"]
set line_main_return [expr $line_main_hello + 2]
+set line_main_callme_2 [expr $line_main_return + 1]
set line_callee4_head [gdb_get_line_number "callee4 ("]
set line_callee4_body [expr $line_callee4_head + 2]
set line_callee4_next [expr $line_callee4_body + 1]
@@ -184,6 +185,25 @@ mi_gdb_test "-interpreter-exec console \"list\"" \
"\~\"$line_main_return\[\\\\t ]*callme \\(1\\);\\\\n\".*\\^done" \
"-interpreter-exec console \"list\" at basics.c:\$line_main_return"
+mi_gdb_test "600-break-insert -t basics.c:$line_main_callme_2" \
+ {600\^done,bkpt=.number="4",type="breakpoint".*\}} \
+ "-break-insert -t basics.c:\$line_main_callme_2"
+
+mi_execute_to "exec-continue" "breakpoint-hit" "main" "" ".*basics.c" \
+ $line_main_callme_2 { "" "disp=\"del\"" } \
+ "-exec-continue to line \$line_main_callme_2"
+
+# Restore the listsize back to the default.
+mi_gdb_test "-interpreter-exec console \"set listsize 10\"" \
+ ".*=cmd-param-changed,param=\"listsize\",value=\"10\".*\\^done" \
+ "-interpreter-exec console \"set listsize 10\""
+
+# "list" should show 10 lines centered on where the program stopped.
+set first_list_line [expr $line_main_callme_2 - 5]
+mi_gdb_test "-interpreter-exec console \"list\"" \
+ ".*\~\"$first_list_line.*\\^done" \
+ "-interpreter-exec console \"list\" at basics.c:\$line_main_return"
+
mi_gdb_test "-interpreter-exec console \"help set args\"" \
{\~"Set argument list to give program being debugged when it is started\.\\nFollow this command with any number of args, to be passed to the program\.".*\^done} \
"-interpreter-exec console \"help set args\""
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index a470427..2b7d78e 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -168,7 +168,8 @@ if ![runto_main] then {
return 0
}
-runto [gdb_get_line_number "Break to end."]
+set lineno [gdb_get_line_number "Break to end."]
+runto $lineno
# Test gdb.decode_line.
gdb_test "python gdb.decode_line(\"main.c:43\")" \
@@ -179,7 +180,7 @@ gdb_test "python print (len(symtab))" "2" "Test decode_line current location"
gdb_test "python print (symtab\[0\])" "None" "Test decode_line expression parse"
gdb_test "python print (len(symtab\[1\]))" "1" "Test decode_line current location"
gdb_test "python print (symtab\[1\]\[0\].symtab)" ".*gdb.python/python.c.*" "Test decode_line current locationn filename"
-gdb_test "python print (symtab\[1\]\[0\].line)" "22" "Test decode_line current location line number"
+gdb_test "python print (symtab\[1\]\[0\].line)" "$lineno" "Test decode_line current location line number"
gdb_py_test_silent_cmd "python symtab = gdb.decode_line(\"python.c:26 if foo\")" "test decode_line python.c:26" 1
gdb_test "python print (len(symtab))" "2" "Test decode_line python.c:26 length"
--
1.7.11.7
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v6] PR gdb/13860: make -interpreter-exec console "list" behave more like "list".
2014-03-18 16:04 ` [RFC v6] " Pedro Alves
@ 2014-05-21 22:16 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-05-21 22:16 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
On 03/18/2014 04:04 PM, Pedro Alves wrote:
> Looking again at this, I think there's a deeper issue here. This "list"
> is the first source listing after a breakpoint stop, and "set listsize 1".
> The breakpoint is at line 62. Why would we list line 57 (and only
> that line) ? The issue is with the line centering done too soon. We should
> be a little more lazy here, like in the patch below.
>
> Let me know what you think.
Tom told me off list he's OK with this version.
I've pushed it in.
Pedro Alves
>
> ---------
> [PATCH] PR gdb/13860: make -interpreter-exec console "list" behave
> more like "list".
>
> I noticed that "list" behaves differently in CLI vs MI. Particularly:
>
> $ ./gdb -nx -q ./testsuite/gdb.mi/mi-cli
> Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli...done.
> (gdb) start
> Temporary breakpoint 1 at 0x40054d: file ../../../src/gdb/testsuite/gdb.mi/basics.c, line 62.
> Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli
>
> Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62
> 62 callee1 (2, "A string argument.", 3.5);
> (gdb) list
> 57 {
> 58 }
> 59
> 60 main ()
> 61 {
> 62 callee1 (2, "A string argument.", 3.5);
> 63 callee1 (2, "A string argument.", 3.5);
> 64
> 65 do_nothing (); /* Hello, World! */
> 66
> (gdb)
>
> Note the list started at line 57. IOW, the program stopped at line
> 62, and GDB centered the list on that.
>
> compare with:
>
> $ ./gdb -nx -q ./testsuite/gdb.mi/mi-cli -i=mi
> =thread-group-added,id="i1"
> ~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli..."
> ~"done.\n"
> (gdb)
> start
> &"start\n"
> ...
> ~"\nTemporary breakpoint "
> ~"1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62\n"
> ~"62\t callee1 (2, \"A string argument.\", 3.5);\n"
> *stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x000000000040054d",func="main",args=[],file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62"},thread-id="1",stopped-threads="all",core="0"
> =breakpoint-deleted,id="1"
> (gdb)
> -interpreter-exec console list
> ~"62\t callee1 (2, \"A string argument.\", 3.5);\n"
> ~"63\t callee1 (2, \"A string argument.\", 3.5);\n"
> ~"64\t\n"
> ~"65\t do_nothing (); /* Hello, World! */\n"
> ~"66\t\n"
> ~"67\t callme (1);\n"
> ~"68\t callme (2);\n"
> ~"69\t\n"
> ~"70\t return 0;\n"
> ~"71\t}\n"
> ^done
> (gdb)
>
> Here the list starts at line 62, where the program was stopped.
>
> This happens because print_stack_frame, called from both normal_stop
> and mi_on_normal_stop, is the function responsible for setting the
> current sal from the selected frame, overrides the PRINT_WHAT
> argument, and only after that does it decide whether to center the
> current sal line or not, based on the overriden value, and it will
> always decide false.
>
> (The print_stack_frame call in mi_on_normal_stop is a little different
> from the call in normal_stop, in that it is an unconditional
> SRC_AND_LOC call. A future patch will make those uniform.)
>
> A previous version of this patch made MI uniform with CLI here, by
> making print_stack_frame also center when MI is active. That changed
> the output of a "list" command in mi-cli.exp, to expect line 57
> instead of 62, as per the example above.
>
> However, looking deeper, that list in question is the first "list"
> after the program stops, and right after the stop, before the "list",
> the test did "set listsize 1". Let's try the same thing with the CLI:
>
> (gdb) start
> 62 callee1 (2, "A string argument.", 3.5);
> (gdb) set listsize 1
> (gdb) list
> 57 {
>
> Huh, that's unexpected. Why the 57? It's because print_stack_frame,
> called in reaction to the breakpoint stop, expecting the next "list"
> to show 10 lines (the listsize at the time) around line 62, sets the
> lines listed range to 57-67 (62 +/- 5). If the user changes the
> listsize before "list", why would we still show that range? Looks
> bogus to me.
>
> So the fix for this whole issue should be delay trying to center the
> listing to until actually listing, so that the correct listsize can be
> taken into account. This makes MI and CLI uniform too, as it deletes
> the center code from print_stack_frame.
>
> A series of tests are added to list.exp to cover this. mi-cli.exp was
> after all correct all along, but it now gains an additional test that
> lists lines with listsize 10, to ensure the centering is consistent
> with CLI's.
>
> One related Python test changed related output -- it's a test that
> prints the line number after stopping for a breakpoint, similar to the
> new list.exp tests. Previously we'd print the stop line minus 5 (due
> to the premature centering), now we print the stop line. I think
> that's a good change.
>
> Tested on x86_64 Fedora 17.
>
> gdb/
> 2014-03-18 Pedro Alves <palves@redhat.com>
>
> * cli/cli-cmds.c (list_command): Handle the first "list" after the
> current source line having changed.
> * frame.h (set_current_sal_from_frame): Remove 'center' parameter.
> * infrun.c (normal_stop): Adjust call to
> set_current_sal_from_frame.
> * source.c (clear_lines_listed_range): New function.
> (set_current_source_symtab_and_line, identify_source_line): Clear
> the lines listed range.
> (line_info): Handle the first "info line" after the current source
> line having changed.
> * stack.c (print_stack_frame): Remove center handling.
> (set_current_sal_from_frame): Remove 'center' parameter. Don't
> center sal.line.
>
> gdb/testsuite/
> 2014-03-18 Pedro Alves <palves@redhat.com>
>
> * gdb.base/list.exp (build_pattern, test_list): New procedures.
> Use them to test variations of "list" after reaching a breakpoint.
> * gdb.mi/mi-cli.exp (line_main_callme_2): New global.
> Test "list" with listsize 10 after reaching a breakpoint.
> * gdb.python/python.exp (decode_line current location line
> number): Adjust expected line number.
> ---
> gdb/cli/cli-cmds.c | 21 +++++++++
> gdb/frame.h | 5 +-
> gdb/infrun.c | 2 +-
> gdb/source.c | 26 ++++++++--
> gdb/source.h | 7 +--
> gdb/stack.c | 14 ++----
> gdb/testsuite/gdb.base/list.exp | 94 +++++++++++++++++++++++++++++++++++++
> gdb/testsuite/gdb.mi/mi-cli.exp | 20 ++++++++
> gdb/testsuite/gdb.python/python.exp | 5 +-
> 9 files changed, 171 insertions(+), 23 deletions(-)
>
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index bfcd975..5c6129d 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -874,6 +874,27 @@ list_command (char *arg, int from_tty)
> {
> set_default_source_symtab_and_line ();
> cursal = get_current_source_symtab_and_line ();
> +
> + /* If this is the first "list" since we've set the current
> + source line, center the listing around that line. */
> + if (get_first_line_listed () == 0)
> + {
> + int first;
> +
> + first = max (cursal.line - get_lines_to_list () / 2, 1);
> +
> + /* A small special case --- if listing backwards, and we
> + should list only one line, list the preceding line,
> + instead of the exact line we've just shown after e.g.,
> + stopping for a breakpoint. */
> + if (arg != NULL && arg[0] == '-'
> + && get_lines_to_list () == 1 && first > 1)
> + first -= 1;
> +
> + print_source_lines (cursal.symtab, first,
> + first + get_lines_to_list (), 0);
> + return;
> + }
> }
>
> /* "l" or "l +" lists next ten lines. */
> diff --git a/gdb/frame.h b/gdb/frame.h
> index e451a93..6a75f0d 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -388,10 +388,9 @@ extern void find_frame_sal (struct frame_info *frame,
> struct symtab_and_line *sal);
>
> /* Set the current source and line to the location given by frame
> - FRAME, if possible. When CENTER is true, adjust so the relevant
> - line is in the center of the next 'list'. */
> + FRAME, if possible. */
>
> -void set_current_sal_from_frame (struct frame_info *, int);
> +void set_current_sal_from_frame (struct frame_info *);
>
> /* Return the frame base (what ever that is) (DEPRECATED).
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index b7c0969..3d3ba55 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -6091,7 +6091,7 @@ normal_stop (void)
> display the frame below, but the current SAL will be incorrect
> during a user hook-stop function. */
> if (has_stack_frames () && !stop_stack_dummy)
> - set_current_sal_from_frame (get_current_frame (), 1);
> + set_current_sal_from_frame (get_current_frame ());
>
> /* Let the user/frontend see the threads as stopped. */
> do_cleanups (old_chain);
> diff --git a/gdb/source.c b/gdb/source.c
> index c112765..c77a4f4 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -133,7 +133,9 @@ show_filename_display_string (struct ui_file *file, int from_tty,
>
> static int last_line_listed;
>
> -/* First line number listed by last listing command. */
> +/* First line number listed by last listing command. If 0, then no
> + source lines have yet been listed since the last time the current
> + source line was changed. */
>
> static int first_line_listed;
>
> @@ -153,6 +155,16 @@ get_first_line_listed (void)
> return first_line_listed;
> }
>
> +/* Clear line listed range. This makes the next "list" center the
> + printed source lines around the current source line. */
> +
> +static void
> +clear_lines_listed_range (void)
> +{
> + first_line_listed = 0;
> + last_line_listed = 0;
> +}
> +
> /* Return the default number of lines to print with commands like the
> cli "list". The caller of print_source_lines must use this to
> calculate the end line and use it in the call to print_source_lines
> @@ -220,6 +232,9 @@ set_current_source_symtab_and_line (const struct symtab_and_line *sal)
> current_source_symtab = sal->symtab;
> current_source_line = sal->line;
>
> + /* Force the next "list" to center around the current line. */
> + clear_lines_listed_range ();
> +
> return cursal;
> }
>
> @@ -1294,9 +1309,8 @@ identify_source_line (struct symtab *s, int line, int mid_statement,
> mid_statement, get_objfile_arch (s->objfile), pc);
>
> current_source_line = line;
> - first_line_listed = line;
> - last_line_listed = line;
> current_source_symtab = s;
> + clear_lines_listed_range ();
> return 1;
> }
> \f
> @@ -1488,7 +1502,11 @@ line_info (char *arg, int from_tty)
> {
> sal.symtab = current_source_symtab;
> sal.pspace = current_program_space;
> - sal.line = last_line_listed;
> + if (last_line_listed != 0)
> + sal.line = last_line_listed;
> + else
> + sal.line = current_source_line;
> +
> sals.nelts = 1;
> sals.sals = (struct symtab_and_line *)
> xmalloc (sizeof (struct symtab_and_line));
> diff --git a/gdb/source.h b/gdb/source.h
> index 61826ea..ad74df9 100644
> --- a/gdb/source.h
> +++ b/gdb/source.h
> @@ -62,9 +62,10 @@ extern const char *symtab_to_filename_for_display (struct symtab *symtab);
> lines. */
> extern void find_source_lines (struct symtab *s, int desc);
>
> -/* Return the first line listed by print_source_lines.
> - Used by command interpreters to request listing from
> - a previous point. */
> +/* Return the first line listed by print_source_lines. Used by
> + command interpreters to request listing from a previous point. If
> + 0, then no source lines have yet been listed since the last time
> + the current source line was changed. */
> extern int get_first_line_listed (void);
>
> /* Return the default number of lines to print with commands like the
> diff --git a/gdb/stack.c b/gdb/stack.c
> index da7d977..0c6923d 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -166,12 +166,10 @@ print_stack_frame (struct frame_info *frame, int print_level,
>
> TRY_CATCH (e, RETURN_MASK_ERROR)
> {
> - int center = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
> -
> print_frame_info (frame, print_level, print_what, 1 /* print_args */,
> set_current_sal);
> if (set_current_sal)
> - set_current_sal_from_frame (frame, center);
> + set_current_sal_from_frame (frame);
> }
> }
>
> @@ -716,17 +714,13 @@ print_frame_args (struct symbol *func, struct frame_info *frame,
> line is in the center of the next 'list'. */
>
> void
> -set_current_sal_from_frame (struct frame_info *frame, int center)
> +set_current_sal_from_frame (struct frame_info *frame)
> {
> struct symtab_and_line sal;
>
> find_frame_sal (frame, &sal);
> - if (sal.symtab)
> - {
> - if (center)
> - sal.line = max (sal.line - get_lines_to_list () / 2, 1);
> - set_current_source_symtab_and_line (&sal);
> - }
> + if (sal.symtab != NULL)
> + set_current_source_symtab_and_line (&sal);
> }
>
> /* If ON, GDB will display disassembly of the next source line when
> diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp
> index 12b2c94..67eef12 100644
> --- a/gdb/testsuite/gdb.base/list.exp
> +++ b/gdb/testsuite/gdb.base/list.exp
> @@ -529,4 +529,98 @@ if [ set_listsize 10 ] then {
> test_only_end
> }
>
> +# Follows tests that require execution.
> +
> +# Build source listing pattern based on a line range spec string. The
> +# range can be specificed as "START-END" indicating all lines in range
> +# (inclusive); or just "LINE", indicating just that line.
> +
> +proc build_pattern { range_spec } {
> + global line_re
> +
> + set range_list [split $range_spec -]
> + set range_list_len [llength $range_list]
> +
> + set range_start [lindex $range_list 0]
> + if { $range_list_len > 2 || $range_list_len < 1} {
> + error "invalid range spec string: $range_spec"
> + } elseif { $range_list_len == 2 } {
> + set range_end [lindex $range_list 1]
> + } else {
> + set range_end $range_start
> + }
> +
> + for {set i $range_start} {$i <= $range_end} {incr i} {
> + append pattern "\r\n$i\[ \t\]\[^\r\n\]*"
> + }
> +
> + verbose -log "pattern $pattern"
> + return $pattern
> +}
> +
> +# Test "list" command invocations right after stopping for an event.
> +# COMMAND is the actual list command, including arguments. LISTSIZE1
> +# and LISTSIZE2 are the listsizes set just before and after running
> +# the program to the stop point. COMMAND is issued twice. The first
> +# time, the lines specificed by LINERANGE1 are expected; the second
> +# time, the lines specified by LINERANGE2 are expected.
> +
> +proc test_list {command listsize1 listsize2 linerange1 linerange2} {
> + with_test_prefix "$command after stop: $listsize1, $listsize2" {
> + global binfile
> +
> + clean_restart $binfile
> + if ![runto_main] then {
> + fail "Can't run to main"
> + return
> + }
> +
> + # Test changing the listsize both before nexting, and after
> + # stopping, but before listing. Only the second listsize
> + # change should affect which lines are listed.
> + gdb_test_no_output "set listsize $listsize1"
> + gdb_test "next" "foo \\(.*"
> + gdb_test_no_output "set listsize $listsize2"
> +
> + set pattern1 [build_pattern $linerange1]
> + set pattern2 [build_pattern $linerange2]
> + gdb_test "$command" "${pattern1}" "$command #1"
> + gdb_test "$command" "${pattern2}" "$command #2"
> + }
> +}
> +
> +
> +# The first "list" should center the listing around line 8, the stop
> +# line.
> +test_list "list" 1 10 "3-12" "13-22"
> +
> +# Likewise.
> +test_list "list" 10 10 "3-12" "13-22"
> +
> +# Likewise, but show only one line. IOW, the first list should show
> +# line 8. Note how the listsize is 10 at the time of the stop, but
> +# before any listing had been requested. That should not affect the
> +# line range that is first listed.
> +test_list "list" 10 1 "8" "9"
> +
> +# Likewise, but show two lines.
> +test_list "list" 10 2 "7-8" "9-10"
> +
> +# Three lines.
> +test_list "list" 10 3 "7-9" "10-12"
> +
> +# Now test backwards. Just like "list", the first "list -" should
> +# center the listing around the stop line.
> +test_list "list -" 10 10 "3-12" "2"
> +
> +# Likewise, but test showing 3 lines at a time.
> +test_list "list -" 10 3 "7-9" "4-6"
> +
> +# 2 lines at a time.
> +test_list "list -" 10 2 "7-8" "5-6"
> +
> +# Test listing one line one. This case is a little special, and
> +# starts showing the previous line immediately.
> +test_list "list -" 10 1 "7" "6"
> +
> remote_exec build "rm -f list0.h"
> diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
> index 5657be6..566d64d 100644
> --- a/gdb/testsuite/gdb.mi/mi-cli.exp
> +++ b/gdb/testsuite/gdb.mi/mi-cli.exp
> @@ -65,6 +65,7 @@ set line_main_head [gdb_get_line_number "main ("]
> set line_main_body [expr $line_main_head + 2]
> set line_main_hello [gdb_get_line_number "Hello, World!"]
> set line_main_return [expr $line_main_hello + 2]
> +set line_main_callme_2 [expr $line_main_return + 1]
> set line_callee4_head [gdb_get_line_number "callee4 ("]
> set line_callee4_body [expr $line_callee4_head + 2]
> set line_callee4_next [expr $line_callee4_body + 1]
> @@ -184,6 +185,25 @@ mi_gdb_test "-interpreter-exec console \"list\"" \
> "\~\"$line_main_return\[\\\\t ]*callme \\(1\\);\\\\n\".*\\^done" \
> "-interpreter-exec console \"list\" at basics.c:\$line_main_return"
>
> +mi_gdb_test "600-break-insert -t basics.c:$line_main_callme_2" \
> + {600\^done,bkpt=.number="4",type="breakpoint".*\}} \
> + "-break-insert -t basics.c:\$line_main_callme_2"
> +
> +mi_execute_to "exec-continue" "breakpoint-hit" "main" "" ".*basics.c" \
> + $line_main_callme_2 { "" "disp=\"del\"" } \
> + "-exec-continue to line \$line_main_callme_2"
> +
> +# Restore the listsize back to the default.
> +mi_gdb_test "-interpreter-exec console \"set listsize 10\"" \
> + ".*=cmd-param-changed,param=\"listsize\",value=\"10\".*\\^done" \
> + "-interpreter-exec console \"set listsize 10\""
> +
> +# "list" should show 10 lines centered on where the program stopped.
> +set first_list_line [expr $line_main_callme_2 - 5]
> +mi_gdb_test "-interpreter-exec console \"list\"" \
> + ".*\~\"$first_list_line.*\\^done" \
> + "-interpreter-exec console \"list\" at basics.c:\$line_main_return"
> +
> mi_gdb_test "-interpreter-exec console \"help set args\"" \
> {\~"Set argument list to give program being debugged when it is started\.\\nFollow this command with any number of args, to be passed to the program\.".*\^done} \
> "-interpreter-exec console \"help set args\""
> diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
> index a470427..2b7d78e 100644
> --- a/gdb/testsuite/gdb.python/python.exp
> +++ b/gdb/testsuite/gdb.python/python.exp
> @@ -168,7 +168,8 @@ if ![runto_main] then {
> return 0
> }
>
> -runto [gdb_get_line_number "Break to end."]
> +set lineno [gdb_get_line_number "Break to end."]
> +runto $lineno
>
> # Test gdb.decode_line.
> gdb_test "python gdb.decode_line(\"main.c:43\")" \
> @@ -179,7 +180,7 @@ gdb_test "python print (len(symtab))" "2" "Test decode_line current location"
> gdb_test "python print (symtab\[0\])" "None" "Test decode_line expression parse"
> gdb_test "python print (len(symtab\[1\]))" "1" "Test decode_line current location"
> gdb_test "python print (symtab\[1\]\[0\].symtab)" ".*gdb.python/python.c.*" "Test decode_line current locationn filename"
> -gdb_test "python print (symtab\[1\]\[0\].line)" "22" "Test decode_line current location line number"
> +gdb_test "python print (symtab\[1\]\[0\].line)" "$lineno" "Test decode_line current location line number"
>
> gdb_py_test_silent_cmd "python symtab = gdb.decode_line(\"python.c:26 if foo\")" "test decode_line python.c:26" 1
> gdb_test "python print (len(symtab))" "2" "Test decode_line python.c:26 length"
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v5 3/8] PR gdb/13860: make "-exec-foo"'s MI output equal to "foo"'s MI output.
2014-03-04 18:33 [RFC v5 0/8] enable target-async by default Tom Tromey
` (4 preceding siblings ...)
2014-03-04 18:33 ` [RFC v5 2/8] PR gdb/13860: make -interpreter-exec console "list" behave more like "list" Tom Tromey
@ 2014-03-04 18:33 ` Tom Tromey
2014-03-04 18:33 ` [RFC v5 8/8] enable target-async by default Tom Tromey
2014-03-04 18:40 ` [RFC v5 6/8] fix py-finish-breakpoint.exp with always-async Tom Tromey
7 siblings, 0 replies; 19+ messages in thread
From: Tom Tromey @ 2014-03-04 18:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
From: Pedro Alves <palves@redhat.com>
Part of PR gdb/13860 is about the mi-solib.exp test's output being
different in sync vs async modes.
sync:
>./gdb -nx -q ./testsuite/gdb.mi/solib-main -ex "set stop-on-solib-events 1" -ex "set target-async off" -i=mi
=thread-group-added,id="i1"
~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main..."
~"done.\n"
(gdb)
&"start\n"
~"Temporary breakpoint 1 at 0x400608: file ../../../src/gdb/testsuite/gdb.mi/solib-main.c, line 21.\n"
=breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000000000400608",func="main",file="../../../src/gdb/testsuite/gdb.mi/solib-main.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/solib-main.c",line="21",times="0",original-location="main"}
~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
=thread-group-started,id="i1",pid="17724"
=thread-created,id="1",group-id="i1"
^running
*running,thread-id="all"
(gdb)
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
~"Stopped due to shared library event (no libraries added or removed)\n"
*stopped,reason="solib-event",frame={addr="0x000000379180f990",func="_dl_debug_state",args=[],from="/lib64/ld-linux-x86-64.so.2"},thread-id="1",stopped-threads="all",core="3"
(gdb)
async:
>./gdb -nx -q ./testsuite/gdb.mi/solib-main -ex "set stop-on-solib-events 1" -ex "set target-async on" -i=mi
=thread-group-added,id="i1"
~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main..."
~"done.\n"
(gdb)
start
&"start\n"
~"Temporary breakpoint 1 at 0x400608: file ../../../src/gdb/testsuite/gdb.mi/solib-main.c, line 21.\n"
=breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000000000400608",func="main",file="../../../src/gdb/testsuite/gdb.mi/solib-main.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/solib-main.c",line="21",times="0",original-location="main"}
~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
=thread-group-started,id="i1",pid="17729"
=thread-created,id="1",group-id="i1"
^running
*running,thread-id="all"
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
(gdb)
*stopped,reason="solib-event",thread-id="1",stopped-threads="all",core="1"
For now, let's focus only on the *stopped event. We see that the
async output is missing frame info. And this causes a test failure in
async mode, as "mi_expect_stop solib-event" wants to see the frame
info.
However, if we compare the event output when a real MI execution
command is used, compared to a CLI command (e.g., run vs -exec-run,
next vs -exec-next, etc.), we see:
>./gdb -nx -q ./testsuite/gdb.mi/solib-main -ex "set stop-on-solib-events 1" -ex "set target-async off" -i=mi
=thread-group-added,id="i1"
~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main..."
~"done.\n"
(gdb)
r
&"r\n"
~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
=thread-group-started,id="i1",pid="17751"
=thread-created,id="1",group-id="i1"
^running
*running,thread-id="all"
(gdb)
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
~"Stopped due to shared library event (no libraries added or removed)\n"
*stopped,reason="solib-event",frame={addr="0x000000379180f990",func="_dl_debug_state",args=[],from="/lib64/ld-linux-x86-64.so.2"},thread-id="1",stopped-threads="all",core="3"
(gdb)
-exec-run
=thread-exited,id="1",group-id="i1"
=thread-group-exited,id="i1"
=library-unloaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",thread-group="i1"
=thread-group-started,id="i1",pid="17754"
=thread-created,id="1",group-id="i1"
^running
*running,thread-id="all"
(gdb)
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
*stopped,reason="solib-event",thread-id="1",stopped-threads="all",core="1"
=thread-selected,id="1"
(gdb)
As seen above, with MI commands, the *stopped event _doesn't_ have
frame info. This is because normal_stop, as commanded by the result
of bpstat_print, skips printing frame info in this case (it's an
"event", not a "breakpoint"), and when the interpreter is MI,
mi_on_normal_stop skips calling print_stack_frame, as the normal_stop
call was already done with the MI uiout. This explains why the async
output is different even with a CLI command. Its because in async
mode, the mi_on_normal_stop path is always taken; it is always reached
with the MI uiout, because the stop is handled from the event loop,
instead of from within `proceed -> wait_for_inferior -> normal_stop'
with the interpreter overridden, as in sync mode.
This patch fixes the issue by making all cases output the same
*stopped event, by factoring out the print code from normal_stop, and
using it from mi_on_normal_stop as well. I chose the *stopped output
without a frame, mainly because that is what you already get if you
use MI execution commands, the commands frontends are supposed to use
(except when implementing a console). This patch makes it simpler to
tweak the MI output differently if desired, as we only have to change
the centralized print_stop_event (taking into account whether the
uiout is MI-like), and all different modes will change accordingly.
Tested on x86_64 Fedora 16, no regressions. The mi-solib.exp test no
longer fails in async mode with this patch, so the patch removes the
kfail.
2013-10-30 Pedro Alves <palves@redhat.com>
PR gdb/13860
* inferior.h (print_stop_event): Declare.
* infrun.c (print_stop_event): New, factored out from ...
(normal_stop): ... this.
* mi/mi-interp.c (mi_on_normal_stop): Use print_stop_event instead
of bpstat_print/print_stack_frame.
2013-10-30 Pedro Alves <palves@redhat.com>
* gdb.mi/mi-solib.exp: Remove gdb/13860 kfail.
* lib/mi-support.exp (mi_expect_stop): Add special handling for
solib-event.
---
gdb/ChangeLog | 9 +++
gdb/inferior.h | 2 +
gdb/infrun.c | 118 ++++++++++++++++++++------------------
gdb/mi/mi-interp.c | 3 +-
gdb/testsuite/ChangeLog | 6 ++
gdb/testsuite/gdb.mi/mi-solib.exp | 4 --
gdb/testsuite/lib/mi-support.exp | 18 +++++-
7 files changed, 97 insertions(+), 63 deletions(-)
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 9f6375e..f365c01 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -195,6 +195,8 @@ extern void start_remote (int from_tty);
extern void normal_stop (void);
+extern void print_stop_event (struct target_waitstatus *ws);
+
extern int signal_stop_state (int);
extern int signal_print_state (int);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 042d5fa..e46f5d4 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5998,6 +5998,68 @@ print_no_history_reason (void)
ui_out_text (current_uiout, "\nNo more reverse-execution history.\n");
}
+/* Print current location without a level number, if we have changed
+ functions or hit a breakpoint. Print source line if we have one.
+ bpstat_print contains the logic deciding in detail what to print,
+ based on the event(s) that just occurred. */
+
+void
+print_stop_event (struct target_waitstatus *ws)
+{
+ int bpstat_ret;
+ int source_flag;
+ int do_frame_printing = 1;
+ struct thread_info *tp = inferior_thread ();
+
+ bpstat_ret = bpstat_print (tp->control.stop_bpstat, ws->kind);
+ switch (bpstat_ret)
+ {
+ case PRINT_UNKNOWN:
+ /* FIXME: cagney/2002-12-01: Given that a frame ID does (or
+ should) carry around the function and does (or should) use
+ that when doing a frame comparison. */
+ if (tp->control.stop_step
+ && frame_id_eq (tp->control.step_frame_id,
+ get_frame_id (get_current_frame ()))
+ && step_start_function == find_pc_function (stop_pc))
+ {
+ /* Finished step, just print source line. */
+ source_flag = SRC_LINE;
+ }
+ else
+ {
+ /* Print location and source line. */
+ source_flag = SRC_AND_LOC;
+ }
+ break;
+ case PRINT_SRC_AND_LOC:
+ /* Print location and source line. */
+ source_flag = SRC_AND_LOC;
+ break;
+ case PRINT_SRC_ONLY:
+ source_flag = SRC_LINE;
+ break;
+ case PRINT_NOTHING:
+ /* Something bogus. */
+ source_flag = SRC_LINE;
+ do_frame_printing = 0;
+ break;
+ default:
+ internal_error (__FILE__, __LINE__, _("Unknown value."));
+ }
+
+ /* The behavior of this routine with respect to the source
+ flag is:
+ SRC_LINE: Print only source line
+ LOCATION: Print only location
+ SRC_AND_LOC: Print location and source line. */
+ if (do_frame_printing)
+ print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
+
+ /* Display the auto-display expressions. */
+ do_displays ();
+}
+
/* Here to return control to GDB when the inferior stops for real.
Print appropriate messages, remove breakpoints, give terminal our modes.
@@ -6120,65 +6182,11 @@ normal_stop (void)
{
select_frame (get_current_frame ());
- /* Print current location without a level number, if
- we have changed functions or hit a breakpoint.
- Print source line if we have one.
- bpstat_print() contains the logic deciding in detail
- what to print, based on the event(s) that just occurred. */
-
/* If --batch-silent is enabled then there's no need to print the current
source location, and to try risks causing an error message about
missing source files. */
if (stop_print_frame && !batch_silent)
- {
- int bpstat_ret;
- int source_flag;
- int do_frame_printing = 1;
- struct thread_info *tp = inferior_thread ();
-
- bpstat_ret = bpstat_print (tp->control.stop_bpstat, last.kind);
- switch (bpstat_ret)
- {
- case PRINT_UNKNOWN:
- /* FIXME: cagney/2002-12-01: Given that a frame ID does
- (or should) carry around the function and does (or
- should) use that when doing a frame comparison. */
- if (tp->control.stop_step
- && frame_id_eq (tp->control.step_frame_id,
- get_frame_id (get_current_frame ()))
- && step_start_function == find_pc_function (stop_pc))
- source_flag = SRC_LINE; /* Finished step, just
- print source line. */
- else
- source_flag = SRC_AND_LOC; /* Print location and
- source line. */
- break;
- case PRINT_SRC_AND_LOC:
- source_flag = SRC_AND_LOC; /* Print location and
- source line. */
- break;
- case PRINT_SRC_ONLY:
- source_flag = SRC_LINE;
- break;
- case PRINT_NOTHING:
- source_flag = SRC_LINE; /* something bogus */
- do_frame_printing = 0;
- break;
- default:
- internal_error (__FILE__, __LINE__, _("Unknown value."));
- }
-
- /* The behavior of this routine with respect to the source
- flag is:
- SRC_LINE: Print only source line
- LOCATION: Print only location
- SRC_AND_LOC: Print location and source line. */
- if (do_frame_printing)
- print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
-
- /* Display the auto-display expressions. */
- do_displays ();
- }
+ print_stop_event (&last);
}
/* Save the function value return registers, if we care.
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 25bf0a1..862beaf 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -441,9 +441,8 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
current_uiout = mi_uiout;
get_last_target_status (&last_ptid, &last);
- bpstat_print (bs, last.kind);
+ print_stop_event (&last);
- print_stack_frame (get_selected_frame (NULL), 0, SRC_AND_LOC, 1);
current_uiout = saved_uiout;
}
diff --git a/gdb/testsuite/gdb.mi/mi-solib.exp b/gdb/testsuite/gdb.mi/mi-solib.exp
index 06fa26b..608d2b7 100644
--- a/gdb/testsuite/gdb.mi/mi-solib.exp
+++ b/gdb/testsuite/gdb.mi/mi-solib.exp
@@ -60,8 +60,4 @@ mi_gdb_test "777-gdb-set stop-on-solib-events 1" "777\\^done" \
# commands still cause the correct MI output to be generated.
mi_run_with_cli
-global async
-if { $async } {
- setup_kfail gdb/13860 *-*-*
-}
mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 1e8fee6..213073a 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1028,6 +1028,8 @@ proc mi_expect_stop { reason func args file line extra test } {
global thread_selected_re
global breakpoint_re
+ set any "\[^\n\]*"
+
set after_stopped ""
set after_reason ""
if { [llength $extra] == 2 } {
@@ -1070,6 +1072,20 @@ proc mi_expect_stop { reason func args file line extra test } {
return
}
+ if { $reason == "solib-event" } {
+ set pattern "\\*stopped,reason=\"solib-event\",thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re|$breakpoint_re)*$prompt_re"
+ verbose -log "mi_expect_stop: expecting: $pattern"
+ gdb_expect {
+ -re "$pattern" {
+ pass "$test"
+ }
+ timeout {
+ fail "$test (unknown output after running)"
+ }
+ }
+ return
+ }
+
set args "\\\[$args\\\]"
set bn ""
@@ -1087,8 +1103,6 @@ proc mi_expect_stop { reason func args file line extra test } {
set a $after_reason
- set any "\[^\n\]*"
-
verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}frame=\{addr=\"$hex\",func=\"$func\",args=$args,(?:file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"|from=\"$file\")\}$after_stopped,thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re|$breakpoint_re)*$prompt_re"
gdb_expect {
-re "\\*stopped,${r}${a}${bn}frame=\{addr=\"$hex\",func=\"$func\",args=$args,(?:file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"|from=\"$file\")\}$after_stopped,thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re|$breakpoint_re)*$prompt_re" {
--
1.8.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread* [RFC v5 8/8] enable target-async by default
2014-03-04 18:33 [RFC v5 0/8] enable target-async by default Tom Tromey
` (5 preceding siblings ...)
2014-03-04 18:33 ` [RFC v5 3/8] PR gdb/13860: make "-exec-foo"'s MI output equal to "foo"'s MI output Tom Tromey
@ 2014-03-04 18:33 ` Tom Tromey
2014-03-04 18:40 ` [RFC v5 6/8] fix py-finish-breakpoint.exp with always-async Tom Tromey
7 siblings, 0 replies; 19+ messages in thread
From: Tom Tromey @ 2014-03-04 18:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This flips the default of maint_async_permitted, causing target async
to be enabled by default.
2014-02-26 Tom Tromey <tromey@redhat.com>
* target.c (maint_async_permitted, maint_async_permitted_1):
Default to 1.
---
gdb/ChangeLog | 5 +++++
gdb/target.c | 4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gdb/target.c b/gdb/target.c
index 6711bd7..369cbb7 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -4201,11 +4201,11 @@ maintenance_print_target_stack (char *cmd, int from_tty)
/* Controls if targets can report that they are async. This is just
for maintainers to use when debugging gdb. */
-int maint_async_permitted = 0;
+int maint_async_permitted = 1;
/* The set command writes to this variable. If the inferior is
executing, maint_async_permitted is *not* updated. */
-static int maint_async_permitted_1 = 0;
+static int maint_async_permitted_1 = 1;
static void
set_maint_async_command (char *args, int from_tty,
--
1.8.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread* [RFC v5 6/8] fix py-finish-breakpoint.exp with always-async
2014-03-04 18:33 [RFC v5 0/8] enable target-async by default Tom Tromey
` (6 preceding siblings ...)
2014-03-04 18:33 ` [RFC v5 8/8] enable target-async by default Tom Tromey
@ 2014-03-04 18:40 ` Tom Tromey
2014-03-20 18:28 ` Pedro Alves
7 siblings, 1 reply; 19+ messages in thread
From: Tom Tromey @ 2014-03-04 18:40 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
With target async enabled, py-finish-breakpoint.exp will trigger an
assertion failure.
The failure occurs because execute_command re-enters the event loop in
some circumstances, and in this case resets the sync_execution flag.
Then later gdb reaches this assertion in normal_stop:
gdb_assert (sync_execution || !target_can_async_p ());
execute_command has a comment explaining why it dispatches events:
/* If the interpreter is in sync mode (we're running a user
command's list, running command hooks or similars), and we
just ran a synchronous command that started the target, wait
for that command to end. */
However, the code did not follow this comment -- it didn't check to
see if the command started the target, just whether the target was
executing a sync command at this point.
This patch fixes the problem by noting whether the target was
executing in sync_execution mode before running the command, and then
augmenting the condition to test this as well.
2013-10-30 Tom Tromey <tromey@redhat.com>
PR gdb/14135:
* top.c (execute_command): Only dispatch events if command
started target.
---
gdb/ChangeLog | 6 ++++++
gdb/top.c | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/gdb/top.c b/gdb/top.c
index e1a1331..fa20025 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -406,6 +406,8 @@ execute_command (char *p, int from_tty)
{
const char *cmd = p;
char *arg;
+ int was_sync = sync_execution;
+
line = p;
/* If trace-commands is set then this will print this command. */
@@ -461,7 +463,7 @@ execute_command (char *p, int from_tty)
command's list, running command hooks or similars), and we
just ran a synchronous command that started the target, wait
for that command to end. */
- if (!interpreter_async && sync_execution)
+ if (!interpreter_async && !was_sync && sync_execution)
{
while (gdb_do_one_event () >= 0)
if (!sync_execution)
--
1.8.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v5 6/8] fix py-finish-breakpoint.exp with always-async
2014-03-04 18:40 ` [RFC v5 6/8] fix py-finish-breakpoint.exp with always-async Tom Tromey
@ 2014-03-20 18:28 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-03-20 18:28 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi,
I somehow neglected to respond to your response to:
https://sourceware.org/ml/gdb-patches/2013-11/msg00298.html
at:
https://sourceware.org/ml/gdb-patches/2013-12/msg00349.html
I investigated this further in the direction of my comments,
and extended the commit log with the resulting details.
Here's what I pushed.
Thanks.
-----------------
From: Tom Tromey <tromey@redhat.com>
Fix py-finish-breakpoint.exp with target async.
With target async enabled, py-finish-breakpoint.exp triggers an
assertion failure.
The failure occurs because execute_command re-enters the event loop in
some circumstances, and in this case resets the sync_execution flag.
Then later GDB reaches this assertion in normal_stop:
gdb_assert (sync_execution || !target_can_async_p ());
In detail:
#1 - A synchronous execution command is run. sync_execution is set.
#2 - A python breakpoint is hit (TARGET_WAITKIND_STOPPED), and the
corresponding Python breakpoint's stop method is executed. When
and while python commands are executed, interpreter_async is
forced to 0.
#3 - The Python stop method happens to execute a not-execution-related
gdb command. In this case, "where 1".
#4 - Seeing that sync_execution is set, execute_command nests a new
event loop (although that wasn't necessary; this is the problem).
#5 - The linux-nat target's pipe in the event loop happens to be
marked. That's normal, due to this in linux_nat_wait:
/* If we requested any event, and something came out, assume there
may be more. If we requested a specific lwp or process, also
assume there may be more. */
The nested event loop thus immediately wakes up and calls
target_wait. No thread is actually executing in the inferior, so
the target returns TARGET_WAITKIND_NO_RESUMED.
#6 - normal_stop is reached. GDB prints "No unwaited-for children
left.", and resets the sync_execution flag (IOW, there are no
resumed threads left, so the synchronous command is considered
completed.) This is already bogus. We were handling a
breakpoint!
#7 - the nested event loop unwinds/ends. GDB is now back to handling
the python stop method (TARGET_WAITKIND_STOPPED), which decides
the breakpoint should stop. normal_stop is called for this
event. However, normal_stop actually works with the _last_
reported target status:
void
normal_stop (void)
{
struct target_waitstatus last;
ptid_t last_ptid;
struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
...
get_last_target_status (&last_ptid, &last);
...
if (last.kind == TARGET_WAITKIND_NO_RESUMED)
{
gdb_assert (sync_execution || !target_can_async_p ());
target_terminal_ours_for_output ();
printf_filtered (_("No unwaited-for children left.\n"));
}
And due to the nesting in execute command, the last event is now
TARGET_WAITKIND_NO_RESUMED, not the actual breakpoint event being
handled. This could be seen to be broken in itself, but we can
leave fixing that for another pass. The assertion is reached, and
fails.
execute_command has a comment explaining when it should synchronously
wait for events:
/* If the interpreter is in sync mode (we're running a user
command's list, running command hooks or similars), and we
just ran a synchronous command that started the target, wait
for that command to end. */
However, the code did not follow this comment -- it didn't check to
see if the command actually started the target, just whether the
target was executing a sync command at this point.
This patch fixes the problem by noting whether the target was
executing in sync_execution mode before running the command, and then
augmenting the condition to test this as well.
2014-03-20 Tom Tromey <tromey@redhat.com>
PR gdb/14135
* top.c (execute_command): Only dispatch events if the command
started the target.
---
gdb/ChangeLog | 6 ++++++
gdb/top.c | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index acb25f8..1db0ee9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
2014-03-20 Tom Tromey <tromey@redhat.com>
+ PR gdb/14135
+ * top.c (execute_command): Only dispatch events if the command
+ started the target.
+
+2014-03-20 Tom Tromey <tromey@redhat.com>
+
PR cli/15718
* infcall.c: Include event-top.h.
(run_inferior_call): Call async_disable_stdin if needed.
diff --git a/gdb/top.c b/gdb/top.c
index e1a1331..fa20025 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -406,6 +406,8 @@ execute_command (char *p, int from_tty)
{
const char *cmd = p;
char *arg;
+ int was_sync = sync_execution;
+
line = p;
/* If trace-commands is set then this will print this command. */
@@ -461,7 +463,7 @@ execute_command (char *p, int from_tty)
command's list, running command hooks or similars), and we
just ran a synchronous command that started the target, wait
for that command to end. */
- if (!interpreter_async && sync_execution)
+ if (!interpreter_async && !was_sync && sync_execution)
{
while (gdb_do_one_event () >= 0)
if (!sync_execution)
--
1.7.11.7
^ permalink raw reply [flat|nested] 19+ messages in thread