[patch] PR python/10344

[patch] PR python/10344

[Phil Muldoon]

http://sourceware.org/bugzilla/show_bug.cgi?id=10344

With GDB, when an inferior stops at a breakpoint (and the stock printing 
options are unchanged), GDB attempts to print out the function name and 
the arguments of the frame.  If any of these argument values triggers a 
Python pretty-printer, then the pretty-printer supplies the output 
format for printing that argument.  While the breakpoint frame printing 
code uses three uiout levels to construct the function and argument 
output, if the argument pretty-printer calls parse_and_eval, a further 
two uiout levels are consumed while the expression is evaluated. In this 
particular case, the parse_and_eval call creates an inferior function 
call to an inferior function which raises a SIGSEGV. GDB catches this 
and tries to report the issue in the inferior function call. As there 
are no further levels for error output (MAX_UI_OUT_LEVELS = 6}, and 
(especially in this PR) if a GDB exception is created it cannot be 
printed and GDB raises an internal error.

I decided to investigate this case on the mailing list, and the 
historical cases for fixing uiout level overflow is either to improve 
the cleanups or just to increment the maximum when cleanups are correct. 
I checked all the cleanups, and all the levels are required at the time 
they are in use. It goes something like this:

-> print frame -- 1st level
---> print arguments -- 2rd level
------> loop through individual arguments, use and disregard another 
level per argument in the list -- 3rd level
---------> One argument has a pretty printer that calls parse_and_eval 
that consumes 2 more levels. -- 5th level
------------> parse_and_eval calls an inferior function which raises a 
signal. Try to report the failure. Out of levels. Raise internal error


As a second thought I looked at how difficult it would be to make the 
uiout level grow and shrink dynamically.   I noted however that uiout 
levels are allocated on the stack, while the streams associated with 
them are lazily malloc'ed when needed (in fact the programmer need do 
this manually, and clean them up too).  I had second thoughts on this 
when I counted the number of times pop and push_level are called in a 
random debugging session. It was in the 1000s (if not 10,000s can't 
remember). That would be a lot of mallocs and frees :( So I decided to 
leave the uiout levels, at least, allocated on the stack at a fixed level.

The only thing I can think to do in this case is just to add one more 
level to the maximum uiout levels. I am not sure why the initial level 
is set at such a small level. The uiout level struct is not particularly 
large. Anyway, what do you think? This fixes the PR.


ChangeLog:

2009-09-23  Phil Muldoon <pmuldoon@redhat.com>

     PR python/10344

     * ui-out.c (MAX_UI_OUT_LEVELS): Increment to 7.

--

diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 19a4644..0224d70 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -44,7 +44,7 @@ struct ui_out_hdr
     is always available.  Stack/nested level 0 is reserved for the
     top-level result. */

-enum { MAX_UI_OUT_LEVELS = 6 };
+enum { MAX_UI_OUT_LEVELS = 7 };

  struct ui_out_level
    {

Re: [Archer] [patch] PR python/10344

[Joel Brobecker]

> The only thing I can think to do in this case is just to add one more  
> level to the maximum uiout levels. I am not sure why the initial level  
> is set at such a small level. The uiout level struct is not particularly  
> large. Anyway, what do you think? This fixes the PR.

The only thing I can think of is protecting ourselves against reaching
the maximum number of open file descriptors? Perhaps it's ridiculously
low on certain systems? I'd have a hard time believing that raising
MAX_UI_OUT_LEVELS from 6 to 7 would make any difference so I would be
inclined to give it a try.  I looked at the history of this constant,
and it was initially defined as 5, and then raised to 6 without much
comment in order to fix PR cli/654.

I forgot how to convert old PR numbers into the new ones (I need to
write this down in my tips-and-tricks file). Anyone remembers?

Thanks,
-- 
Joel

Re: [Archer] [patch] PR python/10344

[Phil Muldoon]

On 09/23/2009 04:44 PM, Joel Brobecker wrote:
>> The only thing I can think to do in this case is just to add one more
>> level to the maximum uiout levels. I am not sure why the initial level
>> is set at such a small level. The uiout level struct is not particularly
>> large. Anyway, what do you think? This fixes the PR.
>>      
> The only thing I can think of is protecting ourselves against reaching
> the maximum number of open file descriptors? Perhaps it's ridiculously
> low on certain systems? I'd have a hard time believing that raising
> MAX_UI_OUT_LEVELS from 6 to 7 would make any difference so I would be
> inclined to give it a try.  I looked at the history of this constant,
> and it was initially defined as 5, and then raised to 6 without much
> comment in order to fix PR cli/654.
>
> I forgot how to convert old PR numbers into the new ones (I need to
> write this down in my tips-and-tricks file). Anyone remembers?
>
>    

http://sourceware.org/bugzilla/show_bug.cgi?id=7759

But yeah, not much in there other than what was on the list. I think, it 
had some vague to do with hurd from reading around the gmane archives.

There are other cases where the limit was exceeded but these exclusively 
refer to bugs with cleanups.

Regards

Phil

Re: [patch] PR python/10344

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> 2009-09-23  Phil Muldoon <pmuldoon@redhat.com>
Phil>     PR python/10344
Phil>     * ui-out.c (MAX_UI_OUT_LEVELS): Increment to 7.

For the record, we discussed this a bit on irc, and this doesn't seem to
be the best solution.  In particular it can fail in other situations;
Phil put more details into the bug.

Tom