* [RFA] fix *stopped for CLI commands
@ 2009-02-06 7:44 Vladimir Prus
2009-02-06 21:11 ` Tom Tromey
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Vladimir Prus @ 2009-02-06 7:44 UTC (permalink / raw)
To: gdb-patches, Nick Roberts, Marc Khouzam
[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]
In gdb 6.8, typing CLI commands in MI session does not produce either *running,
or ^running or *stopped messages. I've checked in some patches previous to
improve that, but *stopped turns out to be not finished yet -- when in non-async
mode, CLI commands such as 'continue' result in *stopped, but it has no frame
information.
The reason is that *stopped is printed by normal_stop observer, and the frame
information is printed by normal_stop function, in infrun.c. When a CLI command
is executed, we switch to the CLI interpreter and CLI uiout. In async mode,
the command execution is done, we switch back to MI uiout, and then we handle
stop event, so infrun.c:normal_stop prints frame into MI uiout and then MI observer
puts that data out. In sync mode, however, infrun.c:normal_stop is called when
the uiout is still CLI uiout, and then MI observer does not have any way to
access the fields printed into CLI uiout.
This patch fixes this by making MI observer print frame again, into MI uiout,
if necessary. It passes all MI tests in (sync,async)x(native,gdbserver) combinations.
How does this look, and are non-MI changes OK? If approved, I'll add a test that
CLI commands result in proper *stopped.
Thanks,
Volodya
[-- Attachment #2: stopped.diff --]
[-- Type: text/x-diff, Size: 8473 bytes --]
commit b9adf85d3252a9ed41c76bad645ca13af4efa027
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Fri Feb 6 10:39:17 2009 +0300
Include frame information for *stopped due to CLI commands.
gdb/doc/
* observer.texi: Add parameter 'print_frame' to normal_stop
observer.
* ada-tasks.c (ada_normal_stop_observer): Adjust prototype.
* infcmd.c (finish_command_continuation): Pass '1' for
'print_frame' parameter to the observer.
* infrun.c (normal_stop): Don't print mi-specific information
here. Pass 'stop_print_frame' to the 'print_frame' parameter
of the observer.
* mi/mi-interp.c (mi_on_normal_stop): Adjust prototype.
If we need to print frame, and current uiout is not the MI one,
print frame again.
gdb/testsuite/
* lib/mi-support.exp (mi_expect_stop): Adjust the order of fields.
(mi_expect_interrupt): Likewise.
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 5176d75..3639472 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -951,7 +951,7 @@ ada_task_list_changed (void)
/* The 'normal_stop' observer notification callback. */
static void
-ada_normal_stop_observer (struct bpstats *unused_args)
+ada_normal_stop_observer (struct bpstats *unused_args, int unused_args2)
{
/* The inferior has been resumed, and just stopped. This means that
our task_list needs to be recomputed before it can be used again. */
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index 636658a..8327fea 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -88,7 +88,7 @@ Send a notification to all @var{event} observers.
The following observable events are defined:
-@deftypefun void normal_stop (struct bpstats *@var{bs})
+@deftypefun void normal_stop (struct bpstats *@var{bs}, int @var{print_frame})
The inferior has stopped for real.
@end deftypefun
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3696f79..188077c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1374,7 +1374,7 @@ finish_command_continuation (void *arg)
next stop will be in the same thread that we started doing a
finish on. This suppressing (or some other replacement means)
should be a thread property. */
- observer_notify_normal_stop (bs);
+ observer_notify_normal_stop (bs, 1 /* print frame */);
suppress_stop_observer = 0;
delete_breakpoint (a->breakpoint);
}
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c11c71a..e50ef49 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4351,22 +4351,6 @@ Further execution is probably impossible.\n"));
internal_error (__FILE__, __LINE__, _("Unknown value."));
}
- if (ui_out_is_mi_like_p (uiout))
- {
-
- ui_out_field_int (uiout, "thread-id",
- pid_to_thread_id (inferior_ptid));
- if (non_stop)
- {
- struct cleanup *back_to = make_cleanup_ui_out_list_begin_end
- (uiout, "stopped-threads");
- ui_out_field_int (uiout, NULL,
- pid_to_thread_id (inferior_ptid));
- do_cleanups (back_to);
- }
- else
- ui_out_field_string (uiout, "stopped-threads", "all");
- }
/* The behavior of this routine with respect to the source
flag is:
SRC_LINE: Print only source line
@@ -4421,9 +4405,10 @@ done:
&& inferior_thread ()->step_multi))
{
if (!ptid_equal (inferior_ptid, null_ptid))
- observer_notify_normal_stop (inferior_thread ()->stop_bpstat);
+ observer_notify_normal_stop (inferior_thread ()->stop_bpstat,
+ stop_print_frame);
else
- observer_notify_normal_stop (NULL);
+ observer_notify_normal_stop (NULL, stop_print_frame);
}
if (target_has_execution)
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 8e2b230..7f52549 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -52,7 +52,7 @@ static void mi1_command_loop (void);
static void mi_insert_notify_hooks (void);
static void mi_remove_notify_hooks (void);
-static void mi_on_normal_stop (struct bpstats *bs);
+static void mi_on_normal_stop (struct bpstats *bs, int print_frame);
static void mi_new_thread (struct thread_info *t);
static void mi_thread_exit (struct thread_info *t);
@@ -322,17 +322,46 @@ mi_inferior_exit (int pid)
}
static void
-mi_on_normal_stop (struct bpstats *bs)
+mi_on_normal_stop (struct bpstats *bs, int print_frame)
{
/* Since this can be called when CLI command is executing,
using cli interpreter, be sure to use MI uiout for output,
not the current one. */
- struct ui_out *uiout = interp_ui_out (top_level_interpreter ());
+ struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
struct mi_interp *mi = top_level_interpreter_data ();
+ if (print_frame)
+ {
+ if (uiout != mi_uiout)
+ {
+ /* The normal_stop function has printed frame information into
+ CLI uiout, or some other non-MI uiout. There's no way we
+ can extra proper fields from random uiout object, so we print
+ the frame again. In practice, this can only happen when running
+ a CLI command in MI. */
+ struct ui_out *saved_uiout = uiout;
+ uiout = mi_uiout;
+ print_stack_frame (get_selected_frame (NULL), 0, SRC_AND_LOC);
+ uiout = saved_uiout;
+ }
+
+ ui_out_field_int (mi_uiout, "thread-id",
+ pid_to_thread_id (inferior_ptid));
+ if (non_stop)
+ {
+ struct cleanup *back_to = make_cleanup_ui_out_list_begin_end
+ (mi_uiout, "stopped-threads");
+ ui_out_field_int (mi_uiout, NULL,
+ pid_to_thread_id (inferior_ptid));
+ do_cleanups (back_to);
+ }
+ else
+ ui_out_field_string (mi_uiout, "stopped-threads", "all");
+ }
+
fputs_unfiltered ("*stopped", raw_stdout);
- mi_out_put (uiout, raw_stdout);
- mi_out_rewind (uiout);
+ mi_out_put (mi_uiout, raw_stdout);
+ mi_out_rewind (mi_uiout);
fputs_unfiltered ("\n", raw_stdout);
gdb_flush (raw_stdout);
}
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index be9b530..f62c240 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1020,13 +1020,13 @@ proc mi_expect_stop { reason func args file line extra test } {
set any "\[^\n\]*"
- verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n($thread_selected_re)?$prompt_re"
+ 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\"\}$after_stopped,thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re)?$prompt_re"
gdb_expect {
- -re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n($thread_selected_re)?$prompt_re" {
+ -re "\\*stopped,${r}${a}${bn}frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped,thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re)?$prompt_re" {
pass "$test"
return $expect_out(2,string)
}
- -re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$any\",args=\[\\\[\{\]$any\[\\\]\}\],file=\"$any\",fullname=\"${fullname_syntax}$any\",line=\"\[0-9\]*\"\}$any\r\n$prompt_re" {
+ -re "\\*stopped,${r}${a}${bn}frame=\{addr=\"$hex\",func=\"$any\",args=\[\\\[\{\]$any\[\\\]\}\],file=\"$any\",fullname=\"${fullname_syntax}$any\",line=\"\[0-9\]*\"\}thread-id=\"$decimal\",stopped-threads=$any\r\n$prompt_re" {
verbose -log "got $expect_out(buffer)"
fail "$test (stopped at wrong place)"
return -1
@@ -1061,9 +1061,9 @@ proc mi_expect_interrupt { test } {
set any "\[^\n\]*"
# A signal can land anywhere, just ignore the location
- verbose -log "mi_expect_interrupt: expecting: \\*stopped,${r},thread-id=\"$decimal\",stopped-threads=$any\r\n$prompt_re"
+ verbose -log "mi_expect_interrupt: expecting: \\*stopped,${r}$any\r\n$prompt_re"
gdb_expect {
- -re "\\*stopped,${r},thread-id=\"$decimal\",stopped-threads=$any\r\n$prompt_re" {
+ -re "\\*stopped,${r}$any\r\n$prompt_re" {
pass "$test"
return 0;
}
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFA] fix *stopped for CLI commands
2009-02-06 7:44 [RFA] fix *stopped for CLI commands Vladimir Prus
@ 2009-02-06 21:11 ` Tom Tromey
2009-02-07 1:04 ` Joel Brobecker
2009-02-12 8:46 ` Vladimir Prus
2009-02-09 20:36 ` Daniel Jacobowitz
2009-02-10 20:50 ` Marc Khouzam
2 siblings, 2 replies; 13+ messages in thread
From: Tom Tromey @ 2009-02-06 21:11 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches, Nick Roberts, Marc Khouzam
>>>>> "Vladimir" == Vladimir Prus <vladimir@codesourcery.com> writes:
Vladimir> This patch fixes this by making MI observer print frame
Vladimir> again, into MI uiout, if necessary. It passes all MI tests
Vladimir> in (sync,async)x(native,gdbserver) combinations. How does
Vladimir> this look, and are non-MI changes OK? If approved, I'll add
Vladimir> a test that CLI commands result in proper *stopped.
I don't understand all the implications of the core change, but I do
like how it moves some MI logic out of the core and into MI.
Vladimir> -@deftypefun void normal_stop (struct bpstats *@var{bs})
Vladimir> +@deftypefun void normal_stop (struct bpstats *@var{bs}, int @var{print_frame})
Vladimir> The inferior has stopped for real.
Vladimir> @end deftypefun
I would like to ask for documentation describing the meaning of the
new argument.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFA] fix *stopped for CLI commands
2009-02-06 21:11 ` Tom Tromey
@ 2009-02-07 1:04 ` Joel Brobecker
2009-02-12 8:46 ` Vladimir Prus
1 sibling, 0 replies; 13+ messages in thread
From: Joel Brobecker @ 2009-02-07 1:04 UTC (permalink / raw)
To: Tom Tromey; +Cc: Vladimir Prus, gdb-patches, Nick Roberts, Marc Khouzam
> Vladimir> This patch fixes this by making MI observer print frame
> Vladimir> again, into MI uiout, if necessary. It passes all MI tests
> Vladimir> in (sync,async)x(native,gdbserver) combinations. How does
> Vladimir> this look, and are non-MI changes OK? If approved, I'll add
> Vladimir> a test that CLI commands result in proper *stopped.
>
> I don't understand all the implications of the core change, but I do
> like how it moves some MI logic out of the core and into MI.
Agreed. Ideally, I'd really like to see all such notifications
be handled by observers, even the CLI ones.
--
Joel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA] fix *stopped for CLI commands
2009-02-06 21:11 ` Tom Tromey
2009-02-07 1:04 ` Joel Brobecker
@ 2009-02-12 8:46 ` Vladimir Prus
2009-02-12 20:37 ` Eli Zaretskii
1 sibling, 1 reply; 13+ messages in thread
From: Vladimir Prus @ 2009-02-12 8:46 UTC (permalink / raw)
To: tromey, Eli Zaretskii; +Cc: gdb-patches, Nick Roberts, Marc Khouzam
[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]
On Saturday 07 February 2009 00:11:27 Tom Tromey wrote:
> >>>>> "Vladimir" == Vladimir Prus <vladimir@codesourcery.com> writes:
>
> Vladimir> This patch fixes this by making MI observer print frame
> Vladimir> again, into MI uiout, if necessary. It passes all MI tests
> Vladimir> in (sync,async)x(native,gdbserver) combinations. How does
> Vladimir> this look, and are non-MI changes OK? If approved, I'll add
> Vladimir> a test that CLI commands result in proper *stopped.
>
> I don't understand all the implications of the core change, but I do
> like how it moves some MI logic out of the core and into MI.
>
> Vladimir> -@deftypefun void normal_stop (struct bpstats *@var{bs})
> Vladimir> +@deftypefun void normal_stop (struct bpstats *@var{bs}, int @var{print_frame})
> Vladimir> The inferior has stopped for real.
> Vladimir> @end deftypefun
>
> I would like to ask for documentation describing the meaning of the
> new argument.
The attached patch does so. Eli, can you see if the doc change is fine?
I've also added testcase, as promised. One detail I forgot to say in the
original email is that this patch does not make *stopped emitted for CLI
identical to *stopped for the corresponding MI commands. In particular,
output for CLI does not include 'reason' field sometimes, and for 'finish'
it will not include the return value. I think this is fine -- CLI commands
are meant to be issued by the user in the console, and therefore user will
see the CLI output from such commands. The only purpose of MI *stopped is
to let MI frontend know that the program is not stopped in a different place.
- Volodya
[-- Attachment #2: stopped.diff --]
[-- Type: text/x-diff, Size: 9491 bytes --]
commit 8fb6bdc851dc83207839e5d06c13925746c211f2
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Fri Feb 6 10:39:17 2009 +0300
Include frame information for *stopped due to CLI commands.
gdb/doc/
* observer.texi: Add parameter 'print_frame' to normal_stop
observer.
* ada-tasks.c (ada_normal_stop_observer): Adjust prototype.
* infcmd.c (finish_command_continuation): Pass '1' for
'print_frame' parameter to the observer.
* infrun.c (normal_stop): Don't print mi-specific information
here. Pass 'stop_print_frame' to the 'print_frame' parameter
of the observer.
* mi/mi-interp.c (mi_on_normal_stop): Adjust prototype.
If we need to print frame, and current uiout is not the MI one,
print frame again.
gdb/testsuite/
* lib/mi-support.exp (mi_expect_stop): Adjust the order of fields.
(mi_expect_interrupt): Likewise.
* gdb.mi/mi-cli.exp: Check that "step" results in proper *stopped
response.
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 5176d75..3639472 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -951,7 +951,7 @@ ada_task_list_changed (void)
/* The 'normal_stop' observer notification callback. */
static void
-ada_normal_stop_observer (struct bpstats *unused_args)
+ada_normal_stop_observer (struct bpstats *unused_args, int unused_args2)
{
/* The inferior has been resumed, and just stopped. This means that
our task_list needs to be recomputed before it can be used again. */
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index 636658a..7ca1f8a 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -88,8 +88,12 @@ Send a notification to all @var{event} observers.
The following observable events are defined:
-@deftypefun void normal_stop (struct bpstats *@var{bs})
-The inferior has stopped for real.
+@deftypefun void normal_stop (struct bpstats *@var{bs}, int @var{print_frame})
+The inferior has stopped for real. The @var{bs} parameter describes
+the breakpoints were are stopped at, if any. The @var{print_frame}
+parameter indicates whether the @value{GDBN} core suggests that the
+location where the inferiour has stopped is known and should be
+reported to the user.
@end deftypefun
@deftypefun void target_changed (struct target_ops *@var{target})
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3696f79..188077c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1374,7 +1374,7 @@ finish_command_continuation (void *arg)
next stop will be in the same thread that we started doing a
finish on. This suppressing (or some other replacement means)
should be a thread property. */
- observer_notify_normal_stop (bs);
+ observer_notify_normal_stop (bs, 1 /* print frame */);
suppress_stop_observer = 0;
delete_breakpoint (a->breakpoint);
}
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c11c71a..e50ef49 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4351,22 +4351,6 @@ Further execution is probably impossible.\n"));
internal_error (__FILE__, __LINE__, _("Unknown value."));
}
- if (ui_out_is_mi_like_p (uiout))
- {
-
- ui_out_field_int (uiout, "thread-id",
- pid_to_thread_id (inferior_ptid));
- if (non_stop)
- {
- struct cleanup *back_to = make_cleanup_ui_out_list_begin_end
- (uiout, "stopped-threads");
- ui_out_field_int (uiout, NULL,
- pid_to_thread_id (inferior_ptid));
- do_cleanups (back_to);
- }
- else
- ui_out_field_string (uiout, "stopped-threads", "all");
- }
/* The behavior of this routine with respect to the source
flag is:
SRC_LINE: Print only source line
@@ -4421,9 +4405,10 @@ done:
&& inferior_thread ()->step_multi))
{
if (!ptid_equal (inferior_ptid, null_ptid))
- observer_notify_normal_stop (inferior_thread ()->stop_bpstat);
+ observer_notify_normal_stop (inferior_thread ()->stop_bpstat,
+ stop_print_frame);
else
- observer_notify_normal_stop (NULL);
+ observer_notify_normal_stop (NULL, stop_print_frame);
}
if (target_has_execution)
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 8e2b230..7f52549 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -52,7 +52,7 @@ static void mi1_command_loop (void);
static void mi_insert_notify_hooks (void);
static void mi_remove_notify_hooks (void);
-static void mi_on_normal_stop (struct bpstats *bs);
+static void mi_on_normal_stop (struct bpstats *bs, int print_frame);
static void mi_new_thread (struct thread_info *t);
static void mi_thread_exit (struct thread_info *t);
@@ -322,17 +322,46 @@ mi_inferior_exit (int pid)
}
static void
-mi_on_normal_stop (struct bpstats *bs)
+mi_on_normal_stop (struct bpstats *bs, int print_frame)
{
/* Since this can be called when CLI command is executing,
using cli interpreter, be sure to use MI uiout for output,
not the current one. */
- struct ui_out *uiout = interp_ui_out (top_level_interpreter ());
+ struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
struct mi_interp *mi = top_level_interpreter_data ();
+ if (print_frame)
+ {
+ if (uiout != mi_uiout)
+ {
+ /* The normal_stop function has printed frame information into
+ CLI uiout, or some other non-MI uiout. There's no way we
+ can extra proper fields from random uiout object, so we print
+ the frame again. In practice, this can only happen when running
+ a CLI command in MI. */
+ struct ui_out *saved_uiout = uiout;
+ uiout = mi_uiout;
+ print_stack_frame (get_selected_frame (NULL), 0, SRC_AND_LOC);
+ uiout = saved_uiout;
+ }
+
+ ui_out_field_int (mi_uiout, "thread-id",
+ pid_to_thread_id (inferior_ptid));
+ if (non_stop)
+ {
+ struct cleanup *back_to = make_cleanup_ui_out_list_begin_end
+ (mi_uiout, "stopped-threads");
+ ui_out_field_int (mi_uiout, NULL,
+ pid_to_thread_id (inferior_ptid));
+ do_cleanups (back_to);
+ }
+ else
+ ui_out_field_string (mi_uiout, "stopped-threads", "all");
+ }
+
fputs_unfiltered ("*stopped", raw_stdout);
- mi_out_put (uiout, raw_stdout);
- mi_out_rewind (uiout);
+ mi_out_put (mi_uiout, raw_stdout);
+ mi_out_rewind (mi_uiout);
fputs_unfiltered ("\n", raw_stdout);
gdb_flush (raw_stdout);
}
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index b77a4d4..b469acb 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -140,6 +140,9 @@ mi_gdb_test "500-stack-select-frame 0" \
{500\^done} \
"-stack-select-frame 0"
+mi_execute_to "interpreter-exec console step" "" "callee4" "" ".*basics.c" "29" \
+ "" "check *stopped from CLI command"
+
# NOTE: cagney/2003-02-03: Not yet.
# mi_gdb_test "-break-insert -t basics.c:$line_main_hello" \
# {.*=breakpoint-create,number="3".*\^done} \
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index be9b530..f62c240 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1020,13 +1020,13 @@ proc mi_expect_stop { reason func args file line extra test } {
set any "\[^\n\]*"
- verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n($thread_selected_re)?$prompt_re"
+ 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\"\}$after_stopped,thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re)?$prompt_re"
gdb_expect {
- -re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n($thread_selected_re)?$prompt_re" {
+ -re "\\*stopped,${r}${a}${bn}frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped,thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re)?$prompt_re" {
pass "$test"
return $expect_out(2,string)
}
- -re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$any\",args=\[\\\[\{\]$any\[\\\]\}\],file=\"$any\",fullname=\"${fullname_syntax}$any\",line=\"\[0-9\]*\"\}$any\r\n$prompt_re" {
+ -re "\\*stopped,${r}${a}${bn}frame=\{addr=\"$hex\",func=\"$any\",args=\[\\\[\{\]$any\[\\\]\}\],file=\"$any\",fullname=\"${fullname_syntax}$any\",line=\"\[0-9\]*\"\}thread-id=\"$decimal\",stopped-threads=$any\r\n$prompt_re" {
verbose -log "got $expect_out(buffer)"
fail "$test (stopped at wrong place)"
return -1
@@ -1061,9 +1061,9 @@ proc mi_expect_interrupt { test } {
set any "\[^\n\]*"
# A signal can land anywhere, just ignore the location
- verbose -log "mi_expect_interrupt: expecting: \\*stopped,${r},thread-id=\"$decimal\",stopped-threads=$any\r\n$prompt_re"
+ verbose -log "mi_expect_interrupt: expecting: \\*stopped,${r}$any\r\n$prompt_re"
gdb_expect {
- -re "\\*stopped,${r},thread-id=\"$decimal\",stopped-threads=$any\r\n$prompt_re" {
+ -re "\\*stopped,${r}$any\r\n$prompt_re" {
pass "$test"
return 0;
}
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFA] fix *stopped for CLI commands
2009-02-12 8:46 ` Vladimir Prus
@ 2009-02-12 20:37 ` Eli Zaretskii
2009-02-14 20:43 ` Vladimir Prus
0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2009-02-12 20:37 UTC (permalink / raw)
To: Vladimir Prus; +Cc: tromey, gdb-patches, nickrob, marc.khouzam
> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Thu, 12 Feb 2009 11:45:59 +0300
> Cc: gdb-patches@sources.redhat.com,
> Nick Roberts <nickrob@snap.net.nz>,
> Marc Khouzam <marc.khouzam@ericsson.com>
>
> The attached patch does so. Eli, can you see if the doc change is fine?
Fine, but I have some comments:
> +@deftypefun void normal_stop (struct bpstats *@var{bs}, int @var{print_frame})
> +The inferior has stopped for real. The @var{bs} parameter describes
^^^^^^^^^
I think we should use "argument", not "parameter" here.
> The @var{print_frame}
> +parameter indicates whether the @value{GDBN} core suggests that the
> +location where the inferiour has stopped is known and should be
> +reported to the user.
That's a mouthful: too many passive tenses. How about this variant:
Second argument @var{print_frame} non-zero means display the
location where the inferior has stopped.
("inferior", not "inferiour" because we use the US English spelling)
> + /* The normal_stop function has printed frame information into
> + CLI uiout, or some other non-MI uiout. There's no way we
> + can extra proper fields from random uiout object, so we print
^^^^^
Did you mean "extract"?
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFA] fix *stopped for CLI commands
2009-02-12 20:37 ` Eli Zaretskii
@ 2009-02-14 20:43 ` Vladimir Prus
2009-02-15 18:22 ` Pedro Alves
0 siblings, 1 reply; 13+ messages in thread
From: Vladimir Prus @ 2009-02-14 20:43 UTC (permalink / raw)
To: Eli Zaretskii, gdb-patches
On Thursday 12 February 2009 23:31:47 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Thu, 12 Feb 2009 11:45:59 +0300
> > Cc: gdb-patches@sources.redhat.com,
> > Nick Roberts <nickrob@snap.net.nz>,
> > Marc Khouzam <marc.khouzam@ericsson.com>
> >
> > The attached patch does so. Eli, can you see if the doc change is fine?
>
> Fine, but I have some comments:
>
> > +@deftypefun void normal_stop (struct bpstats *@var{bs}, int @var{print_frame})
> > +The inferior has stopped for real. The @var{bs} parameter describes
> ^^^^^^^^^
> I think we should use "argument", not "parameter" here.
>
> > The @var{print_frame}
> > +parameter indicates whether the @value{GDBN} core suggests that the
> > +location where the inferiour has stopped is known and should be
> > +reported to the user.
>
> That's a mouthful: too many passive tenses. How about this variant:
>
> Second argument @var{print_frame} non-zero means display the
> location where the inferior has stopped.
>
> ("inferior", not "inferiour" because we use the US English spelling)
>
> > + /* The normal_stop function has printed frame information into
> > + CLI uiout, or some other non-MI uiout. There's no way we
> > + can extra proper fields from random uiout object, so we print
> ^^^^^
> Did you mean "extract"?
Thanks,
I have checked in this patch with the corrections you've suggested.
- Volodya
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFA] fix *stopped for CLI commands
2009-02-14 20:43 ` Vladimir Prus
@ 2009-02-15 18:22 ` Pedro Alves
2009-02-17 19:25 ` Vladimir Prus
0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2009-02-15 18:22 UTC (permalink / raw)
To: gdb-patches; +Cc: Vladimir Prus
Oops, I'm seeing this:
Running ../../../src/gdb/testsuite/gdb.gdb/observer.exp ...
FAIL: gdb.gdb/observer.exp: second observer attached; check second observer counter value
FAIL: gdb.gdb/observer.exp: 1st observer added; check first observer counter value
FAIL: gdb.gdb/observer.exp: 2nd observer added; check first observer counter value
FAIL: gdb.gdb/observer.exp: 2nd observer added; check second observer counter value
FAIL: gdb.gdb/observer.exp: 3rd observer added; check first observer counter value
FAIL: gdb.gdb/observer.exp: 3rd observer added; check second observer counter value
FAIL: gdb.gdb/observer.exp: 3rd observer added; check third observer counter value
FAIL: gdb.gdb/observer.exp: 2nd observer removed; check first observer counter value
FAIL: gdb.gdb/observer.exp: 2nd observer removed; check third observer counter value
FAIL: gdb.gdb/observer.exp: 1st observer removed; check third observer counter value
FAIL: gdb.gdb/observer.exp: three observers added; check first observer counter value
FAIL: gdb.gdb/observer.exp: three observers added; check second observer counter value
FAIL: gdb.gdb/observer.exp: three observers added; check third observer counter value
FAIL: gdb.gdb/observer.exp: third observer removed; check first observer counter value
FAIL: gdb.gdb/observer.exp: third observer removed; check second observer counter value
FAIL: gdb.gdb/observer.exp: second observer removed; check first observer counter value
Due to:
(gdb) PASS: gdb.gdb/observer.exp: second observer attached; reset third observer counter
call observer_notify_normal_stop (0)
Too few arguments in function call.
(gdb) PASS: gdb.gdb/observer.exp: second observer attached; sending notification
(outch, that should have been a FAIL)
The test needs to be adjusted to the new extra argument this
notification takes.
Alternatively, we could use a safer and more future proof test specific
notification in doc/observer.texi, instead of reusing the normal_stop
notification. Say,
@deftypefun void test_notification (int @var{somearg})
For internal testing. Do not use. See testsuite/gdb.gdb/observer.exp.
@end deftypefun
--
Pedro Alves
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFA] fix *stopped for CLI commands
2009-02-15 18:22 ` Pedro Alves
@ 2009-02-17 19:25 ` Vladimir Prus
2009-02-17 19:29 ` Pedro Alves
0 siblings, 1 reply; 13+ messages in thread
From: Vladimir Prus @ 2009-02-17 19:25 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 738 bytes --]
On Sunday 15 February 2009 18:47:28 Pedro Alves wrote:
> Oops, I'm seeing this:
>
> Running ../../../src/gdb/testsuite/gdb.gdb/observer.exp ...
> FAIL: gdb.gdb/observer.exp: second observer attached; check second observer counter value
Apologies, I've shortcutted the testing by running on MI tests, since "obviously" that
was a MI-only change.
> Alternatively, we could use a safer and more future proof test specific
> notification in doc/observer.texi, instead of reusing the normal_stop
> notification. Say,
>
> @deftypefun void test_notification (int @var{somearg})
> For internal testing. Do not use. See testsuite/gdb.gdb/observer.exp.
> @end deftypefun
Here's a patch implementing this approach. OK?
- Volodya
[-- Attachment #2: test_observer.diff --]
[-- Type: text/x-diff, Size: 7705 bytes --]
commit 1160d59db83b2a172c980bb7fa5c9dc9961d1b18
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Tue Feb 17 22:19:34 2009 +0300
Unbreak observer tests.
gdb/
* observer.c (observer_test_first_notification_function)
(observer_test_second_notification_function)
(observer_test_third_notification_function): Adjust prototype.
gdb/doc/
* observer.texi (test_notification): New observer.
gdb/testsuite/
* gdb.gdb/observer.exp: Use test_notification observer, not
normal_stop, everywhere.
(test_normal_stop_notifications): Rename to...
(test_notifications): ...this.
(test_observer_normal_stop): Rename to...
(test_observer): ...this.
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index c8e8b96..04f5034 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -200,3 +200,8 @@ Either @value{GDBN} detached from the inferior, or the inferior
exited. The argument @var{pid} identifies the inferior.
@end deftypefun
+ @deftypefun void test_notification (int @var{somearg})
+This observer is used for internal testing. Do not use.
+See testsuite/gdb.gdb/observer.exp.
+ @end deftypefun
+
diff --git a/gdb/observer.c b/gdb/observer.c
index 8a059c1..106bfcd 100644
--- a/gdb/observer.c
+++ b/gdb/observer.c
@@ -181,19 +181,19 @@ int observer_test_second_observer = 0;
int observer_test_third_observer = 0;
void
-observer_test_first_notification_function (struct bpstats *bs)
+observer_test_first_notification_function (int arg)
{
observer_test_first_observer++;
}
void
-observer_test_second_notification_function (struct bpstats *bs)
+observer_test_second_notification_function (int arg)
{
observer_test_second_observer++;
}
void
-observer_test_third_notification_function (struct bpstats *bs)
+observer_test_third_notification_function (int arg)
{
observer_test_third_observer++;
}
diff --git a/gdb/testsuite/gdb.gdb/observer.exp b/gdb/testsuite/gdb.gdb/observer.exp
index afc6909..e57b91b 100644
--- a/gdb/testsuite/gdb.gdb/observer.exp
+++ b/gdb/testsuite/gdb.gdb/observer.exp
@@ -101,32 +101,32 @@ proc setup_test { executable } {
}
proc attach_first_observer { message } {
- gdb_test "set \$first_obs = observer_attach_normal_stop (&observer_test_first_notification_function)" \
+ gdb_test "set \$first_obs = observer_attach_test_notification (&observer_test_first_notification_function)" \
"" "$message; attach first observer"
}
proc attach_second_observer { message } {
- gdb_test "set \$second_obs = observer_attach_normal_stop (&observer_test_second_notification_function)" \
+ gdb_test "set \$second_obs = observer_attach_test_notification (&observer_test_second_notification_function)" \
"" "$message; attach second observer"
}
proc attach_third_observer { message } {
- gdb_test "set \$third_obs = observer_attach_normal_stop (&observer_test_third_notification_function)" \
+ gdb_test "set \$third_obs = observer_attach_test_notification (&observer_test_third_notification_function)" \
"" "$message; attach third observer"
}
proc detach_first_observer { message } {
- gdb_test "call observer_detach_normal_stop (\$first_obs)" \
+ gdb_test "call observer_detach_test_notification (\$first_obs)" \
"" "$message; detach first observer"
}
proc detach_second_observer { message } {
- gdb_test "call observer_detach_normal_stop (\$second_obs)" \
+ gdb_test "call observer_detach_test_notification (\$second_obs)" \
"" "$message; detach second observer"
}
proc detach_third_observer { message } {
- gdb_test "call observer_detach_normal_stop (\$third_obs)" \
+ gdb_test "call observer_detach_test_notification (\$third_obs)" \
"" "$message; detach third observer"
}
@@ -151,21 +151,21 @@ proc reset_counters { message } {
"$message; reset third observer counter"
}
-proc test_normal_stop_notifications { first second third message args } {
+proc test_notifications { first second third message args } {
# Do any initialization
for {set i 0} {$i < [llength $args]} {incr i} {
[lindex $args $i] $message
}
reset_counters $message
- # Call observer_notify_normal_stop. Note that this procedure
+ # Call observer_notify_test_notification. Note that this procedure
# takes one argument, but this argument is ignored by the observer
# callbacks we have installed. So we just pass an arbitrary value.
- gdb_test "call observer_notify_normal_stop (0)" "" \
+ gdb_test "call observer_notify_test_notification (0)" "" \
"$message; sending notification"
check_counters $first $second $third $message
}
-proc test_observer_normal_stop { executable } {
+proc test_observer { executable } {
set setup_result [setup_test $executable]
if {$setup_result <0} then {
@@ -173,56 +173,56 @@ proc test_observer_normal_stop { executable } {
}
# First, try sending a notification without any observer attached.
- test_normal_stop_notifications 0 0 0 "no observer attached"
+ test_notifications 0 0 0 "no observer attached"
# Now, attach one observer, and send a notification.
- test_normal_stop_notifications 0 1 0 "second observer attached" \
+ test_notifications 0 1 0 "second observer attached" \
attach_second_observer
# Remove the observer, and send a notification.
- test_normal_stop_notifications 0 0 0 "second observer detached" \
+ test_notifications 0 0 0 "second observer detached" \
detach_second_observer
# With a new observer.
- test_normal_stop_notifications 1 0 0 "1st observer added" \
+ test_notifications 1 0 0 "1st observer added" \
attach_first_observer
# With 2 observers.
- test_normal_stop_notifications 1 1 0 "2nd observer added" \
+ test_notifications 1 1 0 "2nd observer added" \
attach_second_observer
# With 3 observers.
- test_normal_stop_notifications 1 1 1 "3rd observer added" \
+ test_notifications 1 1 1 "3rd observer added" \
attach_third_observer
# Remove middle observer.
- test_normal_stop_notifications 1 0 1 "2nd observer removed" \
+ test_notifications 1 0 1 "2nd observer removed" \
detach_second_observer
# Remove first observer.
- test_normal_stop_notifications 0 0 1 "1st observer removed" \
+ test_notifications 0 0 1 "1st observer removed" \
detach_first_observer
# Remove last observer.
- test_normal_stop_notifications 0 0 0 "3rd observer removed" \
+ test_notifications 0 0 0 "3rd observer removed" \
detach_third_observer
# Go back to 3 observers, and remove them in a different order...
- test_normal_stop_notifications 1 1 1 "three observers added" \
+ test_notifications 1 1 1 "three observers added" \
attach_first_observer \
attach_second_observer \
attach_third_observer
# Remove the third observer.
- test_normal_stop_notifications 1 1 0 "third observer removed" \
+ test_notifications 1 1 0 "third observer removed" \
detach_third_observer
# Remove the second observer.
- test_normal_stop_notifications 1 0 0 "second observer removed" \
+ test_notifications 1 0 0 "second observer removed" \
detach_second_observer
# Remove the first observer, no more observers.
- test_normal_stop_notifications 0 0 0 "first observer removed" \
+ test_notifications 0 0 0 "first observer removed" \
detach_first_observer
return 0
@@ -262,7 +262,7 @@ remote_file host delete x$tool
gdb_start
set file [remote_download host $GDB_FULLPATH x$tool]
-set result [test_observer_normal_stop $file];
+set result [test_observer $file];
gdb_exit;
catch "remote_file host delete $file";
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA] fix *stopped for CLI commands
2009-02-06 7:44 [RFA] fix *stopped for CLI commands Vladimir Prus
2009-02-06 21:11 ` Tom Tromey
@ 2009-02-09 20:36 ` Daniel Jacobowitz
2009-02-10 20:50 ` Marc Khouzam
2 siblings, 0 replies; 13+ messages in thread
From: Daniel Jacobowitz @ 2009-02-09 20:36 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches, Nick Roberts, Marc Khouzam
On Fri, Feb 06, 2009 at 10:45:18AM +0300, Vladimir Prus wrote:
> This patch fixes this by making MI observer print frame again, into MI uiout,
> if necessary. It passes all MI tests in (sync,async)x(native,gdbserver) combinations.
> How does this look, and are non-MI changes OK? If approved, I'll add a test that
> CLI commands result in proper *stopped.
Aside from the docs issue, this looks good to me.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RFA] fix *stopped for CLI commands
2009-02-06 7:44 [RFA] fix *stopped for CLI commands Vladimir Prus
2009-02-06 21:11 ` Tom Tromey
2009-02-09 20:36 ` Daniel Jacobowitz
@ 2009-02-10 20:50 ` Marc Khouzam
2009-02-10 20:58 ` Vladimir Prus
2 siblings, 1 reply; 13+ messages in thread
From: Marc Khouzam @ 2009-02-10 20:50 UTC (permalink / raw)
To: Vladimir Prus, gdb-patches, Nick Roberts
> Vladimir Prus
> Sent: Friday, February 06, 2009 2:45 AM
> To: gdb-patches@sources.redhat.com; Nick Roberts; Marc Khouzam
> Subject: [RFA] fix *stopped for CLI commands
>
> This patch fixes this by making MI observer print frame
> again, into MI uiout,
> if necessary.
I've applied the patch and tried it out. It works as expected
but it 'allowed' me to run into another problem.
When issuing a CLI command that triggers a ^running event,
the token id is not part of the ^running event anymore.
You can see this in the summarized session below.
26-exec-run
[...]
26^running <-- token is there for MI command
*running,thread-id="all"
(gdb)
[...]
(gdb)
27run
&"run\n"
[...]
^running <-- token is not there for CLI command
*running,thread-id="all"
(gdb)
I never noticed this before because DSF-GDB never uses CLI commands
that would trigger a ^running. So, to DSF-GDB it looks like the
CLI command never completed.
Thanks
Marc
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA] fix *stopped for CLI commands
2009-02-10 20:50 ` Marc Khouzam
@ 2009-02-10 20:58 ` Vladimir Prus
2009-02-11 14:09 ` Marc Khouzam
0 siblings, 1 reply; 13+ messages in thread
From: Vladimir Prus @ 2009-02-10 20:58 UTC (permalink / raw)
To: Marc Khouzam; +Cc: gdb-patches, Nick Roberts
On Tuesday 10 February 2009 23:44:28 Marc Khouzam wrote:
> > Vladimir Prus
> > Sent: Friday, February 06, 2009 2:45 AM
> > To: gdb-patches@sources.redhat.com; Nick Roberts; Marc Khouzam
> > Subject: [RFA] fix *stopped for CLI commands
> >
> > This patch fixes this by making MI observer print frame
> > again, into MI uiout,
> > if necessary.
>
> I've applied the patch and tried it out. It works as expected
> but it 'allowed' me to run into another problem.
>
> When issuing a CLI command that triggers a ^running event,
> the token id is not part of the ^running event anymore.
> You can see this in the summarized session below.
>
> 26-exec-run
> [...]
> 26^running <-- token is there for MI command
> *running,thread-id="all"
> (gdb)
> [...]
> (gdb)
> 27run
> &"run\n"
> [...]
> ^running <-- token is not there for CLI command
> *running,thread-id="all"
> (gdb)
>
> I never noticed this before because DSF-GDB never uses CLI commands
> that would trigger a ^running. So, to DSF-GDB it looks like the
> CLI command never completed.
Why does DSF use tokens at all? I probably can fix this up, but I am
not sure the tokens are really necessary. Does anybody envision a case
where GDB would report command results out-of-order?
- Volodya
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RFA] fix *stopped for CLI commands
2009-02-10 20:58 ` Vladimir Prus
@ 2009-02-11 14:09 ` Marc Khouzam
0 siblings, 0 replies; 13+ messages in thread
From: Marc Khouzam @ 2009-02-11 14:09 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches, Nick Roberts
> From: Vladimir Prus
> Sent: Tuesday, February 10, 2009 3:58 PM
>
> Why does DSF use tokens at all? I probably can fix this up, but I am
> not sure the tokens are really necessary. Does anybody envision a case
> where GDB would report command results out-of-order?
We used tokens from the very start. CDI uses them too, I believe.
I'm not sure if tokens are really nessary, but it is probably a little
risky to suddenly move away from them.
Marc
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2009-02-17 19:25 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-06 7:44 [RFA] fix *stopped for CLI commands Vladimir Prus
2009-02-06 21:11 ` Tom Tromey
2009-02-07 1:04 ` Joel Brobecker
2009-02-12 8:46 ` Vladimir Prus
2009-02-12 20:37 ` Eli Zaretskii
2009-02-14 20:43 ` Vladimir Prus
2009-02-15 18:22 ` Pedro Alves
2009-02-17 19:25 ` Vladimir Prus
2009-02-17 19:29 ` Pedro Alves
2009-02-09 20:36 ` Daniel Jacobowitz
2009-02-10 20:50 ` Marc Khouzam
2009-02-10 20:58 ` Vladimir Prus
2009-02-11 14:09 ` Marc Khouzam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox