From patchwork Fri Dec 7 20:34:10 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philippe Waroquiers X-Patchwork-Id: 30587 Received: (qmail 32363 invoked by alias); 7 Dec 2018 20:34:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 32329 invoked by uid 89); 7 Dec 2018 20:34:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-27.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=Forward, Hx-languages-length:4679, 1947 X-HELO: mailsec107.isp.belgacom.be Received: from mailsec107.isp.belgacom.be (HELO mailsec107.isp.belgacom.be) (195.238.20.103) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 07 Dec 2018 20:34:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1544214858; x=1575750858; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=/1H4TygzJTUXPaYF1yiqMa6HCf2JZRvZKCL/9mTQ5Sk=; b=TLf76d1WNWc9bzXV/ZJG4UpE3SEKiIhcK6peIDNu3gixLUeD6n16gSxP PKE/S83hswYGW0KwPo+jUpCUVbMPgg==; Received: from 184.205-67-87.adsl-dyn.isp.belgacom.be (HELO md.home) ([87.67.205.184]) by relay.skynet.be with ESMTP/TLS/DHE-RSA-AES128-GCM-SHA256; 07 Dec 2018 21:34:15 +0100 From: Philippe Waroquiers To: gdb-patches@sourceware.org Cc: Philippe Waroquiers Subject: [RFA] Fix leak by using td_ta_delete() to deregister target process and deallocate internal process handle. Date: Fri, 7 Dec 2018 21:34:10 +0100 Message-Id: <20181207203410.29636-1-philippe.waroquiers@skynet.be> MIME-Version: 1.0 X-IsSubscribed: yes 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, std::vector >*, enum_flags, objfile*) (symfile.c:1158) ==25327== by 0x5F5C4A: solib_read_symbols(so_list*, enum_flags) (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-07 Philippe Waroquiers * 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/linux-thread-db.c | 15 +++++++++++++++ gdb/nat/gdb_thread_db.h | 1 + 2 files changed, 16 insertions(+) diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c index 3c0998e02f..b3fe4fb641 100644 --- a/gdb/linux-thread-db.c +++ b/gdb/linux-thread-db.c @@ -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,17 @@ 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; + + err = info->td_ta_delete_p (info->thread_agent); + if (err != TD_OK) + warning (_("Cannot deregister target process: %s"), + thread_db_err_str (err)); + info->thread_agent = NULL; + } + if (info->handle != NULL) dlclose (info->handle); @@ -855,6 +869,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, diff --git a/gdb/nat/gdb_thread_db.h b/gdb/nat/gdb_thread_db.h index b8259c3aa2..618516ed3f 100644 --- a/gdb/nat/gdb_thread_db.h +++ b/gdb/nat/gdb_thread_db.h @@ -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,