This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFC: don't set the pspace on ordinary breakpoints
- From: Pedro Alves <pedro at codesourcery dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Ulrich Weigand <uweigand at de dot ibm dot com>
- Date: Wed, 2 Nov 2011 20:21:02 +0000
- Subject: Re: RFC: don't set the pspace on ordinary breakpoints
- References: <m3lis6h35l.fsf@fleche.redhat.com> <201111021854.42981.pedro@codesourcery.com> <m3aa8ewbm9.fsf@fleche.redhat.com>
On Wednesday 02 November 2011 19:46:54, Tom Tromey wrote:
> Pedro> I'm not clear how this isn't breaking multi-process without
> Pedro> the linespec changes. I mean, if we no longer have a pspace
> Pedro> pointer, breakpoint re-setting will no longer work correctly.
> Pedro> (breakpoint_re_set -> ... -> prepare_re_set_context)
>
> Yeah, it probably does.
> I'm not sure why this doesn't result in any regressions.
Jan pointed at one in gdb.multi/. Not sure if it's related.
> I won't put it in separately... it is mostly useful as a way to break
> off a chunk of the huge linespec patch for readability.
Yeah, thanks for that.
> Pedro> I don't think that's correct. During startup, we disable user
> Pedro> breakpoints, because the symbols haven't been relocated yet.
> Pedro> But, we still need to insert internal breakpoints set at magic
> Pedro> addresses (not through symbols), so that we know when the startup
> Pedro> is done with. Ulrich?
>
> Ok. How would I test this?
The only calls to disable_breakpoints_before_startup/enable_breakpoints_after_startup
are in solib-spu.c, so you'd need testing on Cell.
> Plan B would be to put the startup-disabled state on bp_location.
Maybe keeping your change, but disabling only user breakpoints would
be enough:
@@ -1578,6 +1578,9 @@ should_be_inserted (struct bp_location *bl)
if (!bl->enabled || bl->shlib_disabled || bl->duplicate)
return 0;
-+ if (bl->pspace->executing_startup)
++ if (user_breakpoint_p (bl->owner) && bl->pspace->executing_startup)
+ return 0;
Though I'm not sure of the impact the bkpt_re_set change has.
I'm hoping Ulrich still remembers things from when he added the
startup disabled stuff. :-)
On Wednesday 02 November 2011 19:46:54, Tom Tromey wrote:
> Pedro> Could have been two independent patches.
> Ok, noted.
Thanks. IMO, the executing_startup change IMO could go in
separately, as soon as it is correct, it looks quite independent.
What prompted it, BTW?
--
Pedro Alves