[libbacktrace] Fix memory leak in loop in build_address_map

When failing in build_address_map, we free the unit that's currently being
handled in the loop, but the ones that already have been allocated are leaked.

Fix this by keeping track of allocated units in a vector, and releasing them
upon failure.

Also, now that we have a vector of allocated units, move the freeing upon
failure of the abbrevs associated with each unit to build_address_map, and
remove the now redundant call to free_unit_addrs_vector.

Bootstrapped and reg-tested on x86_64.

2018-12-28  Ian Lance Taylor  <iant@golang.org>
	    Tom de Vries  <tdevries@suse.de>

	PR libbacktrace/88063
	* dwarf.c (free_unit_addrs_vector): Remove.
	(build_address_map): Keep track of allocated units in vector.  Free
	allocated units and corresponding abbrevs upon failure.  Remove now
	redundant call to free_unit_addrs_vector.  Free addrs vector upon
	failure.  Free allocated unit vector.

git-svn-id: https://gcc.gnu.org/svn/gcc/trunk@267443 138bc75d-0d04-0410-961f-82ee72b054a4

2 files changed, 44 insertions, 26 deletions

diff --git a/libbacktrace/ChangeLog b/libbacktrace/ChangeLog
index 60e5f03d0f2..27a0f429331 100644
--- a/libbacktrace/ChangeLog
+++ b/libbacktrace/ChangeLog
@@ -1,3 +1,13 @@
+2018-12-28  Ian Lance Taylor  <iant@golang.org>
+	    Tom de Vries  <tdevries@suse.de>
+
+	PR libbacktrace/88063
+	* dwarf.c (free_unit_addrs_vector): Remove.
+	(build_address_map): Keep track of allocated units in vector.  Free
+	allocated units and corresponding abbrevs upon failure.  Remove now
+	redundant call to free_unit_addrs_vector.  Free addrs vector upon
+	failure.  Free allocated unit vector.
+
 2018-12-28  Tom de Vries  <tdevries@suse.de>
 
 	* dwarf.c (build_address_map): Free addrs vector upon failure.
diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 37a08ca29a8..f3499a9f45a 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -923,21 +923,6 @@ add_unit_addr (struct backtrace_state *state, uintptr_t base_address,
   return 1;
 }
 
-/* Free a unit address vector.  */
-
-static void
-free_unit_addrs_vector (struct backtrace_state *state,
-			struct unit_addrs_vector *vec,
-			backtrace_error_callback error_callback, void *data)
-{
-  struct unit_addrs *addrs;
-  size_t i;
-
-  addrs = (struct unit_addrs *) vec->vec.base;
-  for (i = 0; i < vec->count; ++i)
-    free_abbrevs (state, &addrs[i].u->abbrevs, error_callback, data);
-}
-
 /* Compare unit_addrs for qsort.  When ranges are nested, make the
    smallest one sort last.  */
 
@@ -1448,6 +1433,10 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
 {
   struct dwarf_buf info;
   struct abbrevs abbrevs;
+  struct backtrace_vector units;
+  size_t units_count;
+  size_t i;
+  struct unit **pu;
 
   memset (&addrs->vec, 0, sizeof addrs->vec);
   addrs->count = 0;
@@ -1465,6 +1454,9 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
   info.data = data;
   info.reported_underflow = 0;
 
+  memset (&units, 0, sizeof units);
+  units_count = 0;
+
   memset (&abbrevs, 0, sizeof abbrevs);
   while (info.left > 0)
     {
@@ -1503,10 +1495,20 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
 
       addrsize = read_byte (&unit_buf);
 
+      pu = ((struct unit **)
+	    backtrace_vector_grow (state, sizeof (struct unit *),
+				   error_callback, data, &units));
+      if (pu == NULL)
+	  goto fail;
+
       u = ((struct unit *)
 	   backtrace_alloc (state, sizeof *u, error_callback, data));
       if (u == NULL)
 	goto fail;
+
+      *pu = u;
+      ++units_count;
+
       u->unit_data = unit_buf.buf;
       u->unit_data_len = unit_buf.left;
       u->unit_data_offset = unit_buf.buf - unit_data_start;
@@ -1531,27 +1533,33 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
 				dwarf_ranges, dwarf_ranges_size,
 				is_bigendian, error_callback, data,
 				u, addrs))
-	{
-	  free_abbrevs (state, &u->abbrevs, error_callback, data);
-	  backtrace_free (state, u, sizeof *u, error_callback, data);
-	  goto fail;
-	}
+	goto fail;
 
       if (unit_buf.reported_underflow)
-	{
-	  free_abbrevs (state, &u->abbrevs, error_callback, data);
-	  backtrace_free (state, u, sizeof *u, error_callback, data);
-	  goto fail;
-	}
+	goto fail;
     }
   if (info.reported_underflow)
     goto fail;
 
+  // We only kept the list of units to free them on failure.  On
+  // success the units are retained, pointed to by the entries in
+  // addrs.
+  backtrace_vector_free (state, &units, error_callback, data);
+
   return 1;
 
  fail:
+  if (units_count > 0)
+    {
+      pu = (struct unit **) units.base;
+      for (i = 0; i < units_count; i++)
+	{
+	  free_abbrevs (state, &pu[i]->abbrevs, error_callback, data);
+	  backtrace_free (state, pu[i], sizeof **pu, error_callback, data);
+	}
+      backtrace_vector_free (state, &units, error_callback, data);
+    }
   free_abbrevs (state, &abbrevs, error_callback, data);
-  free_unit_addrs_vector (state, addrs, error_callback, data);
   if (addrs->count > 0)
     {
       backtrace_vector_free (state, &addrs->vec, error_callback, data);