thumb_skip_prologue too adventurous

[Jonathan Larmour <jlarmour@redhat.co.uk>]

Hi Fernando,

You checked in the following change to arm-tdep.c:

revision 1.46
date: 2000/01/28 15:32:04;  author: fnasser;  state: Exp;  lines: +84 -22
2000-01-28  Fernando Nasser  <fnasser@totem.to.cygnus.com>
        * arm-tdep.c (thumb_skip_prologue, thumb_scan_prologue): Add
        support for new style thumb prologues.

However this has broken small functions, e.g. void foo() { for (;;); }  etc.
because such functions contain no prologue. e.g. for foo(), there is only
one instruction which is a branch to itself. However thumb_skip_prologue
will continue past the end of this small function and into the next one.
Thus breakpoints, etc. get set in completely the wrong place.

The essential issue is that thumb_skip_prologue must not be allowed to
continue on after the end of the function, even if it hasn't found the
prologue.

The attached patch certainly fixes things for me, but I'm not sure if it's
the right answer. If it is, please check it in; if not, tell me what I
should do :-).

Thanks,

Jifl

2000-03-18  Jonathan Larmour  <jlarmour@redhat.co.uk>

	* arm-tdep.c (thumb_skip_prologue): Take function end addr argument
	so that we can stop searching for the prologue past the function end
	(arm_skip_prologue): Call thumb_skip_prologue with function end addr


-- 
Red Hat, 35 Cambridge Place, Cambridge, UK. CB2 1NS  Tel: +44 (1223) 728762
"Plan to be spontaneous tomorrow."  ||  These opinions are all my own fault
From jlarmour@redhat.co.uk Sat Mar 18 14:17:00 2000
From: Jonathan Larmour <jlarmour@redhat.co.uk>
To: fnasser@redhat.com, gdb-patches@sourceware.cygnus.com
Subject: Re: thumb_skip_prologue too adventurous
Date: Sat, 18 Mar 2000 14:17:00 -0000
Message-id: <38D40052.AF731E81@redhat.co.uk>
References: <38D3FFC8.32082A85@redhat.co.uk>
X-SW-Source: 2000-03/msg00331.html
Content-length: 1896

Jonathan Larmour wrote:
> 2000-03-18  Jonathan Larmour  <jlarmour@redhat.co.uk>
> 
>         * arm-tdep.c (thumb_skip_prologue): Take function end addr argument
>         so that we can stop searching for the prologue past the function end
>         (arm_skip_prologue): Call thumb_skip_prologue with function end addr

Doh! Patch attached.

Jifl
Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.4
diff -u -5 -p -r1.4 arm-tdep.c
--- arm-tdep.c	2000/02/29 07:23:02	1.4
+++ arm-tdep.c	2000/03/18 22:16:21
@@ -326,20 +326,20 @@ arm_frameless_function_invocation (struc
    When we have found at least one of each class we are done with the prolog.
    Note that the "sub sp, #NN" before the push does not count.
    */
 
 static CORE_ADDR
-thumb_skip_prologue (CORE_ADDR pc)
+thumb_skip_prologue (CORE_ADDR pc, CORE_ADDR func_end)
 {
   CORE_ADDR current_pc;
   int findmask = 0;  	/* findmask:
       			   bit 0 - push { rlist }
 			   bit 1 - mov r7, sp  OR  add r7, sp, #imm  (setting of r7)
       			   bit 2 - sub sp, #simm  OR  add sp, #simm  (adjusting of sp)
 			*/
 
-  for (current_pc = pc; current_pc < pc + 40; current_pc += 2)
+  for (current_pc = pc; current_pc + 2 < func_end && current_pc < pc + 40; current_pc += 2)
     {
       unsigned short insn = read_memory_unsigned_integer (current_pc, 2);
 
       if ((insn & 0xfe00) == 0xb400)	/* push { rlist } */
 	{
@@ -397,11 +397,11 @@ arm_skip_prologue (CORE_ADDR pc)
 	return sal.end;
     }
 
   /* Check if this is Thumb code.  */
   if (arm_pc_is_thumb (pc))
-    return thumb_skip_prologue (pc);
+    return thumb_skip_prologue (pc, func_end);
 
   /* Can't find the prologue end in the symbol table, try it the hard way
      by disassembling the instructions. */
   skip_pc = pc;
   inst = read_memory_integer (skip_pc, 4);
From eliz@delorie.com Sat Mar 18 14:26:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: phdm@macqel.be
Cc: ezannoni@cygnus.com, ac131313@cygnus.com, gdb-patches@sourceware.cygnus.com
Subject: Re: HAVE_POLL is not enough - RFA
Date: Sat, 18 Mar 2000 14:26:00 -0000
Message-id: <200003182226.RAA07052@indy.delorie.com>
References: <200003181028.LAA30913@mail.macqel.be>
X-SW-Source: 2000-03/msg00332.html
Content-length: 9200

> I implemented the runtime poll/select selection.  The patch seems
> huge because of the indentation changes caused by replacing
> 
> #if HAVE_POLL
> 	some_code
> #else
> 	some_other_code
> #endif
> 
> by
> 
> #if HAVE_POLL
> 	if (!use_select)
> 	  {
> 	    some_code
> 	  }
> 	else
> #endif
> 	  {
> 	    some_other_code
> 	  }
> 
> I have tested it on m68k-motorola-sysv and powerpc-ibm-aix4.1.5.0
> 
> OK to apply ?

This patch clashes with the changes I sent for the select branch,
whose goal is to use the standard FD_* macros instead of fiddling with
the individual bits (which is both unnecessary and non-portable).  I
reproduce my patch below.  The references can be found here:

  http://sourceware.cygnus.com/ml/gdb/2000-q1/msg00358.html


Btw, Elena, what's up with this patch?  Why wasn't it commited?  Can I
commit it?


2000-03-04  Eli Zaretskii  <eliz@is.elta.co.il>

	* event-loop.c (top-level) [NO_FD_SET]: Deprecate this branch.
	Print an error at compile time if we are to use select, but FD_SET
	is not available.
	(SELECT_MASK, NBBY, FD_SETSIZE, NFDBITS, MASK_SIZE): Define only
	if HAVE_POLL is not defined and NO_FD_SET *is* defined.
	(create_file_handler) [!HAVE_POLL]: Use FD_SET and FD_CLR.
	(delete_file_handler) [!HAVE_POLL]: Use FD_CLR and FD_ISSET.
	(gdb_wait_for_event) [!HAVE_POLL]: Copy fd_set sets directly
	instead of using memcpy and memset.  Use FD_ISSET.

	* config/i386/xm-go32.h (fd_mask): Remove typedef.


--- gdb/event-loop.c~0	Mon Feb 21 18:17:16 2000
+++ gdb/event-loop.c	Sat Mar  4 14:28:38 2000
@@ -27,6 +27,7 @@
 #include <poll.h>
 #else
 #include <sys/types.h>
+#include <string.h>
 #endif
 #include <errno.h>
 #include <setjmp.h>
@@ -34,9 +35,14 @@
 
 /* Type of the mask arguments to select. */
 
-#ifndef NO_FD_SET
-#define SELECT_MASK fd_set
-#else
+#ifndef HAVE_POLL
+#ifdef NO_FD_SET
+/* All this stuff below is not required if select is used as God(tm)
+   intended, with the FD_* macros.  Are there any implementations of
+   select which don't have FD_SET and other standard FD_* macros?  I
+   don't think there are, but if I'm wrong, we need to catch them.  */
+#error FD_SET must be defined if select function is to be used!
+
 #ifndef _AIX
 typedef long fd_mask;
 #endif
@@ -44,8 +50,7 @@ typedef long fd_mask;
 #define SELECT_MASK void
 #else
 #define SELECT_MASK int
-#endif
-#endif
+#endif /* !_IBMR2 */
 
 /* Define "NBBY" (number of bits per byte) if it's not already defined. */
 
@@ -53,7 +58,6 @@ typedef long fd_mask;
 #define NBBY 8
 #endif
 
-
 /* Define the number of fd_masks in an fd_set */
 
 #ifndef FD_SETSIZE
@@ -71,6 +75,9 @@ typedef long fd_mask;
 #endif
 #define MASK_SIZE howmany(FD_SETSIZE, NFDBITS)
 
+#endif /* NO_FD_SET */
+#endif /* !HAVE_POLL */
+
 
 typedef struct gdb_event gdb_event;
 typedef void (event_handler_func) (int);
@@ -192,10 +199,10 @@ static struct
 
     /* Masks to be used in the next call to select.
        Bits are set in response to calls to create_file_handler. */
-    fd_mask check_masks[3 * MASK_SIZE];
+    fd_set check_masks[3];
 
     /* What file descriptors were found ready by select. */
-    fd_mask ready_masks[3 * MASK_SIZE];
+    fd_set ready_masks[3];
 
     /* Number of valid bits (highest fd value + 1). */
     int num_fds;
@@ -487,10 +494,6 @@ create_file_handler (int fd, int mask, h
 {
   file_handler *file_ptr;
 
-#ifndef HAVE_POLL
-  int index, bit;
-#endif
-
   /* Do we already have a file handler for this file? (We may be
      changing its associated procedure). */
   for (file_ptr = gdb_notifier.first_file_handler; file_ptr != NULL;
@@ -532,23 +535,20 @@ create_file_handler (int fd, int mask, h
 
 #else /* ! HAVE_POLL */
 
-  index = fd / (NBBY * sizeof (fd_mask));
-  bit = 1 << (fd % (NBBY * sizeof (fd_mask)));
-
   if (mask & GDB_READABLE)
-    gdb_notifier.check_masks[index] |= bit;
+    FD_SET (fd, &gdb_notifier.check_masks[0]);
   else
-    gdb_notifier.check_masks[index] &= ~bit;
+    FD_CLR (fd, &gdb_notifier.check_masks[0]);
 
   if (mask & GDB_WRITABLE)
-    (gdb_notifier.check_masks + MASK_SIZE)[index] |= bit;
+    FD_SET (fd, &gdb_notifier.check_masks[1]);
   else
-    (gdb_notifier.check_masks + MASK_SIZE)[index] &= ~bit;
+    FD_CLR (fd, &gdb_notifier.check_masks[1]);
 
   if (mask & GDB_EXCEPTION)
-    (gdb_notifier.check_masks + 2 * (MASK_SIZE))[index] |= bit;
+    FD_SET (fd, &gdb_notifier.check_masks[2]);
   else
-    (gdb_notifier.check_masks + 2 * (MASK_SIZE))[index] &= ~bit;
+    FD_CLR (fd, &gdb_notifier.check_masks[2]);
 
   if (gdb_notifier.num_fds <= fd)
     gdb_notifier.num_fds = fd + 1;
@@ -562,11 +562,10 @@ void
 delete_file_handler (int fd)
 {
   file_handler *file_ptr, *prev_ptr = NULL;
-  int i, j;
+  int i;
+#ifdef HAVE_POLL
+  int j
   struct pollfd *new_poll_fds;
-#ifndef HAVE_POLL
-  int index, bit;
-  unsigned long flags;
 #endif
 
   /* Find the entry for the given file. */
@@ -604,36 +603,26 @@ delete_file_handler (int fd)
 
 #else /* ! HAVE_POLL */
 
-  index = fd / (NBBY * sizeof (fd_mask));
-  bit = 1 << (fd % (NBBY * sizeof (fd_mask)));
-
   if (file_ptr->mask & GDB_READABLE)
-    gdb_notifier.check_masks[index] &= ~bit;
+    FD_CLR (fd, &gdb_notifier.check_masks[0]);
   if (file_ptr->mask & GDB_WRITABLE)
-    (gdb_notifier.check_masks + MASK_SIZE)[index] &= ~bit;
+    FD_CLR (fd, &gdb_notifier.check_masks[1]);
   if (file_ptr->mask & GDB_EXCEPTION)
-    (gdb_notifier.check_masks + 2 * (MASK_SIZE))[index] &= ~bit;
+    FD_CLR (fd, &gdb_notifier.check_masks[2]);
 
   /* Find current max fd. */
 
   if ((fd + 1) == gdb_notifier.num_fds)
     {
-      for (gdb_notifier.num_fds = 0; index >= 0; index--)
+      gdb_notifier.num_fds--;
+      for (i = gdb_notifier.num_fds; i; i--)
 	{
-	  flags = gdb_notifier.check_masks[index]
-	    | (gdb_notifier.check_masks + MASK_SIZE)[index]
-	    | (gdb_notifier.check_masks + 2 * (MASK_SIZE))[index];
-	  if (flags)
-	    {
-	      for (i = (NBBY * sizeof (fd_mask)); i > 0; i--)
-		{
-		  if (flags & (((unsigned long) 1) << (i - 1)))
-		    break;
-		}
-	      gdb_notifier.num_fds = index * (NBBY * sizeof (fd_mask)) + i;
-	      break;
-	    }
+	  if (FD_ISSET (i - 1, &gdb_notifier.check_masks[0])
+	      || FD_ISSET (i - 1, &gdb_notifier.check_masks[1])
+	      || FD_ISSET (i - 1, &gdb_notifier.check_masks[2]))
+	    break;
 	}
+      gdb_notifier.num_fds = i;
     }
 #endif /* HAVE_POLL */
 
@@ -742,10 +731,8 @@ gdb_wait_for_event (void)
   file_handler *file_ptr;
   gdb_event *file_event_ptr;
   int num_found = 0;
+#ifdef HAVE_POLL
   int i;
-
-#ifndef HAVE_POLL
-  int mask, bit, index;
 #endif
 
   /* Make sure all output is done before getting another event. */
@@ -767,20 +754,24 @@ gdb_wait_for_event (void)
     perror_with_name ("Poll");
 
 #else /* ! HAVE_POLL */
-  memcpy (gdb_notifier.ready_masks,
-	  gdb_notifier.check_masks,
-	  3 * MASK_SIZE * sizeof (fd_mask));
+
+  gdb_notifier.ready_masks[0] = gdb_notifier.check_masks[0];
+  gdb_notifier.ready_masks[1] = gdb_notifier.check_masks[1];
+  gdb_notifier.ready_masks[2] = gdb_notifier.check_masks[2];
+
   num_found = select (gdb_notifier.num_fds,
-		      (SELECT_MASK *) & gdb_notifier.ready_masks[0],
-		      (SELECT_MASK *) & gdb_notifier.ready_masks[MASK_SIZE],
-		  (SELECT_MASK *) & gdb_notifier.ready_masks[2 * MASK_SIZE],
-		  gdb_notifier.timeout_valid ? &gdb_notifier.timeout : NULL);
+		      & gdb_notifier.ready_masks[0],
+		      & gdb_notifier.ready_masks[1],
+		      & gdb_notifier.ready_masks[2],
+		      gdb_notifier.timeout_valid
+		      ? &gdb_notifier.timeout : NULL);
 
   /* Clear the masks after an error from select. */
   if (num_found == -1)
     {
-      memset (gdb_notifier.ready_masks,
-	      0, 3 * MASK_SIZE * sizeof (fd_mask));
+      FD_ZERO (&gdb_notifier.ready_masks[0]);
+      FD_ZERO (&gdb_notifier.ready_masks[1]);
+      FD_ZERO (&gdb_notifier.ready_masks[2]);
       /* Dont print anything is we got a signal, let gdb handle it. */
       if (errno != EINTR)
 	perror_with_name ("Select");
@@ -821,19 +812,18 @@ gdb_wait_for_event (void)
     }
 
 #else /* ! HAVE_POLL */
+
   for (file_ptr = gdb_notifier.first_file_handler;
        (file_ptr != NULL) && (num_found > 0);
        file_ptr = file_ptr->next_file)
     {
-      index = file_ptr->fd / (NBBY * sizeof (fd_mask));
-      bit = 1 << (file_ptr->fd % (NBBY * sizeof (fd_mask)));
-      mask = 0;
+      int mask = 0;
 
-      if (gdb_notifier.ready_masks[index] & bit)
+      if (FD_ISSET (file_ptr->fd, &gdb_notifier.ready_masks[0]))
 	mask |= GDB_READABLE;
-      if ((gdb_notifier.ready_masks + MASK_SIZE)[index] & bit)
+      if (FD_ISSET (file_ptr->fd, &gdb_notifier.ready_masks[1]))
 	mask |= GDB_WRITABLE;
-      if ((gdb_notifier.ready_masks + 2 * (MASK_SIZE))[index] & bit)
+      if (FD_ISSET (file_ptr->fd, &gdb_notifier.ready_masks[2]))
 	mask |= GDB_EXCEPTION;
 
       if (!mask)
@@ -851,6 +841,7 @@ gdb_wait_for_event (void)
 	}
       file_ptr->ready_mask = mask;
     }
+
 #endif /* HAVE_POLL */
 
   return 0;
--- gdb/config/i386/xm-go32.h~1	Fri Feb 18 18:18:44 2000
+++ gdb/config/i386/xm-go32.h	Sat Mar  4 13:58:36 2000
@@ -36,4 +36,3 @@
 #define DIRNAME_SEPARATOR ';'
 
 #define HOST_I386
-typedef unsigned char fd_mask;