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

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

[Kevin Buettner]

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.)

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

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

[Elena Zannoni]

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
 > 

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

[Andrew Cagney]

Kevin Buettner wrote:
> 
> I request approval for committing the patch below.

Yes, ok.  I think this can go in using a slight broader understanding
what people think of as an obvious fix.

	Andrew

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

[Kevin Buettner]

On Apr 1,  5:39pm, Kevin Buettner wrote:

> 	* 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.

Committed.