[patch] avoid the crash of gdb+pretty printer on initialized local variables

[patch] avoid the crash of gdb+pretty printer on initialized local variables

[asmwarrior]

[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]

Hi, gdb developers.

When debugger with python pretty printer, I sometimes get the gdb crash
when I try to show the value of uninitialized local variables. As you
know, an uninitialized local variable can contain some random value, so
the pretty printer try to interpret those values, and can cause the gdb
to an long loop or crash.

The patch is just a work-around/hack to handle this problem.
I just first check if the symbol is a local variable, and then check the
current line SAL is smaller than the variable's declaration line. If
true, which means this local variable is OK to show, if not, than I just
skip it.

The first patch try to deal with the "info locals" problem, the local
variable defined later than the current line will be skipped.
The second patch try to skip/filter the same local variables.

For example:

void fun()
{
    wxString *psty = (wxString*) NULL;
    wxString wxStr(L"wxString");
    wxStr += L" Value";
    std::string stdStr("std::string");
    stdStr.append(" value");
    std::map<int, std::string> m;
    m[0] = "000";
    m[1] = "111";     //break point here, we stop here

   

    wxString& wxStrRef = wxStr;
    wxStrRef += L" Ref";
    std::string& stdStrRef = stdStr;
    stdStrRef += " Ref";

    std::list<std::string> l = {"a", "b", "c"};
    std::vector<std::string> v = {"a", "b", "c"};
    std::queue<std::string> q;
    q.push("a");
    q.push("b");

    std::stack<std::string> s;
    s.push("a");
    s.push("b");
}

Now, you have stopped on the breakpoint. the local variable "l,v,q" is
after the breakpoint line.
If you try to run "print v", or "info locals", then gdb will crash (I'm
using gdb cvs build under WindowsXP, mingw, python 2.7)

I believe that this patch will not be applied, because it is just a
hack, right?
I just also CC to mingw maillist hope they have some interests.

Basically, gdb should alive with out any crash in any condition, but
sometimes, I think a workaround is also necessary. I have see that in
QTcreator, when they want to show the contents of a stl container, it do
many sanity check. like:

The length of the std::vector should be positive, and it's size should
be limited.
Also many other sanity checks

asmwarrior
ollydbg from codeblocks forum



[-- Attachment #2: avoid_uninitial_crash_stack.patch --]
[-- Type: text/x-c++, Size: 732 bytes --]

 gdb/stack.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index c51832e..317570d 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1825,6 +1825,10 @@ iterate_over_block_locals (struct block *b,
 {
   struct dict_iterator iter;
   struct symbol *sym;
+  struct frame_info *frame;
+  struct symtab_and_line sal;
+  frame = get_selected_frame (NULL) ;
+  find_frame_sal (frame, &sal);
 
   ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
@@ -1836,6 +1840,8 @@ iterate_over_block_locals (struct block *b,
 	case LOC_COMPUTED:
 	  if (SYMBOL_IS_ARGUMENT (sym))
 	    break;
+	  if(sym->line>= sal.line)
+            break;
 	  (*cb) (SYMBOL_PRINT_NAME (sym), sym, cb_data);
 	  break;
 



[-- Attachment #3: avoid_uninitial_crash_print.patch --]
[-- Type: text/x-c++, Size: 1621 bytes --]

 gdb/symtab.c |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 65e4248..2047351 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1730,6 +1730,8 @@ lookup_block_symbol (const struct block *block, const char *name,
 {
   struct dict_iterator iter;
   struct symbol *sym;
+  struct frame_info *frame;
+  struct symtab_and_line sal;
 
   if (!BLOCK_FUNCTION (block))
     {
@@ -1739,7 +1741,20 @@ lookup_block_symbol (const struct block *block, const char *name,
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
 				     SYMBOL_DOMAIN (sym), domain))
-	    return sym;
+	    {
+	       if(SYMBOL_CLASS (sym)==LOC_LOCAL
+	          ||SYMBOL_CLASS (sym)==LOC_REGISTER
+		  ||SYMBOL_CLASS (sym)==LOC_COMPUTED)	  
+	       {
+		   frame = get_selected_frame (NULL) ;
+		   find_frame_sal (frame, &sal);
+		   if( block == get_frame_block (frame, 0) 
+		      && sym->line >= sal.line)
+			return NULL;
+	       }
+	       return sym;
+	    }
+	    
 	}
       return NULL;
     }
@@ -1763,7 +1778,17 @@ lookup_block_symbol (const struct block *block, const char *name,
 	      sym_found = sym;
 	      if (!SYMBOL_IS_ARGUMENT (sym))
 		{
-		  break;
+		   if(SYMBOL_CLASS (sym)==LOC_LOCAL
+	              ||SYMBOL_CLASS (sym)==LOC_REGISTER
+		      ||SYMBOL_CLASS (sym)==LOC_COMPUTED)
+	           {
+			  frame = get_selected_frame (NULL) ;
+			  find_frame_sal (frame, &sal);
+			  if(block == get_frame_block (frame, 0) 
+			  && sym->line >= sal.line)
+			       sym_found = NULL;
+	           }
+		   break;
 		}
 	    }
 	}

Re: [patch] avoid the crash of gdb+pretty printer on initialized local variables

[Tom Tromey]

>>>>> ">" == ext asmwarrior <asmwarrior@gmail.com> writes:

>> When debugger with python pretty printer, I sometimes get the gdb crash
>> when I try to show the value of uninitialized local variables.

gdb should not crash.

Is this that mingw-builds-only problem that keeps popping up?
I forget, something to do with not using the right gcc flag?

>> The patch is just a work-around/hack to handle this problem.
>> I just first check if the symbol is a local variable, and then check the
>> current line SAL is smaller than the variable's declaration line. If
>> true, which means this local variable is OK to show, if not, than I just
>> skip it.

This is not ok.  First, due to optimization, lines can be smeared
around, leading to the wrong result here.  Second, a variable can be
trashed for many reasons, like other bugs in the code.  In this second
case it is still unacceptable for gdb to crash.

In theory the existing code will detect memory access errors and cause
printing to stop early.  If you have a case where this doesn't work
properly, make a reproducible test case.

>> void fun()
>> {
>>     wxString *psty = (wxString*) NULL;
>>     wxString wxStr(L"wxString");
>>     wxStr += L" Value";
>>     std::string stdStr("std::string");
>>     stdStr.append(" value");
>>     std::map<int, std::string> m;
>>     m[0] = "000";
>>     m[1] = "111";     //break point here, we stop here

This kind of thing is generally not very reproducible.  It relies on
compiler differences, libraries, runtime differences.  Something that is
plain C and just writes bad values directly would be better.

>> If you try to run "print v", or "info locals", then gdb will crash (I'm
>> using gdb cvs build under WindowsXP, mingw, python 2.7)

>> I believe that this patch will not be applied, because it is just a
>> hack, right?

I think it is actively wrong in some cases.

>> Basically, gdb should alive with out any crash in any condition, but
>> sometimes, I think a workaround is also necessary. I have see that in
>> QTcreator, when they want to show the contents of a stl container, it do
>> many sanity check. like:

>> The length of the std::vector should be positive, and it's size should
>> be limited.
>> Also many other sanity checks

Adding a sanity check to the vector printer would probably be ok.
That is maintained in libstdc++.

Checking the size is probably not ok.  gdb lazily fetch sub-objects; and
respects length limits like "set print elements" or the MI equivalent
for dynamic varobjs.  Having the printer check that the length is "short
enough" is generally wrong for this reason.

Tom

Re: [patch] avoid the crash of gdb+pretty printer on initialized local variables

[asmwarrior]

On 2011-12-3 0:50, Tom Tromey wrote:
>>>>>> ">" == ext asmwarrior <asmwarrior@gmail.com> writes:
> 
>>> When debugger with python pretty printer, I sometimes get the gdb crash
>>> when I try to show the value of uninitialized local variables.
> 
> gdb should not crash.
> 
> Is this that mingw-builds-only problem that keeps popping up?

Thanks for the reply and comments.
Yes, I guess it is only the mingw gdb problem, I have no linux at the moment, but in the Codeblocks' forum, I hear no Linux users complained this kind of crash of gdb on linux.


> I forget, something to do with not using the right gcc flag?

My app was build with -O0 and -g, no extra flag is used.

> 
>>> The patch is just a work-around/hack to handle this problem.
>>> I just first check if the symbol is a local variable, and then check the
>>> current line SAL is smaller than the variable's declaration line. If
>>> true, which means this local variable is OK to show, if not, than I just
>>> skip it.
> 
> This is not ok.  First, due to optimization, lines can be smeared
> around, leading to the wrong result here.  Second, a variable can be
> trashed for many reasons, like other bugs in the code.  In this second
> case it is still unacceptable for gdb to crash.
> 
> In theory the existing code will detect memory access errors and cause
> printing to stop early.  If you have a case where this doesn't work
> properly, make a reproducible test case.
Yes, agreed, as I said, it is only a work around. I create this patch to only avoid the crash of gdb under mingw.
The sample code below can easily cause the crash under mingw+gdb+python pretty printer.
> 
>>> void fun()
>>> {
>>>     wxString *psty = (wxString*) NULL;
>>>     wxString wxStr(L"wxString");
>>>     wxStr += L" Value";
>>>     std::string stdStr("std::string");
>>>     stdStr.append(" value");
>>>     std::map<int, std::string> m;
>>>     m[0] = "000";
>>>     m[1] = "111";     //break point here, we stop here
> 

It was not easy to catch such crash problem under windows.
By the way, I found a way to debug gdb under Codeblocks' IDE, see:
http://forums.codeblocks.org/index.php/topic,15618.msg105064.html#msg105064

It has some advantages than just debug gdb under command line. (mentioned in the above link post)

Hopefully it will help some Windows user to easily check the crash problem in the future.

asmwarrior
ollydbg from codeblocks' forum

Fwd: [patch] avoid the crash of gdb+pretty printer on initialized local variables

[Martin Runge]

Hello,

I have seen gdb running into very long loops on Linux, too (Ubuntu
11.04, gdb 7.2 and 7.3). It looks like gdb is frozen, but it uses 100%
CPU. When attaching to that gdb process, I observed gdb fetching a lot
of values (address increasing, but the range was by far too large). I
guess this comes from a pretty printer, asking gdb to fetch too many
values. libstdc++ and Qt4 pretty printers were in use in my case.

Although the error is probably somewhere in the pretty_printers, the
user experience is very confusing. E.g. with Eclipse CDT on top, gdb
does not respond to mi commands any more. As gdb does not give any
"progress" of "still alive" messages via mi to Eclipse, it runs into a
timeout and assumes gdb dead, debug session is broken and needs to be
restarted.

I think, only few people write their pretty printers themselves. Most
get them "somewhere from the Internet", so its hard to guarantee
quality or rely on it.

Would it be a good idea in your eyes, if

- gdb would guarantee a response time to mi commands ( limit the time
spent in a mi command -> stop fetching values when time is over)?

- gdb gave some progress reports "still working, nn% done" in regular
intervals, so mi clients like Eclipse can restart their timeout? For
Example smartcards do so. If asked for a long running operation, the
smartcard first replies by asking the terminal (== card reader) to
extend the timeout before starting work. This can be repeated as often
as neccessary to complete the operation and the terminal gets regular
feedback. The card terminal may not deny the smartcard's wish.

- some possibly long running mi commands could be aborted over mi
(those that run a loop over many small operations, like
pretty-printers)?

best regards
Martin

Re: Fwd: [patch] avoid the crash of gdb+pretty printer on initialized local variables

[André Pönitz]

On Monday 05 December 2011 11:13:25 ext Martin Runge wrote:
> Hello,
> 
> I have seen gdb running into very long loops on Linux, too (Ubuntu
> 11.04, gdb 7.2 and 7.3). It looks like gdb is frozen, but it uses 100%
> CPU. When attaching to that gdb process, I observed gdb fetching a lot
> of values (address increasing, but the range was by far too large). I
> guess this comes from a pretty printer, asking gdb to fetch too many
> values. libstdc++ and Qt4 pretty printers were in use in my case.
> 
> Although the error is probably somewhere in the pretty_printers, the
> user experience is very confusing. E.g. with Eclipse CDT on top, gdb
> does not respond to mi commands any more. As gdb does not give any
> "progress" of "still alive" messages via mi to Eclipse, it runs into a
> timeout and assumes gdb dead, debug session is broken and needs to be
> restarted.
> 
> I think, only few people write their pretty printers themselves. Most
> get them "somewhere from the Internet", so its hard to guarantee
> quality or rely on it.

[Even "good" pretty printers have a hard time to work robustly on 
uninitialized data without being too restrictive.]

> Would it be a good idea in your eyes, if
> 
> - gdb would guarantee a response time to mi commands ( limit the time
> spent in a mi command -> stop fetching values when time is over)?
> 
> - gdb gave some progress reports "still working, nn% done" in regular
> intervals, so mi clients like Eclipse can restart their timeout? For
> Example smartcards do so. If asked for a long running operation, the
> smartcard first replies by asking the terminal (== card reader) to
> extend the timeout before starting work. This can be repeated as often
> as neccessary to complete the operation and the terminal gets regular
> feedback. The card terminal may not deny the smartcard's wish.
> 
> - some possibly long running mi commands could be aborted over mi
> (those that run a loop over many small operations, like
> pretty-printers)?

I think it would be sufficient if a frontend could interrupt a running python
script, see http://sourceware.org/bugzilla/show_bug.cgi?id=12615

Andre'

Re: Fwd: [patch] avoid the crash of gdb+pretty printer on initialized local variables

[Tom Tromey]

>>>>> "Martin" == Martin Runge <martin.runge@web.de> writes:

Martin> I have seen gdb running into very long loops on Linux, too (Ubuntu
Martin> 11.04, gdb 7.2 and 7.3). It looks like gdb is frozen, but it uses 100%
Martin> CPU. When attaching to that gdb process, I observed gdb fetching a lot
Martin> of values (address increasing, but the range was by far too large). I
Martin> guess this comes from a pretty printer, asking gdb to fetch too many
Martin> values. libstdc++ and Qt4 pretty printers were in use in my case.

Martin> Although the error is probably somewhere in the pretty_printers, the
Martin> user experience is very confusing. E.g. with Eclipse CDT on top, gdb
Martin> does not respond to mi commands any more. As gdb does not give any
Martin> "progress" of "still alive" messages via mi to Eclipse, it runs into a
Martin> timeout and assumes gdb dead, debug session is broken and needs to be
Martin> restarted.

Is Eclipse using the range limitation feature of dynamic varobjs?
That is intended to help prevent this sort of thing.

Martin> I think, only few people write their pretty printers themselves. Most
Martin> get them "somewhere from the Internet", so its hard to guarantee
Martin> quality or rely on it.

Yeah.

Martin> Would it be a good idea in your eyes, if
[...]

I am not sure what would be the best solution, but could you file a bug
report for the problem?

Tom

Re: [patch] avoid the crash of gdb+pretty printer on initialized local variables

[Tom Tromey]

>>>>> ">" == ext asmwarrior <asmwarrior@gmail.com> writes:

Tom> I forget, something to do with not using the right gcc flag?

>> My app was build with -O0 and -g, no extra flag is used.

I mean a missing flag when building gdb.  I vaguely recall a need for
-fno-omit-frame-pointer, and that without this, gdb exceptions cause
crashes.

Tom

Re: [patch] avoid the crash of gdb+pretty printer on initialized local variables

[asmwarrior]

On 2011-12-6 5:44, Tom Tromey wrote:
>>>>>> ">" == ext asmwarrior <asmwarrior@gmail.com> writes:
> 
> Tom> I forget, something to do with not using the right gcc flag?
> 
>>> My app was build with -O0 and -g, no extra flag is used.
> 
> I mean a missing flag when building gdb.  I vaguely recall a need for
> -fno-omit-frame-pointer, and that without this, gdb exceptions cause
> crashes.
> 
> Tom

Hi, Tom, thanks.

Yes, gdb build under mingw should have -fno-omit-frame-pointer flag, otherwise, it will crash.

I forget to mention that My gdb was build with -O0 and -g, so I think no optimization was used in building gdb.

The problem is that gdb still crash when it try to pretty-print the uninitialized local variables.

asmwarrior
ollydbg from codeblocks' forum

Re: [patch] avoid the crash of gdb+pretty printer on initialized local variables

[Tom Tromey]

>>>>> ">" == ext asmwarrior <asmwarrior@gmail.com> writes:

>> I forget to mention that My gdb was build with -O0 and -g, so I think
>> no optimization was used in building gdb.

>> The problem is that gdb still crash when it try to pretty-print the
>> uninitialized local variables.

I think the best thing to do is find out why that happens, and fix the
root cause.  gdb should not crash.

Tom