Re: [rfc] Set a breakpoint's type before adjusting its address

[Kevin Buettner <kevinb at redhat dot com>]

On Sat, 28 Apr 2007 17:45:10 -0400
Daniel Jacobowitz <drow@false.org> wrote:

> This happens because the breakpoint's location is already on the
> location chain, right?

Right.

> Alternatively, we could move that from the end
> of allocate_bp_location to the end of set_raw_breakpoint, and avoid
> the inconsistency.

If we can do it, I think it'd be nice to keep the code which allocates
the location together with the code which adds the newly allocated
location to the chain.

I agree that my earlier patch is not very nice in that
adjust_breakpoint_address() was being called with an only partially
initialized location on the location chain.  That patch was a band-aid
in that it initialized those bits which a particular function
(read_memory_nobpt) cared about, but who knows what else might break
if some other function were called.

Appended below is a new patch which calls adjust_breakpoint_address()
prior to allocating the breakoint's location.  What do you think of
this approach?

Kevin

	* breakpoint.c (set_raw_breakpoint): Adjust breakpoint's address
	prior to allocating its location.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.246
diff -u -p -r1.246 breakpoint.c
--- breakpoint.c	13 Apr 2007 13:50:32 -0000	1.246
+++ breakpoint.c	2 May 2007 23:57:06 -0000
@@ -4189,13 +4189,23 @@ struct breakpoint *
 set_raw_breakpoint (struct symtab_and_line sal, enum bptype bptype)
 {
   struct breakpoint *b, *b1;
+  CORE_ADDR adjusted_address;
 
   b = (struct breakpoint *) xmalloc (sizeof (struct breakpoint));
   memset (b, 0, sizeof (*b));
+
+  /* Adjust the breakpoint's address prior to allocating a location.
+     Once we call allocate_bp_location(), that mostly uninitialized
+     location will be placed on the location chain.  Adjustment of the
+     breakpoint may cause read_memory_nobpt() to be called and we do
+     not want its scan of the location chain to find a breakpoint and
+     location that's only been partially initialized.  */
+  adjusted_address = adjust_breakpoint_address (sal.pc, bptype);
+
   b->loc = allocate_bp_location (b, bptype);
   b->loc->requested_address = sal.pc;
-  b->loc->address = adjust_breakpoint_address (b->loc->requested_address,
-                                               bptype);
+  b->loc->address = adjusted_address;
+
   if (sal.symtab == NULL)
     b->source_file = NULL;
   else