Re: -var-create on invalid expression causes seg. fault

Re: -var-create on invalid expression causes seg. fault

[Nick Roberts]

> One of our customers reported GDB crashing when an MI
> variable is created on an invalid address and then referenced.
> I've debugged to the point of finding the cause, but am not
> knowledgable enough on the MI variables to know the right
> way to fix it. I filed a bug report because this needs
> to get fixed in the latest sources. I'd appreciate any
> insight so I can fix it in our 5.2.1 based sources.
> I don't have the time to look deeper, but someone on this
> list is familiar with this stuff I'm sure.


> -var-create on an expression that's invalid (eg. "(*1)")
> creates a variable and retains a ptr in var->value. That
> gets freed by free_all_values() next command. Later a
> -var-update or -var-evaluate-expression on that variable
> dereferences the freed memory, causing a seg. fault.

Since you have 5.2.1 based sources, you could run it under gdb and the
backtrace should show you where the problem occurs e.g. in the directory of
your source, somthing like:

/usr/bin/gdb ./gdb
...
(top-gdb) cd ~/
(top-gdb) run -i=mi myprog
...
(gdb)
-var-create - * *1
&"Cannot access memory at address 0x1\n"
^done,name="var1",numchild="0",type="int"
(gdb) 
-var-update *
&"Cannot access memory at address 0x1\n"
Segmentation fault
(top-gdb) bt
...

> I looked at the GDB 6.3 source and it seems to be the same.

> Now why would anyone try to evaluate *1? It's some tool that
> uses MI, one of our customers reported. I'm not clear on why
> GDB even creates the variable in this case, but it does.
> GDB should report an error, not crash.

GDB in CVS doesn't crash although it does allow a variable object to be set at
a nonsensical address. Perhaps this behaviour should be changed. However, I
don't think people on this list will be generally interested in debugging old
versions of GDB.


Nick

Re: -var-create on invalid expression causes seg. fault

[Ross Morley]

Thanks for your input Nick, but I don't think it's what I need.

I already know exactly where the problem is (I debugged it under gdb).
What I don't know is the right way to fix it.
Also I believe this bug exists in 6.3 because the offending code is
the same, though I haven't tried to reproduce it there.

I should have mentioned that a seg. fault does not always immediately
result. It just depends how the freed memory is used after it's freed.
If it's allocated again and its new owner alters the location that was
var->value to contain 0 or some other bad address, the seg. fault will
occur.  Otherwise the problem may lurk for a long time and show up
later in some obscure way or not at all. So it's a bit hard to reproduce
the seg. fault, but if you read my post and run it under gdb you should
easily be able to verify that a pointer to freed memory is dereferenced.

I'm hoping someone who knows the internals of MI variables can suggest
a good fix that won't break something else. It's not as simple as
clearing that hanging pointer... that will just cause a seg. fault
for sure when the pointer is dereferenced. If no one knows, I'll
probably just have to dive in and educate myself.

Ross


Nick Roberts wrote:

>>-var-create on an expression that's invalid (eg. "(*1)")
>>creates a variable and retains a ptr in var->value. That
>>gets freed by free_all_values() next command. Later a
>>-var-update or -var-evaluate-expression on that variable
>>dereferences the freed memory, causing a seg. fault.
>>    
>>
>
>Since you have 5.2.1 based sources, you could run it under gdb and the
>backtrace should show you where the problem occurs e.g. in the directory of
>your source, somthing like:
>
>/usr/bin/gdb ./gdb
>...
>(top-gdb) cd ~/
>(top-gdb) run -i=mi myprog
>...
>(gdb)
>-var-create - * *1
>&"Cannot access memory at address 0x1\n"
>^done,name="var1",numchild="0",type="int"
>(gdb) 
>-var-update *
>&"Cannot access memory at address 0x1\n"
>Segmentation fault
>(top-gdb) bt
>...
>
>  
>
>>I looked at the GDB 6.3 source and it seems to be the same.
>>    
>>
>
>  
>
>>Now why would anyone try to evaluate *1? It's some tool that
>>uses MI, one of our customers reported. I'm not clear on why
>>GDB even creates the variable in this case, but it does.
>>GDB should report an error, not crash.
>>    
>>
>
>GDB in CVS doesn't crash although it does allow a variable object to be set at
>a nonsensical address. Perhaps this behaviour should be changed. However, I
>don't think people on this list will be generally interested in debugging old
>versions of GDB.
>
>
>Nick
>
>  
>

Re: -var-create on invalid expression causes seg. fault

[Daniel Jacobowitz]

On Sun, Feb 20, 2005 at 12:35:06AM -0800, Ross Morley wrote:
> Thanks for your input Nick, but I don't think it's what I need.
> 
> I already know exactly where the problem is (I debugged it under gdb).
> What I don't know is the right way to fix it.
> Also I believe this bug exists in 6.3 because the offending code is
> the same, though I haven't tried to reproduce it there.
> 
> I should have mentioned that a seg. fault does not always immediately
> result. It just depends how the freed memory is used after it's freed.
> If it's allocated again and its new owner alters the location that was
> var->value to contain 0 or some other bad address, the seg. fault will
> occur.  Otherwise the problem may lurk for a long time and show up
> later in some obscure way or not at all. So it's a bit hard to reproduce
> the seg. fault, but if you read my post and run it under gdb you should
> easily be able to verify that a pointer to freed memory is dereferenced.
> 
> I'm hoping someone who knows the internals of MI variables can suggest
> a good fix that won't break something else. It's not as simple as
> clearing that hanging pointer... that will just cause a seg. fault
> for sure when the pointer is dereferenced. If no one knows, I'll
> probably just have to dive in and educate myself.

Could you give the attached patch a try?  I encountered a similar
problem in mi-var-block.exp when using ARM RVDS, which emits location
lists even at -O0.  It frequently reports variables as "unavailable",
which is an error condition.

The patch sets the variable to an error in the -var-update
"unavailable" case; this never passes any error message on to the user,
but that seems to be the prior art for varobj.

You'll need to use catch_exceptions for your older sources; the patch
is for HEAD.

[I haven't finished testing any of these patches, that's why I haven't
submitted this to gdb-patches yet.]

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-02-23  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (eval.o): Update dependencies.
	* eval.c: Include "ui-out.h" and "exceptions.h".
	(evaluate_subexp_standard): Use TRY_CATCH around value_of_variable.
	Use value_zero if an error occurs when avoiding side effects.
	* varobj.c (varobj_create): Call release_value after evaluate_type.
	(c_value_of_root): Initialize new_val.  Don't release_value a NULL
	value.

Index: src/gdb/eval.c
===================================================================
--- src.orig/gdb/eval.c	2005-02-17 14:29:06.000000000 -0800
+++ src/gdb/eval.c	2005-02-23 07:02:08.000000000 -0800
@@ -37,6 +37,8 @@
 #include "block.h"
 #include "parser-defs.h"
 #include "cp-support.h"
+#include "ui-out.h"
+#include "exceptions.h"
 
 /* This is defined in valops.c */
 extern int overload_resolution;
@@ -437,8 +439,26 @@ evaluate_subexp_standard (struct type *e
 	 value_rtti_target_type () if we are dealing with a pointer
 	 or reference to a base class and print object is on. */
 
-	return value_of_variable (exp->elts[pc + 2].symbol,
-				  exp->elts[pc + 1].block);
+      {
+	volatile struct exception except;
+	struct value *ret = NULL;
+
+	TRY_CATCH (except, RETURN_MASK_ERROR)
+	  {
+	    ret = value_of_variable (exp->elts[pc + 2].symbol,
+				     exp->elts[pc + 1].block);
+	  }
+
+	if (except.reason < 0)
+	  {
+	    if (noside == EVAL_AVOID_SIDE_EFFECTS)
+	      ret = value_zero (SYMBOL_TYPE (exp->elts[pc + 2].symbol), not_lval);
+	    else
+	      throw_exception (except);
+	  }
+
+	return ret;
+      }
 
     case OP_LAST:
       (*pos) += 2;
Index: src/gdb/varobj.c
===================================================================
--- src.orig/gdb/varobj.c	2005-02-17 14:29:06.000000000 -0800
+++ src/gdb/varobj.c	2005-02-23 07:12:25.000000000 -0800
@@ -504,7 +504,10 @@ varobj_create (char *objname,
 	    gdb_value_fetch_lazy (var->value);
 	}
       else
-	var->value = evaluate_type (var->root->exp);
+	{
+	  var->value = evaluate_type (var->root->exp);
+	  release_value (var->value);
+	}
 
       var->type = value_type (var->value);
 
@@ -1873,7 +1876,7 @@ c_name_of_child (struct varobj *parent, 
 static struct value *
 c_value_of_root (struct varobj **var_handle)
 {
-  struct value *new_val;
+  struct value *new_val = NULL;
   struct varobj *var = *var_handle;
   struct frame_info *fi;
   int within_scope;
@@ -1917,11 +1920,12 @@ c_value_of_root (struct varobj **var_han
 	      else
 		var->error = 0;
 	    }
+
+	  release_value (new_val);
 	}
       else
 	var->error = 1;
 
-      release_value (new_val);
       return new_val;
     }
 
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2005-02-14 12:09:02.000000000 -0800
+++ src/gdb/Makefile.in	2005-02-23 07:00:39.000000000 -0800
@@ -1891,7 +1891,7 @@ environ.o: environ.c $(defs_h) $(environ
 eval.o: eval.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \
 	$(value_h) $(expression_h) $(target_h) $(frame_h) $(language_h) \
 	$(f_lang_h) $(cp_abi_h) $(infcall_h) $(objc_lang_h) $(block_h) \
-	$(parser_defs_h) $(cp_support_h)
+	$(parser_defs_h) $(cp_support_h) $(exceptions_h) $(uiout_h)
 event-loop.o: event-loop.c $(defs_h) $(event_loop_h) $(event_top_h) \
 	$(gdb_string_h) $(exceptions_h)
 event-top.o: event-top.c $(defs_h) $(top_h) $(inferior_h) $(target_h) \

Re: -var-create on invalid expression causes seg. fault

[Ross Morley]

Thanks Daniel, very much. That did the trick.
I had to translate the exception handling as you mentioned,
but I managed to get that done and the problem is solved
in our 5.2.1 sources. I have not tried your patch in HEAD.

Ross

---------------
Ross Morley
Tensilica, Inc.
ross@tensilica.com


Daniel Jacobowitz wrote:

>On Sun, Feb 20, 2005 at 12:35:06AM -0800, Ross Morley wrote:
>  
>
>>Thanks for your input Nick, but I don't think it's what I need.
>>
>>I already know exactly where the problem is (I debugged it under gdb).
>>What I don't know is the right way to fix it.
>>Also I believe this bug exists in 6.3 because the offending code is
>>the same, though I haven't tried to reproduce it there.
>>
>>I should have mentioned that a seg. fault does not always immediately
>>result. It just depends how the freed memory is used after it's freed.
>>If it's allocated again and its new owner alters the location that was
>>var->value to contain 0 or some other bad address, the seg. fault will
>>occur.  Otherwise the problem may lurk for a long time and show up
>>later in some obscure way or not at all. So it's a bit hard to reproduce
>>the seg. fault, but if you read my post and run it under gdb you should
>>easily be able to verify that a pointer to freed memory is dereferenced.
>>
>>I'm hoping someone who knows the internals of MI variables can suggest
>>a good fix that won't break something else. It's not as simple as
>>clearing that hanging pointer... that will just cause a seg. fault
>>for sure when the pointer is dereferenced. If no one knows, I'll
>>probably just have to dive in and educate myself.
>>    
>>
>
>Could you give the attached patch a try?  I encountered a similar
>problem in mi-var-block.exp when using ARM RVDS, which emits location
>lists even at -O0.  It frequently reports variables as "unavailable",
>which is an error condition.
>
>The patch sets the variable to an error in the -var-update
>"unavailable" case; this never passes any error message on to the user,
>but that seems to be the prior art for varobj.
>
>You'll need to use catch_exceptions for your older sources; the patch
>is for HEAD.
>
>[I haven't finished testing any of these patches, that's why I haven't
>submitted this to gdb-patches yet.]
>
>  
>

Re: -var-create on invalid expression causes seg. fault

[Ross Morley]

P.S. For when you get you patch tested at the head version,
the bug report is varobj/1873.

Ross

Re: -var-create on invalid expression causes seg. fault

[Nick Roberts]

 > Could you give the attached patch a try?  I encountered a similar
 > problem in mi-var-block.exp when using ARM RVDS, which emits location
 > lists even at -O0.  It frequently reports variables as "unavailable",
 > which is an error condition.
 > 
 > The patch sets the variable to an error in the -var-update
 > "unavailable" case; this never passes any error message on to the user,
 > but that seems to be the prior art for varobj.
 > 
 > You'll need to use catch_exceptions for your older sources; the patch
 > is for HEAD.
 > 
 > [I haven't finished testing any of these patches, that's why I haven't
 > submitted this to gdb-patches yet.]

This is what I get now:

-var-create - * *1 
^done,name="var1",numchild="0",type="int"
(gdb) 
-var-update *
~"varobj.c:1467: internal-error: my_value_equal: Assertion `!value_lazy (val1)' failed.\n"
~"A problem internal to GDB has been detected,\n"
~"further debugging may prove unreliable.\n"
~"Quit this debugging session? (y or n) "

but I would expect "-var-create - * *1" to return:

^error,msg="Cannot access memory at address 0x1"

I might be missing the point, since although I don't understand what you have
done, Ross Morley clearly does.


Nick

-var-create on invalid expression causes seg. fault

[Ross Morley]

Hi,

One of our customers reported GDB crashing when an MI
variable is created on an invalid address and then referenced.
I've debugged to the point of finding the cause, but am not
knowledgable enough on the MI variables to know the right
way to fix it. I filed a bug report because this needs
to get fixed in the latest sources. I'd appreciate any
insight so I can fix it in our 5.2.1 based sources.
I don't have the time to look deeper, but someone on this
list is familiar with this stuff I'm sure.

-var-create on an expression that's invalid (eg. "(*1)")
creates a variable and retains a ptr in var->value. That
gets freed by free_all_values() next command. Later a
-var-update or -var-evaluate-expression on that variable
dereferences the freed memory, causing a seg. fault.

Here's the relevant code snippet from varobj_create():

  /* We definitively need to catch errors here.
     If evaluate_expression succeeds we got the value we wanted.
     But if it fails, we still go on with a call to evaluate_type()  */
  if (gdb_evaluate_expression (var->root->exp, &var->value))
    {
      /* no error */
      release_value (var->value);
      if (VALUE_LAZY (var->value))
    gdb_value_fetch_lazy (var->value);
    }
  else
    var->value = evaluate_type (var->root->exp);

  var->type = VALUE_TYPE (var->value);

gdb_evaluate_expression() fails, and GDB goes on to call
evaluate_type(). I don't think simply clearing var->value
is correct. Maybe release_value()? I don't have the time
to acquire enough knowledge of GDB varobj.

I looked at the GDB 6.3 source and it seems to be the same.

Now why would anyone try to evaluate *1? It's some tool that
uses MI, one of our customers reported. I'm not clear on why
GDB even creates the variable in this case, but it does.
GDB should report an error, not crash.

Thanks for any insight.

---------------
Ross Morley
Tensilica, Inc.
ross@tensilica.com