Fix memory leaks in libunwind unwinder, part 2

Fix memory leaks in libunwind unwinder, part 2

[Andreas Schwab]

This is the second part of the memory leak fix for the libunwind
unwinder.  There is no significant increase in gdb's memory any more.

Andreas.

2007-06-08  Andreas Schwab  <schwab@suse.de>

	* frame-unwind.h (frame_dealloc_cache_ftype): Define.
	(struct frame_unwind): Add dealloc_cache.
	* frame.c (reinit_frame_cache): Call dealloc_cache on all caches.

	* libunwind-frame.h (libunwind_frame_dealloc_cache): Declare.
	* libunwind-frame.c (libunwind_frame_dealloc_cache): Define.
	(libunwind_frame_unwind): Set dealloc_cache.
	* ia64-tdep.c (ia64_libunwind_frame_unwind): Set dealloc_cache.

Index: frame-unwind.h
===================================================================
RCS file: /cvs/src/src/gdb/frame-unwind.h,v
retrieving revision 1.16
diff -u -a -p -r1.16 frame-unwind.h
--- frame-unwind.h	9 Jan 2007 17:58:50 -0000	1.16
+++ frame-unwind.h	8 Jun 2007 14:25:33 -0000
@@ -125,6 +125,10 @@ typedef void (frame_prev_register_ftype)
 typedef CORE_ADDR (frame_prev_pc_ftype) (struct frame_info *next_frame,
 					 void **this_prologue_cache);
 
+/* Deallocate extra memory associated with the frame cache if any.  */
+
+typedef void (frame_dealloc_cache_ftype) (struct frame_info *self,
+					  void *this_cache);
 
 struct frame_unwind
 {
@@ -138,6 +142,7 @@ struct frame_unwind
   const struct frame_data *unwind_data;
   frame_sniffer_ftype *sniffer;
   frame_prev_pc_ftype *prev_pc;
+  frame_dealloc_cache_ftype *dealloc_cache;
 };
 
 /* Register a frame unwinder, _prepending_ it to the front of the
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.227
diff -u -a -p -r1.227 frame.c
--- frame.c	6 Jun 2007 15:26:28 -0000	1.227
+++ frame.c	8 Jun 2007 14:25:33 -0000
@@ -1077,6 +1077,17 @@ frame_observer_target_changed (struct ta
 void
 reinit_frame_cache (void)
 {
+  struct frame_info *fi;
+
+  /* Tear down all frame caches.  */
+  for (fi = current_frame; fi != NULL; fi = fi->prev)
+    {
+      if (fi->prologue_cache && fi->unwind->dealloc_cache)
+	fi->unwind->dealloc_cache (fi, fi->prologue_cache);
+      if (fi->base_cache && fi->base->unwind->dealloc_cache)
+	fi->base->unwind->dealloc_cache (fi, fi->base_cache);
+    }
+
   /* Since we can't really be sure what the first object allocated was */
   obstack_free (&frame_cache_obstack, 0);
   obstack_init (&frame_cache_obstack);
Index: ia64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-tdep.c,v
retrieving revision 1.151
diff -u -a -p -r1.151 ia64-tdep.c
--- ia64-tdep.c	12 May 2007 00:07:45 -0000	1.151
+++ ia64-tdep.c	8 Jun 2007 14:25:33 -0000
@@ -2778,7 +2778,11 @@ static const struct frame_unwind ia64_li
 {
   NORMAL_FRAME,
   ia64_libunwind_frame_this_id,
-  ia64_libunwind_frame_prev_register
+  ia64_libunwind_frame_prev_register,
+  NULL,
+  NULL,
+  NULL,
+  libunwind_frame_dealloc_cache
 };
 
 static const struct frame_unwind *
Index: libunwind-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/libunwind-frame.c,v
retrieving revision 1.15
diff -u -a -p -r1.15 libunwind-frame.c
--- libunwind-frame.c	6 Jun 2007 19:35:02 -0000	1.15
+++ libunwind-frame.c	8 Jun 2007 14:25:34 -0000
@@ -65,6 +65,7 @@ struct libunwind_frame_cache
   CORE_ADDR base;
   CORE_ADDR func_addr;
   unw_cursor_t cursor;
+  unw_addr_space_t as;
 };
 
 /* We need to qualify the function names with a platform-specific prefix to match 
@@ -187,11 +188,20 @@ libunwind_frame_cache (struct frame_info
     }
 
   cache->base = (CORE_ADDR)fp;
+  cache->as = as;
 
   *this_cache = cache;
   return cache;
 }
 
+void
+libunwind_frame_dealloc_cache (struct frame_info *self, void *this_cache)
+{
+  struct libunwind_frame_cache *cache = this_cache;
+  if (cache->as)
+    unw_destroy_addr_space_p (cache->as);
+}
+
 unw_word_t
 libunwind_find_dyn_list (unw_addr_space_t as, unw_dyn_info_t *di, void *arg)
 {
@@ -202,7 +212,11 @@ static const struct frame_unwind libunwi
 {
   NORMAL_FRAME,
   libunwind_frame_this_id,
-  libunwind_frame_prev_register
+  libunwind_frame_prev_register,
+  NULL,
+  NULL,
+  NULL,
+  libunwind_frame_dealloc_cache
 };
 
 /* Verify if there is sufficient libunwind information for the frame to use
Index: libunwind-frame.h
===================================================================
RCS file: /cvs/src/src/gdb/libunwind-frame.h,v
retrieving revision 1.8
diff -u -a -p -r1.8 libunwind-frame.h
--- libunwind-frame.h	12 May 2007 00:07:45 -0000	1.8
+++ libunwind-frame.h	8 Jun 2007 14:25:34 -0000
@@ -53,6 +53,7 @@ void libunwind_frame_prev_register (stru
 				    int regnum, int *optimizedp,
 				    enum lval_type *lvalp, CORE_ADDR *addrp,
 				    int *realnump, gdb_byte *valuep);
+void libunwind_frame_dealloc_cache (struct frame_info *self, void *cache);
 CORE_ADDR libunwind_frame_base_address (struct frame_info *next_frame, void **this_cache);
 
 int libunwind_is_initialized (void);

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Fix memory leaks in libunwind unwinder, part 2

[Daniel Jacobowitz]

On Fri, Jun 08, 2007 at 05:09:41PM +0200, Andreas Schwab wrote:
> This is the second part of the memory leak fix for the libunwind
> unwinder.  There is no significant increase in gdb's memory any more.
> 
> Andreas.
> 
> 2007-06-08  Andreas Schwab  <schwab@suse.de>
> 
> 	* frame-unwind.h (frame_dealloc_cache_ftype): Define.
> 	(struct frame_unwind): Add dealloc_cache.
> 	* frame.c (reinit_frame_cache): Call dealloc_cache on all caches.
> 
> 	* libunwind-frame.h (libunwind_frame_dealloc_cache): Declare.
> 	* libunwind-frame.c (libunwind_frame_dealloc_cache): Define.
> 	(libunwind_frame_unwind): Set dealloc_cache.
> 	* ia64-tdep.c (ia64_libunwind_frame_unwind): Set dealloc_cache.

Thanks, this is OK with one fix.
> +  /* Tear down all frame caches.  */
> +  for (fi = current_frame; fi != NULL; fi = fi->prev)
> +    {
> +      if (fi->prologue_cache && fi->unwind->dealloc_cache)
> +	fi->unwind->dealloc_cache (fi, fi->prologue_cache);
> +      if (fi->base_cache && fi->base->unwind->dealloc_cache)
> +	fi->base->unwind->dealloc_cache (fi, fi->base_cache);
> +    }

Compare with:

  /* Sneaky: If the low-level unwind and high-level base code share a
     common unwinder, let them share the prologue cache.  */
  if (fi->base->unwind == fi->unwind)
    return fi->base->this_base (fi->next, &fi->prologue_cache);
  return fi->base->this_base (fi->next, &fi->base_cache);

I think you shouldn't dealloc the base cache if the two unwinders are
the same, right?

-- 
Daniel Jacobowitz
CodeSourcery

Re: Fix memory leaks in libunwind unwinder, part 2

[Andreas Schwab]

Daniel Jacobowitz <drow@false.org> writes:

> On Fri, Jun 08, 2007 at 05:09:41PM +0200, Andreas Schwab wrote:
>> This is the second part of the memory leak fix for the libunwind
>> unwinder.  There is no significant increase in gdb's memory any more.
>> 
>> Andreas.
>> 
>> 2007-06-08  Andreas Schwab  <schwab@suse.de>
>> 
>> 	* frame-unwind.h (frame_dealloc_cache_ftype): Define.
>> 	(struct frame_unwind): Add dealloc_cache.
>> 	* frame.c (reinit_frame_cache): Call dealloc_cache on all caches.
>> 
>> 	* libunwind-frame.h (libunwind_frame_dealloc_cache): Declare.
>> 	* libunwind-frame.c (libunwind_frame_dealloc_cache): Define.
>> 	(libunwind_frame_unwind): Set dealloc_cache.
>> 	* ia64-tdep.c (ia64_libunwind_frame_unwind): Set dealloc_cache.
>
> Thanks, this is OK with one fix.
>> +  /* Tear down all frame caches.  */
>> +  for (fi = current_frame; fi != NULL; fi = fi->prev)
>> +    {
>> +      if (fi->prologue_cache && fi->unwind->dealloc_cache)
>> +	fi->unwind->dealloc_cache (fi, fi->prologue_cache);
>> +      if (fi->base_cache && fi->base->unwind->dealloc_cache)
>> +	fi->base->unwind->dealloc_cache (fi, fi->base_cache);
>> +    }
>
> Compare with:
>
>   /* Sneaky: If the low-level unwind and high-level base code share a
>      common unwinder, let them share the prologue cache.  */
>   if (fi->base->unwind == fi->unwind)
>     return fi->base->this_base (fi->next, &fi->prologue_cache);
>   return fi->base->this_base (fi->next, &fi->base_cache);
>
> I think you shouldn't dealloc the base cache if the two unwinders are
> the same, right?

In this case the base cache was never used, so there is nothing to
deallocate.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Fix memory leaks in libunwind unwinder, part 2

[Daniel Jacobowitz]

On Tue, Jun 12, 2007 at 04:53:40PM +0200, Andreas Schwab wrote:
> In this case the base cache was never used, so there is nothing to
> deallocate.

Oh, I see how it works.  Patch OK.

-- 
Daniel Jacobowitz
CodeSourcery