Re: [PATCH 5/6] Test tracepoints are installed or not

[Pedro Alves <palves@redhat.com>]

On 12/04/2012 04:44 AM, Yao Qi wrote:
> Test the state of field 'installed'.  This patch should be applied on
> top of this one,
> 
>   [PATCH] Test on =breakpoint-created when reconnect
>   http://sourceware.org/ml/gdb-patches/2012-12/msg00019.html
> 
> gdb/testsuite:
> 
> 2012-12-03  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.trace/mi-tracepoint-changed.exp (test_pending_resolved): Check
> 	'installed' field in '=breakpoint-modified'.
> 	(test_reconnect): Check 'installed' field in
> 	'=breakpoint-modified' and '=breakpoint-created'.
> ---
>  gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp |   20 +++++++++++++++++---
>  1 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
> index a4cef0a..f20ea74 100644
> --- a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
> +++ b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
> @@ -130,10 +130,20 @@ proc test_reconnect { } { with_test_prefix "reconnect" {
>  	    fail "$test: 2"
>  	    exp_continue
>  	}
> -	-re ".*=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*${mi_gdb_prompt}" {
> +	-re ".*=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*installed=\"y\"" {
>  	    # Tracepoint on main is created because it is not defined
>  	    # before and it is from remote stub.
>  	    pass $test
> +	    exp_continue
> +	}
> +	-re ".*=breakpoint-modified,bkpt=\{number=\"2\".*,func=\"marker\".*installed=\"y\"" {
> +	    # Tracepoint on marker is defined.  After the sync, we
> +	    # know that the tracepoint is in remote stub.  Mark it
> +	    # 'installed'.
> +	    pass "tracepoint on marker is installed"
> +	    exp_continue
> +	}
> +	-re ".*${mi_gdb_prompt}" {
>  	}
>  	timeout {
>  	    fail $test
> @@ -187,8 +197,12 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
>  
>      set test "tracepoint on pendfunc2 resolved"
>      gdb_expect {
> -	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\"" {
> +	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"n\"" {
>  	    pass "$test"
> +	    exp_continue
> +	}
> +	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"y\"" {
> +	    pass "tracepoint on pendfunc2 installed"
>  	}

I don't understand this part.  Why do we get two notifications?  Different calls to the
breakpoint_modified observers?  This also relates to the "installed" field being present
or not for pending breakpoints issue I pointed out in a previous patch.  It'd be good to
see (and test perhaps) this field is output correctly in the case of a tracepoint with multiple
locations.

>  	-re ".*${mi_gdb_prompt}$" {
>  	    fail $test
> @@ -210,7 +224,7 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
>      mi_send_resuming_command "exec-continue" "continuing to exit"
>      set test "tracepoint on pendfunc2 becomes pending again"
>      gdb_expect {
> -	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*addr=\"<PENDING>\",.*times=\"0\"" {
> +	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*addr=\"<PENDING>\",.*times=\"0\".*installed=\"n\"" {
>  	    pass "$test"
>  	}

Also related.  It very much feels to me that installed or not is a location property, not a
breakpoint property.  So if the breakpoint is pending, it doesn't have any location at all,
and then it feels strange to me to include an "installed" attribute.  WDYT?

>  	-re ".*${mi_gdb_prompt}$" {
> 

-- 
Pedro Alves