[rfa/6.0] Fix a thinko in dwarf2loc

[rfa/6.0] Fix a thinko in dwarf2loc

[Daniel Jacobowitz]

Any variable with DW_OP_reg* requires a frame to read it - what
symbol_read_needs_frame () really means is "is this variable meaningful when
the program is not running".

This would show up in the testsuite, but GCC doesn't tend to put locals in
registers without optimization or register keywords everywhere.  Symptom is
that a watchpoint was not deleted when we left its scope, since we thought
it was global.

OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-08-01  Daniel Jacobowitz  <drow@mvista.com>

	* dwarf2loc.c (dwarf2_loc_desc_needs_frame): Variables in a
	register do need a frame.

--- gdb-5.3.20030801/gdb/dwarf2loc.c.orig	2003-08-01 14:58:57.000000000 -0400
+++ gdb-5.3.20030801/gdb/dwarf2loc.c	2003-08-01 15:00:05.000000000 -0400
@@ -318,7 +318,7 @@ dwarf2_loc_desc_needs_frame (unsigned ch
 
   free_dwarf_expr_context (ctx);
 
-  return baton.needs_frame;
+  return baton.needs_frame || ctx->in_reg;
 }
 
 static void

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Daniel Jacobowitz]

On Fri, Aug 01, 2003 at 03:32:45PM -0400, Daniel Jacobowitz wrote:
> Any variable with DW_OP_reg* requires a frame to read it - what
> symbol_read_needs_frame () really means is "is this variable meaningful when
> the program is not running".
> 
> This would show up in the testsuite, but GCC doesn't tend to put locals in
> registers without optimization or register keywords everywhere.  Symptom is
> that a watchpoint was not deleted when we left its scope, since we thought
> it was global.
> 
> OK?

Ping...

[Does dwarf2loc fall under the maintenance umbrella of the dwarf2
reader?]

> 2003-08-01  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* dwarf2loc.c (dwarf2_loc_desc_needs_frame): Variables in a
> 	register do need a frame.
> 
> --- gdb-5.3.20030801/gdb/dwarf2loc.c.orig	2003-08-01 14:58:57.000000000 -0400
> +++ gdb-5.3.20030801/gdb/dwarf2loc.c	2003-08-01 15:00:05.000000000 -0400
> @@ -318,7 +318,7 @@ dwarf2_loc_desc_needs_frame (unsigned ch
>  
>    free_dwarf_expr_context (ctx);
>  
> -  return baton.needs_frame;
> +  return baton.needs_frame || ctx->in_reg;
>  }
>  
>  static void
> 
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Daniel Jacobowitz]

On Sun, Aug 17, 2003 at 04:58:22PM -0400, Daniel Jacobowitz wrote:
> On Fri, Aug 01, 2003 at 03:32:45PM -0400, Daniel Jacobowitz wrote:
> > Any variable with DW_OP_reg* requires a frame to read it - what
> > symbol_read_needs_frame () really means is "is this variable meaningful when
> > the program is not running".
> > 
> > This would show up in the testsuite, but GCC doesn't tend to put locals in
> > registers without optimization or register keywords everywhere.  Symptom is
> > that a watchpoint was not deleted when we left its scope, since we thought
> > it was global.
> > 
> > OK?
> 
> Ping...
> 
> [Does dwarf2loc fall under the maintenance umbrella of the dwarf2
> reader?]

Ping?  I would like to fix this bug for 6.0.

> 
> > 2003-08-01  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* dwarf2loc.c (dwarf2_loc_desc_needs_frame): Variables in a
> > 	register do need a frame.
> > 
> > --- gdb-5.3.20030801/gdb/dwarf2loc.c.orig	2003-08-01 14:58:57.000000000 -0400
> > +++ gdb-5.3.20030801/gdb/dwarf2loc.c	2003-08-01 15:00:05.000000000 -0400
> > @@ -318,7 +318,7 @@ dwarf2_loc_desc_needs_frame (unsigned ch
> >  
> >    free_dwarf_expr_context (ctx);
> >  
> > -  return baton.needs_frame;
> > +  return baton.needs_frame || ctx->in_reg;
> >  }
> >  
> >  static void
> > 
> > 
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Daniel Jacobowitz]

On Sun, Aug 24, 2003 at 12:48:55PM -0400, Daniel Jacobowitz wrote:
> On Sun, Aug 17, 2003 at 04:58:22PM -0400, Daniel Jacobowitz wrote:
> > On Fri, Aug 01, 2003 at 03:32:45PM -0400, Daniel Jacobowitz wrote:
> > > Any variable with DW_OP_reg* requires a frame to read it - what
> > > symbol_read_needs_frame () really means is "is this variable meaningful when
> > > the program is not running".
> > > 
> > > This would show up in the testsuite, but GCC doesn't tend to put locals in
> > > registers without optimization or register keywords everywhere.  Symptom is
> > > that a watchpoint was not deleted when we left its scope, since we thought
> > > it was global.
> > > 
> > > OK?
> > 
> > Ping...
> > 
> > [Does dwarf2loc fall under the maintenance umbrella of the dwarf2
> > reader?]
> 
> Ping?  I would like to fix this bug for 6.0.

Jim, Elena?  I hit this bug again in testing the
deprecated_safe_get_selected_frame () patch.

> > > 2003-08-01  Daniel Jacobowitz  <drow@mvista.com>
> > > 
> > > 	* dwarf2loc.c (dwarf2_loc_desc_needs_frame): Variables in a
> > > 	register do need a frame.
> > > 
> > > --- gdb-5.3.20030801/gdb/dwarf2loc.c.orig	2003-08-01 14:58:57.000000000 -0400
> > > +++ gdb-5.3.20030801/gdb/dwarf2loc.c	2003-08-01 15:00:05.000000000 -0400
> > > @@ -318,7 +318,7 @@ dwarf2_loc_desc_needs_frame (unsigned ch
> > >  
> > >    free_dwarf_expr_context (ctx);
> > >  
> > > -  return baton.needs_frame;
> > > +  return baton.needs_frame || ctx->in_reg;
> > >  }
> > >  
> > >  static void
> > > 
> > > 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Sun, Aug 24, 2003 at 12:48:55PM -0400, Daniel Jacobowitz wrote:
 > > On Sun, Aug 17, 2003 at 04:58:22PM -0400, Daniel Jacobowitz wrote:
 > > > On Fri, Aug 01, 2003 at 03:32:45PM -0400, Daniel Jacobowitz wrote:
 > > > > Any variable with DW_OP_reg* requires a frame to read it - what
 > > > > symbol_read_needs_frame () really means is "is this variable meaningful when
 > > > > the program is not running".
 > > > > 
 > > > > This would show up in the testsuite, but GCC doesn't tend to put locals in
 > > > > registers without optimization or register keywords everywhere.  Symptom is
 > > > > that a watchpoint was not deleted when we left its scope, since we thought
 > > > > it was global.
 > > > > 
 > > > > OK?
 > > > 
 > > > Ping...
 > > > 
 > > > [Does dwarf2loc fall under the maintenance umbrella of the dwarf2
 > > > reader?]
 > > 
 > > Ping?  I would like to fix this bug for 6.0.
 > 
 > Jim, Elena?  I hit this bug again in testing the
 > deprecated_safe_get_selected_frame () patch.
 > 
 > > > > 2003-08-01  Daniel Jacobowitz  <drow@mvista.com>
 > > > > 
 > > > > 	* dwarf2loc.c (dwarf2_loc_desc_needs_frame): Variables in a
 > > > > 	register do need a frame.
 > > > > 
 > > > > --- gdb-5.3.20030801/gdb/dwarf2loc.c.orig	2003-08-01 14:58:57.000000000 -0400
 > > > > +++ gdb-5.3.20030801/gdb/dwarf2loc.c	2003-08-01 15:00:05.000000000 -0400
 > > > > @@ -318,7 +318,7 @@ dwarf2_loc_desc_needs_frame (unsigned ch
 > > > >  
 > > > >    free_dwarf_expr_context (ctx);
 > > > >  
 > > > > -  return baton.needs_frame;
 > > > > +  return baton.needs_frame || ctx->in_reg;
 > > > >  }
 > > > >  

Hmmm, the ctx just got freed, you cannot dereference it. Need to use a
local variable.  What you are really saying here is that the
needs_frame variable is not sufficient to describe what's needed. Can
the case be added, so that needs_frame becomes OK? Proabably you
need another need_frame_* function.

elena


 > > > >  static void
 > > > > 
 > > > > 
 > 
 > -- 
 > Daniel Jacobowitz
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Daniel Jacobowitz]

On Tue, Sep 09, 2003 at 05:27:10PM -0400, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > On Sun, Aug 24, 2003 at 12:48:55PM -0400, Daniel Jacobowitz wrote:
>  > > On Sun, Aug 17, 2003 at 04:58:22PM -0400, Daniel Jacobowitz wrote:
>  > > > On Fri, Aug 01, 2003 at 03:32:45PM -0400, Daniel Jacobowitz wrote:
>  > > > > Any variable with DW_OP_reg* requires a frame to read it - what
>  > > > > symbol_read_needs_frame () really means is "is this variable meaningful when
>  > > > > the program is not running".
>  > > > > 
>  > > > > This would show up in the testsuite, but GCC doesn't tend to put locals in
>  > > > > registers without optimization or register keywords everywhere.  Symptom is
>  > > > > that a watchpoint was not deleted when we left its scope, since we thought
>  > > > > it was global.
>  > > > > 
>  > > > > OK?
>  > > > 
>  > > > Ping...
>  > > > 
>  > > > [Does dwarf2loc fall under the maintenance umbrella of the dwarf2
>  > > > reader?]
>  > > 
>  > > Ping?  I would like to fix this bug for 6.0.
>  > 
>  > Jim, Elena?  I hit this bug again in testing the
>  > deprecated_safe_get_selected_frame () patch.
>  > 
>  > > > > 2003-08-01  Daniel Jacobowitz  <drow@mvista.com>
>  > > > > 
>  > > > > 	* dwarf2loc.c (dwarf2_loc_desc_needs_frame): Variables in a
>  > > > > 	register do need a frame.
>  > > > > 
>  > > > > --- gdb-5.3.20030801/gdb/dwarf2loc.c.orig	2003-08-01 14:58:57.000000000 -0400
>  > > > > +++ gdb-5.3.20030801/gdb/dwarf2loc.c	2003-08-01 15:00:05.000000000 -0400
>  > > > > @@ -318,7 +318,7 @@ dwarf2_loc_desc_needs_frame (unsigned ch
>  > > > >  
>  > > > >    free_dwarf_expr_context (ctx);
>  > > > >  
>  > > > > -  return baton.needs_frame;
>  > > > > +  return baton.needs_frame || ctx->in_reg;
>  > > > >  }
>  > > > >  
> 
> Hmmm, the ctx just got freed, you cannot dereference it. Need to use a

Eek, I'm embarassed.  You are so right.

> local variable.  What you are really saying here is that the
> needs_frame variable is not sufficient to describe what's needed. Can
> the case be added, so that needs_frame becomes OK? Proabably you
> need another need_frame_* function.

No, I can't.  The needs_frame_* functions are hooks for things which
are external to the expression evaluator - concepts like "object" and
"thread local storage address" which require information.  But the
evaluator won't actually try to read from the register when parsing a
DW_OP_reg1; it just sets in_reg.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Tue, Sep 09, 2003 at 05:27:10PM -0400, Elena Zannoni wrote:
 > > Daniel Jacobowitz writes:
 > >  > On Sun, Aug 24, 2003 at 12:48:55PM -0400, Daniel Jacobowitz wrote:
 > >  > > On Sun, Aug 17, 2003 at 04:58:22PM -0400, Daniel Jacobowitz wrote:
 > >  > > > On Fri, Aug 01, 2003 at 03:32:45PM -0400, Daniel Jacobowitz wrote:
 > >  > > > > Any variable with DW_OP_reg* requires a frame to read it - what
 > >  > > > > symbol_read_needs_frame () really means is "is this variable meaningful when
 > >  > > > > the program is not running".
 > >  > > > > 
 > >  > > > > This would show up in the testsuite, but GCC doesn't tend to put locals in
 > >  > > > > registers without optimization or register keywords everywhere.  Symptom is
 > >  > > > > that a watchpoint was not deleted when we left its scope, since we thought
 > >  > > > > it was global.
 > >  > > > > 
 > >  > > > > OK?
 > >  > > > 
 > >  > > > Ping...
 > >  > > > 
 > >  > > > [Does dwarf2loc fall under the maintenance umbrella of the dwarf2
 > >  > > > reader?]
 > >  > > 
 > >  > > Ping?  I would like to fix this bug for 6.0.
 > >  > 
 > >  > Jim, Elena?  I hit this bug again in testing the
 > >  > deprecated_safe_get_selected_frame () patch.
 > >  > 
 > >  > > > > 2003-08-01  Daniel Jacobowitz  <drow@mvista.com>
 > >  > > > > 
 > >  > > > > 	* dwarf2loc.c (dwarf2_loc_desc_needs_frame): Variables in a
 > >  > > > > 	register do need a frame.
 > >  > > > > 
 > >  > > > > --- gdb-5.3.20030801/gdb/dwarf2loc.c.orig	2003-08-01 14:58:57.000000000 -0400
 > >  > > > > +++ gdb-5.3.20030801/gdb/dwarf2loc.c	2003-08-01 15:00:05.000000000 -0400
 > >  > > > > @@ -318,7 +318,7 @@ dwarf2_loc_desc_needs_frame (unsigned ch
 > >  > > > >  
 > >  > > > >    free_dwarf_expr_context (ctx);
 > >  > > > >  
 > >  > > > > -  return baton.needs_frame;
 > >  > > > > +  return baton.needs_frame || ctx->in_reg;
 > >  > > > >  }
 > >  > > > >  
 > > 
 > > Hmmm, the ctx just got freed, you cannot dereference it. Need to use a
 > 
 > Eek, I'm embarassed.  You are so right.
 > 
 > > local variable.  What you are really saying here is that the
 > > needs_frame variable is not sufficient to describe what's needed. Can
 > > the case be added, so that needs_frame becomes OK? Proabably you
 > > need another need_frame_* function.
 > 
 > No, I can't.  The needs_frame_* functions are hooks for things which
 > are external to the expression evaluator - concepts like "object" and
 > "thread local storage address" which require information.  But the
 > evaluator won't actually try to read from the register when parsing a
 > DW_OP_reg1; it just sets in_reg.

Oh, ok, I guess save ctx->in_reg in a local variable then.

elena


 > 
 > -- 
 > Daniel Jacobowitz
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Andrew Cagney]

>  > >  > > > > +  return baton.needs_frame || ctx->in_reg;
>  > >  > > > >  }
>  > >  > > > >  
>  > > 
>  > > Hmmm, the ctx just got freed, you cannot dereference it. Need to use a
>  > 
>  > Eek, I'm embarassed.  You are so right.
>  > 
>  > > local variable.  What you are really saying here is that the
>  > > needs_frame variable is not sufficient to describe what's needed. Can
>  > > the case be added, so that needs_frame becomes OK? Proabably you
>  > > need another need_frame_* function.
>  > 
>  > No, I can't.  The needs_frame_* functions are hooks for things which
>  > are external to the expression evaluator - concepts like "object" and
>  > "thread local storage address" which require information.  But the
>  > evaluator won't actually try to read from the register when parsing a
>  > DW_OP_reg1; it just sets in_reg.
> 
> Oh, ok, I guess save ctx->in_reg in a local variable then.
> 
> elena

Thanks for picking this up.  So there's no confusion I'm also ok with 
this on the branch.

Andrew

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Daniel Jacobowitz]

On Tue, Sep 09, 2003 at 06:59:52PM -0400, Andrew Cagney wrote:
> > > >  > > > > +  return baton.needs_frame || ctx->in_reg;
> > > >  > > > >  }
> > > >  > > > >  
> > > > 
> > > > Hmmm, the ctx just got freed, you cannot dereference it. Need to use a
> > > 
> > > Eek, I'm embarassed.  You are so right.
> > > 
> > > > local variable.  What you are really saying here is that the
> > > > needs_frame variable is not sufficient to describe what's needed. Can
> > > > the case be added, so that needs_frame becomes OK? Proabably you
> > > > need another need_frame_* function.
> > > 
> > > No, I can't.  The needs_frame_* functions are hooks for things which
> > > are external to the expression evaluator - concepts like "object" and
> > > "thread local storage address" which require information.  But the
> > > evaluator won't actually try to read from the register when parsing a
> > > DW_OP_reg1; it just sets in_reg.
> >
> >Oh, ok, I guess save ctx->in_reg in a local variable then.
> >
> >elena
> 
> Thanks for picking this up.  So there's no confusion I'm also ok with 
> this on the branch.

Thank you both.  Checked in.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Tue, Sep 09, 2003 at 06:59:52PM -0400, Andrew Cagney wrote:
 > > > > >  > > > > +  return baton.needs_frame || ctx->in_reg;
 > > > > >  > > > >  }
 > > > > >  > > > >  
 > > > > > 
 > > > > > Hmmm, the ctx just got freed, you cannot dereference it. Need to use a
 > > > > 
 > > > > Eek, I'm embarassed.  You are so right.
 > > > > 
 > > > > > local variable.  What you are really saying here is that the
 > > > > > needs_frame variable is not sufficient to describe what's needed. Can
 > > > > > the case be added, so that needs_frame becomes OK? Proabably you
 > > > > > need another need_frame_* function.
 > > > > 
 > > > > No, I can't.  The needs_frame_* functions are hooks for things which
 > > > > are external to the expression evaluator - concepts like "object" and
 > > > > "thread local storage address" which require information.  But the
 > > > > evaluator won't actually try to read from the register when parsing a
 > > > > DW_OP_reg1; it just sets in_reg.
 > > >
 > > >Oh, ok, I guess save ctx->in_reg in a local variable then.
 > > >
 > > >elena
 > > 
 > > Thanks for picking this up.  So there's no confusion I'm also ok with 
 > > this on the branch.
 > 
 > Thank you both.  Checked in.

Just for completeness, can you post the patch?

thanks
elena

 > 
 > -- 
 > Daniel Jacobowitz
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/6.0] Fix a thinko in dwarf2loc

[Daniel Jacobowitz]

On Thu, Sep 11, 2003 at 01:24:57PM -0400, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > On Tue, Sep 09, 2003 at 06:59:52PM -0400, Andrew Cagney wrote:
>  > > > > >  > > > > +  return baton.needs_frame || ctx->in_reg;
>  > > > > >  > > > >  }
>  > > > > >  > > > >  
>  > > > > > 
>  > > > > > Hmmm, the ctx just got freed, you cannot dereference it. Need to use a
>  > > > > 
>  > > > > Eek, I'm embarassed.  You are so right.
>  > > > > 
>  > > > > > local variable.  What you are really saying here is that the
>  > > > > > needs_frame variable is not sufficient to describe what's needed. Can
>  > > > > > the case be added, so that needs_frame becomes OK? Proabably you
>  > > > > > need another need_frame_* function.
>  > > > > 
>  > > > > No, I can't.  The needs_frame_* functions are hooks for things which
>  > > > > are external to the expression evaluator - concepts like "object" and
>  > > > > "thread local storage address" which require information.  But the
>  > > > > evaluator won't actually try to read from the register when parsing a
>  > > > > DW_OP_reg1; it just sets in_reg.
>  > > >
>  > > >Oh, ok, I guess save ctx->in_reg in a local variable then.
>  > > >
>  > > >elena
>  > > 
>  > > Thanks for picking this up.  So there's no confusion I'm also ok with 
>  > > this on the branch.
>  > 
>  > Thank you both.  Checked in.
> 
> Just for completeness, can you post the patch?
> 
> thanks
> elena

Oops.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-09-11  Daniel Jacobowitz  <drow@mvista.com>

	* dwarf2loc.c (dwarf2_loc_desc_needs_frame): Variables in a
	register do need a frame.

Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.12
diff -u -p -r1.12 dwarf2loc.c
--- dwarf2loc.c	22 Jul 2003 18:24:42 -0000	1.12
+++ dwarf2loc.c	11 Sep 2003 14:57:16 -0000
@@ -304,6 +304,7 @@ dwarf2_loc_desc_needs_frame (unsigned ch
 {
   struct needs_frame_baton baton;
   struct dwarf_expr_context *ctx;
+  int in_reg;
 
   baton.needs_frame = 0;
 
@@ -316,9 +317,11 @@ dwarf2_loc_desc_needs_frame (unsigned ch
 
   dwarf_expr_eval (ctx, data, size);
 
+  in_reg = ctx->in_reg;
+
   free_dwarf_expr_context (ctx);
 
-  return baton.needs_frame;
+  return baton.needs_frame || in_reg;
 }
 
 static void