Fix leak by using td_ta_delete() to deregister target process and deallocate internal...
authorPhilippe Waroquiers <philippe.waroquiers@skynet.be>
Fri, 7 Dec 2018 17:13:59 +0000 (18:13 +0100)
committerPhilippe Waroquiers <philippe.waroquiers@skynet.be>
Sat, 8 Dec 2018 16:06:09 +0000 (17:06 +0100)
Valgrind reports the below leak:

==25327== VALGRIND_GDB_ERROR_BEGIN
==25327== 672 bytes in 1 blocks are definitely lost in loss record 2,759 of 3,251
==25327==    at 0x4C2E07C: calloc (vg_replace_malloc.c:752)
==25327==    by 0x7FDCB3E: ???
==25327==    by 0x532A7A: try_thread_db_load_1 (linux-thread-db.c:828)
==25327==    by 0x532A7A: try_thread_db_load(char const*, int) (linux-thread-db.c:997)
==25327==    by 0x53354D: try_thread_db_load_from_sdir (linux-thread-db.c:1074)
==25327==    by 0x53354D: thread_db_load_search (linux-thread-db.c:1129)
==25327==    by 0x53354D: thread_db_load() (linux-thread-db.c:1187)
==25327==    by 0x611AF1: operator() (functional:2127)
==25327==    by 0x611AF1: notify (observable.h:106)
==25327==    by 0x611AF1: symbol_file_add_with_addrs(bfd*, char const*, enum_flags<symfile_add_flag>, std::vector<other_sections, std::allocator<other_sections> >*, enum_flags<objfile_flag>, objfile*) (symfile.c:1158)
==25327==    by 0x5F5C4A: solib_read_symbols(so_list*, enum_flags<symfile_add_flag>) (solib.c:691)
==25327==    by 0x5F6A8B: solib_add(char const*, int, int) (solib.c:1003)
==25327==    by 0x5F6BF7: handle_solib_event() (solib.c:1281)
==25327==    by 0x3D0A94: bpstat_stop_status(address_space const*, unsigned long, thread_info*, target_waitstatus const*, bpstats*) (breakpoint.c:5417)
==25327==    by 0x4FF133: handle_signal_stop(execution_control_state*) (infrun.c:5874)
==25327==    by 0x502C29: handle_inferior_event_1 (infrun.c:5300)
==25327==    by 0x502C29: handle_inferior_event(execution_control_state*) (infrun.c:5335)
==25327==    by 0x5041DB: fetch_inferior_event(void*) (infrun.c:3868)
==25327==    by 0x4A1E7C: gdb_wait_for_event(int) (event-loop.c:859)
...

This leak is created because a call to td_ta_new allocates some resources
that must be freed with td_ta_delete, and that was missing.

With this patch, the nr of GDB executions leaking during regression tests
decreases further from 566 to 380.

Note that the gdbserver equivalent code is properly calling
td_ta_delete: see thread_db_mourn in thread-db.c.

Tests run natively on debian/amd64, and run under valgrind.

gdb/ChangeLog
2018-12-08  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

* linux-thread-db.c (struct thread_db_info): Add td_ta_delete_p.
(thread_db_err_str): Forward declare.
(delete_thread_db_info): Call td_ta_delete_p if available.
(try_thread_db_load_1): Acquire td_ta_delete address.
* nat/gdb_thread_db.h (td_ta_delete_ftype): Declare.

gdb/ChangeLog
gdb/linux-thread-db.c
gdb/nat/gdb_thread_db.h

index df49408b8b6bee3f69736c5d8db679b17bbcc083..ec5a7e483557e520d9a2eda5722cb9a45ae75e3d 100644 (file)
@@ -1,3 +1,11 @@
+2018-12-08  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+       * linux-thread-db.c (struct thread_db_info): Add td_ta_delete_p.
+       (thread_db_err_str): Forward declare.
+       (delete_thread_db_info): Call td_ta_delete_p if available.
+       (try_thread_db_load_1): Acquire td_ta_delete address.
+       * nat/gdb_thread_db.h (td_ta_delete_ftype): Declare.
+
 2018-12-08  Pedro Alves  <palves@redhat.com>
 
        * source.c (forward_search_command): Rename to ...
index 3c0998e02f3956485481c9ae2b0c0fa76fd8125a..3027caa8b529f9ebccea9f4d84b432d2adb1b4eb 100644 (file)
@@ -194,6 +194,7 @@ struct thread_db_info
 
   td_init_ftype *td_init_p;
   td_ta_new_ftype *td_ta_new_p;
+  td_ta_delete_ftype *td_ta_delete_p;
   td_ta_map_lwp2thr_ftype *td_ta_map_lwp2thr_p;
   td_ta_thr_iter_ftype *td_ta_thr_iter_p;
   td_thr_get_info_ftype *td_thr_get_info_p;
@@ -254,6 +255,8 @@ get_thread_db_info (int pid)
   return NULL;
 }
 
+static const char *thread_db_err_str (td_err_e err);
+
 /* When PID has exited or has been detached, we no longer want to keep
    track of it as using libpthread.  Call this function to discard
    thread_db related info related to PID.  Note that this closes
@@ -273,6 +276,16 @@ delete_thread_db_info (int pid)
   if (info == NULL)
     return;
 
+  if (info->thread_agent != NULL && info->td_ta_delete_p != NULL)
+    {
+      td_err_e err = info->td_ta_delete_p (info->thread_agent);
+
+      if (err != TD_OK)
+       warning (_("Cannot deregister process %d from libthread_db: %s"),
+                pid, thread_db_err_str (err));
+      info->thread_agent = NULL;
+    }
+
   if (info->handle != NULL)
     dlclose (info->handle);
 
@@ -855,6 +868,7 @@ try_thread_db_load_1 (struct thread_db_info *info)
   /* These are not essential.  */
   TDB_DLSYM (info, td_thr_tls_get_addr);
   TDB_DLSYM (info, td_thr_tlsbase);
+  TDB_DLSYM (info, td_ta_delete);
 
   /* It's best to avoid td_ta_thr_iter if possible.  That walks data
      structures in the inferior's address space that may be corrupted,
index b8259c3aa27713239535590115a96e57d0bd09af..618516ed3f2fc044ba86b3c8da25b3a02ca1241b 100644 (file)
@@ -41,6 +41,7 @@ typedef td_err_e (td_init_ftype) (void);
 
 typedef td_err_e (td_ta_new_ftype) (struct ps_prochandle * ps,
                                    td_thragent_t **ta);
+typedef td_err_e (td_ta_delete_ftype) (td_thragent_t *ta_p);
 typedef td_err_e (td_ta_map_lwp2thr_ftype) (const td_thragent_t *ta,
                                            lwpid_t lwpid, td_thrhandle_t *th);
 typedef td_err_e (td_ta_thr_iter_ftype) (const td_thragent_t *ta,