Re: [PATCH v4] gdbserver/s390: Switch on tracepoint support.

["Marcin Kościelnicki" <koriakin@0x04.net>]

Ping.

On 15/03/16 19:41, Marcin Kościelnicki wrote:
> Also adds s390 support to gdb.trace testsuite.
>
> gdb/gdbserver/ChangeLog:
>
> 	* linux-s390-low.c (s390_supports_tracepoints): New function.
> 	(struct linux_target_ops): Fill supports_tracepoints hook.
>
> gdb/testsuite/ChangeLog:
>
> 	* gdb.trace/ftrace.exp: Set arg0exp for s390.
> 	* gdb.trace/mi-trace-frame-collected.exp: Expect 4 registers on s390.
> 	* gdb.trace/mi-trace-unavailable.exp: Set pcnum for s390, add gpr0num
> 	variable for GPR 0 instead of assuming it is register 0.
> 	* gdb.trace/trace-common.h: Add s390 fast tracepoint placeholder.
> 	* lib/trace-support.exp: Add s390 registers.
> ---
> This fixes a minor conflict with the powerpc regular tracepoint support
> pushed in the meantime (mi-trace-unavailable.exp now needs gpr0num in
> powerpc branch).
>
>   gdb/gdbserver/ChangeLog                              |  5 +++++
>   gdb/gdbserver/linux-s390-low.c                       | 10 +++++++++-
>   gdb/testsuite/ChangeLog                              |  9 +++++++++
>   gdb/testsuite/gdb.trace/ftrace.exp                   |  5 +++++
>   gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp | 15 ++++++++++++++-
>   gdb/testsuite/gdb.trace/mi-trace-unavailable.exp     | 19 ++++++++++++++-----
>   gdb/testsuite/gdb.trace/trace-common.h               |  8 ++++++++
>   gdb/testsuite/lib/trace-support.exp                  |  4 ++++
>   8 files changed, 68 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
> index dda87b0..29a2e4d 100644
> --- a/gdb/gdbserver/ChangeLog
> +++ b/gdb/gdbserver/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-03-15  Marcin Kościelnicki  <koriakin@0x04.net>
> +
> +	* linux-s390-low.c (s390_supports_tracepoints): New function.
> +	(struct linux_target_ops): Fill supports_tracepoints hook.
> +
>   2016-03-09  Marcin Kościelnicki  <koriakin@0x04.net>
>
>   	* linux-ppc-low.c (ppc_supports_tracepoints): New function.
> diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
> index 7304feb..2bcfa7a 100644
> --- a/gdb/gdbserver/linux-s390-low.c
> +++ b/gdb/gdbserver/linux-s390-low.c
> @@ -745,6 +745,14 @@ s390_regs_info (void)
>     return &regs_info;
>   }
>
> +/* The "supports_tracepoints" linux_target_ops method.  */
> +
> +static int
> +s390_supports_tracepoints (void)
> +{
> +  return 1;
> +}
> +
>   struct linux_target_ops the_low_target = {
>     s390_arch_setup,
>     s390_regs_info,
> @@ -771,7 +779,7 @@ struct linux_target_ops the_low_target = {
>     NULL, /* new_fork */
>     NULL, /* prepare_to_resume */
>     NULL, /* process_qsupported */
> -  NULL, /* supports_tracepoints */
> +  s390_supports_tracepoints,
>     NULL, /* get_thread_area */
>     NULL, /* install_fast_tracepoint_jump_pad */
>     NULL, /* emit_ops */
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index eec8604..9c0fb1b 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,12 @@
> +2016-03-15  Marcin Kościelnicki  <koriakin@0x04.net>
> +
> +	* gdb.trace/ftrace.exp: Set arg0exp for s390.
> +	* gdb.trace/mi-trace-frame-collected.exp: Expect 4 registers on s390.
> +	* gdb.trace/mi-trace-unavailable.exp: Set pcnum for s390, add gpr0num
> +	variable for GPR 0 instead of assuming it is register 0.
> +	* gdb.trace/trace-common.h: Add s390 fast tracepoint placeholder.
> +	* lib/trace-support.exp: Add s390 registers.
> +
>   2016-03-14  Simon Marchi  <simon.marchi@ericsson.com>
>
>   	* gdb.base/default.exp: Add $_as_string to the list of expected
> diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
> index 349f4e2..4736f0f 100644
> --- a/gdb/testsuite/gdb.trace/ftrace.exp
> +++ b/gdb/testsuite/gdb.trace/ftrace.exp
> @@ -244,6 +244,11 @@ if [is_amd64_regs_target] {
>       set arg0exp "\$x0"
>   } elseif [istarget "powerpc*-*-*"] {
>       set arg0exp "\$r3"
> +} elseif { [istarget "s390x-*-*"] } {
> +    set arg0exp "\$r2"
> +} elseif { [istarget "s390-*-*"] } {
> +    # On targets with 64-bit GPRs, avoid the high part.
> +    set arg0exp "(\$r2 & 0xffffffff)"
>   } else {
>       set arg0exp ""
>   }
> diff --git a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
> index 0daaa25..a69c329 100644
> --- a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
> +++ b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
> @@ -91,10 +91,23 @@ proc test_trace_frame_collected { data_source } {
>   	    ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
>   	    "-trace-find frame-number 0"
>
> +	set reg_pattern "\{number=\"$decimal\",value=\"$hex\"\}"
> +
>   	# Test MI command '-trace-frame-collected' dumps only
>   	# collected registers.
> +	if { [istarget "s390*-*-*"] } {
> +	    # Only PC is collected, but on s390 PC is a pseudo-register -
> +	    # collecting it also collects the underlying PSWA register.
> +	    if { "$data_source" != "tfile" } {
> +		set reg_pattern "$reg_pattern,$reg_pattern"
> +	    } else {
> +		# For tfile, PSWM and CC are also guessed.
> +		set reg_pattern "$reg_pattern,$reg_pattern,$reg_pattern,$reg_pattern"
> +	    }
> +	}
> +
>   	mi_gdb_test "-trace-frame-collected" \
> -	    "\\^done,explicit-variables=\\\[\{name=\"gdb_char_test\",value=\".*\"\}\\\],computed-expressions=\\\[\\\],registers=\\\[\{number=\"$decimal\",value=\"$hex\"\}\\\],tvars=\\\[\\\],memory=\\\[\{address=\"$hex\",length=\"1\"\}\\\]"\
> +	    "\\^done,explicit-variables=\\\[\{name=\"gdb_char_test\",value=\".*\"\}\\\],computed-expressions=\\\[\\\],registers=\\\[$reg_pattern\\\],tvars=\\\[\\\],memory=\\\[\{address=\"$hex\",length=\"1\"\}\\\]"\
>   	    "-trace-frame-collected (register)"
>
>   	mi_gdb_test "-trace-find frame-number 1" \
> diff --git a/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp b/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
> index 4228c97..a49cd17 100644
> --- a/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
> +++ b/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
> @@ -130,30 +130,39 @@ proc test_trace_unavailable { data_source } {
>   	    ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"1\",frame=\{.*" \
>   	    "-trace-find frame-number 1"
>
> -	set pcnum 0
> +	set pcnum -1
> +	set gpr0num -1
>   	if [is_amd64_regs_target] {
>   	    set pcnum 16
> +	    set gpr0num 0
>   	} elseif [is_x86_like_target] {
>   	    set pcnum 8
> +	    set gpr0num 0
>   	} elseif [is_aarch64_target] {
>   	    set pcnum 32
> +	    set gpr0num 0
>   	} elseif [istarget "powerpc*-*-*"] {
>   	    set pcnum 64
> +	    set gpr0num 0
> +	} elseif { [istarget "s390*-*-*"] } {
> +	    # Strictly speaking, this is PSWA, not PC.
> +	    set pcnum 1
> +	    set gpr0num 2
>   	} else {
>   	    # Other ports support tracepoint should define the number
>   	    # of its own pc register.
>   	}
>
> -	if { $pcnum != 0 } {
> +	if { $pcnum != -1 } {
>   	    global hex
>   	    # Test that register 0 and PC are displayed, and register
>   	    # 0 is unavailable.
> -	    mi_gdb_test "-data-list-register-values x 0 ${pcnum}" \
> -		".*\\^done,register-values=\\\[\{number=\"0\",value=\"<unavailable>\"\},\{number=\"${pcnum}\",value=\"${hex}\"\}\\\]" \
> +	    mi_gdb_test "-data-list-register-values x ${gpr0num} ${pcnum}" \
> +		".*\\^done,register-values=\\\[\{number=\"${gpr0num}\",value=\"<unavailable>\"\},\{number=\"${pcnum}\",value=\"${hex}\"\}\\\]" \
>   		"-data-list-register-values x"
>
>   	    # Test that only available register PC is displayed.
> -	    mi_gdb_test "-data-list-register-values --skip-unavailable x 0 ${pcnum}" \
> +	    mi_gdb_test "-data-list-register-values --skip-unavailable x ${gpr0num} ${pcnum}" \
>   		".*\\^done,register-values=\\\[\{number=\"${pcnum}\",value=\"${hex}\"\}\\\]" \
>   		"-data-list-register-values --skip-unavailable x"
>   	}
> diff --git a/gdb/testsuite/gdb.trace/trace-common.h b/gdb/testsuite/gdb.trace/trace-common.h
> index fa99fd9..60cf9e8 100644
> --- a/gdb/testsuite/gdb.trace/trace-common.h
> +++ b/gdb/testsuite/gdb.trace/trace-common.h
> @@ -48,6 +48,14 @@ x86_trace_dummy ()
>          "    nop\n" \
>          )
>
> +#elif (defined __s390__)
> +
> +#define FAST_TRACEPOINT_LABEL(name) \
> +  asm ("    .global " SYMBOL(name) "\n" \
> +       SYMBOL(name) ":\n" \
> +       "    mvc 0(8, %r15), 0(%r15)\n" \
> +       )
> +
>   #else
>
>   #error "unsupported architecture for trace tests"
> diff --git a/gdb/testsuite/lib/trace-support.exp b/gdb/testsuite/lib/trace-support.exp
> index 372a595..b307f3f 100644
> --- a/gdb/testsuite/lib/trace-support.exp
> +++ b/gdb/testsuite/lib/trace-support.exp
> @@ -40,6 +40,10 @@ if [is_amd64_regs_target] {
>       set fpreg "r31"
>       set spreg "r1"
>       set pcreg "pc"
> +} elseif { [istarget "s390*-*-*"] } {
> +    set fpreg "r11"
> +    set spreg "r15"
> +    set pcreg "pc"
>   } else {
>       set fpreg "fp"
>       set spreg "sp"
>