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: RFC: fix PR 13987


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


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