Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@codesourcery.com>
To: Tom Tromey <tromey@redhat.com>
Cc: gdb-patches@sourceware.org, Ulrich Weigand <uweigand@de.ibm.com>
Subject: Re: RFC: don't set the pspace on ordinary breakpoints
Date: Wed, 02 Nov 2011 20:21:00 -0000	[thread overview]
Message-ID: <201111022021.03286.pedro@codesourcery.com> (raw)
In-Reply-To: <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


  reply	other threads:[~2011-11-02 20:21 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-27 15:32 Tom Tromey
2011-10-31  1:03 ` Jan Kratochvil
2011-10-31 12:07 ` Yao Qi
2011-11-02 18:55 ` Pedro Alves
2011-11-02 19:47   ` Tom Tromey
2011-11-02 20:21     ` Pedro Alves [this message]
2011-11-03 14:01       ` Tom Tromey
2011-11-03 16:02         ` Pedro Alves
2011-11-03 19:33           ` Ulrich Weigand
2011-11-08 19:32           ` Tom Tromey
2011-11-08 20:23             ` Tom Tromey
2011-11-09 18:24               ` Tom Tromey
2011-11-09 18:30               ` Pedro Alves
2011-11-09 18:41                 ` Tom Tromey
2011-11-10 16:53                   ` Tom Tromey
2011-11-10 17:49                     ` Pedro Alves
2011-11-10 17:57                     ` Ulrich Weigand
2011-11-10 19:27                       ` Tom Tromey
2011-11-10 20:23                         ` Tom Tromey
2011-11-11 12:56                           ` Ulrich Weigand
2011-11-11 14:47                             ` Tom Tromey
2011-11-14 21:12                               ` Tom Tromey
2011-11-16 18:37                               ` Ulrich Weigand
2011-11-16 21:24                                 ` Tom Tromey
2011-11-18 18:31                                   ` Ulrich Weigand
2012-01-02 18:18                                     ` [7.4 regression] Stand-alone Cell debugging broken (Re: RFC: don't set the pspace on ordinary breakpoints) Ulrich Weigand
2012-01-03  3:15                                       ` Joel Brobecker
2012-01-03 20:29                                       ` Tom Tromey
2012-01-04 12:36                                         ` Ulrich Weigand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201111022021.03286.pedro@codesourcery.com \
    --to=pedro@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@redhat.com \
    --cc=uweigand@de.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox