Re: [PATCH] create_internal_breakpoint: Apply gdbarch_skip_entrypoint.

Pedro Alves
2014-11-10T15:45:50Z

On 11/10/2014 12:58 PM, Ulrich Weigand wrote:
> Doug Evans worte:
>
>> This is important because the toolchain will optimize intra-module calls
>> to skip the first two instructions that set up the TOC register.
>> And since gdb currently doesn't to "entry point skipping" for internal
>> breakpoints things work (in particular shlib event breakpoints).
>
> Huh. Yes, that's clearly a bug.
>
>> This patch fixes things by applying entry point skipping to
>> internal breakpoints. Is this the best place to apply entry point
>> skipping for internal breakpoints?
>
> I think this is fine. (The alternative would be push it up into the
> callers, which would have the advantage that you could do it only
> for those callers that get the address from a symbol, and not those
> that e.g. get it from a probe. However, since gdbarch_skip_entrypoint
> is safe even if the address is not equal to a symbol address, it's
> probably preferable to just do the skipping in one place.)

Hmm, not sure. Are there cases that do want a breakpoint
on the specific address that was passed down, even if it was
the entry point address? I'm looking at the watchpoint scope
code:

scope_breakpoint
= create_internal_breakpoint (frame_unwind_caller_arch (frame),
frame_unwind_caller_pc (frame),
bp_watchpoint_scope,
&momentary_breakpoint_ops);

and wondering about a signal coming in just while the mainline code
was going to execute the entry point address.

Should we add a create_internal_breakpoint_at_function method,
and adjust callers that are setting a breakpoint at a function,
rather than a specific address (like probes and the scope
watchpoint), to use that instead [1]? (or the opposite; add a
new create_internal_breakpoint_at_address function?)

I'm particularly thinking of gdbarch_convert_from_func_ptr_addr, and
wondering why create_overlay_event_breakpoint doesn't need to call it.
The jit event breakpoint code doesn't call it either, it seems.
And neither the longjmp even breakpoint, when the breakpoint
it set by function name instead of by probe. Are these just cases
of people not having stumbled on this yet? Or are these event
locations/functions different somehow? Why does the solib-svr4.c
need to call gdbarch_convert_from_func_ptr_addr for its event
functions, while others do not?

[1] something like:

static struct breakpoint *
create_internal_breakpoint_at_function (struct gdbarch *gdbarch,
CORE_ADDR address, enum bptype type,
const struct breakpoint_ops *ops)
{
address = gdbarch_convert_from_func_ptr_addr (..., address, ...);
address = gdbarch_skip_entrypoint (..., address, ...);
return create_internal_breakpoint (..., address, ...)
}

(or instead move the gdbarch calls to a helper function that given
an address, returns the function's breakpoint address?)

Thanks,
Pedro Alves