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]

[patch] Fix leak of bp_jit_event breakpoints


Greetings,

On manual inspection of 'maint info break', I noticed that many instances
of __jit_debug_register_code breakpoint are leaked. For example:

gdb -q --args gdb.base/jit-main gdb.base/jit-solib.so 1
Reading symbols from /usr/local/google/jit-main...done.
(gdb) run
../../../src/gdb/testsuite/gdb.base/jit-main.c:131: libname = gdb.base/jit-solib.so, count = 1

Program exited normally.
(gdb) maint info b
Num     Type           Disp Enb Address            What
-1      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-2      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-4      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-5      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-6      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-7      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-14     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-15     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-16     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-17     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-18     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-19     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
(gdb) run
../../../src/gdb/testsuite/gdb.base/jit-main.c:131: libname = gdb.base/jit-solib.so, count = 1

Program exited normally.
(gdb) maint info b
Num     Type           Disp Enb Address            What
-1      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-2      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-4      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-5      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-6      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-7      jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-20     jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-22     jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-23     jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-24     jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-25     jit events     keep y   0x00000000004009c4 <__jit_debug_register_code> inf 1
        breakpoint already hit 2 times
-32     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-33     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-34     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-35     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-36     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1
-37     longjmp master keep n   0x00007ffff780a8b0 <__libc_siglongjmp> inf 1


That leak is in fact also causing jit.exp test case to fail with gdbserver
(native), which passes after this patch is applied.

Here is a proposed cleanup/fix.


Thanks,

--
Paul Pluzhnikov

2011-01-19  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* jit.c (registering_code): Delete.
	(clear_int): Delete.
	(jit_register_code): Adjust.
	(jit_breakpoint_re_set_internal): New function; move code here ...
	(jit_inferior_init): ... from here.
	(jit_breakpoint_re_set): Adjust.
	* breakpoint.c (breakpoint_re_set_one): Delete bp_jit_event
	breakpoint instead of leaking it.


Index: jit.c
===================================================================
RCS file: /cvs/src/src/gdb/jit.c,v
retrieving revision 1.10
diff -u -p -r1.10 jit.c
--- jit.c	9 Jan 2011 03:08:57 -0000	1.10
+++ jit.c	19 Jan 2011 19:36:04 -0000
@@ -41,15 +41,6 @@ static const char *const jit_descriptor_
 
 static CORE_ADDR jit_descriptor_addr = 0;
 
-/* This is a boolean indicating whether we're currently registering code.  This
-   is used to avoid re-entering the registration code.  We want to check for
-   new JITed every time a new object file is loaded, but we want to avoid
-   checking for new code while we're registering object files for JITed code.
-   Therefore, we flip this variable to 1 before registering new object files,
-   and set it to 0 before returning.  */
-
-static int registering_code = 0;
-
 /* Non-zero if we want to see trace of jit level stuff.  */
 
 static int jit_debug = 0;
@@ -61,14 +52,6 @@ show_jit_debug (struct ui_file *file, in
   fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
 }
 
-/* Helper cleanup function to clear an integer flag like the one above.  */
-
-static void
-clear_int (void *int_addr)
-{
-  *((int *) int_addr) = 0;
-}
-
 struct target_buffer
 {
   CORE_ADDR base;
@@ -278,17 +261,9 @@ JITed symbol file is not an object file,
         ++i;
       }
 
-  /* Raise this flag while we register code so we won't trigger any
-     re-registration.  */
-  registering_code = 1;
-  my_cleanups = make_cleanup (clear_int, &registering_code);
-
   /* This call takes ownership of sai.  */
   objfile = symbol_file_add_from_bfd (nbfd, 0, sai, OBJF_SHARED);
 
-  /* Clear the registering_code flag.  */
-  do_cleanups (my_cleanups);
-
   /* Remember a mapping from entry_addr to objfile.  */
   entry_addr_ptr = xmalloc (sizeof (CORE_ADDR));
   *entry_addr_ptr = entry_addr;
@@ -323,43 +298,51 @@ jit_find_objf_with_entry_addr (CORE_ADDR
   return NULL;
 }
 
-/* (Re-)Initialize the jit breakpoint handler, and register any already
-   created translations.  */
+/* (Re-)Initialize the jit breakpoint handler.
+   Return 0 on success.  */
 
-static void
-jit_inferior_init (struct gdbarch *gdbarch)
+static int
+jit_breakpoint_re_set_internal (struct gdbarch *gdbarch)
 {
   struct minimal_symbol *reg_symbol;
-  struct minimal_symbol *desc_symbol;
   CORE_ADDR reg_addr;
-  struct jit_descriptor descriptor;
-  struct jit_code_entry cur_entry;
-  CORE_ADDR cur_entry_addr;
-
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"jit_inferior_init, registering_code = %d\n",
-			registering_code);
-
-  /* When we register code, GDB resets its breakpoints in case symbols have
-     changed.  That in turn calls this handler, which makes us look for new
-     code again.  To avoid being re-entered, we check this flag.  */
-  if (registering_code)
-    return;
 
   /* Lookup the registration symbol.  If it is missing, then we assume we are
      not attached to a JIT.  */
   reg_symbol = lookup_minimal_symbol (jit_break_name, NULL, NULL);
   if (reg_symbol == NULL)
-    return;
+    return 1;
   reg_addr = SYMBOL_VALUE_ADDRESS (reg_symbol);
   if (reg_addr == 0)
-    return;
+    return 2;
 
   if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog, "jit_inferior_init, reg_addr = %s\n",
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_breakpoint_re_set_internal, reg_addr = %s\n",
 			paddress (gdbarch, reg_addr));
 
+  /* Put a breakpoint in the registration symbol.  */
+  create_jit_event_breakpoint (gdbarch, reg_addr);
+
+  return 0;
+}
+
+/* Register any already created translations.  */
+
+static void
+jit_inferior_init (struct gdbarch *gdbarch)
+{
+  struct minimal_symbol *desc_symbol;
+  struct jit_descriptor descriptor;
+  struct jit_code_entry cur_entry;
+  CORE_ADDR cur_entry_addr;
+
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");
+
+  if (jit_breakpoint_re_set_internal (gdbarch) != 0)
+    return;
+
   /* Lookup the descriptor symbol and cache the addr.  If it is missing, we
      assume we are not attached to a JIT and return early.  */
   desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
@@ -382,9 +365,6 @@ jit_inferior_init (struct gdbarch *gdbar
   if (descriptor.version != 1)
     error (_("Unsupported JIT protocol version in descriptor!"));
 
-  /* Put a breakpoint in the registration symbol.  */
-  create_jit_event_breakpoint (gdbarch, reg_addr);
-
   /* If we've attached to a running program, we need to check the descriptor
      to register any functions that were already generated.  */
   for (cur_entry_addr = descriptor.first_entry;
@@ -416,7 +396,7 @@ jit_inferior_created_hook (void)
 void
 jit_breakpoint_re_set (void)
 {
-  jit_inferior_init (target_gdbarch);
+  jit_breakpoint_re_set_internal (target_gdbarch);
 }
 
 /* Wrapper to match the observer function pointer prototype.  */
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.529
diff -u -p -r1.529 breakpoint.c
--- breakpoint.c	11 Jan 2011 19:39:35 -0000	1.529
+++ breakpoint.c	19 Jan 2011 19:36:04 -0000
@@ -10588,11 +10588,12 @@ breakpoint_re_set_one (void *bint)
       printf_filtered (_("Deleting unknown breakpoint type %d\n"), b->type);
       /* fall through */
       /* Delete overlay event and longjmp master breakpoints; they will be
-	 reset later by breakpoint_re_set.  */
+	 reset later by breakpoint_re_set.  Likewise for jit_event.  */
     case bp_overlay_event:
     case bp_longjmp_master:
     case bp_std_terminate_master:
     case bp_exception_master:
+    case bp_jit_event:
       delete_breakpoint (b);
       break;
 
@@ -10619,7 +10620,6 @@ breakpoint_re_set_one (void *bint)
     case bp_longjmp_resume:
     case bp_exception:
     case bp_exception_resume:
-    case bp_jit_event:
       break;
     }
 


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