Re: remote watchpoint support

Re: remote watchpoint support

[Andrew Cagney]

Mark Salter wrote:
> 
> Ok. Here's a patch that allows a remote target to pass watchpoint
> information back to gdb using the T packet. It also fixes the T
> packet handling to ignore other optional info which may not be
> recognized. This changes the behavior to match the documentation.
> 
> While testing this, I discovered that GDB tries to read data from
> the watched address before the watchpoints are removed. This causes
> another watchpoint exception so the stub returns an error response
> to the read memory request.
> 
> --Mark
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/ChangeLog,v
> retrieving revision 1.744
> diff -p -r1.744 ChangeLog
> *** ChangeLog   2000/10/31 19:35:03     1.744
> --- ChangeLog   2000/11/01 21:06:26
> ***************
> *** 1,3 ****
> --- 1,10 ----
> + 2000-11-01  Mark Salter  <msalter@redhat.com>
> +
> +       * remote.c (remote_wait): Add support to extract optional
> +       watchpoint information from T-packet. Ignore unrecognized
> +       optional info in T-packet.
> +       (remote_async_wait): Ditto.
> +


FYI, changelog entries should be separated out and not included in the
diff.

> Index: remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote.c,v
> retrieving revision 1.26
> diff -p -r1.26 remote.c
> *** remote.c    2000/10/23 22:49:28     1.26
> --- remote.c    2000/11/01 21:06:29
> *************** void _initialize_remote (void);
> *** 204,209 ****
> --- 204,216 ----
> 
>   /* */
> 
> + /* This is set to the data address of the access causing the target
> +  * to stop for a watchpoint. */
> + static CORE_ADDR remote_watch_data_address;
> +
> + /* This is non-zero if taregt stopped for a watchpoint. */
> + static int remote_stopped_by_watchpoint_p;
> +

I've strong reservations about the addition of these two ``global''
variables.  Is it possible to modify things higher up so that their
addition isn't required?

I'm also trying to understand how remote_stopped_data_address() et.al.
gets called.

	Andrew


> *************** struct gdb_ext_thread_info
> *** 963,969 ****
> 
>   #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
> 
> ! char *unpack_varlen_hex (char *buff, int *result);
> 
>   static char *unpack_nibble (char *buf, int *val);
> 
> --- 970,976 ----
> 
>   #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
> 
> ! char *unpack_varlen_hex (char *buff, ULONGEST *result);
> 
>   static char *unpack_nibble (char *buf, int *val);
> 

The ChangeLog doesn't mention this change?  Can I suggest this be
submitted separatly with the tweek that follows:

> --- 2657,2682 ----
>                                p, buf);
>                     if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>                       {
		          ULONGEST thread_num;
> !                       p_temp = unpack_varlen_hex (++p1, &ul);
> !                       thread_num = ul;
>                         record_currthread (thread_num);
>                         p = (unsigned char *) p_temp;
>                       }

Suggest just making threadnum a LONGEST and being done with it.

	Andrew
From ac131313@cygnus.com Mon Nov 06 05:13:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: Stephane Carrez <Stephane.Carrez@worldnet.fr>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH]: Fixes for pseudo regs support
Date: Mon, 06 Nov 2000 05:13:00 -0000
Message-id: <3A06ACF1.AA32DF9C@cygnus.com>
References: <39A5B4A7.8261D654@worldnet.fr>
X-SW-Source: 2000-11/msg00055.html
Content-length: 4443

Stephane Carrez wrote:
> 
> Hi!
> 
> Andrew Cagney wrote:
> > Could I encourage you to persue this alternative.    It isn't that I
> > have something against extra multi-arch entries (ok I do but not in this
> > case :-) but rather that the problem you're describing (registers living
> > in memory space) is far more common than you might think.  Off hand, I
> > can think of at least three targets that have an identical problem.  In
> > those cases people ended up shoving hacks into either/and SIM and the
> > target stub :-(.
> 
> Ok.
> 
> After a close look of the problem, it's not that big or complex.
> 
> I assume that:
>   - Machine dependent parts need not be changed: they must know how
>     to handle pseudo registers (well, only the sh does that).
> 
>   - The remote, monitor, sim stubs need not be changed because they
>     only deal with real hard registers.  They are not aware at all
>     of the existence of pseudo registers.
> 
> Now, what remains is in fact the following places:
> 
>   - The stabs reader performs checks about the validity of the register
>     number.  A pseudo register can be referenced in stabs.
> 
>   - The frame_info structure must save the pseudo registers.
>     The pseudo register (which lies in memory) can be saved by GCC
>     at entry point of a function.

A pseudo register can't lie in memory as it is constructed from one or
more real registers.  The frame info code needs to be able to construct,
on the fly, a pseudo register from a combination of both saved registers
and real registers.

Lets consider a hypothetical architecture with N real registers
(r[0]..r[N]) and M pseudo registers (p0..pN).  Each pseudo register is
contructed using something from all the real registers vis:

	p[i] == pseudo (i, r[0], r[1], ...)

When the program stops, the inner most stack frame is being examined. 
Determining a pseudo value is easy as all the real real registers are
available - their values can be obtained directly from the register
buffer.

When you start to move up and down between frames, however, things get
messy.  Lets look at a typical stack frame.

	main()
		calls outer()

	outer()
		prologue saves r[1]
		being used by main()
		on stack

		calls middle()

	middle()
		prologue saves r[2]
		being used by outer()
		on stack

		calls inner()

	inner()
		prologue saves r[1]
		being used by middle()
		on stack

		somewhere in inner's body.

If the above program halted, inner() would be the currently selected
frame and the register buffer would contain the current registers.

When middle()'s frame is selected, all registers except r[1] are in the
register buffer.  r[1] is found in inner()'s stack frame.

When outer()'s frame is selected, all registers except r[1] and r[2] are
in the register buffer.  r[1] is found in inner()s frame.  r[2] is found
in middle()'s frame.

Or to put it another way, something like:

	real_register (frame, regnr)
		for (f = next_inner (frame); f != NULL; f = next_inner (f))
		  if (REGNR in f->saved_registers)
		    return (saved value of REGNR in f->saved_registers)
		return value from register buffer

(At this point, if you're looking at the code that handles info frame
and looking puzzled, yes, that code is wrong.  It searches in the wrong
direction and has been ever since it was first written.  The SPARC
window code is ok - it should be possible to merge register windows into
this model but I'm leaving that as an exercise for the reader :-)


So? Well the fun beings when you go to compute a pseudo register given a
frame.  In our example, when outer()'s frame is selected. A pseudo for
that frame should be computed using:

	p[i] for frame
		== pseudo (i,
			real_register (frame, 0),
			real_register (frame, 1), ...);

which expanding ends up with:

		== pseudo (i,
			r[0],
			saved value of r[1] from inner() frame,
			saved value of r[2] from middle() frame,
			....);

Right now this just doesn't happen.  Instead:

		pseudo (i, r[0], r[1], r[2], ...)

is computed and then only if you're lucky.

	enjoy,
		Andrew

PS1: I think everything to do with fetching a register should be
parameterized with the frame it is being fetched from.  GDB currently
has one bunch of functions for accessing the registers in the register
buffer and a second set of functions for accessing the registers given a
frame.  I can't see the difference.

PS2: I think the low level register functions should return a ``struct
value''.

Re: remote watchpoint support

[Mark Salter]

>>>>> Andrew Cagney writes:

> FYI, changelog entries should be separated out and not included in the
> diff.

Yup. I think I did that the last time, but had a relapse of expediency
this time.

>> Index: remote.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/remote.c,v
>> retrieving revision 1.26
>> diff -p -r1.26 remote.c
>> *** remote.c    2000/10/23 22:49:28     1.26
>> --- remote.c    2000/11/01 21:06:29
>> *************** void _initialize_remote (void);
>> *** 204,209 ****
>> --- 204,216 ----
>> 
>> /* */
>> 
>> + /* This is set to the data address of the access causing the target
>> +  * to stop for a watchpoint. */
>> + static CORE_ADDR remote_watch_data_address;
>> +
>> + /* This is non-zero if taregt stopped for a watchpoint. */
>> + static int remote_stopped_by_watchpoint_p;
>> +

> I've strong reservations about the addition of these two ``global''
> variables.  Is it possible to modify things higher up so that their
> addition isn't required?

Without a doubt. It would just take time to sort everything out to
make sure existing support isn't broken.

> I'm also trying to understand how remote_stopped_data_address() et.al.
> gets called.

Good luck with that. ;-)

Its all part of bpstat_stop_status(). The inferior stops and gdb tries
to see if the user needs to informed, etc. bpstat_stop_status() walks
through the bp list and for write-only wathcpoints it reads the watched
memory to see if changed. For read-only and access watchpoints, it
calls target_stopped_data_address() which is a macro that usually gets
defined in a tm-xxx.h file. If we add the remote_stopped_by_watchpoint()
and remote_stopped_data_address functions, then the tm-xxx.h additions
for remote watchpoint suppport are pretty simple:

  /* Add HW watchpoint support */

  #define TARGET_HAS_HARDWARE_WATCHPOINTS

  #define TARGET_HW_WATCH_LIMIT 1

  #define TARGET_CAN_USE_HARDWARE_WATCHPOINT(type, cnt, ot) \
	xxx_check_watch_resources (type, cnt, ot)

  /* When a hardware watchpoint fires off the PC will be left at the
     instruction which caused the watchpoint.  It will be necessary for
     GDB to step over the watchpoint. */

  /* Comment out.  On this target, the memory access is issued before
     the watchpoint triggers.*/
  /*#define HAVE_STEPPABLE_WATCHPOINT 1*/

  #define STOPPED_BY_WATCHPOINT(W) \
     remote_stopped_by_watchpoint()

  /* Use these macros for watchpoint insertion/deletion.  */
  #define target_insert_watchpoint(addr, len, type) \
		remote_insert_watchpoint (addr, len, type)
  #define target_remove_watchpoint(addr, len, type) \
		remote_remove_watchpoint (addr, len, type)
  /* Use this to get the data address that triggered a watchpoint */
  #define target_stopped_data_address() \
		remote_stopped_data_address()

  extern int xxx_check_watch_resources (int type, int cnt, int ot);



>> *************** struct gdb_ext_thread_info
>> *** 963,969 ****
>> 
>> #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
>> 
>> ! char *unpack_varlen_hex (char *buff, int *result);
>> 
>> static char *unpack_nibble (char *buf, int *val);
>> 
>> --- 970,976 ----
>> 
>> #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
>> 
>> ! char *unpack_varlen_hex (char *buff, ULONGEST *result);
>> 
>> static char *unpack_nibble (char *buf, int *val);
>> 

> The ChangeLog doesn't mention this change?  Can I suggest this be
> submitted separatly with the tweek that follows:

Oops.

>> --- 2657,2682 ----
>> p, buf);
>> if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>> {
> 		          ULONGEST thread_num;
>> !                       p_temp = unpack_varlen_hex (++p1, &ul);
>> !                       thread_num = ul;
>> record_currthread (thread_num);
>> p = (unsigned char *) p_temp;
>> }

> Suggest just making threadnum a LONGEST and being done with it.

Ok.

--Mark

Re: remote watchpoint support

[Andrew Cagney]

Mark,

Did you ever get a chance to check this (after fixes) in?  Eli was ok 
with the doc changes and I think you addressed my questions.

	Andrew

> Ok. Here's a patch that allows a remote target to pass watchpoint
> information back to gdb using the T packet. It also fixes the T
> packet handling to ignore other optional info which may not be
> recognized. This changes the behavior to match the documentation.
> 
> While testing this, I discovered that GDB tries to read data from
> the watched address before the watchpoints are removed. This causes
> another watchpoint exception so the stub returns an error response
> to the read memory request.
> 
> --Mark
> 
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/ChangeLog,v
> retrieving revision 1.744
> diff -p -r1.744 ChangeLog
> *** ChangeLog	2000/10/31 19:35:03	1.744
> --- ChangeLog	2000/11/01 21:06:26
> ***************
> *** 1,3 ****
> --- 1,10 ----
> + 2000-11-01  Mark Salter  <msalter@redhat.com>
> + 
> + 	* remote.c (remote_wait): Add support to extract optional
> + 	watchpoint information from T-packet. Ignore unrecognized
> + 	optional info in T-packet.
> + 	(remote_async_wait): Ditto.
> + 
>   2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>
> * config/sh/tm-linux.h: New file.  Include generic tm-linux.h, 
> Index: remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote.c,v
> retrieving revision 1.26
> diff -p -r1.26 remote.c
> *** remote.c	2000/10/23 22:49:28	1.26
> --- remote.c	2000/11/01 21:06:29
> *************** void _initialize_remote (void);
> *** 204,209 ****
> --- 204,216 ----
>   
>   /* */
>   
> + /* This is set to the data address of the access causing the target
> +  * to stop for a watchpoint. */
> + static CORE_ADDR remote_watch_data_address;
> + 
> + /* This is non-zero if taregt stopped for a watchpoint. */
> + static int remote_stopped_by_watchpoint_p;
> + 
>   static struct target_ops remote_ops;
>   
>   static struct target_ops extended_remote_ops;
> *************** struct gdb_ext_thread_info
> *** 963,969 ****
>   
>   #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
>   
> ! char *unpack_varlen_hex (char *buff, int *result);
>   
>   static char *unpack_nibble (char *buf, int *val);
>   
> --- 970,976 ----
>   
>   #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
>   
> ! char *unpack_varlen_hex (char *buff, ULONGEST *result);
>   
>   static char *unpack_nibble (char *buf, int *val);
>   
> *************** stub_unpack_int (char *buff, int fieldle
> *** 1084,1090 ****
>   
>   char *
>   unpack_varlen_hex (char *buff,	/* packet to parse */
> ! 		   int *result)
>   {
>     int nibble;
>     int retval = 0;
> --- 1091,1097 ----
>   
>   char *
>   unpack_varlen_hex (char *buff,	/* packet to parse */
> ! 		   ULONGEST *result)
>   {
>     int nibble;
>     int retval = 0;
> *************** remote_wait (int pid, struct target_wait
> *** 2593,2598 ****
> --- 2600,2606 ----
>   {
>     unsigned char *buf = alloca (PBUFSIZ);
>     int thread_num = -1;
> +   ULONGEST ul;
>   
>     status->kind = TARGET_WAITKIND_EXITED;
>     status->value.integer = 0;
> *************** remote_wait (int pid, struct target_wait
> *** 2610,2615 ****
> --- 2618,2625 ----
>         if (target_wait_loop_hook)
>   	(*target_wait_loop_hook) ();
>   
> +       remote_stopped_by_watchpoint_p = 0;
> + 
>         switch (buf[0])
>   	{
>   	case 'E':		/* Error of some sort */
> *************** Packet: '%s'\n",
> *** 2647,2656 ****
>   			       p, buf);
>   		    if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>   		      {
> ! 			p_temp = unpack_varlen_hex (++p1, &thread_num);
>   			record_currthread (thread_num);
>   			p = (unsigned char *) p_temp;
>   		      }
>   		  }
>   		else
>   		  {
> --- 2657,2682 ----
>   			       p, buf);
>   		    if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>   		      {
> ! 			p_temp = unpack_varlen_hex (++p1, &ul);
> ! 			thread_num = ul;
>   			record_currthread (thread_num);
>   			p = (unsigned char *) p_temp;
>   		      }
> + 		    else if ((strncmp ((const char *) p, "watch", p1 - p) == 0)
> + 			     || (strncmp ((const char *) p, "rwatch", p1 - p) == 0)
> + 			     || (strncmp ((const char *) p, "awatch", p1 - p) == 0))
> + 		      {
> + 			remote_stopped_by_watchpoint_p = 1;
> + 			p = unpack_varlen_hex (++p1, &ul);
> + 			remote_watch_data_address = (CORE_ADDR)ul;
> + 		      }
> + 		    else
> + 		      {
> + 			/* silently skip unknown optional info */
> + 			p_temp = (unsigned char *) strchr ((const char *) p1+1, ';');
> + 			if (p_temp)
> + 			  p = p_temp;
> + 		      }
>   		  }
>   		else
>   		  {
> *************** remote_async_wait (int pid, struct targe
> *** 2808,2813 ****
> --- 2834,2840 ----
>   {
>     unsigned char *buf = alloca (PBUFSIZ);
>     int thread_num = -1;
> +   ULONGEST ul;
>   
>     status->kind = TARGET_WAITKIND_EXITED;
>     status->value.integer = 0;
> *************** remote_async_wait (int pid, struct targe
> *** 2831,2836 ****
> --- 2858,2865 ----
>         if (target_wait_loop_hook)
>   	(*target_wait_loop_hook) ();
>   
> +       remote_stopped_by_watchpoint_p = 0;
> + 
>         switch (buf[0])
>   	{
>   	case 'E':		/* Error of some sort */
> *************** Packet: '%s'\n",
> *** 2868,2877 ****
>   			       p, buf);
>   		    if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>   		      {
> ! 			p_temp = unpack_varlen_hex (++p1, &thread_num);
>   			record_currthread (thread_num);
>   			p = (unsigned char *) p_temp;
>   		      }
>   		  }
>   		else
>   		  {
> --- 2897,2922 ----
>   			       p, buf);
>   		    if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>   		      {
> ! 			p_temp = unpack_varlen_hex (++p1, &ul);
> ! 			thread_num = ul;
>   			record_currthread (thread_num);
>   			p = (unsigned char *) p_temp;
>   		      }
> + 		    else if ((strncmp ((const char *) p, "watch", p1 - p) == 0)
> + 			     || (strncmp ((const char *) p, "rwatch", p1 - p) == 0)
> + 			     || (strncmp ((const char *) p, "awatch", p1 - p) == 0))
> + 		      {
> + 			remote_stopped_by_watchpoint_p = 1;
> + 			p = unpack_varlen_hex (++p1, &ul);
> + 			remote_watch_data_address = (CORE_ADDR)ul;
> + 		      }
> + 		    else
> + 		      {
> + 			/* silently skip unknown optional info */
> + 			p_temp = (unsigned char *) strchr ((const char *) p1+1, ';');
> + 			if (p_temp)
> + 			  p = p_temp;
> + 		      }
>   		  }
>   		else
>   		  {
> *************** remote_remove_hw_breakpoint (CORE_ADDR a
> *** 4484,4489 ****
> --- 4529,4551 ----
>       }
>     internal_error ("remote_remove_watchpoint: reached end of function");
>   }
> + 
> + /* Returns data address if stopped by watchpoint.
> +    Zero otherwise. */
> + CORE_ADDR
> + remote_stopped_data_address (void)
> + {
> +     if (remote_stopped_by_watchpoint_p)
> + 	return remote_watch_data_address;
> +     return (CORE_ADDR)0;
> + }
> + 
> + int
> + remote_stopped_by_watchpoint(void)
> + {
> +     return remote_stopped_by_watchpoint_p;
> + }
> + 
>   
>   /* Some targets are only capable of doing downloads, and afterwards
>      they switch to the remote serial protocol.  This function provides
> Index: doc/ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
> retrieving revision 1.51
> diff -p -r1.51 ChangeLog
> *** ChangeLog	2000/10/16 07:34:02	1.51
> --- ChangeLog	2000/11/01 21:06:31
> ***************
> *** 1,3 ****
> --- 1,8 ----
> + 2000-11-01  Mark Salter  <msalter@redhat.com>
> + 
> + 	* gdb.texinfo (Protocol): Document T packet extension to 
> + 	allow watchpoint address reporting.
> + 
>   2000-10-16  Eli Zaretskii  <eliz@is.elta.co.il>
> * gdb.texinfo (Contributors, MIPS Embedded): Minor spelling
> Index: doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.28
> diff -p -r1.28 gdb.texinfo
> *** gdb.texinfo	2000/10/16 07:34:02	1.28
> --- gdb.texinfo	2000/11/01 21:06:40
> *************** conventions is used.
> *** 9390,9399 ****
>   @var{AA} = two hex digit signal number; @var{n...} = register number
>   (hex), @var{r...}  = target byte ordered register contents, size defined
>   by @code{REGISTER_RAW_SIZE}; @var{n...} = @samp{thread}, @var{r...} =
> ! thread process ID, this is a hex integer; @var{n...} = other string not
> ! starting with valid hex digit.  @value{GDBN} should ignore this
> ! @var{n...}, @var{r...} pair and go on to the next.  This way we can
> ! extend the protocol.
>   
>   @item @code{W}@var{AA}
>   @tab
> --- 9390,9400 ----
>   @var{AA} = two hex digit signal number; @var{n...} = register number
>   (hex), @var{r...}  = target byte ordered register contents, size defined
>   by @code{REGISTER_RAW_SIZE}; @var{n...} = @samp{thread}, @var{r...} =
> ! thread process ID, this is a hex integer; @var{n...} = (@samp{watch} | 
> ! @samp{rwatch} | @samp{awatch}, @var{r...} = data address, this is a hex
> ! integer; @var{n...} = other string not starting with valid hex digit.
> ! @value{GDBN} should ignore this @var{n...}, @var{r...} pair and go on
> ! to the next.  This way we can extend the protocol.
>   
>   @item @code{W}@var{AA}
>   @tab
> 
> 

Re: remote watchpoint support

[Mark Salter]

No. There was some concern about the globals (remote_watch_data_address
and remote_stopped_by_watchpoint_p) and I never got the green light. The
globals are ugly and I think we could get rid of them with a little more
effort. I just haven't had time to pursue that.

--Mark


> Mark,
> Did you ever get a chance to check this (after fixes) in?  Eli was ok 
> with the doc changes and I think you addressed my questions.

> 	Andrew

>> Ok. Here's a patch that allows a remote target to pass watchpoint
>> information back to gdb using the T packet. It also fixes the T
>> packet handling to ignore other optional info which may not be
>> recognized. This changes the behavior to match the documentation.
>> 
>> While testing this, I discovered that GDB tries to read data from
>> the watched address before the watchpoints are removed. This causes
>> another watchpoint exception so the stub returns an error response
>> to the read memory request.
>> 
>> --Mark
>> 
>> 
>> Index: ChangeLog
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/ChangeLog,v
>> retrieving revision 1.744
>> diff -p -r1.744 ChangeLog
>> *** ChangeLog	2000/10/31 19:35:03	1.744
>> --- ChangeLog	2000/11/01 21:06:26
>> ***************
>> *** 1,3 ****
>> --- 1,10 ----
>> + 2000-11-01  Mark Salter  <msalter@redhat.com>
>> + 
>> + 	* remote.c (remote_wait): Add support to extract optional
>> + 	watchpoint information from T-packet. Ignore unrecognized
>> + 	optional info in T-packet.
>> + 	(remote_async_wait): Ditto.
>> + 
>> 2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>
>> * config/sh/tm-linux.h: New file.  Include generic tm-linux.h, 
>> Index: remote.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/remote.c,v
>> retrieving revision 1.26
>> diff -p -r1.26 remote.c
>> *** remote.c	2000/10/23 22:49:28	1.26
>> --- remote.c	2000/11/01 21:06:29
>> *************** void _initialize_remote (void);
>> *** 204,209 ****
>> --- 204,216 ----
>> 
>> /* */
>> 
>> + /* This is set to the data address of the access causing the target
>> +  * to stop for a watchpoint. */
>> + static CORE_ADDR remote_watch_data_address;
>> + 
>> + /* This is non-zero if taregt stopped for a watchpoint. */
>> + static int remote_stopped_by_watchpoint_p;
>> + 
>> static struct target_ops remote_ops;
>> 
>> static struct target_ops extended_remote_ops;
>> *************** struct gdb_ext_thread_info
>> *** 963,969 ****
>> 
>> #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
>> 
>> ! char *unpack_varlen_hex (char *buff, int *result);
>> 
>> static char *unpack_nibble (char *buf, int *val);
>> 
>> --- 970,976 ----
>> 
>> #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
>> 
>> ! char *unpack_varlen_hex (char *buff, ULONGEST *result);
>> 
>> static char *unpack_nibble (char *buf, int *val);
>> 
>> *************** stub_unpack_int (char *buff, int fieldle
>> *** 1084,1090 ****
>> 
>> char *
>> unpack_varlen_hex (char *buff,	/* packet to parse */
>> ! 		   int *result)
>> {
>> int nibble;
>> int retval = 0;
>> --- 1091,1097 ----
>> 
>> char *
>> unpack_varlen_hex (char *buff,	/* packet to parse */
>> ! 		   ULONGEST *result)
>> {
>> int nibble;
>> int retval = 0;
>> *************** remote_wait (int pid, struct target_wait
>> *** 2593,2598 ****
>> --- 2600,2606 ----
>> {
>> unsigned char *buf = alloca (PBUFSIZ);
>> int thread_num = -1;
>> +   ULONGEST ul;
>> 
status-> kind = TARGET_WAITKIND_EXITED;
status-> value.integer = 0;
>> *************** remote_wait (int pid, struct target_wait
>> *** 2610,2615 ****
>> --- 2618,2625 ----
>> if (target_wait_loop_hook)
>> (*target_wait_loop_hook) ();
>> 
>> +       remote_stopped_by_watchpoint_p = 0;
>> + 
>> switch (buf[0])
>> {
>> case 'E':		/* Error of some sort */
>> *************** Packet: '%s'\n",
>> *** 2647,2656 ****
>> p, buf);
>> if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>> {
>> ! 			p_temp = unpack_varlen_hex (++p1, &thread_num);
>> record_currthread (thread_num);
>> p = (unsigned char *) p_temp;
>> }
>> }
>> else
>> {
>> --- 2657,2682 ----
>> p, buf);
>> if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>> {
>> ! 			p_temp = unpack_varlen_hex (++p1, &ul);
>> ! 			thread_num = ul;
>> record_currthread (thread_num);
>> p = (unsigned char *) p_temp;
>> }
>> + 		    else if ((strncmp ((const char *) p, "watch", p1 - p) == 0)
>> + 			     || (strncmp ((const char *) p, "rwatch", p1 - p) == 0)
>> + 			     || (strncmp ((const char *) p, "awatch", p1 - p) == 0))
>> + 		      {
>> + 			remote_stopped_by_watchpoint_p = 1;
>> + 			p = unpack_varlen_hex (++p1, &ul);
>> + 			remote_watch_data_address = (CORE_ADDR)ul;
>> + 		      }
>> + 		    else
>> + 		      {
>> + 			/* silently skip unknown optional info */
>> + 			p_temp = (unsigned char *) strchr ((const char *) p1+1, ';');
>> + 			if (p_temp)
>> + 			  p = p_temp;
>> + 		      }
>> }
>> else
>> {
>> *************** remote_async_wait (int pid, struct targe
>> *** 2808,2813 ****
>> --- 2834,2840 ----
>> {
>> unsigned char *buf = alloca (PBUFSIZ);
>> int thread_num = -1;
>> +   ULONGEST ul;
>> 
status-> kind = TARGET_WAITKIND_EXITED;
status-> value.integer = 0;
>> *************** remote_async_wait (int pid, struct targe
>> *** 2831,2836 ****
>> --- 2858,2865 ----
>> if (target_wait_loop_hook)
>> (*target_wait_loop_hook) ();
>> 
>> +       remote_stopped_by_watchpoint_p = 0;
>> + 
>> switch (buf[0])
>> {
>> case 'E':		/* Error of some sort */
>> *************** Packet: '%s'\n",
>> *** 2868,2877 ****
>> p, buf);
>> if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>> {
>> ! 			p_temp = unpack_varlen_hex (++p1, &thread_num);
>> record_currthread (thread_num);
>> p = (unsigned char *) p_temp;
>> }
>> }
>> else
>> {
>> --- 2897,2922 ----
>> p, buf);
>> if (strncmp ((const char *) p, "thread", p1 - p) == 0)
>> {
>> ! 			p_temp = unpack_varlen_hex (++p1, &ul);
>> ! 			thread_num = ul;
>> record_currthread (thread_num);
>> p = (unsigned char *) p_temp;
>> }
>> + 		    else if ((strncmp ((const char *) p, "watch", p1 - p) == 0)
>> + 			     || (strncmp ((const char *) p, "rwatch", p1 - p) == 0)
>> + 			     || (strncmp ((const char *) p, "awatch", p1 - p) == 0))
>> + 		      {
>> + 			remote_stopped_by_watchpoint_p = 1;
>> + 			p = unpack_varlen_hex (++p1, &ul);
>> + 			remote_watch_data_address = (CORE_ADDR)ul;
>> + 		      }
>> + 		    else
>> + 		      {
>> + 			/* silently skip unknown optional info */
>> + 			p_temp = (unsigned char *) strchr ((const char *) p1+1, ';');
>> + 			if (p_temp)
>> + 			  p = p_temp;
>> + 		      }
>> }
>> else
>> {
>> *************** remote_remove_hw_breakpoint (CORE_ADDR a
>> *** 4484,4489 ****
>> --- 4529,4551 ----
>> }
>> internal_error ("remote_remove_watchpoint: reached end of function");
>> }
>> + 
>> + /* Returns data address if stopped by watchpoint.
>> +    Zero otherwise. */
>> + CORE_ADDR
>> + remote_stopped_data_address (void)
>> + {
>> +     if (remote_stopped_by_watchpoint_p)
>> + 	return remote_watch_data_address;
>> +     return (CORE_ADDR)0;
>> + }
>> + 
>> + int
>> + remote_stopped_by_watchpoint(void)
>> + {
>> +     return remote_stopped_by_watchpoint_p;
>> + }
>> + 
>> 
>> /* Some targets are only capable of doing downloads, and afterwards
>> they switch to the remote serial protocol.  This function provides
>> Index: doc/ChangeLog
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
>> retrieving revision 1.51
>> diff -p -r1.51 ChangeLog
>> *** ChangeLog	2000/10/16 07:34:02	1.51
>> --- ChangeLog	2000/11/01 21:06:31
>> ***************
>> *** 1,3 ****
>> --- 1,8 ----
>> + 2000-11-01  Mark Salter  <msalter@redhat.com>
>> + 
>> + 	* gdb.texinfo (Protocol): Document T packet extension to 
>> + 	allow watchpoint address reporting.
>> + 
>> 2000-10-16  Eli Zaretskii  <eliz@is.elta.co.il>
>> * gdb.texinfo (Contributors, MIPS Embedded): Minor spelling
>> Index: doc/gdb.texinfo
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
>> retrieving revision 1.28
>> diff -p -r1.28 gdb.texinfo
>> *** gdb.texinfo	2000/10/16 07:34:02	1.28
>> --- gdb.texinfo	2000/11/01 21:06:40
>> *************** conventions is used.
>> *** 9390,9399 ****
>> @var{AA} = two hex digit signal number; @var{n...} = register number
>> (hex), @var{r...}  = target byte ordered register contents, size defined
>> by @code{REGISTER_RAW_SIZE}; @var{n...} = @samp{thread}, @var{r...} =
>> ! thread process ID, this is a hex integer; @var{n...} = other string not
>> ! starting with valid hex digit.  @value{GDBN} should ignore this
>> ! @var{n...}, @var{r...} pair and go on to the next.  This way we can
>> ! extend the protocol.
>> 
>> @item @code{W}@var{AA}
>> @tab
>> --- 9390,9400 ----
>> @var{AA} = two hex digit signal number; @var{n...} = register number
>> (hex), @var{r...}  = target byte ordered register contents, size defined
>> by @code{REGISTER_RAW_SIZE}; @var{n...} = @samp{thread}, @var{r...} =
>> ! thread process ID, this is a hex integer; @var{n...} = (@samp{watch} | 
>> ! @samp{rwatch} | @samp{awatch}, @var{r...} = data address, this is a hex
>> ! integer; @var{n...} = other string not starting with valid hex digit.
>> ! @value{GDBN} should ignore this @var{n...}, @var{r...} pair and go on
>> ! to the next.  This way we can extend the protocol.
>> 
>> @item @code{W}@var{AA}
>> @tab
>> 
>> 

Re: Remote watchpoint support.

[Michael Snyder]

Mark Salter wrote:
> 
> >>>>> Michael Snyder writes:
> 
> > Mark Salter wrote:
> >>
> >> I'm adding hw break/watchpoint support to a remote target. I'm using
> >> the Z-packet support that is in remote.c to install/remove the
> >> watchpoints and breakpoints. When a watchpoint is triggered by a
> >> read access, GDB needs a way to tell if the target stopped because
> >> of the watchpoint. This is done through a target specific function:
> >>
> >> CORE_ADDR target_stopped_data_address(void)
> >>
> >> which returns the address of the data access which triggered the
> >> watchpoint. If the target did not stop because of a watchpoint,
> >> target_stopped_data_address should return zero. Past implementations
> >> of watchpoints for remote targets have relied on special registers
> >> returned in the 'g' packet to determine the data address.
> >>
> >> Rather than having gdb deal with the debug support registers directly,
> >> I would like to add a remote protocol packet that can be used to
> >> query the target for this address.
> >>
> >> Comments?
> >>
> >> --Mark
> 
> > That really sounds like more of a gdb thing.  xxx-tdep.c should have
> > the knowledge about the registers, though, not remote.c.
> 
> I'm trying to be consistent. Gdb already supports installing and deleting
> hw breakpoints and watchpoints via the Z packet in remote.c. Because the
> watchpoint support also needs to get the data address, it seemed right
> to do that through a packet in remote.c also.
> 
> I'm OK with putting the work of watchpoint support on the gdb side, its less
> work for me on the target side. But if we're going to add knowledge of debug
> support registers into gdb, then we might as well do the installing and
> removing of watchpoints via that mechanism instead of messing with a Z packet.

Oh.  Yes, I see what you mean.  Just ignore me.
J.T. should comment on this.  And maybe Fernando --
weren't you involved with the "z" packet, Fernando?
From jtc@redback.com Mon Oct 30 12:39:00 2000
From: jtc@redback.com (J.T. Conklin)
To: Mark Salter <msalter@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Mon, 30 Oct 2000 12:39:00 -0000
Message-id: <5mg0leoyg6.fsf@jtc.redback.com>
References: <200010301416.e9UEG6m18981@deneb.localdomain>
X-SW-Source: 2000-10/msg00279.html
Content-length: 2732

>>>>> "Mark" == Mark Salter <msalter@redhat.com> writes:
Mark> I'm adding hw break/watchpoint support to a remote target. I'm using
Mark> the Z-packet support that is in remote.c to install/remove the
Mark> watchpoints and breakpoints. When a watchpoint is triggered by a
Mark> read access, GDB needs a way to tell if the target stopped because 
Mark> of the watchpoint. This is done through a target specific function:
Mark>
Mark>   CORE_ADDR target_stopped_data_address(void)
Makr>
Mark> which returns the address of the data access which triggered the
Mark> watchpoint. If the target did not stop because of a watchpoint,
Mark> target_stopped_data_address should return zero. Past implementations
Mark> of watchpoints for remote targets have relied on special registers
Mark> returned in the 'g' packet to determine the data address.
Mark>
Mark> Rather than having gdb deal with the debug support registers directly,
Mark> I would like to add a remote protocol packet that can be used to
Mark> query the target for this address.

I was unware of this macro, yet we've been using hardware watchpoints
on a i386 target using the remote protocol for about a year.  Now that
I've examined the code in breakpoint.c which invokes it, I must assume
that no one has tried to use read or access watchpoints.

I'm not sure I like the protocol change though.  

One thing I dislike about it is that the debug agent never explicitly
tells GDB that a watchpoint has been hit, but rather it is implicitly
coded in the return value of the query.  

The second is the return value of 0 if the target stopped for another
reason.  I (now) know that this is the current behavior of the target_
stopped_data_address() macro, but it use the one address that I think
might be very important to watch on targets where the zero page isn't
unmapped (in which case a reference would cause a exception/trap).
While it might not be possible to fix in GDB's internals for some
time, preserving it in the remote protocol for all time seems like a 
mistake.

I think the way I'd go about this would be to add additional return
codes.  In earlier messages, I have mentioned it would be desirable
for GDB to represent breakpoints and single steps distinctly from
exceptions (e.g., adding a TARGET_WAITKIND_BREAKPOINT and TARGET_
WAITKIND_STEP).  I don't know when (or if) we'll ever get around to
that, but now that I think about it there is no reason why external
protocols could not do so 'ahead' of GDB itself.  For the time being,
gdb would have to re-multiplex them into an SIGTRAP exception, but it
could also extract additional information (such as a watchpoint data
address) at the same time.

Thoughts?

        --jtc

-- 
J.T. Conklin
RedBack Networks
From fnasser@cygnus.com Mon Oct 30 12:53:00 2000
From: Fernando Nasser <fnasser@cygnus.com>
To: Larry Smith <larry@smith-house.org>
Cc: gdb-patches@sources.redhat.com, insight@sources.redhat.com
Subject: Re: [RFA] 100494 fix
Date: Mon, 30 Oct 2000 12:53:00 -0000
Message-id: <39FDDFB0.D995A63E@cygnus.com>
References: <200010272126.RAA02584@ozma.smith-house.org>
X-SW-Source: 2000-10/msg00280.html
Content-length: 2357

Good catch Larry.  But please add a labeled frame around the radiobuttons,
will you?  The target selection dialog expansion box does look weird with
two square buttons and two rounds with no explanation.

"Run method" sounds right for the frame label.

If you really want to fix this, make sure the option is set to "run" and
the buttons greyed if the target "exec" is selected.

Cheers,
Fernando

P.S.: Insight patches go to the insight list, not gdb-patches as the people 
that wrote the code watch it more closely and there is a larger group of
users and contributors in this list that do not subscribe to gdb-patches.




Larry Smith wrote:
> 
> The following patch is for 100494, changing a pair of checkbuttons
> to radiobuttons.  Since the proper behaviour was already enforced
> this is really only cosmetic, and changes no current control flow.
> 
> I apologise for being unaware of proper protocol for doing patches
> on sourceware, this has already been checked in.  I will try to be
> more circumspect in the future...
> 
> regards,
> Larry
> 
> 2000-10-26  Larry Smith  <lsmith@redhat.com>
> 
>         * change targetselection.itb: Run Program and Continue From Last Stop
>         are now radio buttons rather than checkbuttons
> 
> Index: targetselection.itb
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/targetselection.itb,v
> retrieving revision 1.4
> retrieving revision 1.5
> diff -u -r1.4 -r1.5
> --- targetselection.itb 2000/09/12 20:04:11     1.4
> +++ targetselection.itb 2000/10/26 20:47:57     1.5
> @@ -488,11 +488,11 @@
>    checkbutton $frame.load -text {Download Program} -variable $var
> 
>    set var [pref varname gdb/src/run_cont]
> -  checkbutton $frame.cont -text {Continue from Last Stop} -variable $var \
> +  radiobutton $frame.cont -text {Continue from Last Stop} -value 1 -variable $var \
>      -command [code $this set_run run]
> 
>    set var [pref varname gdb/src/run_run]
> -  checkbutton $frame.run -text {Run Program} -variable $var \
> +  radiobutton $frame.run -text {Run Program} -value 1 -variable $var \
>      -command [code $this set_run cont]
> 
>    # The after attaching command entry

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
From jtc@redback.com Mon Oct 30 13:03:00 2000
From: jtc@redback.com (J.T. Conklin)
To: Michael Snyder <msnyder@redhat.com>
Cc: Mark Salter <msalter@redhat.com>, gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Mon, 30 Oct 2000 13:03:00 -0000
Message-id: <5m66maoxdo.fsf@jtc.redback.com>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <39FDCF55.46BD@redhat.com>
X-SW-Source: 2000-10/msg00281.html
Content-length: 1066

>>>>> "Michael" == Michael Snyder <msnyder@redhat.com> writes:

>> Mark Salter wrote:
>> I'm adding hw break/watchpoint support to a remote target. I'm using
>> the Z-packet support that is in remote.c to install/remove the
>> watchpoints and breakpoints. When a watchpoint is triggered by a
>> read access, GDB needs a way to tell if the target stopped because
>> of the watchpoint. This is done through a target specific function:

Michael> That really sounds like more of a gdb thing.  xxx-tdep.c
Michael> should have the knowledge about the registers, though, not
Michael> remote.c.

Well, that's one thing that Mark's change has going for it.  Like the
"Z" packet, the proposed "qWaddr" packet knows nothing about how the
target implements hardware break/watchpoints.  Those details are left
up to the debug agent.  While it's probably processor debug registers,
it could be an external device that speaks the remote debug protocol
and talks to the target over BDM or some other on-chip-debugging tech-
nology.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From brolley@redhat.com Mon Oct 30 13:17:00 2000
From: Dave Brolley <brolley@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: Patch:RFA:sim testsuite: Options Embedded in Test Cases
Date: Mon, 30 Oct 2000 13:17:00 -0000
Message-id: <39FDE57F.A237E2A5@redhat.com>
X-SW-Source: 2000-10/msg00282.html
Content-length: 760

Hi,

The attached patch addresses two issues:

1) Yet another attempt at passing the correct cpu to the assembler
when more than one machine is being tested. This time I've defined a
global variable 'cpu_option' which the caller can set to indicate the
name of the option which specifies the cpu.

2) Resetting options between test cases. Currently options from
previous test cases are not cleared and sometimes they leak into
subsequent tests. Specifically unsetting them seems to be the only way
to make the "if [info exists <option>]" tests work properly.

Tested against m32r (which does not specify 'cpu_option', but has more
than one machine) and two internal ports (one of which uses
'cpu_option' and has mutiple machines to test).

OK to commit?

Dave
From fnasser@cygnus.com Mon Oct 30 14:19:00 2000
From: Fernando Nasser <fnasser@cygnus.com>
To: Michael Snyder <msnyder@redhat.com>
Cc: Mark Salter <msalter@redhat.com>, gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Mon, 30 Oct 2000 14:19:00 -0000
Message-id: <39FDF3FB.12F62A08@cygnus.com>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <39FDCF55.46BD@redhat.com> <200010302018.e9UKIr300876@deneb.localdomain> <39FDD845.680@redhat.com>
X-SW-Source: 2000-10/msg00283.html
Content-length: 2597

Michael Snyder wrote:
> 
> Mark Salter wrote:
> >
> > >>>>> Michael Snyder writes:
> >
> > > Mark Salter wrote:
> > >>
> > >> I'm adding hw break/watchpoint support to a remote target. I'm using
> > >> the Z-packet support that is in remote.c to install/remove the
> > >> watchpoints and breakpoints. When a watchpoint is triggered by a
> > >> read access, GDB needs a way to tell if the target stopped because
> > >> of the watchpoint. This is done through a target specific function:
> > >>
> > >> CORE_ADDR target_stopped_data_address(void)
> > >>
> > >> which returns the address of the data access which triggered the
> > >> watchpoint. If the target did not stop because of a watchpoint,
> > >> target_stopped_data_address should return zero. Past implementations
> > >> of watchpoints for remote targets have relied on special registers
> > >> returned in the 'g' packet to determine the data address.
> > >>
> > >> Rather than having gdb deal with the debug support registers directly,
> > >> I would like to add a remote protocol packet that can be used to
> > >> query the target for this address.
> > >>
> > >> Comments?
> > >>
> > >> --Mark
> >
> > > That really sounds like more of a gdb thing.  xxx-tdep.c should have
> > > the knowledge about the registers, though, not remote.c.
> >
> > I'm trying to be consistent. Gdb already supports installing and deleting
> > hw breakpoints and watchpoints via the Z packet in remote.c. Because the
> > watchpoint support also needs to get the data address, it seemed right
> > to do that through a packet in remote.c also.
> >
> > I'm OK with putting the work of watchpoint support on the gdb side, its less
> > work for me on the target side. But if we're going to add knowledge of debug
> > support registers into gdb, then we might as well do the installing and
> > removing of watchpoints via that mechanism instead of messing with a Z packet.
> 
> Oh.  Yes, I see what you mean.  Just ignore me.
> J.T. should comment on this.  And maybe Fernando --
> weren't you involved with the "z" packet, Fernando?

Yes, I am proposing a gdbarch entry + a target vector entry to deal with
the differences in how things are supported.  My goal was only to be able to
tell the user if we had resources for hardware watchpoints at the time he/she
requested instead of failing when the inferior is run or continued.  I need
it for the GUI.

It seems a little bit orthogonal to what Mark is requesting.




-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
From msnyder@cygnus.com Mon Oct 30 14:30:00 2000
From: Michael Snyder <msnyder@cygnus.com>
To: gdb-patches@sourceware.cygnus.com, kbuettner@cygnus.com
Subject: [PATCH]: Move solib from NATDEPFILES to TDEPFILES for linux.
Date: Mon, 30 Oct 2000 14:30:00 -0000
Message-id: <200010302230.OAA24665@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00284.html
Content-length: 7146

The following change could probably be made for all OS's, but
for now I'll just make it for Linux.  I have moved the modules
solib.c and solib-svr4.c from NATDEPFILES to TDEPFILES, so that
cross-debugging onto a Linux target can support shared libraries.

2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>

        * config/m68k/linux.mh: Remove solib.c, solib-svr4.c from NATDEPFILES.
        * config/powerpc/linux.mh: ditto.
        * config/ia64/linux.mh: ditto.
        * config/i386/linux.mh: ditto.
        * config/alpha/alpha-linux.mh: ditto.
        * config/arm/linux.mh: ditto.
        * config/m68k/linux.mt: Add solib.c, solib-svr4.c to TDEPFILES.
        * config/powerpc/linux.mt: ditto.
        * config/ia64/linux.mt: ditto.
        * config/i386/linux.mt: ditto.
        * config/alpha/alpha-linux.mt: ditto.
        * config/arm/linux.mt: ditto.

Index: config/m68k/linux.mh
===================================================================
RCS file: /cvs/src/src/gdb/config/m68k/linux.mh,v
retrieving revision 1.3
diff -u -p -r1.3 linux.mh
--- linux.mh	2000/10/24 20:05:35	1.3
+++ linux.mh	2000/10/30 22:28:05
@@ -4,7 +4,7 @@ XM_FILE= xm-linux.h
 XDEPFILES=
 
 NAT_FILE= nm-linux.h
-NATDEPFILES= infptrace.o solib.o solib-svr4.o inftarg.o fork-child.o \
+NATDEPFILES= infptrace.o inftarg.o fork-child.o \
 	corelow.o core-aout.o core-regset.o m68klinux-nat.o linux-thread.o
 
 GDBSERVER_DEPFILES= low-linux.o
Index: config/m68k/linux.mt
===================================================================
RCS file: /cvs/src/src/gdb/config/m68k/linux.mt,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 linux.mt
--- linux.mt	1999/04/16 01:34:21	1.1.1.1
+++ linux.mt	2000/10/30 22:28:05
@@ -1,3 +1,3 @@
 # Target: Motorola m68k with a.out and ELF
-TDEPFILES= m68k-tdep.o
+TDEPFILES= m68k-tdep.o solib.o solib-svr4.o 
 TM_FILE= tm-linux.h
Index: config/powerpc/linux.mh
===================================================================
RCS file: /cvs/src/src/gdb/config/powerpc/linux.mh,v
retrieving revision 1.5
diff -u -p -r1.5 linux.mh
--- linux.mh	2000/10/24 20:05:36	1.5
+++ linux.mh	2000/10/30 22:28:05
@@ -5,7 +5,7 @@ XDEPFILES=
 XM_CLIBS=
 
 NAT_FILE= nm-linux.h
-NATDEPFILES= infptrace.o solib.o solib-svr4.o inftarg.o fork-child.o corelow.o \
+NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o \
 core-aout.o core-regset.o ppc-linux-nat.o proc-service.o thread-db.o lin-lwp.o
 
 LOADLIBES = -ldl -rdynamic
Index: config/powerpc/linux.mt
===================================================================
RCS file: /cvs/src/src/gdb/config/powerpc/linux.mt,v
retrieving revision 1.1
diff -u -p -r1.1 linux.mt
--- linux.mt	2000/02/22 01:19:11	1.1
+++ linux.mt	2000/10/30 22:28:05
@@ -1,3 +1,3 @@
 # Target: Motorola PPC on Linux
-TDEPFILES= rs6000-tdep.o ppc-linux-tdep.o
+TDEPFILES= rs6000-tdep.o ppc-linux-tdep.o solib.o solib-svr4.o 
 TM_FILE= tm-linux.h
Index: config/ia64/linux.mh
===================================================================
RCS file: /cvs/src/src/gdb/config/ia64/linux.mh,v
retrieving revision 1.4
diff -u -p -r1.4 linux.mh
--- linux.mh	2000/10/24 20:05:35	1.4
+++ linux.mh	2000/10/30 22:28:05
@@ -4,7 +4,7 @@ XM_FILE= xm-linux.h
 XDEPFILES=
 
 NAT_FILE= nm-linux.h
-NATDEPFILES= infptrace.o solib.o solib-svr4.o inftarg.o fork-child.o corelow.o \
+NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o \
 	core-aout.o core-regset.o ia64-linux-nat.o linux-thread.o lin-thread.o
 
 LOADLIBES = -ldl -rdynamic
Index: config/ia64/linux.mt
===================================================================
RCS file: /cvs/src/src/gdb/config/ia64/linux.mt,v
retrieving revision 1.2
diff -u -p -r1.2 linux.mt
--- linux.mt	2000/04/25 06:36:52	1.2
+++ linux.mt	2000/10/30 22:28:05
@@ -1,5 +1,5 @@
 # Target: Intel IA-64 running GNU/Linux
-TDEPFILES= ia64-tdep.o ia64-linux-tdep.o
+TDEPFILES= ia64-tdep.o ia64-linux-tdep.o solib.o solib-svr4.o 
 TM_FILE= tm-linux.h
 
 GDBSERVER_DEPFILES= low-linux.o
Index: config/i386/linux.mh
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/linux.mh,v
retrieving revision 1.5
diff -u -p -r1.5 linux.mh
--- linux.mh	2000/10/24 20:05:35	1.5
+++ linux.mh	2000/10/30 22:28:05
@@ -4,7 +4,7 @@ XM_FILE= xm-linux.h
 XDEPFILES=
 
 NAT_FILE= nm-linux.h
-NATDEPFILES= infptrace.o solib.o solib-svr4.o inftarg.o fork-child.o corelow.o \
+NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o \
 	core-aout.o i386v-nat.o i386-linux-nat.o i387-nat.o \
 	proc-service.o thread-db.o lin-lwp.o
 
Index: config/i386/linux.mt
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/linux.mt,v
retrieving revision 1.2
diff -u -p -r1.2 linux.mt
--- linux.mt	2000/03/20 20:38:29	1.2
+++ linux.mt	2000/10/30 22:28:05
@@ -1,5 +1,5 @@
 # Target: Intel 386 running GNU/Linux
-TDEPFILES= i386-tdep.o i386-linux-tdep.o i387-tdep.o
+TDEPFILES= i386-tdep.o i386-linux-tdep.o i387-tdep.o solib.o solib-svr4.o 
 TM_FILE= tm-linux.h
 
 GDBSERVER_DEPFILES= low-linux.o
Index: config/alpha/alpha-linux.mh
===================================================================
RCS file: /cvs/src/src/gdb/config/alpha/alpha-linux.mh,v
retrieving revision 1.4
diff -u -p -r1.4 alpha-linux.mh
--- alpha-linux.mh	2000/10/24 20:05:35	1.4
+++ alpha-linux.mh	2000/10/30 22:28:05
@@ -3,7 +3,7 @@ XDEPFILES=
 XM_FILE= xm-alphalinux.h
 NAT_FILE= nm-linux.h
 NATDEPFILES= infptrace.o inftarg.o corelow.o alpha-nat.o \
-	fork-child.o solib.o solib-svr4.o linux-thread.o lin-thread.o 
+	fork-child.o linux-thread.o lin-thread.o 
 
 LOADLIBES = -ldl -rdynamic
 
Index: config/alpha/alpha-linux.mt
===================================================================
RCS file: /cvs/src/src/gdb/config/alpha/alpha-linux.mt,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 alpha-linux.mt
--- alpha-linux.mt	1999/04/16 01:34:14	1.1.1.1
+++ alpha-linux.mt	2000/10/30 22:28:05
@@ -1,3 +1,3 @@
 # Target: Little-endian Alpha
-TDEPFILES= alpha-tdep.o
+TDEPFILES= alpha-tdep.o solib.o solib-svr4.o 
 TM_FILE= tm-alphalinux.h
Index: config/arm/linux.mh
===================================================================
RCS file: /cvs/src/src/gdb/config/arm/linux.mh,v
retrieving revision 1.5
diff -u -p -r1.5 linux.mh
--- linux.mh	2000/10/24 20:05:35	1.5
+++ linux.mh	2000/10/30 22:28:05
@@ -4,7 +4,7 @@ XM_FILE= xm-linux.h
 XDEPFILES=
 
 NAT_FILE= nm-linux.h
-NATDEPFILES= infptrace.o solib.o solib-svr4.o inftarg.o fork-child.o corelow.o \
+NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o \
 	     core-regset.o arm-linux-nat.o linux-thread.o lin-thread.o
 
 LOADLIBES= -ldl -rdynamic
Index: config/arm/linux.mt
===================================================================
RCS file: /cvs/src/src/gdb/config/arm/linux.mt,v
retrieving revision 1.2
diff -u -p -r1.2 linux.mt
--- linux.mt	2000/04/05 15:38:05	1.2
+++ linux.mt	2000/10/30 22:28:05
@@ -1,5 +1,5 @@
 # Target: ARM based machine running GNU/Linux
 TM_FILE= tm-linux.h
-TDEPFILES= arm-tdep.o arm-linux-tdep.o
+TDEPFILES= arm-tdep.o arm-linux-tdep.o solib.o solib-svr4.o 
 
 GDBSERVER_DEPFILES= low-linux.o
From jimb@zwingli.cygnus.com Mon Oct 30 15:00:00 2000
From: Jim Blandy <jimb@zwingli.cygnus.com>
To: Kevin Buettner <kevinb@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH RFA] solib.c relocation improvements
Date: Mon, 30 Oct 2000 15:00:00 -0000
Message-id: <np8zr6hr43.fsf@zwingli.cygnus.com>
References: <1001029002907.ZM8686@ocotillo.lan>
X-SW-Source: 2000-10/msg00285.html
Content-length: 14259

This change is approved.

Kevin Buettner <kevinb@cygnus.com> writes:

> 
> The patch below is based on feedback from Peter Schauer regarding my
> solib.c reorg patch of last weekend.  Specifically, Peter described
> some additional cleanup that would be necessary for making solib.c
> useful for platforms aside from the ones already using it.  I used the
> following remarks from Peter as an outline for making the changes
> contained in the patch:
> 
> > - This code from solib.c:solib_map_sections
> > 
> >   for (p = so->sections; p < so->sections_end; p++)
> >     {
> >       /* Relocate the section binding addresses as recorded in the shared
> >          object's file by the base address to which the object was actually
> >          mapped. */
> >       p->addr += TARGET_SO_LM_ADDR (so);
> >       p->endaddr += TARGET_SO_LM_ADDR (so);
> >       so->lmend = max (p->endaddr, so->lmend);
> >       if (STREQ (p->the_bfd_section->name, ".text"))
> >         {
> >           so->textsection = p;
> >         }
> >     }
> > 
> >   might be too specialized for future targets, as it assumes that all sections
> >   are loaded with the same offset (perhaps I will find some time someday
> >   to convert the horrible AIX shlib code to your scheme, and then we will
> >   definitely need it).
> >   It might be better to perform the relocation via a target_so_ops entry.
> > 
> > - I am not sure if we shouldn't get rid of the TARGET_SO_LM_ADDR and
> >   so->lmend crap altogether.
> > 
> >   solib_address could examine so->sections.
> > 
> >   info_sharedlibrary_command could put out the addresses of the textsection
> >   (or of all sections, perhaps via a new `maint info sharedlibrary' command,
> >   which I could have used quite often in the past).
> > 
> >   The lowest section handling in symbol_add_stub is questionable anyway
> >   and needs some rethinking. I put in some debug output and ran the testsuite
> >   on Solaris2 and Linux x86 targets, and it seems that all the business with
> >   hacking sap->other[lowest_index].addr is not needed anyway, as
> >   sap->other[lowest_index].addr == lowest_addr in all test cases.
> >   I no longer have a SunOS4 platform handy, so perhaps it is needed there.
> 
> I did my own examination of the lowest section handling code in
> symbol_add_stub() and concluded that it was completely unnecessary. 
> Here's what it was doing:
> 
>  - It finds the "lowest" section.  This is always taken to be the
>    .text section if it exists.  Otherwise it is the section with the
>    lowest (unrelocated) address from BFD.
> 
>    In the case of the .text section, the lowest address was taken
>    to be so->textsection->addr.  Note that this is the relocated
>    address of the .text section.
> 
>    In the case of one of the other sections, the lowest address is
>    computed with the following expression:
> 
>      bfd_section_vma (so->abfd, lowest_sect) + TARGET_SO_LM_ADDR (so);
> 
>    I.e, it is the unrelocated address from the BFD section plus the
>    address needed for relocation.  Note that this value could have
>    been obtained from the shared object's section table since
>    solib_map_sections() already did the same work.  (You have to
>    look at build_section_table() and add_to_section_table() to
>    see the bit regarding the BFD section vma though.)
> 
>    In either case, when we compute lowest_addr, it will have the same
>    address as what's already in the section table.  (This was also
>    verified by Peter's empirical study.  See above.)
> 
>  - A section_add_info struct is created from the shared object's
>    section offsets.  This is nothing more than putting the section
>    offsets into a slightly different form so that we can pass them for
>    purposes of doing symbol relocations to symbol_file_add().
> 
>  - Finally, the following line...
> 
> 	sap->other[lowest_index].addr = lowest_addr;
> 
>    is attempting to assign the "lowest" address that we computed
>    earlier to the corresponding address in the section_addr_info
>    struct.
> 
>    But this code is WRONG.  lowest_index is a BFD section index
>    and if you look at the way that the section_addr_info struct
>    is created, you'll see that it is possible for it to be created
>    in such a way that the indices won't match those of the BFD
>    section indices.  (In particular, only those sections whose
>    SEC_ALLOC or SEC_LOAD flags are included in these sections.)
> 
>    In order for this code to be correct, we'd need to have
>    a loop which looks for the bfd section index.
> 
>    If it was correct, however, it would turn out that it isn't
>    doing anything useful since the addr field is already set
>    correctly.
> 
> I've tested these changes on i686-pc-linux-gnu,
> sparc-sun-solaris2.5.1, and sparc-sun-sunos4.1.4.  No regressions. 
> Also, I've implemented a backend for AIX5/IA-64 which uses the
> TARGET_SO_RELOCATE_SECTION_ADDRESSES machinery for sections that are
> relocated by different amounts and it works quite well.  I will add
> this file to the public repository this file as soon as IBM gives
> their approval.
> 
> The one change that I haven't made from Peter's remarks above is
> to add a "maint info sharedlibrary" command.  I agree that this
> would be useful, but Peter has sent me some other comments that
> have some bearing on this issue that should be addressed first.
> 
> Okay to commit?
> 
> 	Changes based on analysis from Peter Schauer:
> 	* solist.h (struct so_list): Remove field lmend.
> 	(struct target_so_ops): Remove field lm_addr.  Add field
> 	relocate_section_addresses.  Add comments for all fields
> 	in this structure
> 	(TARGET_SO_LM_ADDR): Remove.
> 	(TARGET_SO_RELOCATE_SECTION_ADDRESSES): New macro.
> 	* solib-svr4.c (svr4_relocate_section_addresses): New function.
> 	(_initialize_svr4_solib): Remove lm_addr initialization.  Add
> 	initialization for relocate_section_addresses.
> 	* solib.c (solib_map_sections): Invoke 
> 	TARGET_SO_RELOCATE_SECTION_ADDRESSES instead of using now
> 	defunct TARGET_SO_LM_ADDR to relocate the section addresses.
> 	Also, eliminate assignment to the lmend field since this
> 	field no longer exists.
> 	(symbol_add_stub): Remove machinery for determining the lowest
> 	section.
> 	(info_sharedlibrary_command): Print the text section starting
> 	and ending addresses.
> 	(solib_address): Don't use TARGET_SO_LM_ADDR, nor so->lmend to
> 	determine if an address is in a shared object.  Instead, scan
> 	the section table and test against the starting and ending
> 	addresses for each section.
> 
> Index: solib-svr4.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-svr4.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 solib-svr4.c
> --- solib-svr4.c	2000/10/24 20:05:35	1.1
> +++ solib-svr4.c	2000/10/28 22:51:30
> @@ -1567,12 +1567,20 @@ svr4_free_so (struct so_list *so)
>    free (so->lm_info);
>  }
>  
> +static void
> +svr4_relocate_section_addresses (struct so_list *so,
> +                                 struct section_table *sec)
> +{
> +  sec->addr += LM_ADDR (so);
> +  sec->endaddr += LM_ADDR (so);
> +}
> +
>  static struct target_so_ops svr4_so_ops;
>  
>  void
>  _initialize_svr4_solib (void)
>  {
> -  svr4_so_ops.lm_addr = LM_ADDR;
> +  svr4_so_ops.relocate_section_addresses = svr4_relocate_section_addresses;
>    svr4_so_ops.free_so = svr4_free_so;
>    svr4_so_ops.clear_solib = svr4_clear_solib;
>    svr4_so_ops.solib_create_inferior_hook = svr4_solib_create_inferior_hook;
> Index: solib.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 solib.c
> --- solib.c	2000/10/24 20:05:35	1.25
> +++ solib.c	2000/10/28 22:51:32
> @@ -180,9 +180,7 @@ solib_map_sections (PTR arg)
>        /* Relocate the section binding addresses as recorded in the shared
>           object's file by the base address to which the object was actually
>           mapped. */
> -      p->addr += TARGET_SO_LM_ADDR (so);
> -      p->endaddr += TARGET_SO_LM_ADDR (so);
> -      so->lmend = max (p->endaddr, so->lmend);
> +      TARGET_SO_RELOCATE_SECTION_ADDRESSES (so, p);
>        if (STREQ (p->the_bfd_section->name, ".text"))
>  	{
>  	  so->textsection = p;
> @@ -248,9 +246,6 @@ symbol_add_stub (PTR arg)
>  {
>    register struct so_list *so = (struct so_list *) arg;  /* catch_errs bogon */
>    struct section_addr_info *sap;
> -  CORE_ADDR lowest_addr = 0;
> -  int lowest_index;
> -  asection *lowest_sect = NULL;
>  
>    /* Have we already loaded this shared object?  */
>    ALL_OBJFILES (so->objfile)
> @@ -259,35 +254,9 @@ symbol_add_stub (PTR arg)
>  	return 1;
>      }
>  
> -  /* Find the shared object's text segment.  */
> -  if (so->textsection)
> -    {
> -      lowest_addr = so->textsection->addr;
> -      lowest_sect = bfd_get_section_by_name (so->abfd, ".text");
> -      lowest_index = lowest_sect->index;
> -    }
> -  else if (so->abfd != NULL)
> -    {
> -      /* If we didn't find a mapped non zero sized .text section, set
> -         up lowest_addr so that the relocation in symbol_file_add does
> -         no harm.  */
> -      lowest_sect = bfd_get_section_by_name (so->abfd, ".text");
> -      if (lowest_sect == NULL)
> -	bfd_map_over_sections (so->abfd, find_lowest_section,
> -			       (PTR) &lowest_sect);
> -      if (lowest_sect)
> -	{
> -	  lowest_addr = bfd_section_vma (so->abfd, lowest_sect)
> -	    + TARGET_SO_LM_ADDR (so);
> -	  lowest_index = lowest_sect->index;
> -	}
> -    }
> -
>    sap = build_section_addr_info_from_section_table (so->sections,
>                                                      so->sections_end);
>  
> -  sap->other[lowest_index].addr = lowest_addr;
> -
>    so->objfile = symbol_file_add (so->so_name, so->from_tty,
>  				 sap, 0, OBJF_SHARED);
>    free_section_addr_info (sap);
> @@ -610,12 +579,17 @@ info_sharedlibrary_command (char *ignore
>  	    }
>  
>  	  printf_unfiltered ("%-*s", addr_width,
> -                             local_hex_string_custom (
> -			       (unsigned long) TARGET_SO_LM_ADDR (so),
> -	                       addr_fmt));
> +			     so->textsection != NULL 
> +			       ? local_hex_string_custom (
> +			           (unsigned long) so->textsection->addr,
> +	                           addr_fmt)
> +			       : "");
>  	  printf_unfiltered ("%-*s", addr_width,
> -			 local_hex_string_custom ((unsigned long) so->lmend,
> -						  addr_fmt));
> +			     so->textsection != NULL 
> +			       ? local_hex_string_custom (
> +			           (unsigned long) so->textsection->endaddr,
> +	                           addr_fmt)
> +			       : "");
>  	  printf_unfiltered ("%-12s", so->symbols_loaded ? "Yes" : "No");
>  	  printf_unfiltered ("%s\n", so->so_name);
>  	}
> @@ -640,10 +614,7 @@ info_sharedlibrary_command (char *ignore
>  
>     Provides a hook for other gdb routines to discover whether or
>     not a particular address is within the mapped address space of
> -   a shared library.  Any address between the base mapping address
> -   and the first address beyond the end of the last mapping, is
> -   considered to be within the shared library address space, for
> -   our purposes.
> +   a shared library.
>  
>     For example, this routine is called at one point to disable
>     breakpoints which are in shared libraries that are not currently
> @@ -657,8 +628,13 @@ solib_address (CORE_ADDR address)
>  
>    for (so = so_list_head; so; so = so->next)
>      {
> -      if (TARGET_SO_LM_ADDR (so) <= address && address < so->lmend)
> -	return (so->so_name);
> +      struct section_table *p;
> +
> +      for (p = so->sections; p < so->sections_end; p++)
> +	{
> +	  if (p->addr <= address && address < p->endaddr)
> +	    return (so->so_name);
> +	}
>      }
>  
>    return (0);
> Index: solist.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/solist.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 solist.h
> --- solist.h	2000/10/24 20:05:35	1.1
> +++ solist.h	2000/10/28 22:51:32
> @@ -54,7 +54,6 @@ struct so_list
>         are initialized when we actually add it to our symbol tables.  */
>  
>      bfd *abfd;
> -    CORE_ADDR lmend;		/* upper addr bound of mapped object */
>      char symbols_loaded;	/* flag: symbols read in yet? */
>      char from_tty;		/* flag: print msgs? */
>      struct objfile *objfile;	/* objfile for loaded lib */
> @@ -65,12 +64,30 @@ struct so_list
>  
>  struct target_so_ops
>    {
> -    CORE_ADDR (*lm_addr) (struct so_list *so);
> +    /* Adjust the section binding addresses by the base address at
> +       which the object was actually mapped.  */
> +    void (*relocate_section_addresses) (struct so_list *so,
> +                                        struct section_table *);
> +
> +    /* Free the the link map info and any other private data
> +       structures associated with a so_list entry.  */
>      void (*free_so) (struct so_list *so);
> +
> +    /* Reset or free private data structures not associated with
> +       so_list entries.  */
>      void (*clear_solib) (void);
> +
> +    /* Target dependent code to run after child process fork.  */
>      void (*solib_create_inferior_hook) (void);
> +
> +    /* Do additional symbol handling, lookup, etc. after symbols
> +       for a shared object have been loaded.  */
>      void (*special_symbol_handling) (void);
> +
> +    /* Construct a list of the currently loaded shared objects.  */
>      struct so_list *(*current_sos) (void);
> +
> +    /* Find, open, and read the symbols for the main executable.  */
>      int (*open_symbol_file_object) (void *from_ttyp);
>    };
>  
> @@ -79,7 +96,8 @@ void free_so (struct so_list *so);
>  /* FIXME: gdbarch needs to control this variable */
>  extern struct target_so_ops *current_target_so_ops;
>  
> -#define TARGET_SO_LM_ADDR (current_target_so_ops->lm_addr)
> +#define TARGET_SO_RELOCATE_SECTION_ADDRESSES \
> +  (current_target_so_ops->relocate_section_addresses)
>  #define TARGET_SO_FREE_SO (current_target_so_ops->free_so)
>  #define TARGET_SO_CLEAR_SOLIB (current_target_so_ops->clear_solib)
>  #define TARGET_SO_SOLIB_CREATE_INFERIOR_HOOK \
> 
> 
From jtc@redback.com Mon Oct 30 15:15:00 2000
From: jtc@redback.com (J.T. Conklin)
To: Michael Snyder <msnyder@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com, kbuettner@cygnus.com
Subject: Re: [PATCH]: Move solib from NATDEPFILES to TDEPFILES for linux.
Date: Mon, 30 Oct 2000 15:15:00 -0000
Message-id: <5mwvepncpb.fsf@jtc.redback.com>
References: <200010302230.OAA24665@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00286.html
Content-length: 602

>>>>> "Michael" == Michael Snyder <msnyder@cygnus.com> writes:

Michael> The following change could probably be made for all OS's, but
Michael> for now I'll just make it for Linux.  I have moved the modules
Michael> solib.c and solib-svr4.c from NATDEPFILES to TDEPFILES, so that
Michael> cross-debugging onto a Linux target can support shared libraries.

Michael>         * config/m68k/linux.mh: Remove solib.c, solib-svr4.c from NATDEPFILES.
Michael>         * config/m68k/linux.mt: Add solib.c, solib-svr4.c to TDEPFILES.

Those are .o's, not .c's.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From msnyder@redhat.com Mon Oct 30 15:21:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: jtc@redback.com
Cc: Michael Snyder <msnyder@cygnus.com>, gdb-patches@sourceware.cygnus.com, kevinb@cygnus.com
Subject: Re: [PATCH]: Move solib from NATDEPFILES to TDEPFILES for linux.
Date: Mon, 30 Oct 2000 15:21:00 -0000
Message-id: <39FE0249.6944@redhat.com>
References: <200010302230.OAA24665@cleaver.cygnus.com> <5mwvepncpb.fsf@jtc.redback.com>
X-SW-Source: 2000-10/msg00287.html
Content-length: 679

J.T. Conklin wrote:
> 
> >>>>> "Michael" == Michael Snyder <msnyder@cygnus.com> writes:
> 
> Michael> The following change could probably be made for all OS's, but
> Michael> for now I'll just make it for Linux.  I have moved the modules
> Michael> solib.c and solib-svr4.c from NATDEPFILES to TDEPFILES, so that
> Michael> cross-debugging onto a Linux target can support shared libraries.
> 
> Michael>         * config/m68k/linux.mh: Remove solib.c, solib-svr4.c from NATDEPFILES.
> Michael>         * config/m68k/linux.mt: Add solib.c, solib-svr4.c to TDEPFILES.
> 
> Those are .o's, not .c's.

Yeah.  The change is correct, but I suppose the ChangeLog is wrong.
I'll fix it.
From kevinb@cygnus.com Mon Oct 30 15:23:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Michael Snyder <msnyder@cygnus.com>, gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH]: Move solib from NATDEPFILES to TDEPFILES for linux.
Date: Mon, 30 Oct 2000 15:23:00 -0000
Message-id: <1001030232309.ZM16625@ocotillo.lan>
References: <200010302230.OAA24665@cleaver.cygnus.com> <msnyder@cygnus.com>
X-SW-Source: 2000-10/msg00288.html
Content-length: 993

On Oct 30,  2:30pm, Michael Snyder wrote:

> The following change could probably be made for all OS's, but
> for now I'll just make it for Linux.  I have moved the modules
> solib.c and solib-svr4.c from NATDEPFILES to TDEPFILES, so that
> cross-debugging onto a Linux target can support shared libraries.
> 
> 2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>
> 
>         * config/m68k/linux.mh: Remove solib.c, solib-svr4.c from NATDEPFILES.
>         * config/powerpc/linux.mh: ditto.
>         * config/ia64/linux.mh: ditto.
>         * config/i386/linux.mh: ditto.
>         * config/alpha/alpha-linux.mh: ditto.
>         * config/arm/linux.mh: ditto.
>         * config/m68k/linux.mt: Add solib.c, solib-svr4.c to TDEPFILES.
>         * config/powerpc/linux.mt: ditto.
>         * config/ia64/linux.mt: ditto.
>         * config/i386/linux.mt: ditto.
>         * config/alpha/alpha-linux.mt: ditto.
>         * config/arm/linux.mt: ditto.

These changes look good to me.

Kevin
From kevinb@cygnus.com Mon Oct 30 15:36:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Jim Blandy <jimb@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH RFA] solib.c relocation improvements
Date: Mon, 30 Oct 2000 15:36:00 -0000
Message-id: <1001030233552.ZM17074@ocotillo.lan>
References: <1001029002907.ZM8686@ocotillo.lan> <np8zr6hr43.fsf@zwingli.cygnus.com> <jimb@cygnus.com>
X-SW-Source: 2000-10/msg00289.html
Content-length: 1291

On Oct 30,  6:00pm, Jim Blandy wrote:

> This change is approved.

Thanks.

> > 	Changes based on analysis from Peter Schauer:
> > 	* solist.h (struct so_list): Remove field lmend.
> > 	(struct target_so_ops): Remove field lm_addr.  Add field
> > 	relocate_section_addresses.  Add comments for all fields
> > 	in this structure
> > 	(TARGET_SO_LM_ADDR): Remove.
> > 	(TARGET_SO_RELOCATE_SECTION_ADDRESSES): New macro.
> > 	* solib-svr4.c (svr4_relocate_section_addresses): New function.
> > 	(_initialize_svr4_solib): Remove lm_addr initialization.  Add
> > 	initialization for relocate_section_addresses.
> > 	* solib.c (solib_map_sections): Invoke 
> > 	TARGET_SO_RELOCATE_SECTION_ADDRESSES instead of using now
> > 	defunct TARGET_SO_LM_ADDR to relocate the section addresses.
> > 	Also, eliminate assignment to the lmend field since this
> > 	field no longer exists.
> > 	(symbol_add_stub): Remove machinery for determining the lowest
> > 	section.
> > 	(info_sharedlibrary_command): Print the text section starting
> > 	and ending addresses.
> > 	(solib_address): Don't use TARGET_SO_LM_ADDR, nor so->lmend to
> > 	determine if an address is in a shared object.  Instead, scan
> > 	the section table and test against the starting and ending
> > 	addresses for each section.

Committed.
From msnyder@cygnus.com Mon Oct 30 17:02:00 2000
From: Michael Snyder <msnyder@cygnus.com>
To: gdb-patches@sourceware.cygnus.com, kevinb@cygnus.com
Subject: [PATCH]: Extend SVR4 shlib debug changes to SH3 and other targets
Date: Mon, 30 Oct 2000 17:02:00 -0000
Message-id: <200010310102.RAA24876@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00290.html
Content-length: 8254

This change moves the definition of SVR4_SHARED_LIBS into 
config/tm-linux.h, so that it will affect all linuxen.
I then add a new file config/sh/tm-linux.h which includes
config/tm-linux.h (as well as sh/tm-sh.h).  Finally I add
target-specific definitions of fetch_link_map_offsets, so
that shared libraries can be cross-debugged.

2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>

        * config/sh/tm-linux.h: New file.  Include generic tm-linux.h,
        plus tm-sh.h, then define SVR4_FETCH_LINK_MAP_OFFSETS to use
        the sh target function instead of the default link map offsets.
        * config/sh/sh.mt: Add solib.o and solib-svr4.o to TDEPFILES.
        Use sh/tm-linux.h instead of sh/tm-sh.h.
        * sh-tdep.c (sh_linux_svr4_fetch_link_map_offsets):
        New function.  Construct target-specific link map offsets.
        * i386-linux-tdep.c (i386_linux_svr4_fetch_link_map_offsets:
        New function.  Construct target-specific link map offsets.
        * config/i386/tm-linux.h: Use above function instead of default.

2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>

        * config/i386/tm-linux.h: Remove definition of SVR4_SHARED_LIBS,
        and inclusion of solib.h.  Move up into ../tm-linux.h.
        config/tm-linux.h: Define SVR4_SHARED_LIBS, include solib.h.

Index: config/sh/sh.mt
===================================================================
RCS file: /cvs/src/src/gdb/config/sh/sh.mt,v
retrieving revision 1.1.1.1
diff -c -3 -p -r1.1.1.1 sh.mt
*** sh.mt	1999/04/16 01:34:25	1.1.1.1
--- sh.mt	2000/10/31 00:57:12
***************
*** 1,6 ****
  # Target: Hitachi Super-H with ICE and simulator
! TDEPFILES= sh-tdep.o monitor.o sh3-rom.o remote-e7000.o ser-e7kpc.o dsrec.o
! TM_FILE= tm-sh.h
  
  SIM_OBS = remote-sim.o
  SIM = ../sim/sh/libsim.a
--- 1,6 ----
  # Target: Hitachi Super-H with ICE and simulator
! TDEPFILES= sh-tdep.o monitor.o sh3-rom.o remote-e7000.o ser-e7kpc.o dsrec.o solib.o solib-svr4.o
! TM_FILE= tm-linux.h
  
  SIM_OBS = remote-sim.o
  SIM = ../sim/sh/libsim.a
Index: sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.14
diff -c -3 -p -r1.14 sh-tdep.c
*** sh-tdep.c	2000/07/31 16:25:36	1.14
--- sh-tdep.c	2000/10/31 00:57:12
***************
*** 39,44 ****
--- 39,46 ----
  #include "arch-utils.h"
  #include "floatformat.h"
  
+ #include "solib-svr4.h"
+ 
  #undef XMALLOC
  #define XMALLOC(TYPE) ((TYPE*) xmalloc (sizeof (TYPE)))
  
*************** sh_do_registers_info (int regnum, int fp
*** 1797,1802 ****
--- 1799,1848 ----
  	  }
      }
  }
+ 
+ #ifdef SVR4_SHARED_LIBS
+ 
+ /* Fetch (and possibly build) an appropriate link_map_offsets structure
+    for native i386 linux targets using the struct offsets defined in
+    link.h (but without actual reference to that file).
+ 
+    This makes it possible to access i386-linux shared libraries from
+    a gdb that was not built on an i386-linux host (for cross debugging).
+    */
+ 
+ struct link_map_offsets *
+ sh_linux_svr4_fetch_link_map_offsets (void)
+ {
+   static struct link_map_offsets lmo;
+   static struct link_map_offsets *lmp = 0;
+ 
+   if (lmp == 0)
+     {
+       lmp = &lmo;
+ 
+       lmo.r_debug_size = 8;	/* 20 not actual size but all we need */
+ 
+       lmo.r_map_offset = 4;
+       lmo.r_map_size   = 4;
+ 
+       lmo.link_map_size = 20;	/* 552 not actual size but all we need */
+ 
+       lmo.l_addr_offset = 0;
+       lmo.l_addr_size   = 4;
+ 
+       lmo.l_name_offset = 4;
+       lmo.l_name_size   = 4;
+ 
+       lmo.l_next_offset = 12;
+       lmo.l_next_size   = 4;
+ 
+       lmo.l_prev_offset = 16;
+       lmo.l_prev_size   = 4;
+     }
+ 
+     return lmp;
+ }
+ #endif /* SVR4_SHARED_LIBS */
  
  static gdbarch_init_ftype sh_gdbarch_init;
  
Index: i386-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-linux-tdep.c,v
retrieving revision 1.2
diff -c -3 -p -r1.2 i386-linux-tdep.c
*** i386-linux-tdep.c	2000/08/29 23:31:10	1.2
--- i386-linux-tdep.c	2000/10/31 00:57:12
***************
*** 27,32 ****
--- 27,33 ----
  #include "symtab.h"
  #include "symfile.h"
  #include "objfiles.h"
+ #include "solib-svr4.h"	/* for struct link_map_offsets */
  
  \f
  /* Recognizing signal handler frames.  */
*************** i386_linux_skip_solib_resolver (CORE_ADD
*** 372,374 ****
--- 373,417 ----
  
    return 0;
  }
+ 
+ /* Fetch (and possibly build) an appropriate link_map_offsets structure
+    for native i386 linux targets using the struct offsets defined in
+    link.h (but without actual reference to that file).
+ 
+    This makes it possible to access i386-linux shared libraries from
+    a gdb that was not built on an i386-linux host (for cross debugging).
+    */
+ 
+ struct link_map_offsets *
+ i386_linux_svr4_fetch_link_map_offsets (void)
+ {
+   static struct link_map_offsets lmo;
+   static struct link_map_offsets *lmp = 0;
+ 
+   if (lmp == 0)
+     {
+       lmp = &lmo;
+ 
+       lmo.r_debug_size = 8;	/* 20 not actual size but all we need */
+ 
+       lmo.r_map_offset = 4;
+       lmo.r_map_size   = 4;
+ 
+       lmo.link_map_size = 20;	/* 552 not actual size but all we need */
+ 
+       lmo.l_addr_offset = 0;
+       lmo.l_addr_size   = 4;
+ 
+       lmo.l_name_offset = 4;
+       lmo.l_name_size   = 4;
+ 
+       lmo.l_next_offset = 12;
+       lmo.l_next_size   = 4;
+ 
+       lmo.l_prev_offset = 16;
+       lmo.l_prev_size   = 4;
+     }
+ 
+     return lmp;
+ }
+ 
Index: config/i386/tm-linux.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/tm-linux.h,v
retrieving revision 1.8
diff -c -3 -p -r1.8 tm-linux.h
*** tm-linux.h	2000/08/10 17:04:33	1.8
--- tm-linux.h	2000/10/31 00:57:12
***************
*** 30,35 ****
--- 30,39 ----
  #include "i386/tm-i386.h"
  #include "tm-linux.h"
  
+ /* Use target_specific function to define link map offsets.  */
+ extern struct link_map_offsets *i386_linux_svr4_fetch_link_map_offsets (void);
+ #define SVR4_FETCH_LINK_MAP_OFFSETS() i386_linux_svr4_fetch_link_map_offsets ()
+ 
  /* FIXME: kettenis/2000-03-26: We should get rid of this last piece of
     Linux-specific `long double'-support code, probably by adding code
     to valprint.c:print_floating() to recognize various extended
Index: config/tm-linux.h
===================================================================
RCS file: /cvs/src/src/gdb/config/tm-linux.h,v
retrieving revision 1.1.1.1
diff -c -3 -p -r1.1.1.1 tm-linux.h
*** tm-linux.h	1999/12/07 03:56:08	1.1.1.1
--- tm-linux.h	2000/10/31 00:57:12
***************
*** 34,36 ****
--- 34,42 ----
  /* We need this file for the SOLIB_TRAMPOLINE stuff. */
  
  #include "tm-sysv4.h"
+ 
+ /* We define this if link.h is available, because with ELF we use SVR4 style
+    shared libraries.  FIXME: move up into config/tm-linux.h?  */
+ 
+ #define SVR4_SHARED_LIBS
+ #include "solib.h"		/* Support for shared libraries. */

[and here is sh/tm-linux.h]

/* Target-specific definitions for Linux running on a Hitachi Super-H.
   Copyright (C) 2000 Free Software Foundation, Inc.

   This file is part of GDB.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software
   Foundation, Inc., 59 Temple Place - Suite 330,
   Boston, MA 02111-1307, USA.  */

/* Pull in Linux generic defs */
#include "tm-linux.h"

/* Pull in sh-target defs */
#include "sh/tm-sh.h"

/* Use target_specific function to define link map offsets.  */
extern struct link_map_offsets *sh_linux_svr4_fetch_link_map_offsets (void);
#define SVR4_FETCH_LINK_MAP_OFFSETS() sh_linux_svr4_fetch_link_map_offsets ()

Re: Remote watchpoint support.

[Eli Zaretskii]

> From: jtc@redback.com (J.T. Conklin)
> Date: 30 Oct 2000 12:39:53 -0800
> 
> One thing I dislike about it is that the debug agent never explicitly
> tells GDB that a watchpoint has been hit, but rather it is implicitly
> coded in the return value of the query.

I agree that this is not a good design.

> The second is the return value of 0 if the target stopped for another
> reason.  I (now) know that this is the current behavior of the target_
> stopped_data_address() macro, but it use the one address that I think
> might be very important to watch on targets where the zero page isn't
> unmapped (in which case a reference would cause a exception/trap).

I would actually be very happy if we could change the interface of
target_stopped_data_address so that zero would not be treated
specially.  One problem with the current design is that I cannot catch
NULL pointer dereferences on Windows, where the null page cannot be
unmapped from the address space of DJGPP programs.  This prevents me
from finding such bugs by setting a watchpoint at address 0.

> While it might not be possible to fix in GDB's internals for some
> time

What would it take to change that?  Perhaps it would be a good idea to
have the list of the obstacles, in case someone might try to negotiate
them one by one.
From eliz@delorie.com Tue Oct 31 01:09:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: john@Calva.COM
Cc: Peter.Schauer@Regent.E-Technik.TU-Muenchen.DE, gdb-patches@sources.redhat.com
Subject: Re: Patch for gdb5.0; enable hardware watchpoints on UnixWare
Date: Tue, 31 Oct 2000 01:09:00 -0000
Message-id: <200010310906.EAA21817@indy.delorie.com>
References: <NCBBLMGKIKDGJMEOMNMEEEDMHEAA.john@Calva.COM>
X-SW-Source: 2000-10/msg00294.html
Content-length: 1008

> From: "John Hughes" <john@Calva.COM>
> Date: Mon, 30 Oct 2000 17:11:58 +0100
> 
> > But all the i386v4-nat.c code has to be conditionalized on 
> > UNIXWARE (or better yet, moved to a new i386sco-nat.c file), as
> > i386v4-nat.c is used by x86 Solaris as well, and Solaris uses page
> > faulting instead of debug registers for watchpoints, and would not
> > compile with your patches installed.
> 
> Aha.  Here's a fixed version of the patch; the i386v4-nat.c changes
> are conditional on UNIXWARE.

Note that I'm working on a unified watchpoint implementation for all
ia32 targets, based on the code written for the DJGPP port (see
go32-nat.c).  This implementation will allow to watch large areas (up
to 16 bytes) and debug register sharing via reference counts (required
for watching overlapping areas) on all ia32 platforms.

I am trying to make this happen for v5.1, so perhaps changes like this
one should be witheld and not committed, at least until we decide that
I'm not living up to my promises.
From kettenis@wins.uva.nl Tue Oct 31 01:37:00 2000
From: Mark Kettenis <kettenis@wins.uva.nl>
To: msnyder@cygnus.com
Cc: gdb-patches@sourceware.cygnus.com, kevinb@cygnus.com
Subject: Re: [PATCH]: Extend SVR4 shlib debug changes to SH3 and other targets
Date: Tue, 31 Oct 2000 01:37:00 -0000
Message-id: <200010310937.e9V9b4604953@debye.wins.uva.nl>
References: <200010310102.RAA24876@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00295.html
Content-length: 2239

   From: Michael Snyder <msnyder@cygnus.com>
   Date: Mon, 30 Oct 2000 17:02:01 -0800 (PST)

   This change moves the definition of SVR4_SHARED_LIBS into 
   config/tm-linux.h, so that it will affect all linuxen.
   I then add a new file config/sh/tm-linux.h which includes
   config/tm-linux.h (as well as sh/tm-sh.h).  Finally I add
   target-specific definitions of fetch_link_map_offsets, so
   that shared libraries can be cross-debugged.

   2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>

	   * config/sh/tm-linux.h: New file.  Include generic tm-linux.h,
	   plus tm-sh.h, then define SVR4_FETCH_LINK_MAP_OFFSETS to use
	   the sh target function instead of the default link map offsets.
	   * config/sh/sh.mt: Add solib.o and solib-svr4.o to TDEPFILES.
	   Use sh/tm-linux.h instead of sh/tm-sh.h.
	   * sh-tdep.c (sh_linux_svr4_fetch_link_map_offsets):
	   New function.  Construct target-specific link map offsets.
	   * i386-linux-tdep.c (i386_linux_svr4_fetch_link_map_offsets:
	   New function.  Construct target-specific link map offsets.
	   * config/i386/tm-linux.h: Use above function instead of default.

   2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>

	   * config/i386/tm-linux.h: Remove definition of SVR4_SHARED_LIBS,
	   and inclusion of solib.h.  Move up into ../tm-linux.h.
	   config/tm-linux.h: Define SVR4_SHARED_LIBS, include solib.h.

Looks good to me.  One minor nit though:

   Index: config/tm-linux.h
   ===================================================================
   RCS file: /cvs/src/src/gdb/config/tm-linux.h,v
   retrieving revision 1.1.1.1
   diff -c -3 -p -r1.1.1.1 tm-linux.h
   *** tm-linux.h	1999/12/07 03:56:08	1.1.1.1
   --- tm-linux.h	2000/10/31 00:57:12
   ***************
   *** 34,36 ****
   --- 34,42 ----
     /* We need this file for the SOLIB_TRAMPOLINE stuff. */

     #include "tm-sysv4.h"
   + 
   + /* We define this if link.h is available, because with ELF we use SVR4 style
   +    shared libraries.  FIXME: move up into config/tm-linux.h?  */
   + 
   + #define SVR4_SHARED_LIBS
   + #include "solib.h"		/* Support for shared libraries. */

Please update the comment.  The FIXME can go now, and the blurb about
link.h isn't true anymore isn't it?
From john@Calva.COM Tue Oct 31 03:20:00 2000
From: "John Hughes" <john@Calva.COM>
To: "Eli Zaretskii" <eliz@is.elta.co.il>
Cc: <gdb-patches@sources.redhat.com>
Subject: RE: Patch for gdb5.0; enable hardware watchpoints on UnixWare
Date: Tue, 31 Oct 2000 03:20:00 -0000
Message-id: <NCBBLMGKIKDGJMEOMNMEMEELHEAA.john@Calva.COM>
References: <200010310906.EAA21817@indy.delorie.com>
X-SW-Source: 2000-10/msg00296.html
Content-length: 2169

Eli Zaretskii writes:
> Note that I'm working on a unified watchpoint implementation for all
> ia32 targets, based on the code written for the DJGPP port (see
> go32-nat.c).

Ah good.  I saw the stuff in TODO yesterday and was trying to work up
the courage to propose doing this myself.

> I am trying to make this happen for v5.1, so perhaps changes like this
> one should be witheld and not committed, at least until we decide that
> I'm not living up to my promises.

Oh, please ignore my patch then, could you steal the svr42mp specific
bits of it for 5.1?  (debug registers held in pr_family element of
proc status, PCSDEBUG operation to write debug registers).

Looking at version 1.6 of go32-nat.c I've got a couple of comments:

1. Why not zap the waddr arg to go32_..._watchpoint?  It's not used.

2. In go32_insert_aligned_watchpoint (and ...remove_aligned...) you
   have a "switch" to work out the len_bits for the watchpoint, then a
   loop to find an existing watchpoint that might match, then an
   "if" to check for non-aligned watchpoints.  (Which could not have
   matched in the loop).

   It seems to me this could be simplified to:

  ...
  if (addr % len)
    return go32_handle_nonaligned_watchpoint (wp_insert, addr, len, rw);

  switch (len)
  {
  case 4:
    len_bits = DR_LEN_4;
    break;
  case 2:
    len_bits = DR_LEN_2;
    break;
  case 1:
    len_bits = DR_LEN_1;
    break;
  default:
    return go32_handle_nonaligned_watchpoint (wp_insert, addr, len, rw);
  }

  /* Look for an occupied debug register with the same address and the
     same RW and LEN definitions.  If we find one, we can use it for
     this watchpoint as well (and save a register).  */
  ...

3. You're not using the DR_FIRSADDR/DR_LASTADDR defines.

4. You have a define DR_RW_READWRITE, other people (SVR3/SVR4/SVR5) seem
   to have DR_RW_READ with the same value (0x3).

5. Not everybody has DR_CONTROL_MASK.

	#define DR_CONTROL_MASK ((1<<DR_CONTROL_SIZE) - 1)

-- 
John Hughes <john@Calva.COM>, 
        CalvaEDI SA.                            Tel: +33-1-4313-3131
        66 rue du Moulin de la Pointe,          Fax: +33-1-4313-3139
        75013 PARIS.