This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFC: fix PR 13987
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 12 Dec 2012 19:14:35 +0000
- Subject: Re: RFC: fix PR 13987
- References: <87392gzblm.fsf@fleche.redhat.com> <507EEC51.5060306@redhat.com> <873916i6tk.fsf@fleche.redhat.com>
On 10/22/2012 09:35 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> Tom> This patch fixes the bug by changing how the JIT breakpoint is
> Tom> managed. Now it is handled more like the longjmp master
> Tom> breakpoint: deleted and recreated during each re-set. The cache is
> Tom> still used, but now just to hold the relevant minimal symbols used
> Tom> to re-create the breakpoint.
>
> Pedro> Hmm. I'm not sure this is a good approach. It seems to add a
> Pedro> race where we could miss JIT events in non-stop. E.g., if a
> Pedro> thread causes a DSO load, and while we handle that, we do a
> Pedro> breakpoint reset, we remove the jit event breakpoint from the
> Pedro> target for a bit. In this window where the jit even bkpt is not
> Pedro> installed, another thread, still running, could pass by the jit
> Pedro> event breakpoint location, thus missing a notification to GDB.
>
> Thanks. This makes sense to me.
>
> What do you think of the appended? It is basically similar, but rather
> than delete and recreate the JIT breakpoint each time, we instead note
> its address, and then delete and recreate it only when the address
> changes -- basically providing us a simple way to notice the relocation
> caused by PIE.
Sounds like a good idea. Though ...
>
> - jit_inferior_init (gdbarch);
> + addr = SYMBOL_VALUE_ADDRESS (objf_data->register_code);
>
> if (jit_debug)
> fprintf_unfiltered (gdb_stdlog,
> "jit_breakpoint_re_set_internal, "
> "breakpoint_addr = %s\n",
> - paddress (gdbarch, SYMBOL_VALUE_ADDRESS (reg_symbol)));
> + paddress (gdbarch, addr));
> +
> + if (inf_data->cached_code_address == addr)
> + return 1;
> +
> + /* Delete the old breakpoint. */
> + if (inf_data->jit_breakpoint != NULL)
> + delete_breakpoint (inf_data->jit_breakpoint);
>
> /* Put a breakpoint in the registration symbol. */
> - create_jit_event_breakpoint (gdbarch, SYMBOL_VALUE_ADDRESS (reg_symbol));
> + inf_data->cached_code_address = addr;
> + inf_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
... what is taking care of clearing these when the inferior
exits/is detached/etc. or alternatively, restarted? I think the breakpoint
itself is deleted by breakpoint.c, and then this pointer ends up stale?
I think the registry cleanup function only runs when the inferior is
removed/deleted, not when it just exits, right?
Sort of related, while trying to understand how is inferior_data->objfile
cleared in that same situation, I happened to notice that free_objfile_data
calls get_jit_inferior_data, which calls current_inferior. Doesn't look
multi-inferior safe.
> + # Note: compiling without debug info: the library goes through
> + # symbol renaming by munging on its symbol table, and that
> + # wouldn't work for .debug sections. Also, output for "info
> + # function" changes when debug info is resent.
typo: resent->present.
--
Pedro Alves