Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] fix Bug 15180 Agent style dprintf does not respect conditions
@ 2013-04-23 20:00 Hui Zhu
  2013-04-23 20:38 ` Yao Qi
  0 siblings, 1 reply; 2+ messages in thread
From: Hui Zhu @ 2013-04-23 20:00 UTC (permalink / raw)
  To: gdb-patches ml

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

Hi,

This is because in linux_wait_1, it didn't add any check just call
run_breakpoint_commands (event_child->stop_pc);

So I add a check call run_breakpoint_commands only when condition is true.

Because I don't know howto handle the output in gdbserver.  So I
didn't add test for it.

Thanks,
Hui

2013-04-23  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180

	* linux-low.c (linux_wait_1): Add a check before call
	run_breakpoint_commands.

[-- Attachment #2: dprintf-gdbserver-cond.txt --]
[-- Type: text/plain, Size: 1369 bytes --]

--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2313,6 +2313,7 @@ linux_wait_1 (ptid_t ptid,
   int maybe_internal_trap;
   int report_to_gdb;
   int trace_event;
+  int condition_true;
 
   /* Translate generic target options into linux options.  */
   options = __WALL;
@@ -2615,6 +2616,9 @@ Check if we're already there.\n",
       goto retry;
     }
 
+  condition_true = gdb_breakpoint_here (event_child->stop_pc)
+		   && gdb_condition_true_at_breakpoint (event_child->stop_pc);
+
   /* If GDB wanted this thread to single step, we always want to
      report the SIGTRAP, and let GDB handle it.  Watchpoints should
      always be reported.  So should signals we can't explain.  A
@@ -2628,11 +2632,11 @@ Check if we're already there.\n",
 		   || event_child->stopped_by_watchpoint
 		   || (!step_over_finished
 		       && !bp_explains_trap && !trace_event)
-		   || (gdb_breakpoint_here (event_child->stop_pc)
-		       && gdb_condition_true_at_breakpoint (event_child->stop_pc)
+		   || (condition_true
 		       && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
 
-  run_breakpoint_commands (event_child->stop_pc);
+  if (condition_true)
+    run_breakpoint_commands (event_child->stop_pc);
 
   /* We found no reason GDB would want us to stop.  We either hit one
      of our own breakpoints, or finished an internal step GDB

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

* Re: [PATCH] fix Bug 15180 Agent style dprintf does not respect conditions
  2013-04-23 20:00 [PATCH] fix Bug 15180 Agent style dprintf does not respect conditions Hui Zhu
@ 2013-04-23 20:38 ` Yao Qi
  0 siblings, 0 replies; 2+ messages in thread
From: Yao Qi @ 2013-04-23 20:38 UTC (permalink / raw)
  To: Hui Zhu; +Cc: gdb-patches ml

On 04/23/2013 03:14 PM, Hui Zhu wrote:
> +  condition_true = gdb_breakpoint_here (event_child->stop_pc)
> +		   && gdb_condition_true_at_breakpoint (event_child->stop_pc);
> +
>     /* If GDB wanted this thread to single step, we always want to
>        report the SIGTRAP, and let GDB handle it.  Watchpoints should
>        always be reported.  So should signals we can't explain.  A
> @@ -2628,11 +2632,11 @@ Check if we're already there.\n",
>   		   || event_child->stopped_by_watchpoint
>   		   || (!step_over_finished
>   		       && !bp_explains_trap && !trace_event)
> -		   || (gdb_breakpoint_here (event_child->stop_pc)
> -		       && gdb_condition_true_at_breakpoint (event_child->stop_pc)
> +		   || (condition_true
>   		       && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
>
> -  run_breakpoint_commands (event_child->stop_pc);
> +  if (condition_true)
> +    run_breakpoint_commands (event_child->stop_pc);

Hui,
This patch works when there is only one dprintf set on a certain 
address.  However, supposing we set two dprintf breakpoints on the same 
address, one condition is true and the other is false, commands of both 
breakpoint will be executed, which is wrong.  See the session below:

$ ./gdb ./testsuite/gdb.base/dprintf
(gdb) target  remote :1234
(gdb) set breakpoint condition-evaluation target
// ensure that condition is evaluated in the target side.
(gdb) set dprintf-style agent
(gdb) dprintf foo , "true\n"
(gdb) condition 1 main > 1
(gdb) dprintf foo, "false\n"
(gdb) condition 2 main == 0
(gdb) info breakpoints
Num     Type           Disp Enb Address    What
1       dprintf        keep y   0x080484aa in foo at 
../../../../git/gdb/testsuite/gdb.base/dprintf.c:25
         stop only if main > 1 (host evals)
                                ^^^^^^^^^^
// it should be "(target evals)", caused by a separated bug.
         agent-printf "true\n"
2       dprintf        keep y   0x080484aa in foo at 
../../../../git/gdb/testsuite/gdb.base/dprintf.c:25
         stop only if main == 0 (host evals)
         agent-printf "false\n"
(gdb) c
Continuing.
[Inferior 1 (process 4135) exited with code 0152]

In the other console, we can see the output:

kickoff 1234
also to stderr 1234
false
true
false
false
true
false
false
true
false

Child exited with status 106

AFAICS, it is not a bug specific to dprintf, all target-side commands 
are affected by this bug, because the command is not well grouped or 
associated with condition in target side.

-- 
Yao (齐尧)


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

end of thread, other threads:[~2013-04-23  9:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-23 20:00 [PATCH] fix Bug 15180 Agent style dprintf does not respect conditions Hui Zhu
2013-04-23 20:38 ` Yao Qi

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