Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] fix PR 15180 "May only run agent-printf on the target"
@ 2013-06-17 10:12 Hui Zhu
  2013-06-26 18:00 ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Hui Zhu @ 2013-06-17 10:12 UTC (permalink / raw)
  To: gdb-patches ml

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

Hi,

This patch is for PR 15180 too.  Because it depends on patch in
http://sourceware.org/ml/gdb-patches/2013-06/msg00139.html
So I post it in another thread.

It make dprintf_after_condition_true doesn't do actions if dprintf
style is agent.  Because the actions is already did in agent.

The test patch test the issue about 15180.

Thanks,
Hui

2013-06-17  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180
	* breakpoint.c (dprintf_after_condition_true): Check dprintf style
	before do actions.

2013-06-17  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180
	* gdb.base/dprintf-same-addr.c: New file.
	* gdb.base/dprintf-same-addr.exp: New file.

[-- Attachment #2: dprintf-same-addr.txt --]
[-- Type: text/plain, Size: 709 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13465,7 +13465,12 @@ dprintf_after_condition_true (struct bps
   bs->commands = NULL;
   old_chain = make_cleanup_decref_counted_command_line (&tmp_bs.commands);
 
-  bpstat_do_actions_1 (&tmp_bs_p);
+  /* If dprintf-style is set to agent, the actions of dprintf has been
+     executed in agent.  They should not be execued in GDB side.
+     So the actions should be executed only when dprintf-style is not
+     set to agent.  */
+  if (strcmp (dprintf_style, dprintf_style_agent) != 0)
+    bpstat_do_actions_1 (&tmp_bs_p);
 
   /* 'tmp_bs.commands' will usually be NULL by now, but
      bpstat_do_actions_1 may return early without processing the whole

[-- Attachment #3: dprintf-same-addr-test.txt --]
[-- Type: text/plain, Size: 3741 bytes --]

--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-same-addr.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  int i;
+
+  for (i=0; i < 99; i++)
+    {
+      printf("Hello\n");	/* test line  */
+
+      /* Without this line, test with fprintf will not work.  */
+      fprintf (stderr, "world\n");
+    }
+
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-same-addr.exp
@@ -0,0 +1,82 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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 "failed to prepare for dprintf with non-stop" \
+    ${testfile} ${srcfile} {debug}] {
+    return -1
+}
+
+set bp_location [gdb_get_line_number "test line"]
+
+proc continue_dprintf {args} {
+    with_test_prefix $args {
+	global bp_location
+
+	gdb_test "dprintf $bp_location,\"Hello\\n\"" "Dprintf .*"
+	gdb_test_no_output {set $bpnum1=$bpnum}
+	gdb_breakpoint $bp_location
+	gdb_test_no_output {set $bpnum2=$bpnum}
+
+	gdb_test "continue" "Breakpoint \[0-9\]+, main .*" "First continue"
+
+	gdb_test_no_output "condition \$bpnum1 i>10"
+	gdb_test_no_output "condition \$bpnum2 i>15"
+
+	gdb_test "continue" "Breakpoint \[0-9\]+, main .*" "Second continue"
+
+	gdb_test "p i" "\[0-9\]+ = 16" "check i"
+    }
+}
+
+if ![runto main] {
+    return -1
+}
+gdb_test_no_output "set dprintf-style gdb" "Set dprintf style to gdb"
+continue_dprintf "gdb"
+
+if ![target_info exists gdb,noinferiorio] {
+    if ![runto main] {
+	return -1
+    }
+    gdb_test_no_output "set dprintf-style call" "Set dprintf style to call"
+    continue_dprintf "call"
+
+    if ![runto main] {
+	return -1
+    }
+    gdb_test_no_output "set dprintf-function fprintf" "set dprintf-channel stderr"
+    gdb_test_no_output "set dprintf-channel stderr" "set dprintf channel"
+    continue_dprintf "fprintf"
+} else {
+    unsupported "test dprintf-style call"
+}
+
+if ![runto main] {
+    return -1
+}
+gdb_test "dprintf main,\"Hello\\n\"" "Dprintf .*"
+set msg "Set dprintf style to agent"
+gdb_test_multiple "set dprintf-style agent" $msg {
+    -re "warning: Target cannot run dprintf commands.*\r\n$gdb_prompt $" {
+	unsupported $msg
+    }
+    -re ".*$gdb_prompt $" {
+	pass $msg
+	continue_dprintf "agent"
+    }
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fix PR 15180 "May only run agent-printf on the target"
  2013-06-17 10:12 [PATCH] fix PR 15180 "May only run agent-printf on the target" Hui Zhu
@ 2013-06-26 18:00 ` Pedro Alves
  2013-06-30 17:15   ` Hui Zhu
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2013-06-26 18:00 UTC (permalink / raw)
  To: Hui Zhu; +Cc: gdb-patches ml

On 06/17/2013 08:47 AM, Hui Zhu wrote:
> Hi,
> 
> This patch is for PR 15180 too.  Because it depends on patch in
> http://sourceware.org/ml/gdb-patches/2013-06/msg00139.html
> So I post it in another thread.
> 
> It make dprintf_after_condition_true doesn't do actions if dprintf
> style is agent.  Because the actions is already did in agent.

If dprintf style is agent, then the target doesn't report events
for such hits.  If we happen to see a trap reported for the
same address as another breakpoint, the trap is surely not explained
by the dprintf.  IOW, we shouldn't have a bpstat a dprintf w/ style
agent at all.  What is need is to override the breakpoint_hit method
of dprintf_breakpoint_ops to always return false for agent style dprintfs.

Or maybe I'm missing something.  I have a hard time going from
the bug description in the PR to the patch.

> 
> The test patch test the issue about 15180.

Hmm, I get:

Running target native-gdbserver
Using ../../../src/gdb/testsuite/boards/../boards/native-gdbserver.exp as board description file for target.
Using ../../../src/gdb/testsuite/config/gdbserver.exp as tool-and-target-specific interface file.
Running ../../../src/gdb/testsuite/gdb.base/dprintf-same-addr.exp ...
FAIL: gdb.base/dprintf-same-addr.exp: agent: First continue (the program exited)
FAIL: gdb.base/dprintf-same-addr.exp: agent: Second continue (the program is no longer running)
FAIL: gdb.base/dprintf-same-addr.exp: agent: check i

                === gdb Summary ===

# of expected passes            16
# of unexpected failures        3
# of unsupported tests          1



> 
> Thanks,
> Hui
> 
> 2013-06-17  Hui Zhu  <hui@codesourcery.com>
> 
> 	PR gdb/15180
> 	* breakpoint.c (dprintf_after_condition_true): Check dprintf style
> 	before do actions.
> 
> 2013-06-17  Hui Zhu  <hui@codesourcery.com>
> 
> 	PR gdb/15180
> 	* gdb.base/dprintf-same-addr.c: New file.
> 	* gdb.base/dprintf-same-addr.exp: New file.
> 


-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fix PR 15180 "May only run agent-printf on the target"
  2013-06-26 18:00 ` Pedro Alves
@ 2013-06-30 17:15   ` Hui Zhu
  2013-07-01  8:10     ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Hui Zhu @ 2013-06-30 17:15 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches ml

[-- Attachment #1: Type: text/plain, Size: 2773 bytes --]

Hi Pedro,

Thanks for your review.

On Thu, Jun 27, 2013 at 1:58 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/17/2013 08:47 AM, Hui Zhu wrote:
>> Hi,
>>
>> This patch is for PR 15180 too.  Because it depends on patch in
>> http://sourceware.org/ml/gdb-patches/2013-06/msg00139.html
>> So I post it in another thread.
>>
>> It make dprintf_after_condition_true doesn't do actions if dprintf
>> style is agent.  Because the actions is already did in agent.
>
> If dprintf style is agent, then the target doesn't report events
> for such hits.  If we happen to see a trap reported for the
> same address as another breakpoint, the trap is surely not explained
> by the dprintf.  IOW, we shouldn't have a bpstat a dprintf w/ style
> agent at all.  What is need is to override the breakpoint_hit method
> of dprintf_breakpoint_ops to always return false for agent style dprintfs.
>
> Or maybe I'm missing something.  I have a hard time going from
> the bug description in the PR to the patch.

OK.  I add dprintf_check_status in the new patch according to your comments.

>
>>
>> The test patch test the issue about 15180.
>
> Hmm, I get:
>
> Running target native-gdbserver
> Using ../../../src/gdb/testsuite/boards/../boards/native-gdbserver.exp as board description file for target.
> Using ../../../src/gdb/testsuite/config/gdbserver.exp as tool-and-target-specific interface file.
> Running ../../../src/gdb/testsuite/gdb.base/dprintf-same-addr.exp ...
> FAIL: gdb.base/dprintf-same-addr.exp: agent: First continue (the program exited)
> FAIL: gdb.base/dprintf-same-addr.exp: agent: Second continue (the program is no longer running)
> FAIL: gdb.base/dprintf-same-addr.exp: agent: check i
>
>                 === gdb Summary ===
>
> # of expected passes            16
> # of unexpected failures        3
> # of unsupported tests          1
>

I am sorry that didn't talk this test very clear.  It depend on the
patches in http://sourceware.org/ml/gdb-patches/2013-06/msg00367.html
Could you patch those patches and try again?

Thanks,
Hui

2013-06-30  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180
	* breakpoint.c (dprintf_check_status): New function.
	(initialize_breakpoint_ops): Set dprintf_check_status.

2013-06-30  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180
	* gdb.base/dprintf-same-addr.c: New file.
	* gdb.base/dprintf-same-addr.exp: New file.

>
>
>>
>> Thanks,
>> Hui
>>
>> 2013-06-17  Hui Zhu  <hui@codesourcery.com>
>>
>>       PR gdb/15180
>>       * breakpoint.c (dprintf_after_condition_true): Check dprintf style
>>       before do actions.
>>
>> 2013-06-17  Hui Zhu  <hui@codesourcery.com>
>>
>>       PR gdb/15180
>>       * gdb.base/dprintf-same-addr.c: New file.
>>       * gdb.base/dprintf-same-addr.exp: New file.
>>
>
>
> --
> Pedro Alves

[-- Attachment #2: dprintf-same-addr.txt --]
[-- Type: text/plain, Size: 841 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13418,6 +13418,18 @@ dprintf_re_set (struct breakpoint *b)
     update_dprintf_command_list (b);
 }
 
+/* Implement the "check_status" breakpoint_ops method for dprintf.  */
+
+static void
+dprintf_check_status (struct bpstats *bs)
+{
+  if (strcmp (dprintf_style, dprintf_style_agent) == 0)
+    {
+      bs->stop = 0;
+      bs->print_it = print_it_noop;
+    }
+}
+
 /* Implement the "print_recreate" breakpoint_ops method for dprintf.  */
 
 static void
@@ -15958,6 +15970,7 @@ initialize_breakpoint_ops (void)
   ops = &dprintf_breakpoint_ops;
   *ops = bkpt_base_breakpoint_ops;
   ops->re_set = dprintf_re_set;
+  ops->check_status = dprintf_check_status;
   ops->resources_needed = bkpt_resources_needed;
   ops->print_it = bkpt_print_it;
   ops->print_mention = bkpt_print_mention;

[-- Attachment #3: dprintf-same-addr-test.txt --]
[-- Type: text/plain, Size: 3741 bytes --]

--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-same-addr.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  int i;
+
+  for (i=0; i < 99; i++)
+    {
+      printf("Hello\n");	/* test line  */
+
+      /* Without this line, test with fprintf will not work.  */
+      fprintf (stderr, "world\n");
+    }
+
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-same-addr.exp
@@ -0,0 +1,82 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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 "failed to prepare for dprintf with non-stop" \
+    ${testfile} ${srcfile} {debug}] {
+    return -1
+}
+
+set bp_location [gdb_get_line_number "test line"]
+
+proc continue_dprintf {args} {
+    with_test_prefix $args {
+	global bp_location
+
+	gdb_test "dprintf $bp_location,\"Hello\\n\"" "Dprintf .*"
+	gdb_test_no_output {set $bpnum1=$bpnum}
+	gdb_breakpoint $bp_location
+	gdb_test_no_output {set $bpnum2=$bpnum}
+
+	gdb_test "continue" "Breakpoint \[0-9\]+, main .*" "First continue"
+
+	gdb_test_no_output "condition \$bpnum1 i>10"
+	gdb_test_no_output "condition \$bpnum2 i>15"
+
+	gdb_test "continue" "Breakpoint \[0-9\]+, main .*" "Second continue"
+
+	gdb_test "p i" "\[0-9\]+ = 16" "check i"
+    }
+}
+
+if ![runto main] {
+    return -1
+}
+gdb_test_no_output "set dprintf-style gdb" "Set dprintf style to gdb"
+continue_dprintf "gdb"
+
+if ![target_info exists gdb,noinferiorio] {
+    if ![runto main] {
+	return -1
+    }
+    gdb_test_no_output "set dprintf-style call" "Set dprintf style to call"
+    continue_dprintf "call"
+
+    if ![runto main] {
+	return -1
+    }
+    gdb_test_no_output "set dprintf-function fprintf" "set dprintf-channel stderr"
+    gdb_test_no_output "set dprintf-channel stderr" "set dprintf channel"
+    continue_dprintf "fprintf"
+} else {
+    unsupported "test dprintf-style call"
+}
+
+if ![runto main] {
+    return -1
+}
+gdb_test "dprintf main,\"Hello\\n\"" "Dprintf .*"
+set msg "Set dprintf style to agent"
+gdb_test_multiple "set dprintf-style agent" $msg {
+    -re "warning: Target cannot run dprintf commands.*\r\n$gdb_prompt $" {
+	unsupported $msg
+    }
+    -re ".*$gdb_prompt $" {
+	pass $msg
+	continue_dprintf "agent"
+    }
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fix PR 15180 "May only run agent-printf on the target"
  2013-06-30 17:15   ` Hui Zhu
@ 2013-07-01  8:10     ` Pedro Alves
  2013-07-01  8:51       ` Hui Zhu
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2013-07-01  8:10 UTC (permalink / raw)
  To: Hui Zhu; +Cc: gdb-patches ml

On 06/30/2013 04:17 PM, Hui Zhu wrote:
>> > If dprintf style is agent, then the target doesn't report events
>> > for such hits.  If we happen to see a trap reported for the
>> > same address as another breakpoint, the trap is surely not explained
>> > by the dprintf.  IOW, we shouldn't have a bpstat a dprintf w/ style
>> > agent at all.  What is need is to override the breakpoint_hit method
>> > of dprintf_breakpoint_ops to always return false for agent style dprintfs.
>> >
>> > Or maybe I'm missing something.  I have a hard time going from
>> > the bug description in the PR to the patch.
> OK.  I add dprintf_check_status in the new patch according to your comments.
> 

But I suggested the breakpoint_ops->breakpoint_hit not breakpoint_ops->check_status.
An agent-style dprintf is just like a tracepoint.  It can't ever explain
a stop.  See tracepoint_breakpoint_hit.  If it didn't work, please explain why.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fix PR 15180 "May only run agent-printf on the target"
  2013-07-01  8:10     ` Pedro Alves
@ 2013-07-01  8:51       ` Hui Zhu
  2013-07-01 14:29         ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Hui Zhu @ 2013-07-01  8:51 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches ml

[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]

On Mon, Jul 1, 2013 at 4:10 PM, Pedro Alves <palves@redhat.com> wrote:
> On 06/30/2013 04:17 PM, Hui Zhu wrote:
>>> > If dprintf style is agent, then the target doesn't report events
>>> > for such hits.  If we happen to see a trap reported for the
>>> > same address as another breakpoint, the trap is surely not explained
>>> > by the dprintf.  IOW, we shouldn't have a bpstat a dprintf w/ style
>>> > agent at all.  What is need is to override the breakpoint_hit method
>>> > of dprintf_breakpoint_ops to always return false for agent style dprintfs.
>>> >
>>> > Or maybe I'm missing something.  I have a hard time going from
>>> > the bug description in the PR to the patch.
>> OK.  I add dprintf_check_status in the new patch according to your comments.
>>
>
> But I suggested the breakpoint_ops->breakpoint_hit not breakpoint_ops->check_status.
> An agent-style dprintf is just like a tracepoint.  It can't ever explain
> a stop.  See tracepoint_breakpoint_hit.  If it didn't work, please explain why.

OK.  Post a new version according to your comments.
Please help me review it.

Thanks,
Hui

>
> --
> Pedro Alves
>

2013-07-01  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180
	* breakpoint.c (dprintf_breakpoint_hit): New function.
	(initialize_breakpoint_ops): Set dprintf_breakpoint_hit.

2013-07-01  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180
	* gdb.base/dprintf-same-addr.c: New file.
	* gdb.base/dprintf-same-addr.exp: New file.

[-- Attachment #2: dprintf-same-addr.txt --]
[-- Type: text/plain, Size: 904 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13389,6 +13389,19 @@ tracepoint_probe_decode_linespec (struct
 
 static struct breakpoint_ops tracepoint_probe_breakpoint_ops;
 
+/* Implement the "breakpoint_hit" breakpoint_ops method for dprintf.  */
+
+static int
+dprintf_breakpoint_hit (const struct bp_location *bl,
+			struct address_space *aspace, CORE_ADDR bp_addr,
+			const struct target_waitstatus *ws)
+{
+  if (strcmp (dprintf_style, dprintf_style_agent) != 0)
+    return bkpt_breakpoint_hit (bl, aspace, bp_addr, ws);
+
+  return 0;
+}
+
 /* Dprintf breakpoint_ops methods.  */
 
 static void
@@ -15957,6 +15970,7 @@ initialize_breakpoint_ops (void)
 
   ops = &dprintf_breakpoint_ops;
   *ops = bkpt_base_breakpoint_ops;
+  ops->breakpoint_hit = dprintf_breakpoint_hit;
   ops->re_set = dprintf_re_set;
   ops->resources_needed = bkpt_resources_needed;
   ops->print_it = bkpt_print_it;

[-- Attachment #3: dprintf-same-addr-test.txt --]
[-- Type: text/plain, Size: 3741 bytes --]

--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-same-addr.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  int i;
+
+  for (i=0; i < 99; i++)
+    {
+      printf("Hello\n");	/* test line  */
+
+      /* Without this line, test with fprintf will not work.  */
+      fprintf (stderr, "world\n");
+    }
+
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-same-addr.exp
@@ -0,0 +1,82 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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 "failed to prepare for dprintf with non-stop" \
+    ${testfile} ${srcfile} {debug}] {
+    return -1
+}
+
+set bp_location [gdb_get_line_number "test line"]
+
+proc continue_dprintf {args} {
+    with_test_prefix $args {
+	global bp_location
+
+	gdb_test "dprintf $bp_location,\"Hello\\n\"" "Dprintf .*"
+	gdb_test_no_output {set $bpnum1=$bpnum}
+	gdb_breakpoint $bp_location
+	gdb_test_no_output {set $bpnum2=$bpnum}
+
+	gdb_test "continue" "Breakpoint \[0-9\]+, main .*" "First continue"
+
+	gdb_test_no_output "condition \$bpnum1 i>10"
+	gdb_test_no_output "condition \$bpnum2 i>15"
+
+	gdb_test "continue" "Breakpoint \[0-9\]+, main .*" "Second continue"
+
+	gdb_test "p i" "\[0-9\]+ = 16" "check i"
+    }
+}
+
+if ![runto main] {
+    return -1
+}
+gdb_test_no_output "set dprintf-style gdb" "Set dprintf style to gdb"
+continue_dprintf "gdb"
+
+if ![target_info exists gdb,noinferiorio] {
+    if ![runto main] {
+	return -1
+    }
+    gdb_test_no_output "set dprintf-style call" "Set dprintf style to call"
+    continue_dprintf "call"
+
+    if ![runto main] {
+	return -1
+    }
+    gdb_test_no_output "set dprintf-function fprintf" "set dprintf-channel stderr"
+    gdb_test_no_output "set dprintf-channel stderr" "set dprintf channel"
+    continue_dprintf "fprintf"
+} else {
+    unsupported "test dprintf-style call"
+}
+
+if ![runto main] {
+    return -1
+}
+gdb_test "dprintf main,\"Hello\\n\"" "Dprintf .*"
+set msg "Set dprintf style to agent"
+gdb_test_multiple "set dprintf-style agent" $msg {
+    -re "warning: Target cannot run dprintf commands.*\r\n$gdb_prompt $" {
+	unsupported $msg
+    }
+    -re ".*$gdb_prompt $" {
+	pass $msg
+	continue_dprintf "agent"
+    }
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fix PR 15180 "May only run agent-printf on the target"
  2013-07-01  8:51       ` Hui Zhu
@ 2013-07-01 14:29         ` Pedro Alves
  2013-07-01 16:08           ` Hui Zhu
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2013-07-01 14:29 UTC (permalink / raw)
  To: Hui Zhu; +Cc: gdb-patches ml

On 07/01/2013 09:51 AM, Hui Zhu wrote:
> On Mon, Jul 1, 2013 at 4:10 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 06/30/2013 04:17 PM, Hui Zhu wrote:
>>>>> If dprintf style is agent, then the target doesn't report events
>>>>> for such hits.  If we happen to see a trap reported for the
>>>>> same address as another breakpoint, the trap is surely not explained
>>>>> by the dprintf.  IOW, we shouldn't have a bpstat a dprintf w/ style
>>>>> agent at all.  What is need is to override the breakpoint_hit method
>>>>> of dprintf_breakpoint_ops to always return false for agent style dprintfs.
>>>>>
>>>>> Or maybe I'm missing something.  I have a hard time going from
>>>>> the bug description in the PR to the patch.
>>> OK.  I add dprintf_check_status in the new patch according to your comments.
>>>
>>
>> But I suggested the breakpoint_ops->breakpoint_hit not breakpoint_ops->check_status.
>> An agent-style dprintf is just like a tracepoint.  It can't ever explain
>> a stop.  See tracepoint_breakpoint_hit.  If it didn't work, please explain why.
> 
> OK.  Post a new version according to your comments.
> Please help me review it.

So I've now picked the patches from
<http://sourceware.org/ml/gdb-patches/2013-06/msg00367.html>

and then applied these new dprint-same-addr patches on top.

They completely break gdbserver.  :-/  First I tried:

 $ make check RUNTESTFLAGS="--target_board=native-gdbserver dprintf-same-addr.exp"
 ...
 Running target native-gdbserver
 Using ../../../src/gdb/testsuite/boards/../boards/native-gdbserver.exp as board description file for target.
 Using ../../../src/gdb/testsuite/config/gdbserver.exp as tool-and-target-specific interface file.
 Running ../../../src/gdb/testsuite/gdb.base/dprintf-same-addr.exp ...

                 === gdb Summary ===

 /home/pedro/gdb/mygit/build/gdb/testsuite/../../gdb/gdb version  7.6.50.20130701-cvs -nw -nx -data-directory /home/pedro/gdb/mygit/build/gdb/testsuite/../data-directory

 make[1]: Leaving directory `/home/pedro/gdb/mygit/build/gdb/testsuite'
 $

No FAILs/PASSes/UNSUPPORTED/etc., no nothing, is odd.  Turns out
that the program isn't even stopping at main.  That should have
resulted in a FAIL.  (Please fix that.)

Trying some other existing test shows:

 Running target native-gdbserver
 Using ../../../src/gdb/testsuite/boards/../boards/native-gdbserver.exp as board description file for target.
 Using ../../../src/gdb/testsuite/config/gdbserver.exp as tool-and-target-specific interface file.
 Running ../../../src/gdb/testsuite/gdb.base/break.exp ...
 FAIL: gdb.base/break.exp: run until function breakpoint
 FAIL: gdb.base/break.exp: run until breakpoint set at a line number (the program is no longer running)
 FAIL: gdb.base/break.exp: run until file:function(6) breakpoint (the program is no longer running)
 FAIL: gdb.base/break.exp: run until file:function(5) breakpoint (the program is no longer running)
 FAIL: gdb.base/break.exp: run until file:function(4) breakpoint (the program is no longer running)
 FAIL: gdb.base/break.exp: run until file:function(3) breakpoint (the program is no longer running)
 FAIL: gdb.base/break.exp: run until file:function(2) breakpoint (the program is no longer running)
 FAIL: gdb.base/break.exp: run until file:function(1) breakpoint (the program is no longer running)
 FAIL: gdb.base/break.exp: run until quoted breakpoint (the program is no longer running)

...

I didn't bother to run the whole testsuite.

At least with native testing I got:

$ make check RUNTESTFLAGS="dprintf-same-addr.exp"
...
# of expected passes            29
# of unsupported tests          1
...

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fix PR 15180 "May only run agent-printf on the target"
  2013-07-01 14:29         ` Pedro Alves
@ 2013-07-01 16:08           ` Hui Zhu
  0 siblings, 0 replies; 7+ messages in thread
From: Hui Zhu @ 2013-07-01 16:08 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches ml

On Mon, Jul 1, 2013 at 10:29 PM, Pedro Alves <palves@redhat.com> wrote:
> On 07/01/2013 09:51 AM, Hui Zhu wrote:
>> On Mon, Jul 1, 2013 at 4:10 PM, Pedro Alves <palves@redhat.com> wrote:
>>> On 06/30/2013 04:17 PM, Hui Zhu wrote:
>>>>>> If dprintf style is agent, then the target doesn't report events
>>>>>> for such hits.  If we happen to see a trap reported for the
>>>>>> same address as another breakpoint, the trap is surely not explained
>>>>>> by the dprintf.  IOW, we shouldn't have a bpstat a dprintf w/ style
>>>>>> agent at all.  What is need is to override the breakpoint_hit method
>>>>>> of dprintf_breakpoint_ops to always return false for agent style dprintfs.
>>>>>>
>>>>>> Or maybe I'm missing something.  I have a hard time going from
>>>>>> the bug description in the PR to the patch.
>>>> OK.  I add dprintf_check_status in the new patch according to your comments.
>>>>
>>>
>>> But I suggested the breakpoint_ops->breakpoint_hit not breakpoint_ops->check_status.
>>> An agent-style dprintf is just like a tracepoint.  It can't ever explain
>>> a stop.  See tracepoint_breakpoint_hit.  If it didn't work, please explain why.
>>
>> OK.  Post a new version according to your comments.
>> Please help me review it.
>
> So I've now picked the patches from
> <http://sourceware.org/ml/gdb-patches/2013-06/msg00367.html>

Oops, I so sorry that dprintf-remote-cond-server.txt in that url is
not the right version.  Could you help me try the new version in
http://sourceware.org/ml/gdb-patches/2013-07/msg00038.html

Thanks,
Hui

>
> and then applied these new dprint-same-addr patches on top.
>
> They completely break gdbserver.  :-/  First I tried:
>
>  $ make check RUNTESTFLAGS="--target_board=native-gdbserver dprintf-same-addr.exp"
>  ...
>  Running target native-gdbserver
>  Using ../../../src/gdb/testsuite/boards/../boards/native-gdbserver.exp as board description file for target.
>  Using ../../../src/gdb/testsuite/config/gdbserver.exp as tool-and-target-specific interface file.
>  Running ../../../src/gdb/testsuite/gdb.base/dprintf-same-addr.exp ...
>
>                  === gdb Summary ===
>
>  /home/pedro/gdb/mygit/build/gdb/testsuite/../../gdb/gdb version  7.6.50.20130701-cvs -nw -nx -data-directory /home/pedro/gdb/mygit/build/gdb/testsuite/../data-directory
>
>  make[1]: Leaving directory `/home/pedro/gdb/mygit/build/gdb/testsuite'
>  $
>
> No FAILs/PASSes/UNSUPPORTED/etc., no nothing, is odd.  Turns out
> that the program isn't even stopping at main.  That should have
> resulted in a FAIL.  (Please fix that.)
>
> Trying some other existing test shows:
>
>  Running target native-gdbserver
>  Using ../../../src/gdb/testsuite/boards/../boards/native-gdbserver.exp as board description file for target.
>  Using ../../../src/gdb/testsuite/config/gdbserver.exp as tool-and-target-specific interface file.
>  Running ../../../src/gdb/testsuite/gdb.base/break.exp ...
>  FAIL: gdb.base/break.exp: run until function breakpoint
>  FAIL: gdb.base/break.exp: run until breakpoint set at a line number (the program is no longer running)
>  FAIL: gdb.base/break.exp: run until file:function(6) breakpoint (the program is no longer running)
>  FAIL: gdb.base/break.exp: run until file:function(5) breakpoint (the program is no longer running)
>  FAIL: gdb.base/break.exp: run until file:function(4) breakpoint (the program is no longer running)
>  FAIL: gdb.base/break.exp: run until file:function(3) breakpoint (the program is no longer running)
>  FAIL: gdb.base/break.exp: run until file:function(2) breakpoint (the program is no longer running)
>  FAIL: gdb.base/break.exp: run until file:function(1) breakpoint (the program is no longer running)
>  FAIL: gdb.base/break.exp: run until quoted breakpoint (the program is no longer running)
>
> ...
>
> I didn't bother to run the whole testsuite.
>
> At least with native testing I got:
>
> $ make check RUNTESTFLAGS="dprintf-same-addr.exp"
> ...
> # of expected passes            29
> # of unsupported tests          1
> ...
>
> --
> Pedro Alves
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-07-01 16:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-17 10:12 [PATCH] fix PR 15180 "May only run agent-printf on the target" Hui Zhu
2013-06-26 18:00 ` Pedro Alves
2013-06-30 17:15   ` Hui Zhu
2013-07-01  8:10     ` Pedro Alves
2013-07-01  8:51       ` Hui Zhu
2013-07-01 14:29         ` Pedro Alves
2013-07-01 16:08           ` Hui Zhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox