This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[pushed] Plug target side conditions and commands leaks (was: Re: [PATCH] Let gdbserver doesn't tell GDB it support target-side breakpoint conditions and commands if it doesn't support 'Z' packet)
- From: Pedro Alves <palves at redhat dot com>
- To: Hui Zhu <hui_zhu at mentor dot com>
- Cc: gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Fri, 29 Nov 2013 15:05:13 +0000
- Subject: [pushed] Plug target side conditions and commands leaks (was: Re: [PATCH] Let gdbserver doesn't tell GDB it support target-side breakpoint conditions and commands if it doesn't support 'Z' packet)
- Authentication-results: sourceware.org; auth=none
- References: <5265022F dot 8060203 at mentor dot com> <52654A2C dot 9010202 at redhat dot com> <529707C7 dot 4040504 at mentor dot com>
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.
----------
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