This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Fix double free on error while inserting the breakpoint
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 23 Apr 2009 22:36:19 +0200
- Subject: Re: [patch] Fix double free on error while inserting the breakpoint
- References: <20081122214006.GA22076@host0.dyn.jankratochvil.net> <m3eivkthb6.fsf@fleche.redhat.com>
On Thu, 23 Apr 2009 01:11:25 +0200, Tom Tromey wrote:
> I like this but I am unsure whether it is ok to move the call to
> update_global_location_list past the call to mention.
As in this case the breakpoint is pending and thus it was created by
set_raw_breakpoint_without_location and so update_global_location_list is
a nop for it. So I do not think it is a problem to move it.
> Jan> + /* Have already BREAKPOINT_CHAIN discarded as we may get an exception while
> Jan> + inserting the breakpoints which would double-free the resources both by
> Jan> + BREAKPOINT_CHAIN now and during DELETE_BREAKPOINT in the future. */
> Jan> + update_global_location_list (1);
>
> I found this comment pretty hard to follow. I think the code would be
> pretty clear without it.
The ordering of
discard_cleanups (breakpoint_chain);
vs.
update_global_location_list (1);
I find important to note there. At least tried if the new comment will pass.
Thanks,
Jan
2009-04-23 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix double free on error inserting the breakpoint instruction.
* breakpoint.c (create_breakpoints): Move the
update_global_location_list call to ...
(break_command_really): ... here together with the second local call
both unified after all the cleanups.
--- gdb/breakpoint.c 31 Mar 2009 16:44:17 -0000 1.390
+++ gdb/breakpoint.c 23 Apr 2009 20:13:26 -0000
@@ -5458,8 +5458,6 @@ create_breakpoints (struct symtabs_and_l
cond_string, type, disposition,
thread, task, ignore_count, ops, from_tty, enabled);
}
-
- update_global_location_list (1);
}
/* Parse ARG which is assumed to be a SAL specification possibly
@@ -5800,7 +5798,6 @@ break_command_really (char *arg, char *c
b->ops = ops;
b->enable_state = enabled ? bp_enabled : bp_disabled;
- update_global_location_list (1);
mention (b);
}
@@ -5812,6 +5809,9 @@ break_command_really (char *arg, char *c
discard_cleanups (breakpoint_chain);
/* But cleanup everything else. */
do_cleanups (old_chain);
+
+ /* error call may happen here - have BREAKPOINT_CHAIN already discarded. */
+ update_global_location_list (1);
}
/* Set a breakpoint.