[v4] gdbserver/s390: Switch on tracepoint support.
Commit Message
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(-)
Comments
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 ®s_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"
>
On Tue, Mar 15 2016, 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).
Please also document this new feature in the NEWS file.
[...]
> 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"
Without having looked into the details I wonder why we can't use the
default, fp, sp, and pc. Also, I'd slightly prefer initializing these
variables with their default values first and then adjusting them per
target as required. (This is just a matter of taste.)
Otherwise the patch looks good to me. But I think a global- or
testsuite-maintainer should approve the changes to the testsuite.
Thanks,
Andreas
On 23/03/16 16:25, Andreas Arnez wrote:
> On Tue, Mar 15 2016, 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).
>
> Please also document this new feature in the NEWS file.
OK, I'll submit another patch adding the NEWS entry (it should go after
the fast tracepoints).
>
> [...]
>
>> 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"
>
> Without having looked into the details I wonder why we can't use the
> default, fp, sp, and pc. Also, I'd slightly prefer initializing these
> variables with their default values first and then adjusting them per
> target as required. (This is just a matter of taste.)
These are user registers, which are not supported by the
target-independent tracepoint code:
'sp' is a user-register; GDB cannot yet trace user-register contents.
>
> Otherwise the patch looks good to me. But I think a global- or
> testsuite-maintainer should approve the changes to the testsuite.
>
> Thanks,
> Andreas
>
@@ -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.
@@ -745,6 +745,14 @@ s390_regs_info (void)
return ®s_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 */
@@ -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
@@ -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 ""
}
@@ -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" \
@@ -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"
}
@@ -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"
@@ -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"