Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Add breakpoint_created observer to update tracepoint_count.
@ 2012-11-07  0:50 Yao Qi
  2012-11-07 18:13 ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2012-11-07  0:50 UTC (permalink / raw)
  To: gdb-patches

Hi,
I noticed a bug that 'tracepoint_count' is out of date if tracepoint
is created by MI command '-break-insert -a'.  This patch is to
install 'update_tracepoint_count' to observer 'breakpoint_created',
and 'update_tracepoint_count' can update 'tracepoint_count' if B
is a tracepoint.

Regression tested on x86_64-linux.  Is it OK?

gdb:

2012-11-06  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (set_tracepoint_count): Renamed to ...
	(update_tracepoint_count): ... it.  New.
	(trace_command): Don't call set_tracepoint_count.  Re-indent.
	(strace_command, ftrace_command):
	(create_tracepoint_from_upload): Likewise.
	(_initialize_breakpoint): Call observer_attach_breakpoint_created
	to install update_tracepoint_count to observer
	'breakpoint_created'.
---
 gdb/breakpoint.c |   85 +++++++++++++++++++++++++++--------------------------
 1 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 092d81e..0f505a9 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14980,12 +14980,17 @@ catch_syscall_completer (struct cmd_list_element *cmd,
 
 /* Tracepoint-specific operations.  */
 
-/* Set tracepoint count to NUM.  */
+/* Update 'tracepoint_count' if B is a tracepoint.  */
+
 static void
-set_tracepoint_count (int num)
+update_tracepoint_count (struct breakpoint *b)
 {
-  tracepoint_count = num;
-  set_internalvar_integer (lookup_internalvar ("tpnum"), num);
+  if (is_tracepoint (b))
+    {
+      tracepoint_count = breakpoint_count;
+      set_internalvar_integer (lookup_internalvar ("tpnum"),
+			       breakpoint_count);
+    }
 }
 
 static void
@@ -14999,35 +15004,33 @@ trace_command (char *arg, int from_tty)
   else
     ops = &tracepoint_breakpoint_ops;
 
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 static void
 ftrace_command (char *arg, int from_tty)
 {
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_fast_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 &tracepoint_breakpoint_ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_fast_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     &tracepoint_breakpoint_ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 /* strace command implementation.  Creates a static tracepoint.  */
@@ -15044,18 +15047,17 @@ strace_command (char *arg, int from_tty)
   else
     ops = &tracepoint_breakpoint_ops;
 
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_static_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_static_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 /* Set up a fake reader function that gets command lines from a linked
@@ -15124,8 +15126,6 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 			  CREATE_BREAKPOINT_FLAGS_INSERTED))
     return NULL;
 
-  set_tracepoint_count (breakpoint_count);
-  
   /* Get the tracepoint we just created.  */
   tp = get_tracepoint (tracepoint_count);
   gdb_assert (tp != NULL);
@@ -15898,6 +15898,7 @@ _initialize_breakpoint (void)
   observer_attach_solib_unloaded (disable_breakpoints_in_unloaded_shlib);
   observer_attach_inferior_exit (clear_syscall_counts);
   observer_attach_memory_changed (invalidate_bp_value_on_memory_change);
+  observer_attach_breakpoint_created (update_tracepoint_count);
 
   breakpoint_objfile_key
     = register_objfile_data_with_cleanup (NULL, free_breakpoint_probes);
-- 
1.7.7.6


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

* Re: [PATCH] Add breakpoint_created observer to update tracepoint_count.
  2012-11-07  0:50 [PATCH] Add breakpoint_created observer to update tracepoint_count Yao Qi
@ 2012-11-07 18:13 ` Pedro Alves
  2012-11-09  1:09   ` Yao Qi
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-11-07 18:13 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 11/07/2012 12:49 AM, Yao Qi wrote:
> Hi,
> I noticed a bug that 'tracepoint_count' is out of date if tracepoint
> is created by MI command '-break-insert -a'.  This patch is to
> install 'update_tracepoint_count' to observer 'breakpoint_created',
> and 'update_tracepoint_count' can update 'tracepoint_count' if B
> is a tracepoint.
> 
> Regression tested on x86_64-linux.  Is it OK?
> 

No need to install an observer for a notification that is emitted in the same
module the new observer is in.  This is internals of the breakpoints module.
All set_breakpoint_count's calls are centralized in install_breakpoint, through
set_breakpoint_number.  All but break-range's, that is.  I don't recall why
that doesn't use install_breakpoint.  Maybe it should.

We should be able to put 'if (is_tracepoint) set_tracepoint_count()' in
install_breakpoint too.

-- 
Pedro Alves


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

* Re: [PATCH] Add breakpoint_created observer to update tracepoint_count.
  2012-11-07 18:13 ` Pedro Alves
@ 2012-11-09  1:09   ` Yao Qi
  2012-11-09  2:28     ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2012-11-09  1:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 11/08/2012 02:13 AM, Pedro Alves wrote:
> No need to install an observer for a notification that is emitted in the same
> module the new observer is in.  This is internals of the breakpoints module.
> All set_breakpoint_count's calls are centralized in install_breakpoint, through
> set_breakpoint_number.  All but break-range's, that is.  I don't recall why
> that doesn't use install_breakpoint.  Maybe it should.
> 

My original thought is to move tracepoint-related code from
breakpoint.c to tracepoint.c, so register a breakpoint_created observer
in tracepoint.c to update 'tracepoint_count'.  Unfortunately,
tracepoint commands use some breakpoint internal macros that prevent
moving them to tracepoint.c, so I move the observer register code back
to breakpoint.c.

Yes, it is not necessary to register a observer to update some state in
the same module.

> We should be able to put 'if (is_tracepoint) set_tracepoint_count()' in
> install_breakpoint too.

Hmmm, that sounds the right place to update 'tracepoint_count'.  How
about patch below?  A new test is added, without this fix, this test
fails.

-- 
Yao

gdb:

2012-11-09  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c: Declare set_tracepoint_count.
	(install_breakpoint): Call set_tracepoint_count if B is a
	tracepoint.
	(trace_command): Don't call set_tracepoint_count.  Re-indent.
	(strace_command, ftrace_command):
	(create_tracepoint_from_upload): Likewise.

gdb/testsuite:

2012-11-09  Yao Qi  <yao@codesourcery.com>

	* gdb.mi/mi-break.exp (test_abreak_creation): New.
---
 gdb/breakpoint.c                  |   75 ++++++++++++++++++-------------------
 gdb/testsuite/gdb.mi/mi-break.exp |   10 +++++
 2 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 092d81e..3763a04 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -261,6 +261,8 @@ static void disable_trace_command (char *, int);
 
 static void trace_pass_command (char *, int);
 
+static void set_tracepoint_count (int num);
+
 static int is_masked_watchpoint (const struct breakpoint *b);
 
 static struct bp_location **get_first_locp_gte_addr (CORE_ADDR address);
@@ -8319,6 +8321,8 @@ install_breakpoint (int internal, struct breakpoint *b, int update_gll)
 {
   add_to_breakpoint_chain (b);
   set_breakpoint_number (internal, b);
+  if (is_tracepoint (b))
+    set_tracepoint_count (breakpoint_count);
   if (!internal)
     mention (b);
   observer_notify_breakpoint_created (b);
@@ -14999,35 +15003,33 @@ trace_command (char *arg, int from_tty)
   else
     ops = &tracepoint_breakpoint_ops;
 
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 static void
 ftrace_command (char *arg, int from_tty)
 {
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_fast_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 &tracepoint_breakpoint_ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_fast_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     &tracepoint_breakpoint_ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 /* strace command implementation.  Creates a static tracepoint.  */
@@ -15044,18 +15046,17 @@ strace_command (char *arg, int from_tty)
   else
     ops = &tracepoint_breakpoint_ops;
 
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_static_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_static_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 /* Set up a fake reader function that gets command lines from a linked
@@ -15124,8 +15125,6 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 			  CREATE_BREAKPOINT_FLAGS_INSERTED))
     return NULL;
 
-  set_tracepoint_count (breakpoint_count);
-  
   /* Get the tracepoint we just created.  */
   tp = get_tracepoint (tracepoint_count);
   gdb_assert (tp != NULL);
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 573f484..e40f94d 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -150,6 +150,14 @@ proc test_rbreak_creation_and_listing {} {
 	    "delete temp breakpoints"
 }
 
+proc test_abreak_creation {} {
+    mi_gdb_test "522-break-insert -a main" \
+	"522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
+	"break-insert -a operation"
+
+    mi_gdb_test "p \$tpnum" ".* = 10.*" "print \$tpnum"
+}
+
 proc test_ignore_count {} {
     global mi_gdb_prompt
     global line_callme_body
@@ -256,5 +264,7 @@ test_disabled_creation
 
 test_breakpoint_commands
 
+test_abreak_creation
+
 mi_gdb_exit
 return 0
-- 
1.7.7.6


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

* Re: [PATCH] Add breakpoint_created observer to update tracepoint_count.
  2012-11-09  1:09   ` Yao Qi
@ 2012-11-09  2:28     ` Pedro Alves
  2012-11-09  7:26       ` Yao Qi
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-11-09  2:28 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 11/09/2012 01:09 AM, Yao Qi wrote:
> On 11/08/2012 02:13 AM, Pedro Alves wrote:
>> > No need to install an observer for a notification that is emitted in the same
>> > module the new observer is in.  This is internals of the breakpoints module.
>> > All set_breakpoint_count's calls are centralized in install_breakpoint, through
>> > set_breakpoint_number.  All but break-range's, that is.  I don't recall why
>> > that doesn't use install_breakpoint.  Maybe it should.
>> > 
> My original thought is to move tracepoint-related code from
> breakpoint.c to tracepoint.c, so register a breakpoint_created observer
> in tracepoint.c to update 'tracepoint_count'.  Unfortunately,
> tracepoint commands use some breakpoint internal macros that prevent
> moving them to tracepoint.c, so I move the observer register code back
> to breakpoint.c.

I see.

> 
> Yes, it is not necessary to register a observer to update some state in
> the same module.
> 
>> > We should be able to put 'if (is_tracepoint) set_tracepoint_count()' in
>> > install_breakpoint too.
> Hmmm, that sounds the right place to update 'tracepoint_count'.  How
> about patch below?  A new test is added, without this fix, this test
> fails.

Looks good.  Further comments below.

> 2012-11-09  Yao Qi  <yao@codesourcery.com>
>
> 	* gdb.mi/mi-break.exp (test_abreak_creation): New.

I suggest:

 	* gdb.mi/mi-break.exp (test_abreak_creation): New procedure.
	(top level): Call it.

And here ...

> +proc test_abreak_creation {} {
> +    mi_gdb_test "522-break-insert -a main" \
> +	"522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
> +	"break-insert -a operation"
> +
> +    mi_gdb_test "p \$tpnum" ".* = 10.*" "print \$tpnum"
> +}
> +

... I suggest checking tpnum before creating the tracepoint too

BTW, do we need "10.*", or is "10" good enough?  "10.*" matches 100
or other bogus numbers too.  And usually ".*" at the start of the
regex is not necessary.

So all in all (untested):

    mi_gdb_test "p \$tpnum" " = void" "\$tpnum before tracepoint"

    mi_gdb_test "522-break-insert -a main" \
	"522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
	"break-insert -a operation"

    mi_gdb_test "p \$tpnum" " = 10" "\$tpnum after tracepoint"


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

* Re: [PATCH] Add breakpoint_created observer to update tracepoint_count.
  2012-11-09  2:28     ` Pedro Alves
@ 2012-11-09  7:26       ` Yao Qi
  2012-11-09  9:54         ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2012-11-09  7:26 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 11/09/2012 10:28 AM, Pedro Alves wrote:
> I suggest:
> 
>   	* gdb.mi/mi-break.exp (test_abreak_creation): New procedure.
> 	(top level): Call it.
> 
> And here ...
> 
>> >+proc test_abreak_creation {} {
>> >+    mi_gdb_test "522-break-insert -a main" \
>> >+	"522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
>> >+	"break-insert -a operation"
>> >+
>> >+    mi_gdb_test "p \$tpnum" ".* = 10.*" "print \$tpnum"
>> >+}
>> >+
> ... I suggest checking tpnum before creating the tracepoint too
> 
> BTW, do we need "10.*", or is "10" good enough?  "10.*" matches 100
> or other bogus numbers too.  And usually ".*" at the start of the
> regex is not necessary.
> 
> So all in all (untested):
> 
>      mi_gdb_test "p \$tpnum" " = void" "\$tpnum before tracepoint"
> 
>      mi_gdb_test "522-break-insert -a main" \
> 	"522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
> 	"break-insert -a operation"
> 
>      mi_gdb_test "p \$tpnum" " = 10" "\$tpnum after tracepoint"

OK.  I adjust test here a little to use MI commands to get the value of
$tpnum (before and after creating tracepoint) to avoid using regexp.

Committed as below.

-- 
Yao

gdb:

2012-11-09  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c: Declare set_tracepoint_count.
	(install_breakpoint): Call set_tracepoint_count if B is a
	tracepoint.
	(trace_command): Don't call set_tracepoint_count.  Re-indent.
	(strace_command, ftrace_command):
	(create_tracepoint_from_upload): Likewise.

gdb/testsuite:

2012-11-09  Yao Qi  <yao@codesourcery.com>

	* gdb.mi/mi-break.exp (test_abreak_creation): New procedure.
	(top level): Call it.
---
 gdb/breakpoint.c                  |   75 ++++++++++++++++++-------------------
 gdb/testsuite/gdb.mi/mi-break.exp |   20 ++++++++++
 2 files changed, 57 insertions(+), 38 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 092d81e..3763a04 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -261,6 +261,8 @@ static void disable_trace_command (char *, int);
 
 static void trace_pass_command (char *, int);
 
+static void set_tracepoint_count (int num);
+
 static int is_masked_watchpoint (const struct breakpoint *b);
 
 static struct bp_location **get_first_locp_gte_addr (CORE_ADDR address);
@@ -8319,6 +8321,8 @@ install_breakpoint (int internal, struct breakpoint *b, int update_gll)
 {
   add_to_breakpoint_chain (b);
   set_breakpoint_number (internal, b);
+  if (is_tracepoint (b))
+    set_tracepoint_count (breakpoint_count);
   if (!internal)
     mention (b);
   observer_notify_breakpoint_created (b);
@@ -14999,35 +15003,33 @@ trace_command (char *arg, int from_tty)
   else
     ops = &tracepoint_breakpoint_ops;
 
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 static void
 ftrace_command (char *arg, int from_tty)
 {
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_fast_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 &tracepoint_breakpoint_ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_fast_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     &tracepoint_breakpoint_ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 /* strace command implementation.  Creates a static tracepoint.  */
@@ -15044,18 +15046,17 @@ strace_command (char *arg, int from_tty)
   else
     ops = &tracepoint_breakpoint_ops;
 
-  if (create_breakpoint (get_current_arch (),
-			 arg,
-			 NULL, 0, NULL, 1 /* parse arg */,
-			 0 /* tempflag */,
-			 bp_static_tracepoint /* type_wanted */,
-			 0 /* Ignore count */,
-			 pending_break_support,
-			 ops,
-			 from_tty,
-			 1 /* enabled */,
-			 0 /* internal */, 0))
-    set_tracepoint_count (breakpoint_count);
+  create_breakpoint (get_current_arch (),
+		     arg,
+		     NULL, 0, NULL, 1 /* parse arg */,
+		     0 /* tempflag */,
+		     bp_static_tracepoint /* type_wanted */,
+		     0 /* Ignore count */,
+		     pending_break_support,
+		     ops,
+		     from_tty,
+		     1 /* enabled */,
+		     0 /* internal */, 0);
 }
 
 /* Set up a fake reader function that gets command lines from a linked
@@ -15124,8 +15125,6 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 			  CREATE_BREAKPOINT_FLAGS_INSERTED))
     return NULL;
 
-  set_tracepoint_count (breakpoint_count);
-  
   /* Get the tracepoint we just created.  */
   tp = get_tracepoint (tracepoint_count);
   gdb_assert (tp != NULL);
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 573f484..999fef0 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -150,6 +150,24 @@ proc test_rbreak_creation_and_listing {} {
 	    "delete temp breakpoints"
 }
 
+proc test_abreak_creation {} {
+    mi_create_varobj tpnum \$tpnum "create local variable tpnum"
+    # Test that $tpnum is not set before creating a tracepoint.
+    mi_gdb_test "521-var-evaluate-expression tpnum" \
+	"521\\^done,value=\"void\"" "eval tpnum before tracepoint"
+
+    mi_gdb_test "522-break-insert -a main" \
+	"522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
+	"break-insert -a operation"
+
+    mi_gdb_test "523-var-update tpnum" \
+	"523\\^done,changelist=\\\[\{name=\"tpnum\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\\\]" \
+	"update tpnum"
+    # Test that $tpnum is updated after creating a tracepoint.
+    mi_gdb_test "524-var-evaluate-expression tpnum" \
+	"524\\^done,value=\"10\"" "eval tpnum after tracepoint"
+}
+
 proc test_ignore_count {} {
     global mi_gdb_prompt
     global line_callme_body
@@ -256,5 +274,7 @@ test_disabled_creation
 
 test_breakpoint_commands
 
+test_abreak_creation
+
 mi_gdb_exit
 return 0
-- 
1.7.7.6


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

* Re: [PATCH] Add breakpoint_created observer to update tracepoint_count.
  2012-11-09  7:26       ` Yao Qi
@ 2012-11-09  9:54         ` Pedro Alves
  0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2012-11-09  9:54 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 11/09/2012 07:26 AM, Yao Qi wrote:

> OK.  I adjust test here a little to use MI commands to get the value of
> $tpnum (before and after creating tracepoint) to avoid using regexp.
> 
> Committed as below.

Excellent, thanks.

> +    mi_create_varobj tpnum \$tpnum "create local variable tpnum"
> +    # Test that $tpnum is not set before creating a tracepoint.
> +    mi_gdb_test "521-var-evaluate-expression tpnum" \
> +	"521\\^done,value=\"void\"" "eval tpnum before tracepoint"
> +
> +    mi_gdb_test "522-break-insert -a main" \
> +	"522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
> +	"break-insert -a operation"
> +
> +    mi_gdb_test "523-var-update tpnum" \
> +	"523\\^done,changelist=\\\[\{name=\"tpnum\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\\\]" \
> +	"update tpnum"

Interesting that type_changed is false.  Looks like a bug.

-- 
Pedro Alves


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

end of thread, other threads:[~2012-11-09  9:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-07  0:50 [PATCH] Add breakpoint_created observer to update tracepoint_count Yao Qi
2012-11-07 18:13 ` Pedro Alves
2012-11-09  1:09   ` Yao Qi
2012-11-09  2:28     ` Pedro Alves
2012-11-09  7:26       ` Yao Qi
2012-11-09  9:54         ` Pedro Alves

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