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]

Re: [pushed] Plug target side conditions and commands leaks


On 11/29/13 23:05, Pedro Alves wrote:
On 11/28/2013 09:07 AM, Hui Zhu wrote:
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8187,6 +8187,14 @@ static int
   remote_insert_breakpoint (struct gdbarch *gdbarch,
   			  struct bp_target_info *bp_tgt)
   {
+  int have_target_target_side_commands = 0;
+
+  /* Check bp_tgt->tcommands in this place because
+     remote_add_target_side_commands will release bp_tgt->tcommands.  */

Well, that's quite fragile, isn't it.  If Z packets have been found
to not be supported, then nothing is releasing bp_tgt->tcommands
(and bp_tgt->conditions).

+
+  if (!VEC_empty (agent_expr_p, bp_tgt->tcommands))
+    have_target_target_side_commands = 1;
+

I've pushed this patch below.

Cool!  Thanks for this patch.

Best,
Hui


----------
Plug target side conditions and commands leaks.

The memory management of bp_location->target_info.conditions|tcommands
is currently a little fragile.  If the target reports support for
target conditions or commands, and then target side breakpoint support
is disabled, or some error is thrown before remote_add_target_side_XXX
is called, we'll leak these lists.  This patch makes us free these
lists when the locations are deleted, and also, just before recreating
the commands|conditions lists.

Tested on x86_64 Fedora 17, native and gdbserver.

gdb/
2013-11-29  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (build_target_condition_list): Release previous
	conditions.
	(build_target_command_list): Release previous commands.
	(bp_location_dtor): Release target conditions and commands.
	* remote.c (remote_add_target_side_condition): Don't release
	conditions.
	(remote_add_target_side_commands): Don't release commands.
---

  gdb/ChangeLog    |   10 ++++++++++
  gdb/breakpoint.c |    9 +++++++++
  gdb/remote.c     |    4 ----
  3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e86f5c3..5f0626c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2013-11-29  Pedro Alves  <palves@redhat.com>
+
+	* breakpoint.c (build_target_condition_list): Release previous
+	conditions.
+	(build_target_command_list): Release previous commands.
+	(bp_location_dtor): Release target conditions and commands.
+	* remote.c (remote_add_target_side_condition): Don't release
+	conditions.
+	(remote_add_target_side_commands): Don't release commands.
+
  2013-11-29  Yao Qi  <yao@codesourcery.com>
  	    Pedro Alves  <palves@redhat.com>

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 897b664..111660f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2096,6 +2096,9 @@ build_target_condition_list (struct bp_location *bl)
    int modified = bl->needs_update;
    struct bp_location *loc;

+  /* Release conditions left over from a previous insert.  */
+  VEC_free (agent_expr_p, bl->target_info.conditions);
+
    /* This is only meaningful if the target is
       evaluating conditions and if the user has
       opted for condition evaluation on the target's
@@ -2287,6 +2290,9 @@ build_target_command_list (struct bp_location *bl)
    int modified = bl->needs_update;
    struct bp_location *loc;

+  /* Release commands left over from a previous insert.  */
+  VEC_free (agent_expr_p, bl->target_info.tcommands);
+
    /* For now, limit to agent-style dprintf breakpoints.  */
    if (bl->owner->type != bp_dprintf
        || strcmp (dprintf_style, dprintf_style_agent) != 0)
@@ -12734,6 +12740,9 @@ bp_location_dtor (struct bp_location *self)
    if (self->cond_bytecode)
      free_agent_expr (self->cond_bytecode);
    xfree (self->function_name);
+
+  VEC_free (agent_expr_p, self->target_info.conditions);
+  VEC_free (agent_expr_p, self->target_info.tcommands);
  }

  static const struct bp_location_ops bp_location_ops =
diff --git a/gdb/remote.c b/gdb/remote.c
index 186c058..be54450 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8151,8 +8151,6 @@ remote_add_target_side_condition (struct gdbarch *gdbarch,
  	buf = pack_hex_byte (buf, aexpr->buf[i]);
        *buf = '\0';
      }
-
-  VEC_free (agent_expr_p, bp_tgt->conditions);
    return 0;
  }

@@ -8183,8 +8181,6 @@ remote_add_target_side_commands (struct gdbarch *gdbarch,
  	buf = pack_hex_byte (buf, aexpr->buf[i]);
        *buf = '\0';
      }
-
-  VEC_free (agent_expr_p, bp_tgt->tcommands);
  }

  /* Insert a breakpoint.  On targets that have software breakpoint



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