* [PATCH v2] gdb: xtensa: don't supply registers if they are not present
@ 2023-02-24 6:28 Alexey Lapshin via Gdb-patches
2023-02-24 19:55 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Lapshin via Gdb-patches @ 2023-02-24 6:28 UTC (permalink / raw)
To: gdb-patches
Cc: Alexey Gerenkov, tom, Anton Maklakov, Ivan Grokhotkov, jcmvbkbc
When parsing a core file on hardware configurations without the
zero-overhead loop option (e.g. ESP32-S2 chip), GDB used to assert
while trying to call 'raw_supply' for lbeg, lend, lcount registers,
even though they were not set.
This was because:
regnum == -1 was used to indicate "supply all registers"
lbeg_regnum == -1 was used to indicate "lbeg register not present"
regnum == lbeg_regnum check was considered successful
---
gdb/xtensa-tdep.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 092ae088932..66e5baae7a5 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -801,6 +801,11 @@ xtensa_register_reggroup_p (struct gdbarch
*gdbarch,
return 1;
}
+static inline bool
+is_reg_raw_supplied (int check_regnum, int regnum)
+{
+ return check_regnum > 0 && (regnum == check_regnum || regnum == -1);
+}
/* Supply register REGNUM from the buffer specified by GREGS and LEN
in the general-purpose register set REGSET to register cache
@@ -824,22 +829,22 @@ xtensa_supply_gregset (const struct regset
*regset,
rc->raw_supply (gdbarch_pc_regnum (gdbarch), (char *) ®s->pc);
if (regnum == gdbarch_ps_regnum (gdbarch) || regnum == -1)
rc->raw_supply (gdbarch_ps_regnum (gdbarch), (char *) ®s->ps);
- if (regnum == tdep->wb_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->wb_regnum, regnum))
rc->raw_supply (tdep->wb_regnum,
(char *) ®s->windowbase);
- if (regnum == tdep->ws_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->ws_regnum, regnum))
rc->raw_supply (tdep->ws_regnum,
(char *) ®s->windowstart);
- if (regnum == tdep->lbeg_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->lbeg_regnum, regnum))
rc->raw_supply (tdep->lbeg_regnum,
(char *) ®s->lbeg);
- if (regnum == tdep->lend_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->lend_regnum, regnum))
rc->raw_supply (tdep->lend_regnum,
(char *) ®s->lend);
- if (regnum == tdep->lcount_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->lcount_regnum, regnum))
rc->raw_supply (tdep->lcount_regnum,
(char *) ®s->lcount);
- if (regnum == tdep->sar_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->sar_regnum, regnum))
rc->raw_supply (tdep->sar_regnum,
(char *) ®s->sar);
if (regnum >=tdep->ar_base
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] gdb: xtensa: don't supply registers if they are not present
2023-02-24 6:28 [PATCH v2] gdb: xtensa: don't supply registers if they are not present Alexey Lapshin via Gdb-patches
@ 2023-02-24 19:55 ` Tom Tromey
2023-05-16 4:17 ` [PATCH v3] gdb: xtensa: fix registers supply if they " Alexey Lapshin via Gdb-patches
0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2023-02-24 19:55 UTC (permalink / raw)
To: Alexey Lapshin via Gdb-patches
Cc: Alexey Lapshin, Alexey Gerenkov, tom, Anton Maklakov,
Ivan Grokhotkov, jcmvbkbc
>>>>> "Alexey" == Alexey Lapshin via Gdb-patches <gdb-patches@sourceware.org> writes:
Alexey> +static inline bool
Alexey> +is_reg_raw_supplied (int check_regnum, int regnum)
In gdb, it's customary to have a comment before a function that
explains its purpose.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3] gdb: xtensa: fix registers supply if they not present
2023-02-24 19:55 ` Tom Tromey
@ 2023-05-16 4:17 ` Alexey Lapshin via Gdb-patches
2024-04-03 14:49 ` Alexey Lapshin
2024-04-09 17:59 ` John Baldwin
0 siblings, 2 replies; 5+ messages in thread
From: Alexey Lapshin via Gdb-patches @ 2023-05-16 4:17 UTC (permalink / raw)
To: gdb-patches, tom
Cc: Alexey Gerenkov, Anton Maklakov, Ivan Grokhotkov, jcmvbkbc
When parsing a core file on hardware configurations without the
zero-overhead loop option (e.g. ESP32-S2 chip), GDB used to assert
while trying to call 'raw_supply' for lbeg, lend, lcount registers,
even though they were not set.
This was because:
regnum == -1 was used to indicate "supply all registers"
lbeg_regnum == -1 was used to indicate "lbeg register not present"
regnum == lbeg_regnum check was considered successful
---
gdb/xtensa-tdep.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index a47a2987499..af0f5c86bab 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -803,6 +803,18 @@ xtensa_register_reggroup_p (struct gdbarch *gdbarch,
}
+/* Check if register raw supplied
+ Note:
+ - check_regnum == -1 means register is not present.
+ - regnum == -1 means all present registers are supplied. */
+
+static inline bool
+is_reg_raw_supplied (int check_regnum, int regnum)
+{
+ return check_regnum > 0 && (regnum == check_regnum || regnum == -1);
+}
+
+
/* Supply register REGNUM from the buffer specified by GREGS and LEN
in the general-purpose register set REGSET to register cache
REGCACHE. If REGNUM is -1 do this for all registers in REGSET. */
@@ -825,22 +837,22 @@ xtensa_supply_gregset (const struct regset *regset,
rc->raw_supply (gdbarch_pc_regnum (gdbarch), (char *) ®s->pc);
if (regnum == gdbarch_ps_regnum (gdbarch) || regnum == -1)
rc->raw_supply (gdbarch_ps_regnum (gdbarch), (char *) ®s->ps);
- if (regnum == tdep->wb_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->wb_regnum, regnum))
rc->raw_supply (tdep->wb_regnum,
(char *) ®s->windowbase);
- if (regnum == tdep->ws_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->ws_regnum, regnum))
rc->raw_supply (tdep->ws_regnum,
(char *) ®s->windowstart);
- if (regnum == tdep->lbeg_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->lbeg_regnum, regnum))
rc->raw_supply (tdep->lbeg_regnum,
(char *) ®s->lbeg);
- if (regnum == tdep->lend_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->lend_regnum, regnum))
rc->raw_supply (tdep->lend_regnum,
(char *) ®s->lend);
- if (regnum == tdep->lcount_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->lcount_regnum, regnum))
rc->raw_supply (tdep->lcount_regnum,
(char *) ®s->lcount);
- if (regnum == tdep->sar_regnum || regnum == -1)
+ if (is_reg_raw_supplied (tdep->sar_regnum, regnum))
rc->raw_supply (tdep->sar_regnum,
(char *) ®s->sar);
if (regnum >=tdep->ar_base
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3] gdb: xtensa: fix registers supply if they not present
2023-05-16 4:17 ` [PATCH v3] gdb: xtensa: fix registers supply if they " Alexey Lapshin via Gdb-patches
@ 2024-04-03 14:49 ` Alexey Lapshin
2024-04-09 17:59 ` John Baldwin
1 sibling, 0 replies; 5+ messages in thread
From: Alexey Lapshin @ 2024-04-03 14:49 UTC (permalink / raw)
To: gdb-patches, tom
Cc: Alexey Gerenkov, Anton Maklakov, Ivan Grokhotkov, jcmvbkbc
Ping
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] gdb: xtensa: fix registers supply if they not present
2023-05-16 4:17 ` [PATCH v3] gdb: xtensa: fix registers supply if they " Alexey Lapshin via Gdb-patches
2024-04-03 14:49 ` Alexey Lapshin
@ 2024-04-09 17:59 ` John Baldwin
1 sibling, 0 replies; 5+ messages in thread
From: John Baldwin @ 2024-04-09 17:59 UTC (permalink / raw)
To: Alexey Lapshin, gdb-patches, tom
Cc: Alexey Gerenkov, Anton Maklakov, Ivan Grokhotkov, jcmvbkbc
On 5/16/23 12:17 AM, Alexey Lapshin via Gdb-patches wrote:
> When parsing a core file on hardware configurations without the
> zero-overhead loop option (e.g. ESP32-S2 chip), GDB used to assert
> while trying to call 'raw_supply' for lbeg, lend, lcount registers,
> even though they were not set.
>
> This was because:
> regnum == -1 was used to indicate "supply all registers"
> lbeg_regnum == -1 was used to indicate "lbeg register not present"
> regnum == lbeg_regnum check was considered successful
> ---
> gdb/xtensa-tdep.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
> index a47a2987499..af0f5c86bab 100644
> --- a/gdb/xtensa-tdep.c
> +++ b/gdb/xtensa-tdep.c
> @@ -803,6 +803,18 @@ xtensa_register_reggroup_p (struct gdbarch *gdbarch,
> }
>
>
> +/* Check if register raw supplied
> + Note:
> + - check_regnum == -1 means register is not present.
> + - regnum == -1 means all present registers are supplied. */
> +
> +static inline bool
> +is_reg_raw_supplied (int check_regnum, int regnum)
> +{
> + return check_regnum > 0 && (regnum == check_regnum || regnum == -1);
> +}
> +
> +
> /* Supply register REGNUM from the buffer specified by GREGS and LEN
> in the general-purpose register set REGSET to register cache
> REGCACHE. If REGNUM is -1 do this for all registers in REGSET. */
> @@ -825,22 +837,22 @@ xtensa_supply_gregset (const struct regset *regset,
> rc->raw_supply (gdbarch_pc_regnum (gdbarch), (char *) ®s->pc);
> if (regnum == gdbarch_ps_regnum (gdbarch) || regnum == -1)
> rc->raw_supply (gdbarch_ps_regnum (gdbarch), (char *) ®s->ps);
> - if (regnum == tdep->wb_regnum || regnum == -1)
> + if (is_reg_raw_supplied (tdep->wb_regnum, regnum))
> rc->raw_supply (tdep->wb_regnum,
> (char *) ®s->windowbase);
> - if (regnum == tdep->ws_regnum || regnum == -1)
> + if (is_reg_raw_supplied (tdep->ws_regnum, regnum))
> rc->raw_supply (tdep->ws_regnum,
> (char *) ®s->windowstart);
> - if (regnum == tdep->lbeg_regnum || regnum == -1)
> + if (is_reg_raw_supplied (tdep->lbeg_regnum, regnum))
> rc->raw_supply (tdep->lbeg_regnum,
> (char *) ®s->lbeg);
> - if (regnum == tdep->lend_regnum || regnum == -1)
> + if (is_reg_raw_supplied (tdep->lend_regnum, regnum))
> rc->raw_supply (tdep->lend_regnum,
> (char *) ®s->lend);
> - if (regnum == tdep->lcount_regnum || regnum == -1)
> + if (is_reg_raw_supplied (tdep->lcount_regnum, regnum))
> rc->raw_supply (tdep->lcount_regnum,
> (char *) ®s->lcount);
> - if (regnum == tdep->sar_regnum || regnum == -1)
> + if (is_reg_raw_supplied (tdep->sar_regnum, regnum))
> rc->raw_supply (tdep->sar_regnum,
> (char *) ®s->sar);
> if (regnum >=tdep->ar_base
This looks ok to me, but is a bit ununusal. For other GDB arches we tend to add
helper methods to the tdep class to indicate if an optional feature is present
and then use those helpers in functions like this. For example, aarch64 has an
optional TLS register set with the following class members:
/* Target-dependent structure in gdbarch. */
struct aarch64_gdbarch_tdep : gdbarch_tdep_base
{
...
/* TLS registers. This is -1 if the TLS registers are not available. */
int tls_regnum_base = 0;
int tls_register_count = 0;
bool has_tls() const
{
return tls_regnum_base != -1;
}
...
};
Code that supports the TLS register set uses the has_tls function to instead
of comparing tls_regnum_base against -1 explicitly, e.g. in aarch64-fbsd-nat.c:
void
aarch64_fbsd_nat_target::fetch_registers (struct regcache *regcache,
int regnum)
{
...
gdbarch *gdbarch = regcache->arch ();
aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
if (tdep->has_tls ())
fetch_regset<uint64_t> (regcache, regnum, NT_ARM_TLS,
&aarch64_fbsd_tls_regset, tdep->tls_regnum_base);
}
Looking at the xtensa-tdep bits more though, I don't think there is a clear
analog to this for the way xtensa_rmap[] works (it is not clear to me how
a xtensa gdbarch can ever have a subset of registers given that the same
static xtensa_rmap[] is always passed to the tdep object constructed in
xtensa_gdbarch_init and xtensa_derive_tdep is called immediately after the
constructor, so it seems like lbeg_regnum should always be set?)
--
John Baldwin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-09 18:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-24 6:28 [PATCH v2] gdb: xtensa: don't supply registers if they are not present Alexey Lapshin via Gdb-patches
2023-02-24 19:55 ` Tom Tromey
2023-05-16 4:17 ` [PATCH v3] gdb: xtensa: fix registers supply if they " Alexey Lapshin via Gdb-patches
2024-04-03 14:49 ` Alexey Lapshin
2024-04-09 17:59 ` John Baldwin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox