Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Josef Wolf]

Hello folks!

(This mail got somewhat lengthy. Sorry for that. But I am trapped withhin
gdb internals and cant see the exit :)

The story began as I started to bring the bdm-patches for the motorola
cpu32 (which is m68k-based) up to the current gdb-5.3 release. If anyone
is curious what those patches are about, they are available at
http://cmp.felk.cvut.cz/~pisa/m683xx/gdb-5.2.1-bdm-patches-pi1.tar.gz
They are based on gdb-5.2.1.

Needless to say that the patches did not work with gdb-5.3. Too much
restructuring was done from gdb-5.2.1 up to gdb-5.3. So I did a
binary search through the CVS-checkins to identify the checkin on
which they stopped working. It turned out that they stopped working
after the checkin from Andrew Cagney at "2002-06-08 18:57 UTC".
The relevant change was that default_get_saved_register() was deleted
and replaced with generic_unwind_get_saved_register()

The problem with this replacement was that default_get_saved_register()
semantically did

     if (frame==NULL)
         *addrp=REGISTER_BYTE(regnum);

while generic_unwind_get_saved_register() semantically did

     if (frame==NULL)
         *addrp=0;

This confused value_of_register() because it stores this address into
the value struct, so that callers can calculate the register number from
this address. This results in clobbering of all the register contents of
the target because everything seemed to be in register 0.

This little patch (which is relative to the cvs version
 -D "2002-06-08 19:00 UTC") fixed that problem:

----------snip---------
diff -ud gdb/gdb/frame.c gdb.patched/gdb/frame.c
--- gdb/gdb/frame.c  2003-07-29 15:42:37.000000000 +0200
+++ gdb.patched/gdb/frame.c     2003-07-30 15:00:37.000000000 +0200
@@ -150,6 +150,8 @@
   else
     frame_register_unwind (frame->next, regnum, optimizedp, lvalp, addrp,
 			   &realnumx, raw_buffer);
+
+  if (*addrp == 0) *addrp = REGISTER_BYTE (regnum);
 }
 
 void
----------snip---------

This leads to my first set of questions: Is my understanding about the
problem correct? Or should the bdm patches be changed to provide a
different way to get the value of the registers?


But the never ending story goes on: After applying this patch, I went ahead.
Again, the bdm-patch stopped working after the checkin from Grace Sainsbury
at -D "2002-07-02 18:13 UTC" which introduced GDB_MULTI_ARCH_PARTIAL into
the m68k-based targets. This time, the following patch fixed the problem:

----------snip---------
diff -urd gdb/gdb/config/m68k/tm-m68k.h gdb.patched/gdb/config/m68k/tm-m68k.h
--- gdb/gdb/config/m68k/tm-m68k.h	2003-07-31 18:14:59.000000000 +0200
+++ gdb.patched/gdb/config/m68k/tm-m68k.h	2003-07-31 18:22:32.000 +0200
@@ -21,7 +21,7 @@
 
 #include "regcache.h"
 
-#define GDB_MULTI_ARCH GDB_MULTI_ARCH_PARTIAL
+#define GDB_MULTI_ARCH 0
 
 /* Generic 68000 stuff, to be included by other tm-*.h files.  */
 
diff -urd gdb/gdb/m68k-tdep.c gdb.patched/gdb/m68k-tdep.c
--- gdb/gdb/m68k-tdep.c	2003-07-31 18:15:46.000 +0200
+++ gdb.patched/gdb/m68k-tdep.c	2003-07-31 18:21:56.00 +0200
@@ -60,14 +60,14 @@
   E_SP_REGNUM = 15,		/* Contains address of top of stack */
   E_PS_REGNUM = 16,		/* Contains processor status */
   E_PC_REGNUM = 17,		/* Contains program counter */
-  E_FP0_REGNUM = 18,		/* Floating point register 0 */
+  E_FP0_REGNUM = 26,		/* Floating point register 0 */
   E_FPC_REGNUM = 26,		/* 68881 control register */
   E_FPS_REGNUM = 27,		/* 68881 status register */
   E_FPI_REGNUM = 28
 };
 
-#define REGISTER_BYTES_FP (16*4 + 8 + 8*12 + 3*4)
-#define REGISTER_BYTES_NOFP (16*4 + 8)
+#define REGISTER_BYTES_FP (16*4 + 8 + 9*4 + 8*12 + 3*4)
+#define REGISTER_BYTES_NOFP (16*4 + 8 + 9*4 + 8*12 + 3*4)
 
 #define NUM_FREGS (NUM_REGS-24)
 
----------snip---------

That is, there are two problems. First is that the bdm-enabled target have
some additional registers. Those registers are

  "pcc", "usp", "ssp", "sfc", "dfc", "atemp", "far", "vbr",

which are placed between "pc" and "fp0". Most of those registers are
standard m68k registers, so I wonder how E_FP0_REGNUM can be set to 18?
Could it be that the bdm-patches have a broken register mapping? Or is
it that gdb dont want to know anything about vbr, ssp, sfc etc/pp?

The second problem is that I have disabled the MULTI_ARCH configuration.
If I understand correctly, you gdb-gurus want to go towards MULTI_ARCH.
So it would be silly if I would not try to go along with you into the
MULTI_ARCH direction. But I must confess that I have no clue what the
requirements are for that.


Currently, I can successfully build and run cvs snapshot from 2002-07-09.
AFAICS, frame handling changed very much from 2002-07-09 up to gdb-5.3.
So I expect a lot of additional hurdles on this way. And I am not sure
whether I will be able to take those hurdles without some helping hand.
So please, if anybody can give me some hints, they will be welcome :)

Thanks!

-- 
Please visit and sign http://petition-eurolinux.org and http://www.ffii.org
-- Josef Wolf -- jw@raven.inka.de --

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Josef Wolf]

Hello!

On Fri, Aug 01, 2003 at 12:35:14AM +0200, Josef Wolf wrote:

Could it be that noone have noticed my first mail? IMHO, at least this:

[ ... ]
> The relevant change was that default_get_saved_register() was deleted
> and replaced with generic_unwind_get_saved_register()
> 
> The problem with this replacement was that default_get_saved_register()
> semantically did
> 
>      if (frame==NULL)
>          *addrp=REGISTER_BYTE(regnum);
> 
> while generic_unwind_get_saved_register() semantically did
> 
>      if (frame==NULL)
>          *addrp=0;
> 
> This confused value_of_register() because it stores this address into
> the value struct, so that callers can calculate the register number from
> this address. This results in clobbering of all the register contents of
> the target because everything seemed to be in register 0.

seems to be a bug in gdb. And this bug is still present in gdb-5.3. Maybe
one of the gdb-gurus could clarify this?

-- 
Please visit and sign http://petition-eurolinux.org and http://www.ffii.org
-- Josef Wolf -- jw@raven.inka.de --

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Andrew Cagney]

> Hello folks!
> 
> (This mail got somewhat lengthy. Sorry for that. But I am trapped withhin
> gdb internals and cant see the exit :)
> 
> The story began as I started to bring the bdm-patches for the motorola
> cpu32 (which is m68k-based) up to the current gdb-5.3 release. If anyone
> is curious what those patches are about, they are available at
> http://cmp.felk.cvut.cz/~pisa/m683xx/gdb-5.2.1-bdm-patches-pi1.tar.gz
> They are based on gdb-5.2.1.
> 
> Needless to say that the patches did not work with gdb-5.3. Too much
> restructuring was done from gdb-5.2.1 up to gdb-5.3. So I did a
> binary search through the CVS-checkins to identify the checkin on
> which they stopped working. It turned out that they stopped working
> after the checkin from Andrew Cagney at "2002-06-08 18:57 UTC".
> The relevant change was that default_get_saved_register() was deleted
> and replaced with generic_unwind_get_saved_register()
> 
> The problem with this replacement was that default_get_saved_register()
> semantically did
> 
>      if (frame==NULL)
>          *addrp=REGISTER_BYTE(regnum);
> 
> while generic_unwind_get_saved_register() semantically did
> 
>      if (frame==NULL)
>          *addrp=0;

Yes, your analysis here is correct.  I believe it's been fixed in 
current sources though.  Both frame.c:frame_register and 
sentinel-frame.c:sentinel_frame_prev_register correctly are setting the 
register's offset.

(and GDB is desperatly wants to eliminate that hack).

> This leads to my first set of questions: Is my understanding about the
> problem correct? Or should the bdm patches be changed to provide a
> different way to get the value of the registers?
> 
> 
> But the never ending story goes on: After applying this patch, I went ahead.
> Again, the bdm-patch stopped working after the checkin from Grace Sainsbury
> at -D "2002-07-02 18:13 UTC" which introduced GDB_MULTI_ARCH_PARTIAL into
> the m68k-based targets. This time, the following patch fixed the problem:
> 
> ----------snip---------
> diff -urd gdb/gdb/config/m68k/tm-m68k.h gdb.patched/gdb/config/m68k/tm-m68k.h
> --- gdb/gdb/config/m68k/tm-m68k.h	2003-07-31 18:14:59.000000000 +0200
> +++ gdb.patched/gdb/config/m68k/tm-m68k.h	2003-07-31 18:22:32.000 +0200
> @@ -21,7 +21,7 @@
>  
>  #include "regcache.h"
>  
> -#define GDB_MULTI_ARCH GDB_MULTI_ARCH_PARTIAL
> +#define GDB_MULTI_ARCH 0
>  
>  /* Generic 68000 stuff, to be included by other tm-*.h files.  */
>  
> diff -urd gdb/gdb/m68k-tdep.c gdb.patched/gdb/m68k-tdep.c
> --- gdb/gdb/m68k-tdep.c	2003-07-31 18:15:46.000 +0200
> +++ gdb.patched/gdb/m68k-tdep.c	2003-07-31 18:21:56.00 +0200
> @@ -60,14 +60,14 @@
>    E_SP_REGNUM = 15,		/* Contains address of top of stack */
>    E_PS_REGNUM = 16,		/* Contains processor status */
>    E_PC_REGNUM = 17,		/* Contains program counter */
> -  E_FP0_REGNUM = 18,		/* Floating point register 0 */
> +  E_FP0_REGNUM = 26,		/* Floating point register 0 */
>    E_FPC_REGNUM = 26,		/* 68881 control register */
>    E_FPS_REGNUM = 27,		/* 68881 status register */
>    E_FPI_REGNUM = 28
>  };
>  
> -#define REGISTER_BYTES_FP (16*4 + 8 + 8*12 + 3*4)
> -#define REGISTER_BYTES_NOFP (16*4 + 8)
> +#define REGISTER_BYTES_FP (16*4 + 8 + 9*4 + 8*12 + 3*4)
> +#define REGISTER_BYTES_NOFP (16*4 + 8 + 9*4 + 8*12 + 3*4)
>  
>  #define NUM_FREGS (NUM_REGS-24)
>  
> ----------snip---------
> 
> That is, there are two problems. First is that the bdm-enabled target have
> some additional registers. Those registers are
> 
>   "pcc", "usp", "ssp", "sfc", "dfc", "atemp", "far", "vbr",
> 
> which are placed between "pc" and "fp0". Most of those registers are
> standard m68k registers, so I wonder how E_FP0_REGNUM can be set to 18?
> Could it be that the bdm-patches have a broken register mapping? Or is
> it that gdb dont want to know anything about vbr, ssp, sfc etc/pp?

[I'm guessing here]

The bdm code should use supply_register(REGNUM, BUF).  If it is still 
using the [deprecated_]registers array then it will first need to be 
updated so that it instead uses supply_register / collect_register.

With that done, the code's dependency on the raw register buffer layout 
should have been eliminated.  That should in turn make it possible to 
add these new registers to the end (instead of the middle) of the 
register.table.

The question then becomes one of should they be included by default. 
For the answer to that, ask Andreas Schwab [To:ed] who is the linux m68k 
maintainer.

> The second problem is that I have disabled the MULTI_ARCH configuration.
> If I understand correctly, you gdb-gurus want to go towards MULTI_ARCH.
> So it would be silly if I would not try to go along with you into the
> MULTI_ARCH direction. But I must confess that I have no clue what the
> requirements are for that.

The ability to disable multi-arch is about to be deleted from current 
sources.  Yes you'll have a problem.

> Currently, I can successfully build and run cvs snapshot from 2002-07-09.
> AFAICS, frame handling changed very much from 2002-07-09 up to gdb-5.3.
> So I expect a lot of additional hurdles on this way. And I am not sure
> whether I will be able to take those hurdles without some helping hand.
> So please, if anybody can give me some hints, they will be welcome :)

The current m68k sources are very up-to-date so hopefully the only 
problems you encounter are restricted to bdm.

Andrew

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Andreas Schwab]

Josef Wolf <jw@raven.inka.de> writes:

|> diff -urd gdb/gdb/m68k-tdep.c gdb.patched/gdb/m68k-tdep.c
|> --- gdb/gdb/m68k-tdep.c	2003-07-31 18:15:46.000 +0200
|> +++ gdb.patched/gdb/m68k-tdep.c	2003-07-31 18:21:56.00 +0200
|> @@ -60,14 +60,14 @@
|>    E_SP_REGNUM = 15,		/* Contains address of top of stack */
|>    E_PS_REGNUM = 16,		/* Contains processor status */
|>    E_PC_REGNUM = 17,		/* Contains program counter */
|> -  E_FP0_REGNUM = 18,		/* Floating point register 0 */
|> +  E_FP0_REGNUM = 26,		/* Floating point register 0 */
|>    E_FPC_REGNUM = 26,		/* 68881 control register */

There are 7 registers between fp0 and fpc, namely the other floating point
registers fp1 to fp7.  You can't just renumber fp0 without renumbering the
others.

|> That is, there are two problems. First is that the bdm-enabled target have
|> some additional registers. Those registers are
|> 
|>   "pcc", "usp", "ssp", "sfc", "dfc", "atemp", "far", "vbr",
|> 
|> which are placed between "pc" and "fp0". Most of those registers are
|> standard m68k registers,

But they are supervisor-only registers (btw, I have never heard of the
pcc, atemp and far registers, they don't exist on any of the variants of
the m68k family I know of).

|> so I wonder how E_FP0_REGNUM can be set to 18?

Theses numbers are basically arbitrary, AFAIK.

|> Could it be that the bdm-patches have a broken register mapping? Or is
|> it that gdb dont want to know anything about vbr, ssp, sfc etc/pp?

The latter.  No user level program can ever use these registers, so gdb
does not need to know anything about them.  But if you want gdb to be
able to debug supervisor level programs you can just give these registers
any number beyond the currently defined range.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Josef Wolf]

On Wed, Aug 06, 2003 at 08:22:32PM +0200, Andreas Schwab wrote:

Thanks for your reply, Andreas!

> Josef Wolf <jw@raven.inka.de> writes:
> 
> |> diff -urd gdb/gdb/m68k-tdep.c gdb.patched/gdb/m68k-tdep.c
> |> --- gdb/gdb/m68k-tdep.c	2003-07-31 18:15:46.000 +0200
> |> +++ gdb.patched/gdb/m68k-tdep.c	2003-07-31 18:21:56.00 +0200
> |> @@ -60,14 +60,14 @@
> |>    E_SP_REGNUM = 15,		/* Contains address of top of stack */
> |>    E_PS_REGNUM = 16,		/* Contains processor status */
> |>    E_PC_REGNUM = 17,		/* Contains program counter */
> |> -  E_FP0_REGNUM = 18,		/* Floating point register 0 */
> |> +  E_FP0_REGNUM = 26,		/* Floating point register 0 */
> |>    E_FPC_REGNUM = 26,		/* 68881 control register */
> 
> There are 7 registers between fp0 and fpc, namely the other floating point
> registers fp1 to fp7.  You can't just renumber fp0 without renumbering the
> others.

This specific target (CPU32) doesn't have any floating point registers,
so it shouldn't hurt in the case when BDM is used. I would like to bring
the BDM-patches as close as possible to the generic m68k code. But I was
somewhat confused, so I made this quick-n-dirty hack.

> |> That is, there are two problems. First is that the bdm-enabled target have
> |> some additional registers. Those registers are
> |> 
> |>   "pcc", "usp", "ssp", "sfc", "dfc", "atemp", "far", "vbr",
> |> 
> |> which are placed between "pc" and "fp0". Most of those registers are
> |> standard m68k registers,
> 
> But they are supervisor-only registers

Ough? Does that mean that gdb should not be used to debug supervisor mode?
Is this a political decision or are there technical reasons?

> (btw, I have never heard of the pcc, atemp and far registers, they don't
> exist on any of the variants of the m68k family I know of).

FAR: Fault Address Register. Contains the address of the faulting bus
     cycle.
PCC: Current Instruction Program Counter. This can differ from the user
     visible PC due to instruction pipelining.
ATEMP: A TEMPorary register. This one can be used to distinguish between
       a double bus fault and a BGND instruction (which turns the CPU into
       BDM-Mode)

These three registers are not user-visible. They are BDM-specific. So you
are probably right that gdb doesn't need to know about them. But they can
come handy when you are debugging your hardware.

> |> so I wonder how E_FP0_REGNUM can be set to 18?
> 
> Theses numbers are basically arbitrary, AFAIK.

But there are places like 

  for (regnum = FP0_REGNUM + 7; regnum >= FP0_REGNUM; regnum--)
    [ ... ]

or

  for (regnum = FP_REGNUM - 1; regnum >= 0; regnum--)
    [ ... ]


so they seemed not to be very arbitrary to me.

BTW: I still don't have a clue what this MULTI_ARCH stuff is all about and
     how multi-arch targets are meant to be adopted. But it seems to me
     that the BDM patches should be brought towards this multi-arch
     structure on the long run. Does any overview and/or introduction into
     this topic exist? Something like a Add-A-New-Target-To-Gdb.HOWTO maybe?

Thanks!

-- 
Please visit and sign http://petition-eurolinux.org and http://www.ffii.org
-- Josef Wolf -- jw@raven.inka.de --

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Josef Wolf]

On Wed, Aug 06, 2003 at 12:09:40PM -0400, Andrew Cagney wrote:

Thanks for your reply, Andrew!

> >The problem with this replacement was that default_get_saved_register()
> >semantically did
> >
> >     if (frame==NULL)
> >         *addrp=REGISTER_BYTE(regnum);
> >
> >while generic_unwind_get_saved_register() semantically did
> >
> >     if (frame==NULL)
> >         *addrp=0;
> 
> Yes, your analysis here is correct.  I believe it's been fixed in 
> current sources though.

It is present in gdb-5.3. AFAICS, most of the targets are already using
or are beeing moved to use generic_unwind_get_saved_register(). How coud
this bug exist for more than a year and noone noticed?

> Both frame.c:frame_register and 
> sentinel-frame.c:sentinel_frame_prev_register correctly are setting the 
> register's offset.

You are talking about current (pre-6.0) code? Both functions don't exist
in 5.3.

> (and GDB is desperatly wants to eliminate that hack).

Could you please activate verbose mode? Honestly, I did not get what you
try to say here. Do you want to eliminate generic_unwind_get_saved_register
or frame_register/sentinel_frame_prev_register?

[ ... ]
> [I'm guessing here]
> 
> The bdm code should use supply_register(REGNUM, BUF).

OK, this is what it does.

> If it is still 
> using the [deprecated_]registers array then it will first need to be 
> updated so that it instead uses supply_register / collect_register.

It uses supply_register but not collect_register. This looks strange to
me, I would have expected that both would be used.

> With that done, the code's dependency on the raw register buffer layout 
> should have been eliminated.  That should in turn make it possible to 
> add these new registers to the end (instead of the middle) of the 
> register.table.

OK, I'll try it.

> The question then becomes one of should they be included by default. 
> For the answer to that, ask Andreas Schwab [To:ed] who is the linux m68k 
> maintainer.

From Andreas' mail I deduced that it doesn't make much sense to include
them at all. So I am considering to throw them into the bit-bucket. OTOH,
I think at least vbr/usp/ssp/sfc/dfc should be included in the generic
m68k code. I still don't understand why they are not supported. After all,
every m68k based cpu that is worth to be used has those registers.

> >The second problem is that I have disabled the MULTI_ARCH configuration.
> >If I understand correctly, you gdb-gurus want to go towards MULTI_ARCH.
> >So it would be silly if I would not try to go along with you into the
> >MULTI_ARCH direction. But I must confess that I have no clue what the
> >requirements are for that.
> The ability to disable multi-arch is about to be deleted from current 
> sources.  Yes you'll have a problem.

This is exaclty the answer that I expected :-) But then, what needs to be
done to bring a new/existing target towards the MULTI_ARCH world?

> >Currently, I can successfully build and run cvs snapshot from 2002-07-09.
> >AFAICS, frame handling changed very much from 2002-07-09 up to gdb-5.3.
> >So I expect a lot of additional hurdles on this way. And I am not sure
> >whether I will be able to take those hurdles without some helping hand.
> >So please, if anybody can give me some hints, they will be welcome :)
> 
> The current m68k sources are very up-to-date so hopefully the only 
> problems you encounter are restricted to bdm.

It turned out that 5.3 branched off before the changes I was afraid of
appeared on the trunk. But this doesn't solve my problem, it just delays
it to gdb-6.0 :-)

Thanks!

-- 
Please visit and sign http://petition-eurolinux.org and http://www.ffii.org
-- Josef Wolf -- jw@raven.inka.de --

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Andreas Schwab]

Josef Wolf <jw@raven.inka.de> writes:

> Ough? Does that mean that gdb should not be used to debug supervisor mode?

GDB was designed to debug user level programs, which have no way to
see supervisor-only registers.  So from GDB's point of view they don't
exist.  That does not mean that GDB on m68k can not be extended to
handle them, you just need a way to get their contents from the target
(no current m68k target provides that).

>> Theses numbers are basically arbitrary, AFAIK.
>
> But there are places like 

By arbitrary I mean not externally imposed.  The numbers are chosen
for convenience inside GDB, and exposed only to the target interface,
where they are suitably translated when communicating with the target.

> BTW: I still don't have a clue what this MULTI_ARCH stuff is all about and
>      how multi-arch targets are meant to be adopted.

MULTI_ARCH means that every target architecture hooks in through the
gdbarch structure at runtime, instead of hard coding the interface
characteristics at compile time.  Targets that set GDB_MULTI_ARCH to
GDB_MULTI_ARCH_PARTIAL are not quite there yet, but the basic support
is present.

Andreas.

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Andrew Cagney]

> Josef Wolf <jw@raven.inka.de> writes:
> 
> 
>> Ough? Does that mean that gdb should not be used to debug supervisor mode?
> 
> 
> GDB was designed to debug user level programs, which have no way to
> see supervisor-only registers.  So from GDB's point of view they don't
> exist.  That does not mean that GDB on m68k can not be extended to
> handle them, you just need a way to get their contents from the target
> (no current m68k target provides that).

Originally yes.  Since then (was the MIPS first?) people have been 
adding architecture variants that include the supervisor registers.  The 
native register set, though, still only includes user-visible registers.

Josef,
The m68hc11 is hopefully a straight forward enough embedded architecture 
to see how it works.
Do you have an FSF copyright assignment for GDB?  And, would you know 
the history of these BDM patches - who their author was?

Andrew

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Andrew Cagney]

Do you have a a copyright ssignment/disclaimer, and do you know who owns 
the code you're working on - can it be contributed to the FSF?

> On Wed, Aug 06, 2003 at 12:09:40PM -0400, Andrew Cagney wrote:

> You are talking about current (pre-6.0) code? Both functions don't exist
> in 5.3.
> 
> 
>> (and GDB is desperatly wants to eliminate that hack).
> 
> 
> Could you please activate verbose mode? Honestly, I did not get what you
> try to say here. Do you want to eliminate generic_unwind_get_saved_register
> or frame_register/sentinel_frame_prev_register?

The method register_offset_hack, and the way GDB assumes that a value's 
location can be described by an offset into a register buffer.  This 
isn't sufficient.  A value may be in multiple registers.

> The problem with this replacement was that default_get_saved_register()
>> >semantically did
>> >
>> >     if (frame==NULL)
>> >         *addrp=REGISTER_BYTE(regnum);
>> >
>> >while generic_unwind_get_saved_register() semantically did
>> >
>> >     if (frame==NULL)
>> >         *addrp=0;
> 
>> 
>> Yes, your analysis here is correct.  I believe it's been fixed in 
>> current sources though.
> 
> 
> It is present in gdb-5.3. AFAICS, most of the targets are already using
> or are beeing moved to use generic_unwind_get_saved_register(). How coud
> this bug exist for more than a year and noone noticed?

It was noticed and fixed in the mainline, and a test case was added. 
 From memory, the problem only appears when modifying the target, and 
people only do that occasionally.

>> [I'm guessing here]
>> 
>> The bdm code should use supply_register(REGNUM, BUF).
> 
> 
> OK, this is what it does.
> 
> 
>> If it is still 
>> using the [deprecated_]registers array then it will first need to be 
>> updated so that it instead uses supply_register / collect_register.
> 
> 
> It uses supply_register but not collect_register. This looks strange to
> me, I would have expected that both would be used.

collect_register was a relatively recent addition to GDB.

>> The question then becomes one of should they be included by default. 
>> For the answer to that, ask Andreas Schwab [To:ed] who is the linux m68k 
>> maintainer.
> 
> 
>From Andreas' mail I deduced that it doesn't make much sense to include
> them at all. So I am considering to throw them into the bit-bucket. OTOH,
> I think at least vbr/usp/ssp/sfc/dfc should be included in the generic
> m68k code. I still don't understand why they are not supported. After all,
> every m68k based cpu that is worth to be used has those registers.

It's reasonable to add an embedded architecture variant but not as the 
default.  Study targets like m68hc11 which support cpu variants.

Andrew

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Josef Wolf]

On Thu, Aug 07, 2003 at 08:08:11AM +0200, Andreas Schwab wrote:
> Josef Wolf <jw@raven.inka.de> writes:
> 
> > Ough? Does that mean that gdb should not be used to debug supervisor mode?
> 
> GDB was designed to debug user level programs, which have no way to
> see supervisor-only registers.  So from GDB's point of view they don't
> exist.  That does not mean that GDB on m68k can not be extended to
> handle them, you just need a way to get their contents from the target
> (no current m68k target provides that).

IMHO, seeing this registers would be a great benefit when you go behind
user-only code (e.g in the embedded world). If I understand correctly, it
would not be a great deal to support them. So why not to support?

> By arbitrary I mean not externally imposed.  The numbers are chosen
> for convenience inside GDB, and exposed only to the target interface,
> where they are suitably translated when communicating with the target.

But this means you need to know the "inside meanings" when you want to
assign different numbers. And that is exactly my problem. I don't know
those "insight meanings" and I can't find any place where the
"insight meanings" are described.

> > BTW: I still don't have a clue what this MULTI_ARCH stuff is all about and
> >      how multi-arch targets are meant to be adopted.
> 
> MULTI_ARCH means that every target architecture hooks in through the
> gdbarch structure at runtime, instead of hard coding the interface
> characteristics at compile time.  Targets that set GDB_MULTI_ARCH to
> GDB_MULTI_ARCH_PARTIAL are not quite there yet, but the basic support
> is present.

But then I still have no idea how to add a new MULTI_ARCH target. AFAICS
the gdbarch.[ch] files are generated automatically. That introduces a lot
of confusion to me.

-- 
Please visit and sign http://petition-eurolinux.org and http://www.ffii.org
-- Josef Wolf -- jw@raven.inka.de --

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Josef Wolf]

On Thu, Aug 07, 2003 at 10:34:53AM -0400, Andrew Cagney wrote:
> The m68hc11 is hopefully a straight forward enough embedded architecture 
> to see how it works.

I'll take a look at it.

> Do you have an FSF copyright assignment for GDB?

?!? Could you be a little more vebose please? I don't understand this
question.

> And, would you know the history of these BDM patches - who their author
> was?

Originally, they were written by Eric Norum. They are currently maintained
by Pavel Pisa. The GPL is attached to the files, so I assume that I have the
right to hunt for those bugs.

-- 
Please visit and sign http://petition-eurolinux.org and http://www.ffii.org
-- Josef Wolf -- jw@raven.inka.de --

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Josef Wolf]

On Sat, Aug 09, 2003 at 10:31:14AM -0400, Andrew Cagney wrote:
> Do you have a a copyright ssignment/disclaimer, and do you know who owns 
> the code you're working on - can it be contributed to the FSF?

The files claim to be GPL.

> >On Wed, Aug 06, 2003 at 12:09:40PM -0400, Andrew Cagney wrote:
> 
> >You are talking about current (pre-6.0) code? Both functions don't exist
> >in 5.3.
> >
> >
> >>(and GDB is desperatly wants to eliminate that hack).
> >
> >
> >Could you please activate verbose mode? Honestly, I did not get what you
> >try to say here. Do you want to eliminate generic_unwind_get_saved_register
> >or frame_register/sentinel_frame_prev_register?
> 
> The method register_offset_hack, and the way GDB assumes that a value's 
> location can be described by an offset into a register buffer.  This 
> isn't sufficient.  A value may be in multiple registers.

Oh, I see.... But then, what does this mean to porters? What do they need to
do when they want to stay on the bleeding edge of gdb?

-- 
Please visit and sign http://petition-eurolinux.org and http://www.ffii.org
-- Josef Wolf -- jw@raven.inka.de --

Re: Need Help for bringing m68k-based bdm target-patches form gdb-5.2.1 to gdb-5.3

[Andreas Schwab]

Josef Wolf <jw@raven.inka.de> writes:

|> IMHO, seeing this registers would be a great benefit when you go behind
|> user-only code (e.g in the embedded world). If I understand correctly, it
|> would not be a great deal to support them. So why not to support?

If you have a target that provides them, sure.

|> But this means you need to know the "inside meanings" when you want to
|> assign different numbers. And that is exactly my problem. I don't know
|> those "insight meanings" and I can't find any place where the
|> "insight meanings" are described.

Just add the new registers after the existing one, not inbetween.  Simple
as that.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."