Re: [RFC] TARGET_ADJUST_BREAKPOINT_ADDRESS - patch 1 of 4

[Andrew Cagney <ac131313 at redhat dot com>]

>    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