Obvious fix in values.c?

Obvious fix in values.c?

[Jim Ingham]

Do folks agree that this falls under the obvious fix rule?  It doesn't 
seem safe to assume that all demangled names will contain a space (and 
in fact, gdb crashed on someone when the assumption was proved false...).

2002-04-19  Jim Ingham <jingham@apple.com>

	* values.c (value_headof): Don't assume all demangled names contain 
a space.

Index: values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.34
diff -p -r1.34 values.c
*** values.c    29 Jan 2002 03:08:26 -0000      1.34
--- values.c    19 Apr 2002 21:24:45 -0000
*************** value_headof (struct value *in_arg, stru
*** 1043,1048 ****
--- 1043,1049 ----
     struct symbol *sym;
     char *demangled_name;
     struct minimal_symbol *msymbol;
+   char *tmp;

     btype = TYPE_VPTR_BASETYPE (dtype);
     CHECK_TYPEDEF (btype);
*************** value_headof (struct value *in_arg, stru
*** 1080,1086 ****
           return in_arg;
         }
     demangled_name = cplus_demangle(demangled_name,DMGL_ANSI);
!   *(strchr (demangled_name, ' ')) = '\0';

     sym = lookup_symbol (demangled_name, 0, VAR_NAMESPACE, 0, 0);
     if (sym == NULL)
--- 1081,1089 ----
           return in_arg;
         }
     demangled_name = cplus_demangle(demangled_name,DMGL_ANSI);
!   tmp = strchr (demangled_name, ' ');
!   if (tmp)
!     *tmp = '\0';

     sym = lookup_symbol (demangled_name, 0, VAR_NAMESPACE, 0, 0);
     if (sym == NULL)

--
Jim Ingham                                   jingham@apple.com
Developer Tools - gdb
Apple Computer

Re: Obvious fix in values.c?

[Daniel Jacobowitz]

On Fri, Apr 19, 2002 at 02:28:44PM -0700, Jim Ingham wrote:
> Do folks agree that this falls under the obvious fix rule?  It doesn't 
> seem safe to assume that all demangled names will contain a space (and 
> in fact, gdb crashed on someone when the assumption was proved false...).
> 
> 2002-04-19  Jim Ingham <jingham@apple.com>
> 
> 	* values.c (value_headof): Don't assume all demangled names contain 
> a space.

Could I please get you to solve this properly?  Kill the one and only
use of this function in printcmd.c via value_from_vtable_info.  If you
simply cast the pointer to a pointer to the proper base type, found via
value_rtti_type, this function can die.  It should just work (and/or be
more useful than the current behavior).

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: Obvious fix in values.c?

[Jim Ingham]

Daniel,

I'll take a look at it after Apple's WWDC (mid-May).  I won't have time 
till then.

Jim

On Friday, April 19, 2002, at 03:58  PM, Daniel Jacobowitz wrote:

> On Fri, Apr 19, 2002 at 02:28:44PM -0700, Jim Ingham wrote:
>> Do folks agree that this falls under the obvious fix rule?  It doesn't
>> seem safe to assume that all demangled names will contain a space (and
>> in fact, gdb crashed on someone when the assumption was proved 
>> false...).
>>
>> 2002-04-19  Jim Ingham <jingham@apple.com>
>>
>> 	* values.c (value_headof): Don't assume all demangled names contain
>> a space.
>
> Could I please get you to solve this properly?  Kill the one and only
> use of this function in printcmd.c via value_from_vtable_info.  If you
> simply cast the pointer to a pointer to the proper base type, found via
> value_rtti_type, this function can die.  It should just work (and/or be
> more useful than the current behavior).
>
> --
> Daniel Jacobowitz                           Carnegie Mellon University
> MontaVista Software                         Debian GNU/Linux Developer
>
--
Jim Ingham                                   jingham@apple.com
Developer Tools - gdb
Apple Computer

Re: Obvious fix in values.c?

[Andrew Cagney]

> Daniel,
> 
> I'll take a look at it after Apple's WWDC (mid-May).  I won't have time till then.

Jim,

Can you create a change-request then?

Andrew

Re: Obvious fix in values.c?

[Jim Ingham]

It would be better for Daniel to do this, since I don't really even have 
time to figure out what he wants me to do yet (though I'll be happy to 
do it when I do have time...)

Jim

On Friday, April 19, 2002, at 04:43  PM, Andrew Cagney wrote:

>> Daniel,
>> I'll take a look at it after Apple's WWDC (mid-May).  I won't have 
>> time till then.
>
> Jim,
>
> Can you create a change-request then?
>
> Andrew
>
>
>
--
Jim Ingham                                   jingham@apple.com
Developer Tools - gdb
Apple Computer

Re: Obvious fix in values.c?

[Daniel Jacobowitz]

On Fri, Apr 19, 2002 at 06:58:37PM -0400, Daniel Jacobowitz wrote:
> On Fri, Apr 19, 2002 at 02:28:44PM -0700, Jim Ingham wrote:
> > Do folks agree that this falls under the obvious fix rule?  It doesn't 
> > seem safe to assume that all demangled names will contain a space (and 
> > in fact, gdb crashed on someone when the assumption was proved false...).
> > 
> > 2002-04-19  Jim Ingham <jingham@apple.com>
> > 
> > 	* values.c (value_headof): Don't assume all demangled names contain 
> > a space.
> 
> Could I please get you to solve this properly?  Kill the one and only
> use of this function in printcmd.c via value_from_vtable_info.  If you
> simply cast the pointer to a pointer to the proper base type, found via
> value_rtti_type, this function can die.  It should just work (and/or be
> more useful than the current behavior).

Rather than file a change request, I did it myself.

I can't tell when the code to check objectprint in c_val_print came
along; the ChangeLogs are a little unclear.  However, it is superior to
the matching code in printcmd.c, and completely obsoletes it.  I've
killed the block in printcmd, and also the value_headof and
value_from_vtable_info functions.  And the TODO entry telling me to do
this.  The crash point above is deleted and 'set print object 1'
continues to work in all my testcases.

I've committed this.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2002-05-11  Daniel Jacobowitz  <drow@mvista.com>

	* TODO: Remove value_headof/value_from_vtable_info comment.
	* printcmd.c (print_command_1): Don't call value_from_vtable_info.
	* values.c (value_headof, value_from_vtable_info): Delete.
	* value.h (value_from_vtable_info): Delete prototype.

Index: TODO
===================================================================
RCS file: /cvs/src/src/gdb/TODO,v
retrieving revision 1.95
diff -u -p -r1.95 TODO
--- TODO	13 Jan 2002 16:16:58 -0000	1.95
+++ TODO	11 May 2002 23:27:51 -0000
@@ -247,10 +247,6 @@ New languages come onto the scene all th
 
 Re: Various C++ things
 
-value_headof/value_from_vtable_info are worthless, and should be
-removed.  The one place in printcmd.c that uses it should use the RTTI
-functions.
-
 RTTI for g++ should be using the typeinfo functions rather than the
 vtables.  The typeinfo functions are always at offset 4 from the
 beginning of the vtable, and are always right. The vtables will have
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.38
diff -u -p -r1.38 printcmd.c
--- printcmd.c	5 Apr 2002 22:04:41 -0000	1.38
+++ printcmd.c	11 May 2002 23:27:54 -0000
@@ -923,25 +923,6 @@ print_command_1 (char *exp, int inspect,
       old_chain = make_cleanup (free_current_contents, &expr);
       cleanup = 1;
       val = evaluate_expression (expr);
-
-      /* C++: figure out what type we actually want to print it as.  */
-      type = VALUE_TYPE (val);
-
-      if (objectprint
-	  && (TYPE_CODE (type) == TYPE_CODE_PTR
-	      || TYPE_CODE (type) == TYPE_CODE_REF)
-	  && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT
-	      || TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_UNION))
-	{
-	  struct value *v;
-
-	  v = value_from_vtable_info (val, TYPE_TARGET_TYPE (type));
-	  if (v != 0)
-	    {
-	      val = v;
-	      type = VALUE_TYPE (val);
-	    }
-	}
     }
   else
     val = access_value_history (0);
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.29
diff -u -p -r1.29 value.h
--- value.h	17 Mar 2002 01:32:54 -0000	1.29
+++ value.h	11 May 2002 23:27:55 -0000
@@ -405,9 +405,6 @@ extern struct value *value_repeat (struc
 
 extern struct value *value_subscript (struct value *array, struct value *idx);
 
-extern struct value *value_from_vtable_info (struct value *arg,
-					     struct type *type);
-
 extern struct value *value_being_returned (struct type *valtype,
 					   char *retbuf, int struct_return);
 
Index: values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.34
diff -u -p -r1.34 values.c
--- values.c	29 Jan 2002 03:08:26 -0000	1.34
+++ values.c	11 May 2002 23:27:57 -0000
@@ -41,8 +41,6 @@ void _initialize_values (void);
 
 /* Prototypes for local functions. */
 
-static struct value *value_headof (struct value *, struct type *, struct type *);
-
 static void show_values (char *, int);
 
 static void show_convenience (char *, int);
@@ -1020,93 +1018,6 @@ value_fn_field (struct value **arg1p, st
   return v;
 }
 
-/* ARG is a pointer to an object we know to be at least
-   a DTYPE.  BTYPE is the most derived basetype that has
-   already been searched (and need not be searched again).
-   After looking at the vtables between BTYPE and DTYPE,
-   return the most derived type we find.  The caller must
-   be satisfied when the return value == DTYPE.
-
-   FIXME-tiemann: should work with dossier entries as well.
-   NOTICE - djb: I see no good reason at all to keep this function now that
-   we have RTTI support. It's used in literally one place, and it's
-   hard to keep this function up to date when it's purpose is served
-   by value_rtti_type efficiently.
-   Consider it gone for 5.1. */
-
-static struct value *
-value_headof (struct value *in_arg, struct type *btype, struct type *dtype)
-{
-  /* First collect the vtables we must look at for this object.  */
-  struct value *arg;
-  struct value *vtbl;
-  struct symbol *sym;
-  char *demangled_name;
-  struct minimal_symbol *msymbol;
-
-  btype = TYPE_VPTR_BASETYPE (dtype);
-  CHECK_TYPEDEF (btype);
-  arg = in_arg;
-  if (btype != dtype)
-      arg = value_cast (lookup_pointer_type (btype), arg);
-  if (TYPE_CODE (VALUE_TYPE (arg)) == TYPE_CODE_REF)
-      {
-	  /*
-	   * Copy the value, but change the type from (T&) to (T*).
-	   * We keep the same location information, which is efficient,
-	   * and allows &(&X) to get the location containing the reference.
-	   */
-	  arg = value_copy (arg);
-	  VALUE_TYPE (arg) = lookup_pointer_type (TYPE_TARGET_TYPE (VALUE_TYPE (arg)));
-      }
-  if (VALUE_ADDRESS(value_field (value_ind(arg), TYPE_VPTR_FIELDNO (btype)))==0)
-      return arg;
-
-  vtbl = value_ind (value_field (value_ind (arg), TYPE_VPTR_FIELDNO (btype)));
-  /* Turn vtable into typeinfo function */
-  VALUE_OFFSET(vtbl)+=4;
-
-  msymbol = lookup_minimal_symbol_by_pc ( value_as_address(value_ind(vtbl)) );
-  if (msymbol == NULL
-      || (demangled_name = SYMBOL_NAME (msymbol)) == NULL)
-      {
-	  /* If we expected to find a vtable, but did not, let the user
-	     know that we aren't happy, but don't throw an error.
-	     FIXME: there has to be a better way to do this.  */
-	  struct type *error_type = (struct type *) xmalloc (sizeof (struct type));
-	  memcpy (error_type, VALUE_TYPE (in_arg), sizeof (struct type));
-	  TYPE_NAME (error_type) = savestring ("suspicious *", sizeof ("suspicious *"));
-	  VALUE_TYPE (in_arg) = error_type;
-	  return in_arg;
-      }
-  demangled_name = cplus_demangle(demangled_name,DMGL_ANSI);
-  *(strchr (demangled_name, ' ')) = '\0';
-
-  sym = lookup_symbol (demangled_name, 0, VAR_NAMESPACE, 0, 0);
-  if (sym == NULL)
-      error ("could not find type declaration for `%s'", demangled_name);
-
-  arg = in_arg;
-  VALUE_TYPE (arg) = lookup_pointer_type (SYMBOL_TYPE (sym));
-  return arg;
-}
-
-/* ARG is a pointer object of type TYPE.  If TYPE has virtual
-   function tables, probe ARG's tables (including the vtables
-   of its baseclasses) to figure out the most derived type that ARG
-   could actually be a pointer to.  */
-
-struct value *
-value_from_vtable_info (struct value *arg, struct type *type)
-{
-  /* Take care of preliminaries.  */
-  if (TYPE_VPTR_FIELDNO (type) < 0)
-    fill_in_vptr_fieldno (type);
-  if (TYPE_VPTR_FIELDNO (type) < 0)
-    return 0;
-
-  return value_headof (arg, 0, type);
-}
 \f
 /* Unpack a field FIELDNO of the specified TYPE, from the anonymous object at
    VALADDR.