* [PATCH 1/3] S390: Enable re-attaching with native-extended-gdbserver
2018-03-19 15:31 [PATCH 0/3] S390: Some gdbserver fixes Andreas Arnez
@ 2018-03-19 15:32 ` Andreas Arnez
2018-03-21 19:39 ` Simon Marchi
2018-03-19 15:33 ` [PATCH 2/3] S390: gdbserver: Don't write guarded storage registers Andreas Arnez
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Andreas Arnez @ 2018-03-19 15:32 UTC (permalink / raw)
To: gdb-patches
On s390x, when running attach.exp with native-extended-gdbserver,
gdbserver crashes in find_regno like this:
.../regcache.c:252: A problem internal to GDBserver has been detected.
Unknown register tdb0 requested
On the GDB side it looks like this:
(gdb) attach 31568
Attaching to process 31568
Remote connection closed
The test case attempts to attach to a new process via the already running
gdbserver. Thus s390_arch_setup is called a second time, and that's where
the problem occurs. In order to determine the word width (32 or 64 bits),
s390_arch_setup reads the pswm register through the regcache. For that it
uses a temporary tdesc which is supposed to work for all s390 targets,
since the actual tdesc has not been determined yet. But in this second
round this doesn't work, because s390_regsets has been updated already and
now contains regsets not described by the temporary tdesc, such as the one
containing tdb0.
This is fixed by rearranging the logic in s390_arch_setup.
gdb/gdbserver/ChangeLog:
* linux-s390-low.c (s390_get_hwcap): Replace tdesc parameter by
the word size. Add comment.
(s390_get_wordsize): New function.
(s390_arch_setup): No longer select a temporary tdesc to fetch the
pswm with it. Instead, use s390_get_wordsize to determine the
word size first and derive the correct tdesc from that directly.
---
gdb/gdbserver/linux-s390-low.c | 97 +++++++++++++++++++++++-------------------
1 file changed, 53 insertions(+), 44 deletions(-)
diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
index 451f42c7c9..203accdaaa 100644
--- a/gdb/gdbserver/linux-s390-low.c
+++ b/gdb/gdbserver/linux-s390-low.c
@@ -486,10 +486,12 @@ s390_set_pc (struct regcache *regcache, CORE_ADDR newpc)
}
}
+/* Get HWCAP from AUXV, using the given WORDSIZE. Return the HWCAP, or
+ zero if not found. */
+
static unsigned long
-s390_get_hwcap (const struct target_desc *tdesc)
+s390_get_hwcap (int wordsize)
{
- int wordsize = register_size (tdesc, 0);
gdb_byte *data = (gdb_byte *) alloca (2 * wordsize);
int offset = 0;
@@ -514,6 +516,27 @@ s390_get_hwcap (const struct target_desc *tdesc)
return 0;
}
+/* Determine the word size for the given PID, in bytes. */
+
+#ifdef __s390x__
+static int
+s390_get_wordsize (int pid)
+{
+ errno = 0;
+ PTRACE_XFER_TYPE pswm = ptrace (PTRACE_PEEKUSER, pid,
+ (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) 0);
+ if (errno != 0) {
+ warning (_("Couldn't determine word size, assuming 64-bit.\n"));
+ return 8;
+ }
+ /* Derive word size from extended addressing mode (PSW bit 31). */
+ return pswm & (1L << 32) ? 8 : 4;
+}
+#else
+#define s390_get_wordsize(pid) 4
+#endif
+
static int
s390_check_regset (int pid, int regset, int regsize)
{
@@ -540,49 +563,32 @@ s390_arch_setup (void)
const struct target_desc *tdesc;
struct regset_info *regset;
- /* Check whether the kernel supports extra register sets. */
+ /* Determine word size and HWCAP. */
int pid = pid_of (current_thread);
+ int wordsize = s390_get_wordsize (pid);
+ unsigned long hwcap = s390_get_hwcap (wordsize);
+
+ /* Check whether the kernel supports extra register sets. */
int have_regset_last_break
= s390_check_regset (pid, NT_S390_LAST_BREAK, 8);
int have_regset_system_call
= s390_check_regset (pid, NT_S390_SYSTEM_CALL, 4);
- int have_regset_tdb = s390_check_regset (pid, NT_S390_TDB, 256);
- int have_regset_vxrs = s390_check_regset (pid, NT_S390_VXRS_LOW, 128)
- && s390_check_regset (pid, NT_S390_VXRS_HIGH, 256);
- int have_regset_gs = s390_check_regset (pid, NT_S390_GS_CB, 32)
- && s390_check_regset (pid, NT_S390_GS_BC, 32);
-
- /* Assume 31-bit inferior process. */
- if (have_regset_system_call)
- tdesc = tdesc_s390_linux32v2;
- else if (have_regset_last_break)
- tdesc = tdesc_s390_linux32v1;
- else
- tdesc = tdesc_s390_linux32;
+ int have_regset_tdb
+ = (s390_check_regset (pid, NT_S390_TDB, 256)
+ && (hwcap & HWCAP_S390_TE) != 0);
+ int have_regset_vxrs
+ = (s390_check_regset (pid, NT_S390_VXRS_LOW, 128)
+ && s390_check_regset (pid, NT_S390_VXRS_HIGH, 256)
+ && (hwcap & HWCAP_S390_VX) != 0);
+ int have_regset_gs
+ = (s390_check_regset (pid, NT_S390_GS_CB, 32)
+ && s390_check_regset (pid, NT_S390_GS_BC, 32)
+ && (hwcap & HWCAP_S390_GS) != 0);
- /* On a 64-bit host, check the low bit of the (31-bit) PSWM
- -- if this is one, we actually have a 64-bit inferior. */
{
#ifdef __s390x__
- unsigned int pswm;
- struct regcache *regcache = new_register_cache (tdesc);
-
- fetch_inferior_registers (regcache, find_regno (tdesc, "pswm"));
- collect_register_by_name (regcache, "pswm", &pswm);
- free_register_cache (regcache);
-
- if (pswm & 1)
+ if (wordsize == 8)
{
- if (have_regset_tdb)
- have_regset_tdb =
- (s390_get_hwcap (tdesc_s390x_linux64v2) & HWCAP_S390_TE) != 0;
- if (have_regset_vxrs)
- have_regset_vxrs =
- (s390_get_hwcap (tdesc_s390x_linux64v2) & HWCAP_S390_VX) != 0;
- if (have_regset_gs)
- have_regset_gs =
- (s390_get_hwcap (tdesc_s390x_linux64v2) & HWCAP_S390_GS) != 0;
-
if (have_regset_gs)
tdesc = tdesc_s390x_gs_linux64;
else if (have_regset_vxrs)
@@ -602,16 +608,9 @@ s390_arch_setup (void)
using the full 64-bit GPRs. */
else
#endif
- if (s390_get_hwcap (tdesc) & HWCAP_S390_HIGH_GPRS)
+ if (hwcap & HWCAP_S390_HIGH_GPRS)
{
have_hwcap_s390_high_gprs = 1;
- if (have_regset_tdb)
- have_regset_tdb = (s390_get_hwcap (tdesc) & HWCAP_S390_TE) != 0;
- if (have_regset_vxrs)
- have_regset_vxrs = (s390_get_hwcap (tdesc) & HWCAP_S390_VX) != 0;
- if (have_regset_gs)
- have_regset_gs = (s390_get_hwcap (tdesc) & HWCAP_S390_GS) != 0;
-
if (have_regset_gs)
tdesc = tdesc_s390_gs_linux64;
else if (have_regset_vxrs)
@@ -626,6 +625,16 @@ s390_arch_setup (void)
else
tdesc = tdesc_s390_linux64;
}
+ else
+ {
+ /* Assume 31-bit inferior process. */
+ if (have_regset_system_call)
+ tdesc = tdesc_s390_linux32v2;
+ else if (have_regset_last_break)
+ tdesc = tdesc_s390_linux32v1;
+ else
+ tdesc = tdesc_s390_linux32;
+ }
have_hwcap_s390_vx = have_regset_vxrs;
}
--
2.14.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] S390: Enable re-attaching with native-extended-gdbserver
2018-03-19 15:32 ` [PATCH 1/3] S390: Enable re-attaching with native-extended-gdbserver Andreas Arnez
@ 2018-03-21 19:39 ` Simon Marchi
2018-03-22 9:07 ` Andreas Arnez
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2018-03-21 19:39 UTC (permalink / raw)
To: Andreas Arnez, gdb-patches
I am just looking through this to have an idea of what's happening
and found one nit:
> +/* Determine the word size for the given PID, in bytes. */
> +
> +#ifdef __s390x__
> +static int
> +s390_get_wordsize (int pid)
> +{
> + errno = 0;
> + PTRACE_XFER_TYPE pswm = ptrace (PTRACE_PEEKUSER, pid,
> + (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) 0);
> + if (errno != 0) {
> + warning (_("Couldn't determine word size, assuming 64-bit.\n"));
> + return 8;
> + }
GNU-style curly braces.
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] S390: Enable re-attaching with native-extended-gdbserver
2018-03-21 19:39 ` Simon Marchi
@ 2018-03-22 9:07 ` Andreas Arnez
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Arnez @ 2018-03-22 9:07 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Wed, Mar 21 2018, Simon Marchi wrote:
> I am just looking through this to have an idea of what's happening
> and found one nit:
>
>> +/* Determine the word size for the given PID, in bytes. */
>> +
>> +#ifdef __s390x__
>> +static int
>> +s390_get_wordsize (int pid)
>> +{
>> + errno = 0;
>> + PTRACE_XFER_TYPE pswm = ptrace (PTRACE_PEEKUSER, pid,
>> + (PTRACE_TYPE_ARG3) 0,
>> + (PTRACE_TYPE_ARG4) 0);
>> + if (errno != 0) {
>> + warning (_("Couldn't determine word size, assuming 64-bit.\n"));
>> + return 8;
>> + }
>
> GNU-style curly braces.
Oops...
Thanks, pushed a fix.
--
Andreas
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] S390: gdbserver: Don't write guarded storage registers
2018-03-19 15:31 [PATCH 0/3] S390: Some gdbserver fixes Andreas Arnez
2018-03-19 15:32 ` [PATCH 1/3] S390: Enable re-attaching with native-extended-gdbserver Andreas Arnez
@ 2018-03-19 15:33 ` Andreas Arnez
2018-03-19 15:33 ` [PATCH 3/3] S390: Make IPA recognize tdescs with guarded storage Andreas Arnez
2018-03-21 16:43 ` [PATCH 0/3] S390: Some gdbserver fixes Andreas Arnez
3 siblings, 0 replies; 7+ messages in thread
From: Andreas Arnez @ 2018-03-19 15:33 UTC (permalink / raw)
To: gdb-patches
On S390, the guarded storage register set is only valid if guarded storage
is active. Reading/writing the register set yields errors if this is not
the case. Then gdbserver emits warnings like these:
Warning: ptrace(regsets_store_inferior_registers): No data available
Apart from confusing the user, this can also lead to test case failures
due to unexpected output. To suppress this, make the guarded storage
regsets read-only for now.
gdb/gdbserver/ChangeLog:
* linux-s390-low.c (s390_fill_gs): Remove function.
(s390_fill_gsbc): Remove function.
(s390_regsets): Set fill functions for the guarded storage regsets
to NULL.
---
gdb/gdbserver/linux-s390-low.c | 29 +++++------------------------
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
index 203accdaaa..7de70f3a38 100644
--- a/gdb/gdbserver/linux-s390-low.c
+++ b/gdb/gdbserver/linux-s390-low.c
@@ -374,16 +374,6 @@ s390_store_vxrs_high (struct regcache *regcache, const void *buf)
supply_register (regcache, v16 + i, (const char *) buf + 16 * i);
}
-static void
-s390_fill_gs (struct regcache *regcache, void *buf)
-{
- int gsd = find_regno (regcache->tdesc, "gsd");
- int i;
-
- for (i = 0; i < 3; i++)
- collect_register (regcache, gsd + i, (char *) buf + 8 * (i + 1));
-}
-
static void
s390_store_gs (struct regcache *regcache, const void *buf)
{
@@ -394,16 +384,6 @@ s390_store_gs (struct regcache *regcache, const void *buf)
supply_register (regcache, gsd + i, (const char *) buf + 8 * (i + 1));
}
-static void
-s390_fill_gsbc (struct regcache *regcache, void *buf)
-{
- int bc_gsd = find_regno (regcache->tdesc, "bc_gsd");
- int i;
-
- for (i = 0; i < 3; i++)
- collect_register (regcache, bc_gsd + i, (char *) buf + 8 * (i + 1));
-}
-
static void
s390_store_gsbc (struct regcache *regcache, const void *buf)
{
@@ -432,10 +412,11 @@ static struct regset_info s390_regsets[] = {
EXTENDED_REGS, s390_fill_vxrs_low, s390_store_vxrs_low },
{ PTRACE_GETREGSET, PTRACE_SETREGSET, NT_S390_VXRS_HIGH, 0,
EXTENDED_REGS, s390_fill_vxrs_high, s390_store_vxrs_high },
- { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_S390_GS_CB, 0,
- EXTENDED_REGS, s390_fill_gs, s390_store_gs },
- { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_S390_GS_BC, 0,
- EXTENDED_REGS, s390_fill_gsbc, s390_store_gsbc },
+ /* Guarded storage registers are read-only. */
+ { PTRACE_GETREGSET, -1, NT_S390_GS_CB, 0, EXTENDED_REGS,
+ NULL, s390_store_gs },
+ { PTRACE_GETREGSET, -1, NT_S390_GS_BC, 0, EXTENDED_REGS,
+ NULL, s390_store_gsbc },
NULL_REGSET
};
--
2.14.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] S390: Make IPA recognize tdescs with guarded storage
2018-03-19 15:31 [PATCH 0/3] S390: Some gdbserver fixes Andreas Arnez
2018-03-19 15:32 ` [PATCH 1/3] S390: Enable re-attaching with native-extended-gdbserver Andreas Arnez
2018-03-19 15:33 ` [PATCH 2/3] S390: gdbserver: Don't write guarded storage registers Andreas Arnez
@ 2018-03-19 15:33 ` Andreas Arnez
2018-03-21 16:43 ` [PATCH 0/3] S390: Some gdbserver fixes Andreas Arnez
3 siblings, 0 replies; 7+ messages in thread
From: Andreas Arnez @ 2018-03-19 15:33 UTC (permalink / raw)
To: gdb-patches
The in-process agent does not handle tdescs with guarded storage yet.
This is fixed.
gdb/gdbserver/ChangeLog:
* linux-s390-ipa.c (get_ipa_tdesc): Add handling for
S390_TDESC_GS.
* linux-s390-low.c (s390_get_ipa_tdesc_idx): Likewise.
(initialize_low_tracepoint): Call init_registers_s390x_gs_linux64
and init_registers_s390_gs_linux64.
---
gdb/gdbserver/linux-s390-ipa.c | 13 +++++++++++--
gdb/gdbserver/linux-s390-low.c | 4 ++++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/gdb/gdbserver/linux-s390-ipa.c b/gdb/gdbserver/linux-s390-ipa.c
index 707d3384f0..bf9addb17a 100644
--- a/gdb/gdbserver/linux-s390-ipa.c
+++ b/gdb/gdbserver/linux-s390-ipa.c
@@ -119,7 +119,7 @@ static const int s390_linux64_ft_collect_regmap[] = {
FT_VR (28), FT_VR (29), FT_VR (30), FT_VR (31),
};
-/* Used for s390-te-linux64, s390-tevx-linux64. */
+/* Used for s390-te-linux64, s390-tevx-linux64, and s390-gs-linux64. */
static const int s390_te_linux64_ft_collect_regmap[] = {
/* 32-bit PSWA and PSWM. */
@@ -207,7 +207,8 @@ static const int s390x_ft_collect_regmap[] = {
FT_VR (28), FT_VR (29), FT_VR (30), FT_VR (31),
};
-/* Used for s390x-te-linux64, s390x-tevx-linux64. */
+/* Used for s390x-te-linux64, s390x-tevx-linux64, and
+ s390x-gs-linux64. */
static const int s390x_te_ft_collect_regmap[] = {
/* 64-bit PSWA and PSWM. */
@@ -335,6 +336,9 @@ get_ipa_tdesc (int idx)
case S390_TDESC_TEVX:
SET_REGMAP(s390x_te_ft_collect_regmap, 0);
return tdesc_s390x_tevx_linux64;
+ case S390_TDESC_GS:
+ SET_REGMAP(s390x_te_ft_collect_regmap, 0);
+ return tdesc_s390x_gs_linux64;
#else
case S390_TDESC_32:
SET_REGMAP(s390_linux32_ft_collect_regmap, 0);
@@ -363,6 +367,9 @@ get_ipa_tdesc (int idx)
case S390_TDESC_TEVX:
SET_REGMAP(s390_te_linux64_ft_collect_regmap, 0);
return tdesc_s390_tevx_linux64;
+ case S390_TDESC_GS:
+ SET_REGMAP(s390_te_ft_collect_regmap, 0);
+ return tdesc_s390_gs_linux64;
#endif
default:
internal_error (__FILE__, __LINE__,
@@ -440,6 +447,7 @@ initialize_low_tracepoint (void)
init_registers_s390x_te_linux64 ();
init_registers_s390x_vx_linux64 ();
init_registers_s390x_tevx_linux64 ();
+ init_registers_s390x_gs_linux64 ();
#else
init_registers_s390_linux32 ();
init_registers_s390_linux32v1 ();
@@ -450,5 +458,6 @@ initialize_low_tracepoint (void)
init_registers_s390_te_linux64 ();
init_registers_s390_vx_linux64 ();
init_registers_s390_tevx_linux64 ();
+ init_registers_s390_gs_linux64 ();
#endif
}
diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
index 7de70f3a38..cc219e2ba9 100644
--- a/gdb/gdbserver/linux-s390-low.c
+++ b/gdb/gdbserver/linux-s390-low.c
@@ -1431,6 +1431,8 @@ s390_get_ipa_tdesc_idx (void)
return S390_TDESC_VX;
if (tdesc == tdesc_s390x_tevx_linux64)
return S390_TDESC_TEVX;
+ if (tdesc == tdesc_s390x_gs_linux64)
+ return S390_TDESC_GS;
#endif
if (tdesc == tdesc_s390_linux32)
@@ -1451,6 +1453,8 @@ s390_get_ipa_tdesc_idx (void)
return S390_TDESC_VX;
if (tdesc == tdesc_s390_tevx_linux64)
return S390_TDESC_TEVX;
+ if (tdesc == tdesc_s390_gs_linux64)
+ return S390_TDESC_GS;
return 0;
}
--
2.14.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] S390: Some gdbserver fixes
2018-03-19 15:31 [PATCH 0/3] S390: Some gdbserver fixes Andreas Arnez
` (2 preceding siblings ...)
2018-03-19 15:33 ` [PATCH 3/3] S390: Make IPA recognize tdescs with guarded storage Andreas Arnez
@ 2018-03-21 16:43 ` Andreas Arnez
3 siblings, 0 replies; 7+ messages in thread
From: Andreas Arnez @ 2018-03-21 16:43 UTC (permalink / raw)
To: gdb-patches
On Mon, Mar 19 2018, Andreas Arnez wrote:
> Some fixes for test suite FAILs on s390(x) with native-gdbserver or
> native-extended-gdbserver.
>
> Andreas Arnez (3):
> S390: Enable re-attaching with native-extended-gdbserver
> S390: gdbserver: Don't write guarded storage registers
> S390: Make IPA recognize tdescs with guarded storage
>
> gdb/gdbserver/linux-s390-ipa.c | 13 ++++-
> gdb/gdbserver/linux-s390-low.c | 130 ++++++++++++++++++++---------------------
> 2 files changed, 73 insertions(+), 70 deletions(-)
I've pushed these now.
--
Andreas
^ permalink raw reply [flat|nested] 7+ messages in thread