[RESEND] gdb: xtensa: fix register counters for xtensa-linux

Message ID 20190113014248.28071-1-jcmvbkbc@gmail.com
State New, archived
Headers

Commit Message

Max Filippov Jan. 13, 2019, 1:42 a.m. UTC
  Commit 37d9e0623102 ("gdb: xtensa: handle privileged registers") changed
how the tdep->num_regs and tdep->num_pseudo_regs are calculated, but
didn't update these numbers in the gdbarch for the xtensa-linux target.
As a result xtensa-linux-gdb behaves as xtensa-elf-gdb and cannot
communicate with the linux gdbserver.
Fix tdep->num_pseudo_regs calculation and call set_gdbarch_num_regs and
set_gdbarch_num_pseudo_regs in xtensa_linux_init_abi.

gdb/
2018-11-16  Max Filippov  <jcmvbkbc@gmail.com>

	* xtensa-linux-tdep.c (xtensa_linux_init_abi): Update
	tdep->num_pseudo_regs. Add calls to set_gdbarch_num_regs and
	set_gdbarch_num_pseudo_regs.
---
 gdb/xtensa-linux-tdep.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
  

Comments

Simon Marchi Jan. 13, 2019, 5:56 a.m. UTC | #1
On 2019-01-12 8:42 p.m., Max Filippov wrote:
> Commit 37d9e0623102 ("gdb: xtensa: handle privileged registers") changed

> how the tdep->num_regs and tdep->num_pseudo_regs are calculated, but

> didn't update these numbers in the gdbarch for the xtensa-linux target.

> As a result xtensa-linux-gdb behaves as xtensa-elf-gdb and cannot

> communicate with the linux gdbserver.

> Fix tdep->num_pseudo_regs calculation and call set_gdbarch_num_regs and

> set_gdbarch_num_pseudo_regs in xtensa_linux_init_abi.

> 

> gdb/

> 2018-11-16  Max Filippov  <jcmvbkbc@gmail.com>

> 

> 	* xtensa-linux-tdep.c (xtensa_linux_init_abi): Update

> 	tdep->num_pseudo_regs. Add calls to set_gdbarch_num_regs and

> 	set_gdbarch_num_pseudo_regs.

> ---

>  gdb/xtensa-linux-tdep.c | 8 +++++++-

>  1 file changed, 7 insertions(+), 1 deletion(-)

> 

> diff --git a/gdb/xtensa-linux-tdep.c b/gdb/xtensa-linux-tdep.c

> index 1764b953a00b..796143c6699b 100644

> --- a/gdb/xtensa-linux-tdep.c

> +++ b/gdb/xtensa-linux-tdep.c

> @@ -101,7 +101,13 @@ xtensa_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)

>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

>  

>    if (tdep->num_nopriv_regs < tdep->num_regs)

> -    tdep->num_regs = tdep->num_nopriv_regs;

> +    {

> +      tdep->num_pseudo_regs += tdep->num_regs - tdep->num_nopriv_regs;

> +      tdep->num_regs = tdep->num_nopriv_regs;

> +

> +      set_gdbarch_num_regs (gdbarch, tdep->num_regs);

> +      set_gdbarch_num_pseudo_regs (gdbarch, tdep->num_pseudo_regs);

> +    }

>  

>    linux_init_abi (info, gdbarch);

>  

> 


Hi Max.

I am just a bit puzzled by the num_pseudo_regs computation (especially the +=).
Can you explain it quickly?

Thanks,

Simon
  
Max Filippov Jan. 13, 2019, 8:36 a.m. UTC | #2
On Sat, Jan 12, 2019 at 9:56 PM Simon Marchi <simon.marchi@ericsson.com> wrote:
>
> On 2019-01-12 8:42 p.m., Max Filippov wrote:
> > Commit 37d9e0623102 ("gdb: xtensa: handle privileged registers") changed
> > how the tdep->num_regs and tdep->num_pseudo_regs are calculated, but
> > didn't update these numbers in the gdbarch for the xtensa-linux target.
> > As a result xtensa-linux-gdb behaves as xtensa-elf-gdb and cannot
> > communicate with the linux gdbserver.
> > Fix tdep->num_pseudo_regs calculation and call set_gdbarch_num_regs and
> > set_gdbarch_num_pseudo_regs in xtensa_linux_init_abi.
> >
> > gdb/
> > 2018-11-16  Max Filippov  <jcmvbkbc@gmail.com>
> >
> >       * xtensa-linux-tdep.c (xtensa_linux_init_abi): Update
> >       tdep->num_pseudo_regs. Add calls to set_gdbarch_num_regs and
> >       set_gdbarch_num_pseudo_regs.
> > ---
> >  gdb/xtensa-linux-tdep.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/gdb/xtensa-linux-tdep.c b/gdb/xtensa-linux-tdep.c
> > index 1764b953a00b..796143c6699b 100644
> > --- a/gdb/xtensa-linux-tdep.c
> > +++ b/gdb/xtensa-linux-tdep.c
> > @@ -101,7 +101,13 @@ xtensa_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> >    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> >
> >    if (tdep->num_nopriv_regs < tdep->num_regs)
> > -    tdep->num_regs = tdep->num_nopriv_regs;
> > +    {
> > +      tdep->num_pseudo_regs += tdep->num_regs - tdep->num_nopriv_regs;
> > +      tdep->num_regs = tdep->num_nopriv_regs;
> > +
> > +      set_gdbarch_num_regs (gdbarch, tdep->num_regs);
> > +      set_gdbarch_num_pseudo_regs (gdbarch, tdep->num_pseudo_regs);
> > +    }
> >
> >    linux_init_abi (info, gdbarch);
> >
> >
>
> Hi Max.
>
> I am just a bit puzzled by the num_pseudo_regs computation (especially the +=).
> Can you explain it quickly?

In the original code (prior to 37d9e0623102) num_regs was the smallest of
the number of the first pseudo register or the first privileged register, and
num_pseudo_regs was the total number of registers minus num_regs.
The register table is constructed so that pseudo registers are always at the
end of it, so num_regs was always equal to num_nonpriv_regs.
I'd like to restore this in xtensa-linux gdb, and what I do is I increase
num_pseudo_regs by the difference of num_regs and num_nonpriv regs
and set num_regs equal to num_nonpriv_regs to maintain the above equations.
  
Simon Marchi Jan. 13, 2019, 4:32 p.m. UTC | #3
On 2019-01-13 03:36, Max Filippov wrote:
> In the original code (prior to 37d9e0623102) num_regs was the smallest 
> of
> the number of the first pseudo register or the first privileged 
> register, and
> num_pseudo_regs was the total number of registers minus num_regs.
> The register table is constructed so that pseudo registers are always 
> at the
> end of it, so num_regs was always equal to num_nonpriv_regs.
> I'd like to restore this in xtensa-linux gdb, and what I do is I 
> increase
> num_pseudo_regs by the difference of num_regs and num_nonpriv regs
> and set num_regs equal to num_nonpriv_regs to maintain the above 
> equations.

"num_regs == num_nonpriv_regs": is this only true for Linux, because we 
don't have access to privileged registers (and therefore there are 0 
nonpriv registers)?  For bare-metal, num_regs would be greater than 
num_nonpriv_regs?

Simon

-
  
Max Filippov Jan. 13, 2019, 7:33 p.m. UTC | #4
On Sun, Jan 13, 2019 at 8:32 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2019-01-13 03:36, Max Filippov wrote:
> > In the original code (prior to 37d9e0623102) num_regs was the smallest
> > of
> > the number of the first pseudo register or the first privileged
> > register, and
> > num_pseudo_regs was the total number of registers minus num_regs.
> > The register table is constructed so that pseudo registers are always
> > at the
> > end of it, so num_regs was always equal to num_nonpriv_regs.
> > I'd like to restore this in xtensa-linux gdb, and what I do is I
> > increase
> > num_pseudo_regs by the difference of num_regs and num_nonpriv regs
> > and set num_regs equal to num_nonpriv_regs to maintain the above
> > equations.
>
> "num_regs == num_nonpriv_regs": is this only true for Linux, because we
> don't have access to privileged registers (and therefore there are 0
> nonpriv registers)?

Correct.

>  For bare-metal, num_regs would be greater than num_nonpriv_regs?

Correct.
  
Simon Marchi Jan. 13, 2019, 7:43 p.m. UTC | #5
On 2019-01-13 14:33, Max Filippov wrote:
> On Sun, Jan 13, 2019 at 8:32 AM Simon Marchi <simon.marchi@polymtl.ca> 
> wrote:
>> 
>> On 2019-01-13 03:36, Max Filippov wrote:
>> > In the original code (prior to 37d9e0623102) num_regs was the smallest
>> > of
>> > the number of the first pseudo register or the first privileged
>> > register, and
>> > num_pseudo_regs was the total number of registers minus num_regs.
>> > The register table is constructed so that pseudo registers are always
>> > at the
>> > end of it, so num_regs was always equal to num_nonpriv_regs.
>> > I'd like to restore this in xtensa-linux gdb, and what I do is I
>> > increase
>> > num_pseudo_regs by the difference of num_regs and num_nonpriv regs
>> > and set num_regs equal to num_nonpriv_regs to maintain the above
>> > equations.
>> 
>> "num_regs == num_nonpriv_regs": is this only true for Linux, because 
>> we
>> don't have access to privileged registers (and therefore there are 0
>> nonpriv registers)?
> 
> Correct.
> 
>>  For bare-metal, num_regs would be greater than num_nonpriv_regs?
> 
> Correct.

Ok. For the record, the patch LGTM, but I am not sure if you are waiting 
for a review from Woody in CC?

Simon
  
Max Filippov Jan. 13, 2019, 8:07 p.m. UTC | #6
On Sun, Jan 13, 2019 at 11:43 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2019-01-13 14:33, Max Filippov wrote:
> > On Sun, Jan 13, 2019 at 8:32 AM Simon Marchi <simon.marchi@polymtl.ca>
> > wrote:
> >>
> >> On 2019-01-13 03:36, Max Filippov wrote:
> >> > In the original code (prior to 37d9e0623102) num_regs was the smallest
> >> > of
> >> > the number of the first pseudo register or the first privileged
> >> > register, and
> >> > num_pseudo_regs was the total number of registers minus num_regs.
> >> > The register table is constructed so that pseudo registers are always
> >> > at the
> >> > end of it, so num_regs was always equal to num_nonpriv_regs.
> >> > I'd like to restore this in xtensa-linux gdb, and what I do is I
> >> > increase
> >> > num_pseudo_regs by the difference of num_regs and num_nonpriv regs
> >> > and set num_regs equal to num_nonpriv_regs to maintain the above
> >> > equations.
> >>
> >> "num_regs == num_nonpriv_regs": is this only true for Linux, because
> >> we
> >> don't have access to privileged registers (and therefore there are 0
> >> nonpriv registers)?
> >
> > Correct.
> >
> >>  For bare-metal, num_regs would be greater than num_nonpriv_regs?
> >
> > Correct.
>
> Ok. For the record, the patch LGTM, but I am not sure if you are waiting
> for a review from Woody in CC?

I'm cc'ing Woody as he's the maintainer of the xtensa gdb in
Cadence/Tensilica. This patch is a resend of an old bugfix,
I guess he'd reply if there was any concern first time I sent it.
  
Max Filippov Jan. 13, 2019, 9:36 p.m. UTC | #7
On Sun, Jan 13, 2019 at 11:43 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> Ok. For the record, the patch LGTM, but I am not sure if you are waiting
> for a review from Woody in CC?

Thanks for the review. I've committed this change to master.

-- Max
  

Patch

diff --git a/gdb/xtensa-linux-tdep.c b/gdb/xtensa-linux-tdep.c
index 1764b953a00b..796143c6699b 100644
--- a/gdb/xtensa-linux-tdep.c
+++ b/gdb/xtensa-linux-tdep.c
@@ -101,7 +101,13 @@  xtensa_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
   if (tdep->num_nopriv_regs < tdep->num_regs)
-    tdep->num_regs = tdep->num_nopriv_regs;
+    {
+      tdep->num_pseudo_regs += tdep->num_regs - tdep->num_nopriv_regs;
+      tdep->num_regs = tdep->num_nopriv_regs;
+
+      set_gdbarch_num_regs (gdbarch, tdep->num_regs);
+      set_gdbarch_num_pseudo_regs (gdbarch, tdep->num_pseudo_regs);
+    }
 
   linux_init_abi (info, gdbarch);