Re: [RFC] TARGET_ADJUST_BREAKPOINT_ADDRESS - patch 1 of 4

Andrew Cagney
2003-10-08T18:26:51Z

> Date: Mon, 6 Oct 2003 08:36:58 -0700
> From: Kevin Buettner <kevinb@redhat.com>
>
> On Oct 6, 2:49pm, Mark Kettenis wrote:
>
> > The patch below should be non-controversial. It merely adds the
> > TARGET_ADJUST_BREAKPOINT_ADDRESS method. Code which uses it (the
> > possibly controversial bit) will come in patch #3. Documentation will
> > be in the next patch, #2. Finally, target specific code which
> > requires TARGET_ADJUST_BREAKPOINT_ADDRESS will be posted in patch #4.
> >
> > Do we really need the "TARGET" in the name of the new method? It made
> > me think that this was something that was going to be added to the
> > target-vector instead of the architecture vector.
>
> How about just "ADJUST_BREAKPOINT_ADDRESS" ?
>
> Sounds fine to me :-). Thanks,

Then ADJUST_BREAKPOINT_ADDRESS is the name.

[snip]
> To review the criticism of the previous TARGET_ADJUST_BREAKPOINT
> patch (from three years ago!), see:

Since then, the architecture vector has gained "m" and "M" - i.e.,
strictly multi-arch - and the new method will need to be one of those.
Looking at:

+/* Call TARGET_ADJUST_BREAKPOINT_ADDRESS and print warning if an address
+ adjustment was made. Returns the adjusted address. */
+
+static CORE_ADDR
+adjust_breakpoint_address (CORE_ADDR bpaddr)
+{
+ CORE_ADDR adjusted_bpaddr;
+
+ adjusted_bpaddr = TARGET_ADJUST_BREAKPOINT_ADDRESS (bpaddr);
+
+ if (adjusted_bpaddr != bpaddr)
+ breakpoint_adjustment_warning (bpaddr, adjusted_bpaddr, 0, 0);
+
+ return adjusted_bpaddr;
+}
+

I've a strong preference for "M" - method with predicate - since, for
everyone but an frv developer, this code is irrelevant.

Having a predicate would let adjust_breakpoint_address be written in a
style that reflects this atypical code path:

if (gdbarch_adjust_..._p())
adjusted = ...
if (changed)
warning ...

and the average (non-frv) developer wouldn't be distracted by the change.

Other than that, the architect method is ok.

Andrew

PS:

I wonder if the other parts of your change slam straight into DanielJ's
two teer breakpoint effect (assuming I remember his long ago intent) vis:
- the high-level user specified breakpoint (requested_address?)
- the low-level corresponding list of real breakpoints (address?)

I also wonder if PPC64 GDB could use this to do the descriptor ->
function address transformation that I described in:
http://sources.redhat.com/ml/gdb-patches/2003-09/msg00415.html

Andrew