[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.

Co-Authored-By: Tom de Vries <tdevries@suse.de>
From-SVN: r267443

diff --git a/libbacktrace/ChangeLog b/libbacktrace/ChangeLog
index 60e5f03d0f2b30559eadb2aa07ccf3b14ff8e09e..27a0f429331f0b05af3cfc7e2eaddc27a9fbb3c6 100644 (file)
--- 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 37a08ca29a8bb4b847d4f1602d921fb2f7898c6b..f3499a9f45a6021c39cfce9696aa6846a1aa0b08 100644 (file)
--- 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);