public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
To: gdb-patches@sourceware.org
Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Subject: [RFA] Fix regcache leak, and avoid possible regcache access after detach.
Date: Sat, 16 Feb 2019 20:58:00 -0000	[thread overview]
Message-ID: <20190216205810.1253-1-philippe.waroquiers@skynet.be> (raw)

Valgrind reports leaks like the below in various tests,
e.g. gdb.threads/attach-slow-waitpid.exp, gdb.ada/task_switch_in_core.exp, ...

Fix the leak by clearing the regcache when detaching from an inferior.
Note that these leaks are 'created' when GDB exits,
when the regcache::current_regcache is destroyed : the elements
of the forward_list are pointers, and the 'pointed to' memory is not
deleted by the forward_list destructor.

Nevertheless, fixing this leak is good as it makes a bunch of
tests 'leak clean'.

Also, it seems strange to keep a register cache for a process from
which GDB detached : it is not clear if this cache is still valid
after detach.  And effectively, when clearing only the regcache,
(and not the frame cache), then the frame cache was still 'pointing'
at this regcache and was used when switching to the child process
in the test gdb.threads/watchpoint-fork.exp, which seems strange.

So, we solve the leak and avoid possible accesses to the regcache
and frame cache of the detached inferior, by clearing both the
regcache and the frame cache.

Tested on debian/amd64, natively and under Valgrind.

==27679== VALGRIND_GDB_ERROR_BEGIN
==27679== 1,123 (72 direct, 1,051 indirect) bytes in 1 blocks are definitely lost in loss record 2,942 of 3,400
==27679==    at 0x4C2C4CC: operator new(unsigned long) (vg_replace_malloc.c:344)
==27679==    by 0x5CDF71: get_thread_arch_aspace_regcache(ptid_t, gdbarch*, address_space*) (regcache.c:330)
==27679==    by 0x5CE12A: get_thread_regcache (regcache.c:366)
==27679==    by 0x5CE12A: get_current_regcache() (regcache.c:372)
==27679==    by 0x4FF63D: post_create_inferior(target_ops*, int) (infcmd.c:452)
==27679==    by 0x43AF62: core_target_open(char const*, int) (corelow.c:458)
==27679==    by 0x408B68: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1892)
==27679==    by 0x66B399: execute_command(char const*, int) (top.c:630)
==27679==    by 0x4B4D2B: command_handler(char const*) (event-top.c:583)
==27679==    by 0x4B505C: command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) (event-top.c:770)
==27679==    by 0x4B3EA3: gdb_rl_callback_handler(char*) (event-top.c:213)
==27679==    by 0x4E68AC2: rl_callback_read_char (callback.c:283)
==27679==    by 0x4B3DDD: gdb_rl_callback_read_char_wrapper_noexcept() (event-top.c:175)
==27679==    by 0x4B3E48: gdb_rl_callback_read_char_wrapper(void*) (event-top.c:192)
==27679==    by 0x4B43EF: stdin_event_handler(int, void*) (event-top.c:511)
==27679==    by 0x4B31BC: gdb_wait_for_event(int) (event-loop.c:859)
==27679==    by 0x4B32F3: gdb_do_one_event() [clone .part.4] (event-loop.c:347)
==27679==    by 0x4B3484: gdb_do_one_event (common-exceptions.h:219)
==27679==    by 0x4B3484: start_event_loop() (event-loop.c:371)
==27679==    by 0x549967: captured_command_loop() (main.c:330)
==27679==    by 0x54A95C: captured_main (main.c:1176)
==27679==    by 0x54A95C: gdb_main(captured_main_args*) (main.c:1192)
==27679==    by 0x297517: main (gdb.c:32)

gdb/ChangeLog
2019-02-16  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* target.c (target_detach): Clear the regcache and the
	frame cache.
---
 gdb/target.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/gdb/target.c b/gdb/target.c
index 116510e8cb..0f10628590 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2010,6 +2010,8 @@ target_preopen (int from_tty)
 void
 target_detach (inferior *inf, int from_tty)
 {
+  ptid_t pid_ptid = ptid_t (inf->pid);
+
   /* As long as some to_detach implementations rely on the current_inferior
      (either directly, or indirectly, like through target_gdbarch or by
      reading memory), INF needs to be the current inferior.  When that
@@ -2029,6 +2031,15 @@ target_detach (inferior *inf, int from_tty)
   prepare_for_detach ();
 
   current_top_target ()->detach (inf, from_tty);
+
+  /* After we have detached, clear the register cache for this inferior.  */
+  registers_changed_ptid (pid_ptid);
+
+  /* We have to ensure we have no frame cached left.  Normally,
+     registers_changed_ptid (pid_ptid) calls reinit_frame_cache when
+     inferior_ptid matches pid_ptid, but in our case, it does not
+     call it, as inferior_pid has been reset.  */
+  reinit_frame_cache ();
 }
 
 void
-- 
2.20.1

             reply	other threads:[~2019-02-16 20:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-16 20:58 Philippe Waroquiers [this message]
2019-02-19 23:44 ` Kevin Buettner
2019-02-27 21:44   ` Philippe Waroquiers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190216205810.1253-1-philippe.waroquiers@skynet.be \
    --to=philippe.waroquiers@skynet.be \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).