Macro code crasher on re-run

Macro code crasher on re-run

[Daniel Jacobowitz]

There's a bug in default_macro_scope.  I was in the middle of working on
something else when GDB crashed, so I can't fix it now, and Jim probably can
think of a better fix than I can - hence this message :) Or I'll get to it
in a few days.

Here's the problem.  I've just typed "run".

82      struct macro_scope *
83      default_macro_scope (void)
84      {
85        struct symtab_and_line sal;
86        struct macro_source_file *main;
87        struct macro_scope *ms;
88
89        /* If there's a selected frame, use its PC.  */
90        if (deprecated_selected_frame)
91          sal = find_pc_line (get_frame_pc (deprecated_selected_frame), 0);

So deprecated_selected_frame is NULL...

92
93        /* If the target has any registers at all, then use its PC.  Why we
94           would have registers but no stack, I'm not sure.  */
95        else if (target_has_registers)
96          sal = find_pc_line (read_pc (), 0);

And target_has_registers is false.

97
98        /* If all else fails, fall back to the current listing position.  */
99        else
100         {
101           /* Don't call select_source_symtab here.  That can raise an
102              error if symbols aren't loaded, but GDB calls the expression
103              evaluator in all sorts of contexts.
104
105              For example, commands like `set width' call the expression
106              evaluator to evaluate their numeric arguments.  If the
107              current language is C, then that may call this function to
108              choose a scope for macro expansion.  If you don't have any
109              symbol files loaded, then get_current_or_default would raise an
110              error.  But `set width' shouldn't raise an error just because
111              it can't decide which scope to macro-expand its argument in.  */
112           struct symtab_and_line cursal = 
113                             get_current_source_symtab_and_line ();
114           
115           sal.symtab = cursal.symtab;
116           sal.line = cursal.line;
117         }

So we initialize just the symtab and line pointers.
118
119       return sal_macro_scope (sal);
120     }


39        if (! sal.symtab
40            || ! sal.symtab->macro_table)
41          return 0;

Oops, uninitialized memory read.  That else case can't work; Jim, should we
just return 0 from default_macro_scope if the target isn't running, or
is there a function I don't see somewhere to find the macrotab and
initialize the rest of the symtab?  Should it be "sal = cursal"?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Macro code crasher on re-run

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> There's a bug in default_macro_scope.

Only one?!?  :)

> 97
> 98        /* If all else fails, fall back to the current listing position.  */
> 99        else
> 100         {
> 101           /* Don't call select_source_symtab here.  That can raise an
> 102              error if symbols aren't loaded, but GDB calls the expression
> 103              evaluator in all sorts of contexts.
> 104
> 105              For example, commands like `set width' call the expression
> 106              evaluator to evaluate their numeric arguments.  If the
> 107              current language is C, then that may call this function to
> 108              choose a scope for macro expansion.  If you don't have any
> 109              symbol files loaded, then get_current_or_default would raise an
> 110              error.  But `set width' shouldn't raise an error just because
> 111              it can't decide which scope to macro-expand its argument in.  */
> 112           struct symtab_and_line cursal = 
> 113                             get_current_source_symtab_and_line ();
> 114           
> 115           sal.symtab = cursal.symtab;
> 116           sal.line = cursal.line;
> 117         }
> 
> So we initialize just the symtab and line pointers.
> 118
> 119       return sal_macro_scope (sal);
> 120     }
> 
> 
> 39        if (! sal.symtab
> 40            || ! sal.symtab->macro_table)
> 41          return 0;
> 
> Oops, uninitialized memory read.  That else case can't work; Jim, should we
> just return 0 from default_macro_scope if the target isn't running, or
> is there a function I don't see somewhere to find the macrotab and
> initialize the rest of the symtab?  Should it be "sal = cursal"?

I don't follow.  All default_macro_scope's callers check for null
return; it's documented to return zero at times.  So you must be
talking about that code in sal_macro_scope itself.  Line 39 refers to
sal.symtab, initialized by line 115, so you must be talking about line
40.  But sal.symtab must be non-zero, or else we wouldn't reach the
right operand of the ||.

Or should I sleep and try again?

Re: Macro code crasher on re-run

[Daniel Jacobowitz]

On Sun, Nov 02, 2003 at 11:48:05PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > There's a bug in default_macro_scope.
> 
> Only one?!?  :)

One at a time, one at a time.

> 
> > 97
> > 98        /* If all else fails, fall back to the current listing position.  */
> > 99        else
> > 100         {
> > 101           /* Don't call select_source_symtab here.  That can raise an
> > 102              error if symbols aren't loaded, but GDB calls the expression
> > 103              evaluator in all sorts of contexts.
> > 104
> > 105              For example, commands like `set width' call the expression
> > 106              evaluator to evaluate their numeric arguments.  If the
> > 107              current language is C, then that may call this function to
> > 108              choose a scope for macro expansion.  If you don't have any
> > 109              symbol files loaded, then get_current_or_default would raise an
> > 110              error.  But `set width' shouldn't raise an error just because
> > 111              it can't decide which scope to macro-expand its argument in.  */
> > 112           struct symtab_and_line cursal = 
> > 113                             get_current_source_symtab_and_line ();
> > 114           
> > 115           sal.symtab = cursal.symtab;
> > 116           sal.line = cursal.line;
> > 117         }
> > 
> > So we initialize just the symtab and line pointers.
> > 118
> > 119       return sal_macro_scope (sal);
> > 120     }
> > 
> > 
> > 39        if (! sal.symtab
> > 40            || ! sal.symtab->macro_table)
> > 41          return 0;
> > 
> > Oops, uninitialized memory read.  That else case can't work; Jim, should we
> > just return 0 from default_macro_scope if the target isn't running, or
> > is there a function I don't see somewhere to find the macrotab and
> > initialize the rest of the symtab?  Should it be "sal = cursal"?
> 
> I don't follow.  All default_macro_scope's callers check for null
> return; it's documented to return zero at times.  So you must be
> talking about that code in sal_macro_scope itself.  Line 39 refers to
> sal.symtab, initialized by line 115, so you must be talking about line
> 40.  But sal.symtab must be non-zero, or else we wouldn't reach the
> right operand of the ||.
> 
> Or should I sleep and try again?
> 

Right.  sal.symtab is nonzero.  sal.symtab->macro_table, however, is
uninitialized.  The short-circuit if on line 40 does not trigger, but
the remainder of sal_macro_scope assumes that sal.symtab->macro_table
is valid.

In my case it happens to be 0xB.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Macro code crasher on re-run

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> > I don't follow.  All default_macro_scope's callers check for null
> > return; it's documented to return zero at times.  So you must be
> > talking about that code in sal_macro_scope itself.  Line 39 refers to
> > sal.symtab, initialized by line 115, so you must be talking about line
> > 40.  But sal.symtab must be non-zero, or else we wouldn't reach the
> > right operand of the ||.
> > 
> > Or should I sleep and try again?
> > 
> 
> Right.  sal.symtab is nonzero.  sal.symtab->macro_table, however, is
> uninitialized.  The short-circuit if on line 40 does not trigger, but
> the remainder of sal_macro_scope assumes that sal.symtab->macro_table
> is valid.
> 
> In my case it happens to be 0xB.

Every symtab's macro_table field should be initialized, at least to
zero.  Which symtab is it?  Who creates it?

Re: Macro code crasher on re-run

[Daniel Jacobowitz]

On Mon, Nov 03, 2003 at 03:59:41PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > > I don't follow.  All default_macro_scope's callers check for null
> > > return; it's documented to return zero at times.  So you must be
> > > talking about that code in sal_macro_scope itself.  Line 39 refers to
> > > sal.symtab, initialized by line 115, so you must be talking about line
> > > 40.  But sal.symtab must be non-zero, or else we wouldn't reach the
> > > right operand of the ||.
> > > 
> > > Or should I sleep and try again?
> > > 
> > 
> > Right.  sal.symtab is nonzero.  sal.symtab->macro_table, however, is
> > uninitialized.  The short-circuit if on line 40 does not trigger, but
> > the remainder of sal_macro_scope assumes that sal.symtab->macro_table
> > is valid.
> > 
> > In my case it happens to be 0xB.
> 
> Every symtab's macro_table field should be initialized, at least to
> zero.  Which symtab is it?  Who creates it?

If you thought I wasn't making sense, you were right.  I really needed
some sleep when I wrote that.

My best guess is that the returned symtab belongs to the old (already
unloaded) binary.  Which would put the bug somewhere else.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Macro code crasher on re-run

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> On Mon, Nov 03, 2003 at 03:59:41PM -0500, Jim Blandy wrote:
> > 
> > Daniel Jacobowitz <drow@mvista.com> writes:
> > > > I don't follow.  All default_macro_scope's callers check for null
> > > > return; it's documented to return zero at times.  So you must be
> > > > talking about that code in sal_macro_scope itself.  Line 39 refers to
> > > > sal.symtab, initialized by line 115, so you must be talking about line
> > > > 40.  But sal.symtab must be non-zero, or else we wouldn't reach the
> > > > right operand of the ||.
> > > > 
> > > > Or should I sleep and try again?
> > > > 
> > > 
> > > Right.  sal.symtab is nonzero.  sal.symtab->macro_table, however, is
> > > uninitialized.  The short-circuit if on line 40 does not trigger, but
> > > the remainder of sal_macro_scope assumes that sal.symtab->macro_table
> > > is valid.
> > > 
> > > In my case it happens to be 0xB.
> > 
> > Every symtab's macro_table field should be initialized, at least to
> > zero.  Which symtab is it?  Who creates it?
> 
> If you thought I wasn't making sense, you were right.  I really needed
> some sleep when I wrote that.
> 
> My best guess is that the returned symtab belongs to the old (already
> unloaded) binary.  Which would put the bug somewhere else.

I haven't debugged it (only Daniel J. has seen the problem, as far as
I know), but I kind of suspect this change:

2003-10-22  Andrew Cagney  <cagney@redhat.com>

	* target.c (target_close): New function.
	(debug_to_close): Use "target_close".
	(push_target): Use "target_close".
	(unpush_target): Use "target_close".
	(pop_target): Use "target_close".
	* target.h (struct target_ops): Add "to_xclose".
	(target_open): Delete macro.  Move comment to "to_open".
	(target_close): Replace macro with function that takes a target.
	* top.c (quit_target): Pass "current_target" to "target_close".

The to_close method of the "exec" target calls free_objfile, which
assumes that its callers call clear_symtab_users at some appropriate
time.  This change seems to have introduced several new callers to
target_close, so it may have broken that assumption.

Having free_objfile require its callers to take care of things this
way is not very nice.

Re: Macro code crasher on re-run

[Daniel Jacobowitz]

On Mon, Nov 03, 2003 at 04:33:44PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > On Mon, Nov 03, 2003 at 03:59:41PM -0500, Jim Blandy wrote:
> > > 
> > > Daniel Jacobowitz <drow@mvista.com> writes:
> > > > > I don't follow.  All default_macro_scope's callers check for null
> > > > > return; it's documented to return zero at times.  So you must be
> > > > > talking about that code in sal_macro_scope itself.  Line 39 refers to
> > > > > sal.symtab, initialized by line 115, so you must be talking about line
> > > > > 40.  But sal.symtab must be non-zero, or else we wouldn't reach the
> > > > > right operand of the ||.
> > > > > 
> > > > > Or should I sleep and try again?
> > > > > 
> > > > 
> > > > Right.  sal.symtab is nonzero.  sal.symtab->macro_table, however, is
> > > > uninitialized.  The short-circuit if on line 40 does not trigger, but
> > > > the remainder of sal_macro_scope assumes that sal.symtab->macro_table
> > > > is valid.
> > > > 
> > > > In my case it happens to be 0xB.
> > > 
> > > Every symtab's macro_table field should be initialized, at least to
> > > zero.  Which symtab is it?  Who creates it?
> > 
> > If you thought I wasn't making sense, you were right.  I really needed
> > some sleep when I wrote that.
> > 
> > My best guess is that the returned symtab belongs to the old (already
> > unloaded) binary.  Which would put the bug somewhere else.
> 
> I haven't debugged it (only Daniel J. has seen the problem, as far as
> I know), but I kind of suspect this change:
> 
> 2003-10-22  Andrew Cagney  <cagney@redhat.com>
> 
> 	* target.c (target_close): New function.
> 	(debug_to_close): Use "target_close".
> 	(push_target): Use "target_close".
> 	(unpush_target): Use "target_close".
> 	(pop_target): Use "target_close".
> 	* target.h (struct target_ops): Add "to_xclose".
> 	(target_open): Delete macro.  Move comment to "to_open".
> 	(target_close): Replace macro with function that takes a target.
> 	* top.c (quit_target): Pass "current_target" to "target_close".
> 
> The to_close method of the "exec" target calls free_objfile, which
> assumes that its callers call clear_symtab_users at some appropriate
> time.  This change seems to have introduced several new callers to
> target_close, so it may have broken that assumption.
> 
> Having free_objfile require its callers to take care of things this
> way is not very nice.

I was using the 6.0 branch when I found the problem, so that's not it. 
It may be a related problem in a caller of free_objfile, though.  Or
reread_objfile?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Macro code crasher on re-run

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> I was using the 6.0 branch when I found the problem, so that's not it. 
> It may be a related problem in a caller of free_objfile, though.  Or
> reread_objfile?

You mean reread_symbols?  It wouldn't be that, because that calls
clear_symtab_users itself.  It's certainly a caller of free_objfile.

I think this is just going to take some debugging.  Can you tell me
how to reproduce it?

Re: Macro code crasher on re-run

[Daniel Jacobowitz]

On Mon, Nov 03, 2003 at 06:50:38PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > I was using the 6.0 branch when I found the problem, so that's not it. 
> > It may be a related problem in a caller of free_objfile, though.  Or
> > reread_objfile?
> 
> You mean reread_symbols?  It wouldn't be that, because that calls
> clear_symtab_users itself.  It's certainly a caller of free_objfile.
> 
> I think this is just going to take some debugging.  Can you tell me
> how to reproduce it?

Develop a habit for recompiling programs while you're debugging them,
and use dwarf2 information.  I didn't even have -g3 macro debug
information at all.  Eventually you'll crash, either in the macro or
frame code.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Macro code crasher on re-run

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:

> On Mon, Nov 03, 2003 at 06:50:38PM -0500, Jim Blandy wrote:
> > 
> > Daniel Jacobowitz <drow@mvista.com> writes:
> > > I was using the 6.0 branch when I found the problem, so that's not it. 
> > > It may be a related problem in a caller of free_objfile, though.  Or
> > > reread_objfile?
> > 
> > You mean reread_symbols?  It wouldn't be that, because that calls
> > clear_symtab_users itself.  It's certainly a caller of free_objfile.
> > 
> > I think this is just going to take some debugging.  Can you tell me
> > how to reproduce it?
> 
> Develop a habit for recompiling programs while you're debugging them,
> and use dwarf2 information.  I didn't even have -g3 macro debug
> information at all.  Eventually you'll crash, either in the macro or
> frame code.

I did see some crashes last week, doing what you suggest, but I was in
a crunch and didn't have time to investigate.  I'll see if I can do so
now.

Re: Macro code crasher on re-run

[David Carlton]

On 03 Nov 2003 16:33:44 -0500, Jim Blandy <jimb@redhat.com> said:

> I haven't debugged it (only Daniel J. has seen the problem, as far as
> I know)

I've been getting occasional crashes when I'm in the middle of
debugging a program, I recompile it, and then I tell GDB to run it
again and that yes, I want to reload it.  I did once run GDB on itself
when this happened (or maybe looked at a core file, I can't remember),
and macro stuff was involved somehow.  So I may have seen the same
problem that Daniel's seeing, from a source base that's older than the
patch you mentioned.

David Carlton
carlton@kealia.com

Re: Macro code crasher on re-run

[Andrew Cagney]

> 
> I haven't debugged it (only Daniel J. has seen the problem, as far as
> I know), but I kind of suspect this change:
> 
> 2003-10-22  Andrew Cagney  <cagney@redhat.com>
> 
> 	* target.c (target_close): New function.
> 	(debug_to_close): Use "target_close".
> 	(push_target): Use "target_close".
> 	(unpush_target): Use "target_close".
> 	(pop_target): Use "target_close".
> 	* target.h (struct target_ops): Add "to_xclose".
> 	(target_open): Delete macro.  Move comment to "to_open".
> 	(target_close): Replace macro with function that takes a target.
> 	* top.c (quit_target): Pass "current_target" to "target_close".

That's unlikely.  The change re-aranged deck chairs.

Andrew