[patch/rfc] allocate retbuf with xmalloc() instead of alloca()

[Andrew Cagney <ac131313 at ges dot redhat dot com>]

(more regcache stuff, found a memory leak)

When making an inferior function call, GDB ends up saving the register 
state (from the end of the call) in a buffer.  The buffer is currently 
allocated using alloca().  This patch changes things so that the buffer 
is allocated using xmalloc().  Cleanups are then used to recover the space.

A follow-on patch will replace the xmalloc() with regcache_xmalloc() and 
cause all sorts of fallout.  The objective here is to just implement the 
alloca() -> xmalloc() conversion step.

I'll check it in, in a few days.

comments?
Andrew

2002-07-01  Andrew Cagney  <ac131313@redhat.com>

	* valops.c (hand_function_call): Move declaration of retbuf to
	start of function, allocate using malloc, add a cleanup but before
	the inf_status cleanup, cleanup the buffer.  Rename local variable
	old_chain to inf_status_cleanup.

Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.61
diff -u -r1.61 valops.c
--- valops.c	18 Jun 2002 23:41:02 -0000	1.61
+++ valops.c	1 Jul 2002 21:53:40 -0000
@@ -1316,8 +1316,10 @@
   struct type *value_type;
   unsigned char struct_return;
   CORE_ADDR struct_addr = 0;
+  char *retbuf;
+  struct cleanup *retbuf_cleanup;
   struct inferior_status *inf_status;
-  struct cleanup *old_chain;
+  struct cleanup *inf_status_cleanup;
   CORE_ADDR funaddr;
   int using_gcc;		/* Set to version of gcc in use, or zero if not gcc */
   CORE_ADDR real_pc;
@@ -1333,8 +1335,18 @@
   if (!target_has_execution)
     noprocess ();
 
+  /* Create a cleanup chain that contains the retbuf (buffer
+     containing the register values).  This chain is create BEFORE the
+     inf_status chain so that the inferior status can cleaned up
+     (restored or discarded) without having the retbuf freed.  */
+  retbuf = xmalloc (REGISTER_BYTES);
+  retbuf_cleanup = make_cleanup (xfree, retbuf);
+
+  /* A cleanup for the inferior status.  Create this AFTER the retbuf
+     so that this can be discarded or applied without interfering with
+     the regbuf.  */
   inf_status = save_inferior_status (1);
-  old_chain = make_cleanup_restore_inferior_status (inf_status);
+  inf_status_cleanup = make_cleanup_restore_inferior_status (inf_status);
 
   /* PUSH_DUMMY_FRAME is responsible for saving the inferior registers
      (and POP_FRAME for restoring them).  (At least on most machines)
@@ -1656,7 +1668,6 @@
     SAVE_DUMMY_FRAME_TOS (sp);
 
   {
-    char *retbuf = (char*) alloca (REGISTER_BYTES);
     char *name;
     struct symbol *symbol;
 
@@ -1715,11 +1726,12 @@
 	  {
 	    /* The user wants to stay in the frame where we stopped (default).*/
 
-	    /* If we did the cleanups, we would print a spurious error
-	       message (Unable to restore previously selected frame),
-	       would write the registers from the inf_status (which is
-	       wrong), and would do other wrong things.  */
-	    discard_cleanups (old_chain);
+	    /* If we restored the inferior status (via the cleanup),
+	       we would print a spurious error message (Unable to
+	       restore previously selected frame), would write the
+	       registers from the inf_status (which is wrong), and
+	       would do other wrong things.  */
+	    discard_cleanups (inf_status_cleanup);
 	    discard_inferior_status (inf_status);
 
 	    /* FIXME: Insert a bunch of wrap_here; name can be very long if it's
@@ -1737,11 +1749,12 @@
       {
 	/* We hit a breakpoint inside the FUNCTION. */
 
-	/* If we did the cleanups, we would print a spurious error
-	   message (Unable to restore previously selected frame),
-	   would write the registers from the inf_status (which is
-	   wrong), and would do other wrong things.  */
-	discard_cleanups (old_chain);
+	/* If we restored the inferior status (via the cleanup), we
+	   would print a spurious error message (Unable to restore
+	   previously selected frame), would write the registers from
+	   the inf_status (which is wrong), and would do other wrong
+	   things.  */
+	discard_cleanups (inf_status_cleanup);
 	discard_inferior_status (inf_status);
 
 	/* The following error message used to say "The expression
@@ -1761,7 +1774,10 @@
       }
 
     /* If we get here the called FUNCTION run to completion. */
-    do_cleanups (old_chain);
+
+    /* Restore the inferior status, via its cleanup.  At this stage,
+       leave the RETBUF alone.  */
+    do_cleanups (inf_status_cleanup);
 
     /* Figure out the value returned by the function.  */
 /* elz: I defined this new macro for the hppa architecture only.
@@ -1774,10 +1790,17 @@
 
 #ifdef VALUE_RETURNED_FROM_STACK
     if (struct_return)
-      return (struct value *) VALUE_RETURNED_FROM_STACK (value_type, struct_addr);
+      {
+	do_cleanups (retbuf_cleanup);
+	return VALUE_RETURNED_FROM_STACK (value_type, struct_addr);
+      }
 #endif
 
-    return value_being_returned (value_type, retbuf, struct_return);
+    {
+      struct value *retval = value_being_returned (value_type, retbuf, struct_return);
+      do_cleanups (retbuf_cleanup);
+      return retval;
+    }
   }
 }