This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFAv2] Fix leak in displaced step.


Valgrind reports a definite leak of displaced->step_saved_copy
(full leak stack trace below).

This patch fixes the leak by only allocating a new step_saved_copy
if the process displaced_step_inferior_state does not yet have one,
and by freeing it when the displaced_step_inferior_state of a process
is removed, when the inferior exits.

Regtested on debian/amd64 + step-over-syscall.exp rerun under valgrind.

==4736== VALGRIND_GDB_ERROR_BEGIN
==4736== 128 bytes in 8 blocks are definitely lost in loss record 980 of 3,108
==4736==    at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==4736==    by 0x41B627: xmalloc (common-utils.c:44)
==4736==    by 0x50D4E3: displaced_step_prepare_throw (infrun.c:1837)
==4736==    by 0x50D4E3: displaced_step_prepare (infrun.c:1898)
==4736==    by 0x50D4E3: resume_1 (infrun.c:2545)
==4736==    by 0x50D4E3: resume(gdb_signal) (infrun.c:2741)
==4736==    by 0x50DCD5: keep_going_pass_signal(execution_control_state*) (infrun.c:7793)
==4736==    by 0x50E903: process_event_stop_test(execution_control_state*) (infrun.c:6843)
==4736==    by 0x510925: handle_signal_stop(execution_control_state*) (infrun.c:6176)
==4736==    by 0x513F79: handle_inferior_event_1 (infrun.c:5354)
==4736==    by 0x513F79: handle_inferior_event(execution_control_state*) (infrun.c:5389)
==4736==    by 0x51541B: fetch_inferior_event(void*) (infrun.c:3916)
==4736==    by 0x4B3EEC: gdb_wait_for_event(int) (event-loop.c:859)
==4736==    by 0x4B3FF6: gdb_do_one_event() [clone .part.4] (event-loop.c:322)
==4736==    by 0x4B41B4: gdb_do_one_event (common-exceptions.h:219)
==4736==    by 0x4B41B4: start_event_loop() (event-loop.c:371)
==4736==    by 0x551237: captured_command_loop() (main.c:330)
==4736==    by 0x55222C: captured_main (main.c:1177)
==4736==    by 0x55222C: gdb_main(captured_main_args*) (main.c:1193)
==4736==    by 0x29B4F7: main (gdb.c:32)
==4736==
==4736== VALGRIND_GDB_ERROR_END

gdb/ChangeLog

2018-11-11  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* infrun.c (displaced_step_inferior_state): Explain why step_saved_copy
	is sometimes needed after the step-over is finished.
	(remove_displaced_stepping_state): xfree step_saved_copy.
	(displaced_step_clear): Add note that explains why we do not xfree
	step_saved_copy here.
---
 gdb/infrun.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9473d1f20f..1c40cb2e0f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1510,7 +1510,13 @@ struct displaced_step_inferior_state
      made.  */
   CORE_ADDR step_original, step_copy;
 
-  /* Saved contents of copy area.  */
+  /* Saved contents of copy area.  In most cases, we could get rid
+     of this copy when the displaced single-step is finished, after
+     having restored the content, when setting step_thread to nullptr.
+     However, we need to keep this content in case the step-over is
+     over a fork syscall: in such a case, the step-over was done in
+     the parent, but we also have to restore the copy area content
+     in the child, after the parent has finished the step-over.  */
   gdb_byte *step_saved_copy;
 };
 
@@ -1638,6 +1644,11 @@ remove_displaced_stepping_state (inferior *inf)
       if (it->inf == inf)
 	{
 	  *prev_next_p = it->next;
+	  if (it->step_saved_copy != NULL)
+	    {
+	      xfree (it->step_saved_copy);
+	      it->step_saved_copy = NULL;
+	    }
 	  xfree (it);
 	  return;
 	}
@@ -1709,6 +1720,10 @@ displaced_step_clear (struct displaced_step_inferior_state *displaced)
 
   delete displaced->step_closure;
   displaced->step_closure = NULL;
+
+  /* Note: we cannot xfree (displaced->step_saved_copy), as this
+     is needed to restore the content in the child, if
+     the step-over was over a fork syscall.  */
 }
 
 static void
@@ -1834,7 +1849,11 @@ displaced_step_prepare_throw (thread_info *tp)
     }
 
   /* Save the original contents of the copy area.  */
-  displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
+  if (displaced->step_saved_copy == NULL)
+    displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
+  /* Even if we have not allocated step_saved_copy now, make a
+     (temporary) cleanup for it, in case the setup below fails to
+     complete the copy.  */
   ignore_cleanups = make_cleanup (free_current_contents,
 				  &displaced->step_saved_copy);
   status = target_read_memory (copy, displaced->step_saved_copy, len);
-- 
2.19.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]