This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Prevent runaway (infinite loop) in mips-tdep.c


On Wed, Apr 21, 2004 at 05:58:09PM +0000, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> >On Wed, Apr 21, 2004 at 12:20:53AM +0000, Michael Snyder wrote:
> >
> >>Hi Andrew,
> >>
> >>This change will prevent the caller(s) of mips_mdebug_frame_id from
> >>infinite-looping when we get badly lost on the stack frame.
> >>
> >
> >
> >>2004-04-21  Michael Snyder  <msnyder@redhat.com>
> >>
> >>	* mips-tdep.c (mips_mdebug_frame_cache): Call error to prevent
> >>	infinite looping by caller.
> >>	(heuristic_proc_start): Warning() already prefixes "Warning: ".
> >
> >
> >Since I have some patches to make this "I'm not sure how" case into a
> >working part of the unwinder, I don't much like this.  They got hung up
> >on the question of how to trust proc_desc's when we might be in the
> >prologue.
> >
> >One time where this case occurs is in backtracing from a NULL pointer
> >dereference, which happens in the GDB testsuite now.
> 
> OK -- can I see your patches?  Maybe they'll solve the runaway condition
> I'm experiencing, and if not, maybe I can fix it in a way that's
> consistant with what you're doing.

What I've attached is a diff against GDB 6.1.  No promises about
quality or "correctness".

diff -urp -x objdir -x '*~' gdb-2004-03-01-cvs.orig/gdb/mips-linux-tdep.c gdb-2004-03-01-cvs/gdb/mips-linux-tdep.c
--- gdb-2004-03-01-cvs.orig/gdb/mips-linux-tdep.c	2004-03-02 16:56:42.000000000 -0500
+++ gdb-2004-03-01-cvs/gdb/mips-linux-tdep.c	2004-03-03 18:29:25.000000000 -0500
@@ -85,7 +85,7 @@ mips_linux_get_longjmp_target (CORE_ADDR
 			  buf, TARGET_PTR_BIT / TARGET_CHAR_BIT))
     return 0;
 
-  *pc = extract_unsigned_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT);
+  *pc = extract_signed_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT);
 
   return 1;
 }
@@ -379,7 +379,7 @@ mips64_linux_get_longjmp_target (CORE_AD
 			  buf, TARGET_PTR_BIT / TARGET_CHAR_BIT))
     return 0;
 
-  *pc = extract_unsigned_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT);
+  *pc = extract_signed_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT);
 
   return 1;
 }
@@ -866,11 +866,8 @@ static void
 set_reg_offset (struct trad_frame_saved_reg *saved_regs,
 		int regno, CORE_ADDR offset)
 {
-  if (saved_regs[regno].addr == 0)
-    {
-      saved_regs[regno + 0 * NUM_REGS].addr = offset;
-      saved_regs[regno + 1 * NUM_REGS].addr = offset;
-    }
+  saved_regs[regno + 0 * NUM_REGS].addr = offset;
+  saved_regs[regno + 1 * NUM_REGS].addr = offset;
 }
 
 /* There are two possible layouts for a signal frame:
@@ -945,7 +942,7 @@ mips_linux_sigframe_find_saved_regs (COR
 				     int sigframe_kind)
 {
   int ireg, reg_position;
-  CORE_ADDR sigcontext_base = frame - SIGFRAME_CODE_OFFSET;
+  CORE_ADDR sigcontext_base = frame;
   const struct mips_regnum *regs;
 
   if (sigframe_kind == 1)
@@ -1067,7 +1064,7 @@ mips64_linux_sigframe_find_saved_regs (C
 {
   const struct mips_regnum *regs;
   int ireg, reg_position;
-  CORE_ADDR sigcontext_base = frame - SIGFRAME_CODE_OFFSET;
+  CORE_ADDR sigcontext_base = frame;
 
   if (sigframe_kind == 3)
     sigcontext_base += N64_SIGFRAME_SIGCONTEXT_OFFSET;
@@ -1191,7 +1188,7 @@ mips_make_sigtramp_cache (struct frame_i
 
   cache = frame_obstack_zalloc (sizeof (struct mips_prologue_cache));
 
-  cache->prev_sp = frame_unwind_register_unsigned (next_frame, SP_REGNUM);
+  cache->prev_sp = frame_unwind_register_signed (next_frame, NUM_REGS + SP_REGNUM);
 
   cache->saved_regs = trad_frame_alloc_saved_regs (next_frame);
 
@@ -1201,10 +1198,14 @@ mips_make_sigtramp_cache (struct frame_i
   mips_linux_sigframe_find_saved_regs (cache->prev_sp, cache->saved_regs,
 				       cache->kind);
 
+#if 0
   /* Is this right? */
   cache->prev_sp
     = read_memory_integer (cache->saved_regs[SP_REGNUM].addr,
-			   DEPRECATED_REGISTER_RAW_SIZE (SP_REGNUM));
+			   register_size (current_gdbarch, SP_REGNUM));
+  trad_frame_set_value (cache->saved_regs, SP_REGNUM, cache->prev_sp);
+  /* What about NUM_REGS + SP_REGNUM? */
+#endif
 
   return cache;
 }
diff -urp -x objdir -x '*~' gdb-2004-03-01-cvs.orig/gdb/mips-tdep.c gdb-2004-03-01-cvs/gdb/mips-tdep.c
--- gdb-2004-03-01-cvs.orig/gdb/mips-tdep.c	2004-03-02 16:56:42.000000000 -0500
+++ gdb-2004-03-01-cvs/gdb/mips-tdep.c	2004-03-04 14:51:06.000000000 -0500
@@ -1503,19 +1505,26 @@ mips_mdebug_frame_cache (struct frame_in
   /* Get the mdebug proc descriptor.  */
   proc_desc = find_proc_desc (frame_pc_unwind (next_frame), next_frame, 1);
   if (proc_desc == NULL)
-    /* I'm not sure how/whether this can happen.  Normally when we
-       can't find a proc_desc, we "synthesize" one using
-       heuristic_proc_desc and set the saved_regs right away.  */
-    return cache;
-
-  /* Extract the frame's base.  */
-  cache->base = (frame_unwind_register_signed (next_frame, NUM_REGS + PROC_FRAME_REG (proc_desc))
-		 + PROC_FRAME_OFFSET (proc_desc) - PROC_FRAME_ADJUST (proc_desc));
+    {
+      /* This can happen at least at PC == 0.  */
+      cache->base = frame_unwind_register_signed (next_frame, NUM_REGS + SP_REGNUM);
+      cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->pc]
+	= cache->saved_regs[NUM_REGS + RA_REGNUM];
+      return cache;
+    }
+
+  /* Extract the frame's base.  Assume that the first instruction sets it
+     up.  */
+  if (frame_pc_unwind (next_frame) == PROC_LOW_ADDR (proc_desc))
+    cache->base = frame_unwind_register_signed (next_frame, NUM_REGS + SP_REGNUM);
+  else
+    cache->base = (frame_unwind_register_signed (next_frame, NUM_REGS + PROC_FRAME_REG (proc_desc))
+		   + PROC_FRAME_OFFSET (proc_desc) - PROC_FRAME_ADJUST (proc_desc));
 
   kernel_trap = PROC_REG_MASK (proc_desc) & 1;
   gen_mask = kernel_trap ? 0xFFFFFFFF : PROC_REG_MASK (proc_desc);
   float_mask = kernel_trap ? 0xFFFFFFFF : PROC_FREG_MASK (proc_desc);
-  
+
   /* In any frame other than the innermost or a frame interrupted by a
      signal, we assume that all registers have been saved.  This
      assumes that all register saves in a function happen before the
@@ -1681,7 +1690,8 @@ mips_mdebug_frame_this_id (struct frame_
      glibc syscall stubs does not work.  Eventually MIPS will use
      allow us to solve this with DWARF unwind information instead.  */
   if (frame_relative_level (next_frame) >= 0
-      && !trad_frame_addr_p (info->saved_regs, PC_REGNUM))
+      && get_frame_type (next_frame) == NORMAL_FRAME
+      && !trad_frame_addr_p (info->saved_regs, NUM_REGS + PC_REGNUM))
     {
       (*this_id) = null_frame_id;
       return;
@@ -2471,7 +2481,8 @@ find_proc_desc (CORE_ADDR pc, struct fra
        */
       /* If the next frame is a signal handler trampoline, then we also
 	 need to do this, because we might be in the prologue.  */
-      if (next_frame == NULL || get_frame_type (next_frame) == SIGTRAMP_FRAME)
+      if (frame_relative_level (next_frame) < 0
+	  || get_frame_type (next_frame) != NORMAL_FRAME)
 	{
 	  struct symtab_and_line val;
 	  struct symbol *proc_symbol =
@@ -2489,7 +2500,10 @@ find_proc_desc (CORE_ADDR pc, struct fra
 		heuristic_proc_desc (PROC_LOW_ADDR (proc_desc),
 				     pc, next_frame, cur_frame);
 	      if (found_heuristic)
-		proc_desc = found_heuristic;
+		{
+		  PROC_REG_OFFSET (found_heuristic) = PROC_REG_OFFSET (proc_desc);
+		  proc_desc = found_heuristic;
+		}
 	    }
 	}
     }
@@ -3123,6 +3137,11 @@ mips_n32n64_push_dummy_call (struct gdba
 
       val = (char *) VALUE_CONTENTS (arg);
 
+      /* FIXME drow/2004-03-04: One unimplemented rule from the
+	 N32/N64 ABI is that any 64-bit chunk of a structure which
+	 consists of a double-precision floating point field (but not
+	 a double member of a union) will be passed in an FP reg
+	 instead of an integer reg.  */
       if (fp_register_arg_p (typecode, arg_type)
 	  && float_argreg <= MIPS_LAST_FP_ARG_REGNUM)
 	{
@@ -3147,13 +3166,6 @@ mips_n32n64_push_dummy_call (struct gdba
 	  /* Copy the argument to general registers or the stack in
 	     register-sized pieces.  Large arguments are split between
 	     registers and stack.  */
-	  /* Note: structs whose size is not a multiple of
-	     mips_regsize() are treated specially: Irix cc passes them
-	     in registers where gcc sometimes puts them on the stack.
-	     For maximum compatibility, we will put them in both
-	     places.  */
-	  int odd_sized_struct = ((len > mips_saved_regsize (tdep))
-				  && (len % mips_saved_regsize (tdep) != 0));
 	  /* Note: Floating-point values that didn't fit into an FP
 	     register are only written to memory.  */
 	  while (len > 0)
@@ -3169,7 +3181,6 @@ mips_n32n64_push_dummy_call (struct gdba
 
 	      /* Write this portion of the argument to the stack.  */
 	      if (argreg > MIPS_LAST_ARG_REGNUM
-		  || odd_sized_struct
 		  || fp_register_arg_p (typecode, arg_type))
 		{
 		  /* Should shorter than int integer values be
@@ -3209,15 +3220,10 @@ mips_n32n64_push_dummy_call (struct gdba
 		    }
 		  write_memory (addr, val, partial_len);
 		}
-
-	      /* Note!!! This is NOT an else clause.  Odd sized
-	         structs may go thru BOTH paths.  Floating point
-	         arguments will not.  */
-	      /* Write this portion of the argument to a general
-	         purpose register.  */
-	      if (argreg <= MIPS_LAST_ARG_REGNUM
-		  && !fp_register_arg_p (typecode, arg_type))
+	      else
 		{
+		  /* Write this portion of the argument to a general
+		     purpose register.  */
 		  LONGEST regval =
 		    extract_unsigned_integer (val, partial_len);
 
@@ -3290,10 +3296,10 @@ mips_n32n64_return_value (struct gdbarch
 			  void *readbuf, const void *writebuf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      || TYPE_CODE (type) == TYPE_CODE_UNION
-      || TYPE_CODE (type) == TYPE_CODE_ARRAY
-      || TYPE_LENGTH (type) > 2 * mips_saved_regsize (tdep))
+  if ((TYPE_CODE (type) == TYPE_CODE_STRUCT
+       || TYPE_CODE (type) == TYPE_CODE_UNION
+       || TYPE_CODE (type) == TYPE_CODE_ARRAY)
+      && TYPE_LENGTH (type) > 2 * mips_saved_regsize (tdep))
     return RETURN_VALUE_STRUCT_CONVENTION;
   else if (TYPE_CODE (type) == TYPE_CODE_FLT
 	   && TYPE_LENGTH (type) == 16
@@ -3366,6 +3372,29 @@ mips_n32n64_return_value (struct gdbarch
          mips_xfer_lower.  */
       int offset;
       int regnum;
+
+      /* This is... rough.  If we have a non-trivial copy constructor or
+         destructor, use reference.  This should be in common code.  */
+      int i, j;
+      for (i = 0; i < TYPE_NFN_FIELDS (type); i++)
+	for (j = 0; j < TYPE_FN_FIELDLIST_LENGTH (type, i); j++)
+	  {
+	    struct fn_field *fn = TYPE_FN_FIELDLIST1 (type, i);
+	    if (TYPE_FN_FIELD_ARTIFICIAL (fn, j))
+	      continue;
+	    if (TYPE_FN_FIELDLIST_NAME (type, i)[0] == '~')
+	      return RETURN_VALUE_STRUCT_CONVENTION;
+	    if (strncmp (TYPE_NAME (type),
+			 TYPE_FN_FIELDLIST_NAME (type, i),
+			 strlen (TYPE_NAME (type))) == 0
+		&& (TYPE_FN_FIELDLIST_NAME (type, i)[strlen (TYPE_NAME (type))] == '\0'
+		    || TYPE_FN_FIELDLIST_NAME (type, i)[strlen (TYPE_NAME (type))] == '<')
+		&& TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fn, j)) == 2
+		&& TYPE_CODE (TYPE_FIELD_TYPE (TYPE_FN_FIELD_TYPE (fn, j), 1)) == TYPE_CODE_REF
+		&& TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (TYPE_FN_FIELD_TYPE (fn, j), 1)) == type)
+	      return RETURN_VALUE_STRUCT_CONVENTION;
+	  }
+
       for (offset = 0, regnum = V0_REGNUM;
 	   offset < TYPE_LENGTH (type);
 	   offset += register_size (current_gdbarch, regnum), regnum++)


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]