C++ support improvement patch

Thomas Richter thor@mail.math.tu-berlin.de
Mon Jan 23 09:50:00 GMT 2006


Hi folks,

please find a patch to gdb-6.4 (release version) attached. To apply
this patch, you need to get the current release (not CVS) version
of gdb, and apply it to the "gdb" subdirectory of the project.

This patch does the following:

o) GDB finds now members within the multiple-inheritance hierarchy of
a class. That is, if "this" points to a class "Foo" inherited from
"Bar" and "Baz", and "Baz" contains a member "me", then "print me"
will succeed now. gdb 6.4 and before just returned an "unknown symbol"
here, erraneously. This is because it was confused by a "class typedef"
g++ inserted here for subclasses in the dwarf-2 symbol table, which is
now cleverly avoided. The patch then, however, needs to be clever to
resolve expressions like this->Subclass::member, which - after a bit
of work - now works nicely as it should. It seems to me that it worked
more or less by accident before (Subclass was resolved as the constructor
name, not as the class name.)

o) For TAB-expansion, it removes the ":" as word-delimiter for
readline, thus allowing to find names in namespaces. The current
behaivour here is completely unusable for C++. The patch could
be better, though, it should read template argument delimiters < >
and function argument delimiters ( ) as "quotation characters",
though it doesn't. Though it's a huge improvement compared to
previous versions where TAB expansion was just not usable for C++.

o) GDB now avoids to set multiple breakpoints to the same location.
This goes in conjunction with another bug, namely that of being unable
to find constructors of multiply-inherited classes. Without the current
patch, setting a breakpoint for a constructor results in multiple breakpoint
locations found, all of them identical, and none of them actually 
beeing breaked on. With the patch applied, GDB at least finds only one
of the locations (good), but still doesn't break there (bad). This bug
seems partially caused by g++ not emmiting mangled names for constructors.

o) GDB does no longer print the character and the value for integer values
of size one. It only prints the character. The reason for this is that
with the ddd frontend, ddd parses the output of "info breakpoints" to
set or reset breakpoints (e.g. when moving them in the sources). Without
the patch, the syntax of the output is no longer valid, and is not parsable
by gdb again.

Finally, a comment: I have been unable to resolve the "breakpoint in
constructor" bug. This is a long-standing issue that remained open for
quite a while. Most likely, this is caused by two independent bugs:
i) gdb relies on the "MIPS linkage name" of members, though according to
the g++ folks it should not,
ii) g++ may create separate constructors depending on whehter the class is
constructed by itself, or as part of a subclass. gdb seems to be unaware
of this. I've no idea how to resolve this.

I would be thankful to receive commends on the patch,

thanks,
	Thomas

Now for the patch:

/* snip */

diff -Naur gdb_old/c-exp.y gdb/c-exp.y
--- gdb_old/c-exp.y	2005-09-20 10:55:55.000000000 +0200
+++ gdb/c-exp.y	2006-01-09 23:04:35.000000000 +0100
@@ -1752,6 +1752,11 @@
     /* Call lookup_symtab, not lookup_partial_symtab, in case there are
        no psymtabs (coff, xcoff, or some future change to blow away the
        psymtabs once once symbols are read).  */
+    /*
+    ** THOR : FIXME: lookup_symbol may also return LOC_BLOCK in case
+    ** we are really looking for a base-class of a class where in fact
+    ** we should then expect LOC_BLOCK.
+    */
     if (sym && SYMBOL_CLASS (sym) == LOC_BLOCK)
       {
 	yylval.ssym.sym = sym;
diff -Naur gdb_old/c-valprint.c gdb/c-valprint.c
--- gdb_old/c-valprint.c	2005-05-09 23:20:30.000000000 +0200
+++ gdb/c-valprint.c	2006-01-21 23:08:29.000000000 +0100
@@ -392,6 +392,11 @@
 	}
       else
 	{
+	  /*
+	  ** FIX THOR: Do not print *both*. This confuses ddd as the resulting
+	  ** output string is no longer parsable
+	  */
+	  if (TYPE_LENGTH(type) != 1)
 	  val_print_type_code_int (type, valaddr + embedded_offset, stream);
 	  /* C and C++ has no single byte int type, char is used instead.
 	     Since we don't know whether the value is really intended to
@@ -399,7 +404,7 @@
 	     equivalent as well. */
 	  if (TYPE_LENGTH (type) == 1)
 	    {
-	      fputs_filtered (" ", stream);
+	      /* fputs_filtered (" ", stream); also removed, THOR */
 	      LA_PRINT_CHAR ((unsigned char) unpack_long (type, valaddr + embedded_offset),
 			     stream);
 	    }
diff -Naur gdb_old/dwarf2read.c gdb/dwarf2read.c
--- gdb_old/dwarf2read.c	2005-11-04 03:58:31.000000000 +0100
+++ gdb/dwarf2read.c	2006-01-15 16:14:35.000000000 +0100
@@ -3524,7 +3524,7 @@
       if (strcmp (fip->fnfieldlists[i].name, fieldname) == 0)
 	break;
     }
-
+  
   /* Create new list element if necessary.  */
   if (i < fip->nfnfields)
     flp = &fip->fnfieldlists[i];
diff -Naur gdb_old/language.c gdb/language.c
--- gdb_old/language.c	2005-10-03 23:21:20.000000000 +0200
+++ gdb/language.c	2006-01-21 22:48:12.000000000 +0100
@@ -1054,7 +1054,13 @@
 char *
 default_word_break_characters (void)
 {
+  /*
+  ** FIX THOR: No, do *NOT* use : as word-break character,
+  ** as this would disallow looking for namespace-scoped names
+  ** as in A::B::x
   return " \t\n!@#$%^&*()+=|~`}{[]\"';:?/>.<,-";
+  */
+  return " \t\n!@#$%^&*()+=|~`}{[]\"';?/>.<,-";
 }
 
 /* Print the index of array elements using the C99 syntax.  */
diff -Naur gdb_old/linespec.c gdb/linespec.c
--- gdb_old/linespec.c	2005-04-26 16:57:21.000000000 +0200
+++ gdb/linespec.c	2006-01-14 01:29:23.000000000 +0100
@@ -236,15 +236,16 @@
 		method_name = dem_opname;
 	    }
 
-	  if (strcmp_iw (name, method_name) == 0)
+	  if (strcmp_iw (name, method_name) == 0) {
 	    /* Find all the overloaded methods with that name.  */
 	    i1 += add_matching_methods (method_counter, t,
 					sym_arr + i1);
-	  else if (strncmp (class_name, name, name_len) == 0
-		   && (class_name[name_len] == '\0'
-		       || class_name[name_len] == '<'))
+	  } else if (strncmp (class_name, name, name_len) == 0
+	      && (class_name[name_len] == '\0'
+		  || class_name[name_len] == '<')) {
 	    i1 += add_constructors (method_counter, t,
 				    sym_arr + i1);
+	  }
 	}
     }
 
@@ -283,7 +284,7 @@
     {
       struct fn_field *f;
       char *phys_name;
-
+      
       f = TYPE_FN_FIELDLIST1 (t, method_counter);
 
       if (TYPE_FN_FIELD_STUB (f, field_counter))
@@ -305,13 +306,29 @@
       if (is_destructor_name (phys_name) != 0)
 	continue;
 
-      sym_arr[i1] = lookup_symbol (phys_name,
-				   NULL, VAR_DOMAIN,
-				   (int *) NULL,
-				   (struct symtab **) NULL);
-      if (sym_arr[i1])
-	i1++;
-      else
+      /* FIX THOR: Do not add duplicate symbols here. */
+      {
+	struct symbol *sym;
+	
+	sym = lookup_symbol (phys_name,
+			     NULL, VAR_DOMAIN,
+			     (int *) NULL,
+			     (struct symtab **) NULL);
+
+	if (sym) {
+	  int i2;
+	  for(i2 = 0;i2 < i1;i2++) {
+	    if (sym_arr[i2] == sym)
+	      break;
+	  }
+	  if (i2 == i1) {
+	    sym_arr[i1] = sym;
+	    i1++;
+	  }
+	}
+      }
+      /* The follownig is not used anyhow.
+	else */
 	{
 	  /* This error message gets printed, but the method
 	     still seems to be found
diff -Naur gdb_old/symtab.c gdb/symtab.c
--- gdb_old/symtab.c	2005-03-08 05:34:44.000000000 +0100
+++ gdb/symtab.c	2006-01-21 22:49:07.000000000 +0100
@@ -1075,7 +1075,7 @@
 
 static struct symbol *
 lookup_symbol_aux (const char *name, const char *linkage_name,
-		   const struct block *block, const domain_enum domain,
+		   const struct block *block, domain_enum domain,
 		   int *is_a_field_of_this, struct symtab **symtab)
 {
   struct symbol *sym;
@@ -1091,6 +1091,10 @@
   /* Search specified block and its superiors.  Don't search
      STATIC_BLOCK or GLOBAL_BLOCK.  */
 
+  /*
+  ** FIXME: Here provide a special search to lookup for 
+  ** TYPEs instead of constructor calls.
+  */
   sym = lookup_symbol_aux_local (name, linkage_name, block, domain,
 				 symtab);
   if (sym != NULL)
@@ -1106,10 +1110,36 @@
 
       if (v && check_field (v, name))
 	{
+	  /* THOR FIX: check_field no longer returns true in case name is
+	  ** a subclass (and not a field) of this, however this creates
+	  ** more problems. The lexer does not say (or even know) whether
+	  ** the type passed in is expected to be a member, or a type.
+	  ** And in case it is a type, this code would have found the
+	  ** constructor (mistakenly), identifying it as a member of *this.
+	  **
+	  ** However, now there's a second problem here, namely the code
+	  ** below then finds the constructor (and not the typedef) because
+	  ** that's what is first in the debug info, and then cannot deduce
+	  ** the right thing for the lexer: If A is a base of this, then
+	  ** this->A::x no longer works because A is found as method, not
+	  ** as type, and A::x no longer works, because A is not found
+	  ** as a field of this (correctly) but neither as a base of this.
+	  **
+	  ** What should happen here is to test whether A is a valid base
+	  ** of this, and then return the type, not the constructor,
+	  ** set is_a_field_of_this, and return a non_NULL symbol.
+	  */
 	  *is_a_field_of_this = 1;
 	  if (symtab != NULL)
 	    *symtab = NULL;
 	  return NULL;
+	} else if (v && check_base(v, name)) {
+	  /*
+	  ** Now all we have to archive is to enforce that lookup_block_symbol
+	  ** finds really a LOC_TYPEDEF, not a LOC_BLOCK. Yummie!
+	  */
+	  domain = STRUCT_DOMAIN;
+	  *is_a_field_of_this = 1;
 	}
     }
 
@@ -1284,7 +1314,6 @@
 	       looking in the statics even though the psymtab claimed
 	       the symbol was global, or vice-versa. It's possible
 	       that the psymtab gets it wrong in some cases.  */
-
 	    /* FIXME: carlton/2002-09-30: Should we really do that?
 	       If that happens, isn't it likely to be a GDB error, in
 	       which case we should fix the GDB error rather than
@@ -1295,10 +1324,16 @@
 				       block_index == GLOBAL_BLOCK ?
 				       STATIC_BLOCK : GLOBAL_BLOCK);
 	    sym = lookup_block_symbol (block, name, linkage_name, domain);
+	    /*
+	    ** THOR: I don't know why this fails, but it does. I should investigate
+	    ** later.
 	    if (!sym)
 	      error (_("Internal: %s symbol `%s' found in %s psymtab but not in symtab.\n%s may be an inlined function, or may be a template function\n(if a template, try specifying an instantiation: %s<type>)."),
 		     block_index == GLOBAL_BLOCK ? "global" : "static",
 		     name, ps->filename, name, name);
+	    */
+	    if (!sym)
+	      return NULL;
 	  }
 	if (symtab != NULL)
 	  *symtab = s;
@@ -1788,8 +1823,10 @@
 	{
 	  if (SYMBOL_DOMAIN (sym) == domain
 	      && (linkage_name != NULL
-		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
-	    return sym;
+		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1)) { /* FIX THOR */
+	    if (domain != VAR_DOMAIN || sym->aclass != LOC_TYPEDEF) /* Must *NOT* use type forwards here */
+	      return sym;
+	  } /* FIX THOR end */
 	}
       return NULL;
     }
@@ -3539,7 +3576,11 @@
 	   which are in symbols.  */
 	while (p > text)
 	  {
-	    if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == '\0')
+	    /*
+	    ** FIX THOR: Also allow : as character here to check for
+	    ** namespaced symbols as in A::B::x
+	    */
+	    if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == ':' || p[-1] == '\0')
 	      --p;
 	    else
 	      break;
diff -Naur gdb_old/valops.c gdb/valops.c
--- gdb_old/valops.c	2005-05-27 06:39:32.000000000 +0200
+++ gdb/valops.c	2006-01-10 00:26:12.000000000 +0100
@@ -91,6 +91,8 @@
 					   int static_offset);
 
 static int check_field_in (struct type *, const char *);
+static int check_base_in (struct type *type, const char *name);
+static int check_field_in_aux (struct type *type, const char *name,int cons_des);
 
 static struct value *value_struct_elt_for_reference (struct type *domain,
 						     int offset,
@@ -2226,10 +2228,21 @@
 /* Helper function for check_field: Given TYPE, a structure/union,
    return 1 if the component named NAME from the ultimate
    target structure/union is defined, otherwise, return 0. */
-
 static int
 check_field_in (struct type *type, const char *name)
 {
+  return check_field_in_aux(type,name,1);
+}
+
+/* FIX THOR: Helper function for check_field: Given TYPE, a structure/union,
+   return 1 if the component named NAME from the ultimate
+   target structure/union is defined, otherwise, return 0.
+   Unlike the above, this one can be told to ignore constructors
+   and destructors.
+*/
+static int
+check_field_in_aux (struct type *type, const char *name, int check_for_constructors)
+{
   int i;
 
   for (i = TYPE_NFIELDS (type) - 1; i >= TYPE_N_BASECLASSES (type); i--)
@@ -2245,24 +2258,107 @@
   /* Destructors are a special case.  */
   if (destructor_name_p (name, type))
     {
-      int m_index, f_index;
-
-      return get_destructor_fn_field (type, &m_index, &f_index);
+      if (check_for_constructors) {
+	int m_index, f_index;
+	
+	return get_destructor_fn_field (type, &m_index, &f_index);
+      }
+      return 0;
     }
 
   for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; --i)
     {
-      if (strcmp_iw (TYPE_FN_FIELDLIST_NAME (type, i), name) == 0)
-	return 1;
+      if (strcmp_iw (TYPE_FN_FIELDLIST_NAME (type, i), name) == 0) {
+	if (check_for_constructors) {
+	  return 1;
+	} else {
+	  /*
+	  ** Do not return constructors here.
+	  */
+	  if (TYPE_NAME (type) == NULL || strcmp_iw (TYPE_NAME(type), name))
+	    return 1;
+	}
+      }
     }
-
+  
+  /*
+  ** Old code
   for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--)
     if (check_field_in (TYPE_BASECLASS (type, i), name))
       return 1;
+  */
+  
+  /* FIX THOR : Must convert the base type to the
+  ** proper real base type with check_typedef.
+  */
+  for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--) {
+    struct type *t = check_typedef (TYPE_BASECLASS (type, i)); 
+    /* Be a bit more careful here: It might be that name is
+    ** found as a constructor of a sub-class. This is not
+    ** really desirable.
+    */
+    if (check_field_in_aux (t, name, 0)) {
+      return 1;
+    }
+  }
+  
+  return 0;
+}
+
+/* FIX THOR: Check whether name is a proper base class of type.
+*/
+int
+check_base (struct value *arg1, const char *name)
+{
+  struct type *t;
+
+  arg1 = coerce_array (arg1);
+
+  t = value_type (arg1);
 
+  /* Follow pointers until we get to a non-pointer.  */
+
+  for (;;)
+    {
+      CHECK_TYPEDEF (t);
+      if (TYPE_CODE (t) != TYPE_CODE_PTR && TYPE_CODE (t) != TYPE_CODE_REF)
+	break;
+      t = TYPE_TARGET_TYPE (t);
+    }
+
+  if (TYPE_CODE (t) == TYPE_CODE_MEMBER)
+    error (_("not implemented: member type in check_base"));
+
+  if (TYPE_CODE (t) != TYPE_CODE_STRUCT
+      && TYPE_CODE (t) != TYPE_CODE_UNION)
+    error (_("Internal error: `this' is not an aggregate"));
+
+  return check_base_in (t, name);
+}
+
+static int
+check_base_in (struct type *type, const char *name)
+{
+  int i;
+
+  for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; --i)
+    {
+      if (strcmp_iw (TYPE_FN_FIELDLIST_NAME (type, i), name) == 0) {
+	if (TYPE_NAME (type) && strcmp_iw (TYPE_NAME(type), name) == 0)
+	  return 1;
+      }
+    }
+  
+  for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--) {
+    struct type *t = check_typedef (TYPE_BASECLASS (type, i));
+    if (check_base_in(t,name))
+      return 1;
+  }
+  
   return 0;
 }
 
+/* FIX THOR end */
 
 /* C++: Given ARG1, a value of type (pointer to a)* structure/union,
    return 1 if the component named NAME from the ultimate
diff -Naur gdb_old/value.h gdb/value.h
--- gdb_old/value.h	2005-05-27 06:39:33.000000000 +0200
+++ gdb/value.h	2006-01-10 00:25:03.000000000 +0100
@@ -496,6 +496,7 @@
 				  struct ui_file *stream);
 
 extern int check_field (struct value *, const char *);
+extern int check_base (struct value *, const char *name);
 
 extern void typedef_print (struct type *type, struct symbol *news,
 			   struct ui_file *stream);

/* snip */



More information about the Gdb-patches mailing list