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] |
On Thu, Jan 27, 2011 at 4:44 AM, Pedro Alves <pedro@codesourcery.com> wrote: Thanks for your comments. >> +struct jit_inferior_data { > > Put the { on it's own line, please. Done. >> +static struct jit_inferior_data * >> +get_jit_inferior_data (void) > > Please add a small describing blurb over functions. Done. >> + ?if (inf_data->breakpoint != NULL) >> + ? ?{ >> + ? ? ?if (inf_data->breakpoint_addr == inf_data->breakpoint->loc->address) >> + ? ? ? /* Same location as on last run. ?Existing breakpoint is good. ?*/ >> + ? ? ? return 0; > > I'm a little warry about this optimization. ?For example, > we should probably also compare other things, like > gdbarch and location's pspace|aspace. ?Is it > a significant difference if we always delete the breakpoint > (here or perhaps on inferior exit?) I have not measured this, and optimization pass will be needed later anyway for the quadratic slowdown (reported by Vyacheslav earlier). Let's worry about correctness first. > There's at least one problem to solve: on "exec", > update_breakpoints_after_exec deletes bp_jit_event > breakpoints, effectively making it so that your > inf_data->breakpoint pointer becomes stale. I am in a twisty maze of passages, all alike ;-( > There may > be other paths that delete the breakpoint behind jit.c's > back. ?One solution would be to get rid of the breakpoint > pointer in jit.c, and add a remove_jit_event_breakpoints > function, modelled on remove_solib_event_breakpoints. Done. I am calling remove_jit_event_breakpoints from inferior_create_observer (removing breakpoints doesn't work from inferior_exit_hook :-( > But > if you want to come up with other solutions, I'd be happy > to consider them. ?I'm thinking that we should delete the > jit breakpoint (and perhaps more) whenever the executable > changes (say, the "file" command), which is kind of > a similar case of an "exec", so maybe we should install > an executable_changed observer as well. ?Not sure that > covers all we need. I think this is covered now -- after "file", if we attach or run, inferior_create_observer will delete the old breakpoint. Thanks, -- Paul Pluzhnikov 2011-01-27 Paul Pluzhnikov <ppluzhnikov@google.com> * breakpoint.h (remove_jit_event_breakpoints): New prototype. * breakpoint.c (remove_jit_event_breakpoints): New function. * jit.c (jit_descriptor_addr): Delete. (registering_code): Delete. (clear_int): Delete. (jit_inferior_data): New variable. (struct jit_inferior_data): New type. (get_jit_inferior_data): New function. (jit_inferior_data_cleanup): New function. (jit_read_descriptor): Adjust. (jit_register_code): Adjust. (jit_breakpoint_re_set_internal): New function; move code here ... (jit_inferior_init): ... from here. (jit_breakpoint_re_set): Adjust. (jit_inferior_created_observer): Adjust. (jit_inferior_exit_hook): Adjust. (jit_event_handler): Adjust. (_initialize_jit): Adjust.
Attachment:
gdb-jit-leak-20110127.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |