This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [hjl@lucon.org: PATCH: Fix restarting breakpoint (Re: gdb 5.2 removes the conditional breakpoints)]



Daniel Jacobowitz <drow@mvista.com> writes:
> Well... which is more intuitive?  If I step over a line in an
> optimized program, and step until I end up on it again (i.e. it was
> divided into parts) and type 'break', I think that ending up at $PC
> again is more natural.  But I see a good argument either way.

I agree that what you describe is more natural.  I see now that the
sal's symtab and line are merely the line *containing* the given PC;
the PC could be on any instruction attributed to that line.  I don't
see any way to allow such a breakpoint to float the way it should when
the user recompiles.  So unless you see a better solution, this patch
is approved.  A regression test would be nice.


> On Thu, May 16, 2002 at 05:43:24PM -0500, Jim Blandy wrote:
> > 
> > Regarding the change to create_breakpoints: would it be possible to
> > choose an addr string more expressive than `*HEX-ADDRESS'?  For
> > example, if you could use the symtab and line from sals, then the
> > breakpoint would float better.
> > 
> > Daniel Jacobowitz <drow@mvista.com> writes:
> > 
> > > This patch was proposed for 5.2 far too late in the release cycle, but
> > > then it simply got lost.  Could someone please review it?  I just ran
> > > into the bug again, and it's really annoying.
> > > 
> > > Summary: Breakpoints which were set by "break\n", using an implicit
> > > source location, are lost across multiple runs.
> > > 
> > > 
> > > From: "H . J . Lu" <hjl@lucon.org>
> > > Subject: PATCH: Fix restarting breakpoint (Re: gdb 5.2 removes the conditional breakpoints)
> > > To: Michael Veksler <veksler@il.ibm.com>
> > > Cc: gdb@sources.redhat.com, Andrew Cagney <ac131313@cygnus.com>
> > > Date: Wed, 17 Apr 2002 17:33:49 -0700
> > > 
> > > 
> > > On Wed, Apr 17, 2002 at 04:57:56PM -0700, H . J . Lu wrote:
> > > > On Wed, Apr 10, 2002 at 10:52:51AM +0300, Michael Veksler wrote:
> > > > > /References/: <20020322095020.A12445@lucon.org 
> > > > > <http://sources.redhat.com/ml/gdb/2002-03/msg00196.html> > 
> > > > > <3C9B76F5.6050809@cygnus.com 
> > > > > <http://sources.redhat.com/ml/gdb/2002-03/msg00198.html> >
> > > > > 
> > > > > 
> > > > > On Fri, Mar 22, 2002 at 01:24:53PM -0500, Andrew Cagney wrote:
> > > > > > > When I do
> > > > > > > 
> > > > > > > (gdb) b 100
> > > > > > > (gdb) cond 1 i == 3
> > > > > > > (gdb) r
> > > > > > > (gdb) r
> > > > > > > 
> > > > > > > gdb 5.2 will remove the conditional breakpoints on Linux/x86 after I
> > > > > > > restart the debug session. Am I the only one who sees it?
> > > > > > 
> > > > > > It would be very helpful if you could illustrate this problem by 
> > > > > > submitting a real testcase.  That way people can run it and check 
> > > > > > before/after effects on various platforms and GDB releases.
> > > > > > 
> > > > > 
> > > > > Here are the instructions for reproducing this annoying problem:
> > > > > 
> > > > >     // Debugged source:
> > > > >     typedef int operation(int val);
> > > > > 
> > > > >     int f(operation * op, int value)
> > > > >     {
> > > > >         return op(value);
> > > > >     }
> > > > > 
> > > > >     int nop(int val)
> > > > >     {
> > > > >        return val;
> > > > >     }
> > > > > 
> > > > >     int main()
> > > > >     {
> > > > >         return f(nop, 5);
> > > > >     }
> > > > >     // End source
> > > > > 
> > > > > Compile it on Linux using gcc 3.0.4 or  redhat's 2.96 (did not test it 
> > > > > on other versions).
> > > > > (gdb) b main
> > > > > (gdb) r
> > > > > Breakpoint 1, main () at t.c:15
> > > > > 15         return f(nop, 5);
> > > > > (gdb) s
> > > > > f (op=0x8048448 <nop>, value=5) at t.c:5
> > > > > 5          return op(value);
> > > > > (gdb) b
> > > > > Breakpoint 2 at 0x8048432: file t.c, line 5.
> > > > 
> > > > Thanks for the testcase. Basically, we deleted all break points set
> > > > with "break" when we restart. It is a very bad regression from gdb
> > > > 4.17. Here is a patch. May I check it into gdb 5.2?
> > > > 
> > > > Thanks.
> > > > 
> > > > 
> > > > H.J.
> > > > ----
> > > > 2002-04-17  H.J. Lu  (hjl@gnu.org)
> > > > 
> > > > 	* breakpoint.c (create_thread_event_breakpoint): Use xasprintf.
> > > > 	(create_breakpoints): Make sure the addr_string field is not
> > > > 	NULL.
> > > > 
> > > 
> > > Here is an update. It just uses
> > > 
> > > 	xasprintf (&b->addr_string, "*0x%s", paddr (b->address));
> > > 
> > > H.J.
> > > ---
> > > 2002-04-17  H.J. Lu  (hjl@gnu.org)
> > > 
> > > 	* breakpoint.c (create_thread_event_breakpoint): Use xasprintf.
> > > 	(create_breakpoints): Make sure the addr_string field is not
> > > 	NULL.
> > > 
> > > --- gdb/breakpoint.c.break	Wed Mar  6 22:30:42 2002
> > > +++ gdb/breakpoint.c	Wed Apr 17 17:29:58 2002
> > > @@ -3859,14 +3859,12 @@ struct breakpoint *
> > >  create_thread_event_breakpoint (CORE_ADDR address)
> > >  {
> > >    struct breakpoint *b;
> > > -  char addr_string[80];		/* Surely an addr can't be longer than that. */
> > >  
> > >    b = create_internal_breakpoint (address, bp_thread_event);
> > >    
> > >    b->enable_state = bp_enabled;
> > >    /* addr_string has to be used or breakpoint_re_set will delete me.  */
> > > -  sprintf (addr_string, "*0x%s", paddr (b->address));
> > > -  b->addr_string = xstrdup (addr_string);
> > > +  xasprintf (&b->addr_string, "*0x%s", paddr (b->address));
> > >  
> > >    return b;
> > >  }
> > > @@ -4422,7 +4420,12 @@ create_breakpoints (struct symtabs_and_l
> > >  	b->number = breakpoint_count;
> > >  	b->cond = cond[i];
> > >  	b->thread = thread;
> > > -	b->addr_string = addr_string[i];
> > > +	if (addr_string[i])
> > > +	  b->addr_string = addr_string[i];
> > > +	else
> > > +	  /* addr_string has to be used or breakpoint_re_set will delete
> > > +	     me.  */
> > > +	  xasprintf (&b->addr_string, "*0x%s", paddr (b->address));
> > >  	b->cond_string = cond_string[i];
> > >  	b->ignore_count = ignore_count;
> > >  	b->enable_state = bp_enabled;
> > > 
> > > 
> > > ----------
> > > 
> > > 
> > > -- 
> > > Daniel Jacobowitz                           Carnegie Mellon University
> > > MontaVista Software                         Debian GNU/Linux Developer
> > 
> 
> -- 
> Daniel Jacobowitz                           Carnegie Mellon University
> MontaVista Software                         Debian GNU/Linux Developer


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]