Re: [PATCH RFA] Use cleanup machinery to invoke ui_out_list_end()

[Elena Zannoni <ezannoni at cygnus dot com>]

Kevin Buettner writes:
 > I request approval for committing the patch below.
 > 
 > It fixes one of the problems leading to the internal error shown
 > in the session below.  (This is on IA-64.)
 > 
 >     (gdb) file testsuite/gdb.base/break
 >     Reading symbols from testsuite/gdb.base/break...done.
 >     (gdb) b 81
 >     Breakpoint 1 at 0x4000000000000ad1: file /saguaro1/sourceware/src/gdb/testsuite/gdb.base/break.c, line 81.
 >     (gdb) r
 >     Starting program: /home/kev/sourceware-bld/gdb/testsuite/gdb.base/break 
 >     720
 > 
 >     Breakpoint 1, main (argc=1, argv=0x80000ffffffffa08, envp=0x80000ffffffffa18)
 > 	at /saguaro1/sourceware/src/gdb/testsuite/gdb.base/break.c:81
 >     81          marker1 ();
 >     (gdb) b marker2
 >     Breakpoint 2 at 0x4000000000000930: file /saguaro1/sourceware/src/gdb/testsuite/gdb.base/break.c, line 49.
 >     (gdb) print marker2(99)
 > 
 >     Breakpoint 2, marker2 (a=Cannot access memory at address 0xc000000000001d3f
 >     )
 > 	at /saguaro1/sourceware/src/gdb/testsuite/gdb.base/break.c:49
 >     49      int marker2 (a) int a; { return (1); }
 >     The program being debugged stopped while in a function called from GDB.
 >     When the function (marker2) is done executing, GDB will silently
 >     stop (instead of continuing to evaluate the expression containing
 >     the function call).
 >     (gdb) bt
 >     #0  marker2 (a=Cannot access memory at address 0xc000000000001d3f
 >     ) at /saguaro1/sourceware/src/gdb/testsuite/gdb.base/break.c:49
 >     #1  <function called from gdb>
 >     /saguaro1/sourceware/src/gdb/ui-out.c:269: gdb-internal-error: list depth exceeded; only 4 levels of lists can be nested.
 >     #2  0x4000000000000ad0 in main (
 >     An internal GDB error was detected.  This may make further
 >     debugging unreliable.  Continue this debugging session? (y or n) 
 > 
 > There are actually (at least) two problems.  One of them is that the
 > IA-64 prologue scanner does not correctly find the end of the prologue
 > for marker2.  As a result, the register/offset pair that it uses to
 > find the parameter ``a'' is not correct for the point at which GDB
 > places the breakpoint.  This is what causes the "Cannot access memory
 > at address ..." error.
 > 
 > The other problem (which the patch below addresses) is the way that
 > GDB recovers from this error.  A call to ui_out_list_begin() must be
 > treated the same way that we treat a memory allocation since it is
 > allocating a resource, in this case it is simply a nesting level.  The
 > author of the UI list code decided to make nesting levels a scarce
 > resource (probably to catch exactly this sort of problem) so we must
 > be prepared to "free up" any allocated nesting level when an error
 > occurs.  The patch below uses the cleanup machinery to do so.  (BTW,
 > there are a number of other occurrences of ui_out_list_end() which
 > likely deserve similar treatment.  I've taken care of only two of
 > them.  I'll volunteer to fix the others if the maintainers will
 > preapprove this activity.)
 > 

Ah, yes. This cleanup function was introduced only recently by Mark
Kettenis.  I think you are right, we should match the
ui_out_list_begin calls with this function if there is the possibility
of error being called.  I can look at the mi/*.c files, I suspect some
of those calls suffer from the same problem.


Elena


 > Okay to commit?
 > 
 > 	* printcmd.c (print_frame_args): Use a cleanup to invoke
 > 	ui_out_list_end() so that the list count nesting flag will
 > 	be decremented properly when an error occurs.
 > 	* stack.c (print_frame): Likewise.
 > 
 > Index: printcmd.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/printcmd.c,v
 > retrieving revision 1.18
 > diff -u -p -r1.18 printcmd.c
 > --- printcmd.c	2001/03/14 16:42:30	1.18
 > +++ printcmd.c	2001/03/31 23:15:15
 > @@ -1790,7 +1790,7 @@ print_frame_args (struct symbol *func, s
 >    /* Number of ints of arguments that we have printed so far.  */
 >    int args_printed = 0;
 >  #ifdef UI_OUT
 > -  struct cleanup *old_chain;
 > +  struct cleanup *old_chain, *list_chain;
 >    struct ui_stream *stb;
 >  
 >    stb = ui_out_stream_new (uiout);
 > @@ -1909,6 +1909,7 @@ print_frame_args (struct symbol *func, s
 >        annotate_arg_begin ();
 >  
 >        ui_out_list_begin (uiout, NULL);
 > +      list_chain = make_cleanup_ui_out_list_end (uiout);
 >        fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
 >  			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 >        ui_out_field_stream (uiout, "name", stb);
 > @@ -1951,7 +1952,8 @@ print_frame_args (struct symbol *func, s
 >        else
 >  	ui_out_text (uiout, "???");
 >  
 > -      ui_out_list_end (uiout);
 > +      /* Invoke ui_out_list_end.  */
 > +      do_cleanups (list_chain);
 >  #else
 >  	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 >  		     VALUE_ADDRESS (val),
 > Index: stack.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/stack.c,v
 > retrieving revision 1.18
 > diff -u -p -r1.18 stack.c
 > --- stack.c	2001/03/27 20:36:24	1.18
 > +++ stack.c	2001/03/31 23:15:17
 > @@ -580,15 +580,20 @@ print_frame (struct frame_info *fi, 
 >    if (args)
 >      {
 >        struct print_args_args args;
 > +#ifdef UI_OUT
 > +      struct cleanup *args_list_chain;
 > +#endif
 >        args.fi = fi;
 >        args.func = func;
 >        args.stream = gdb_stdout;
 >  #ifdef UI_OUT
 >        ui_out_list_begin (uiout, "args");
 > +      args_list_chain = make_cleanup_ui_out_list_end (uiout);
 >        catch_errors (print_args_stub, &args, "", RETURN_MASK_ALL);
 >        /* FIXME: args must be a list. If one argument is a string it will
 >  		 have " that will not be properly escaped.  */
 > -      ui_out_list_end (uiout);
 > +      /* Invoke ui_out_list_end.  */
 > +      do_cleanups (args_list_chain);
 >  #else
 >        catch_errors (print_args_stub, &args, "", RETURN_MASK_ALL);
 >  #endif
 >