Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] c++/13615
@ 2012-09-12 23:24 Keith Seitz
  2012-09-26 20:52 ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Keith Seitz @ 2012-09-12 23:24 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

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

Hi,

$SUBJECT deals with various symbol lookup failures with inherited 
typedefs.  Tom reported these failures in both the expression parser and 
gdb.lookup_type (part of the python support).

The fix for the expression parser is pretty straightforward. We already 
use cp_lookup_nested_symbol. Simply adding a routine to search through 
base classes is sufficient to make this work.

The python bug is a little trickier. In this case, gdb.lookup_type calls 
lookup_typename from gdbtypes.c, which is essentially a simple call to 
lookup_symbol.

Here's the big change: With this patch, the C++ version of lookup_symbol 
(cp_lookup_symbol_nonlocal) will now return a symbol for which a base 
class defines the name. For example:

class A
{
public:
   typedef int value_type;
};

class B : public A
{
};

Before this patch: lookup_symbol ("B::value_type", VAR_DOMAIN) ==> NULL
After this patch:  lookup_symbol ("B::value_type", VAR_DOMAIN) ==> 
symbol for A::value_type.

I don't know if cp_lookup_symbol_nonlocal was intentionally designed 
this way or not, but IMO, lookup_symbol was virtualized for languages so 
that they could "do the right thing." For C++, I think searching base 
classes is the "right thing." (TM)

No regressions on x86_64-linux and native gdbserver.

Keith

ChangeLog
2012-09-12  Keith Seitz  <keiths@redhat.com>

	c++/13615
	* cp-namespace.c (cp_lookup_symbol_nonlocal): If no symbol
	is found, search through any base classes to find it.
	(find_symbol_in_baseclass): New function.
	(cp_lookup_nested_symbol): Search through base classes, too.

testsuite/ChangeLog
2012-09-12  Keith Seitz  <keiths@redhat.com>

	c++/13615
	* gdb.cp/derivation.cc (A): Add a typedef.
	(B): Use A::value_type instead of int.  Change all references.
	(D): Use value_type instead of int.  Change all references.
	(E): Likewise.
	(F); Likewise.
	(Z): New class.
	(ZZ): New class.
	(main): Add instances of Z and ZZ.
	* gdb.cp/derivation.exp: Update typedef changes in tests.
	Add missing tests for class G.
	Add tests for class typedefs both before and after starting
	the inferior.
	* lib/cp-support.exp (cp_test_ptype_class): Add support for
	typedefs.

[-- Attachment #2: 13615.patch --]
[-- Type: text/x-patch, Size: 14764 bytes --]

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index e2291a9..4cff199 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -252,6 +252,11 @@ cp_lookup_symbol_nonlocal (const char *name,
 			   const domain_enum domain)
 {
   struct symbol *sym;
+  char *klass, *nested;
+  const char *lookup_name;
+  int prefix_len;
+  struct cleanup *cleanup;
+  struct symbol *klass_sym;
   const char *scope = block_scope (block);
 
   sym = lookup_namespace_scope (name, block,
@@ -259,8 +264,41 @@ cp_lookup_symbol_nonlocal (const char *name,
   if (sym != NULL)
     return sym;
 
-  return cp_lookup_symbol_namespace (scope, name,
-				     block, domain);
+  sym = cp_lookup_symbol_namespace (scope, name,
+				    block, domain);
+  if (sym != NULL)
+    return sym;
+
+  /* A simple lookup failed.  Check if the symbol was defined in
+     a base class.  */
+
+  /* Find the class and method/member names.  */
+  cleanup = demangle_for_lookup (name, language_cplus, &lookup_name);
+
+  /* Find the name of the class and the name of the method, variable, etc.  */
+  prefix_len = cp_entire_prefix_len (lookup_name);
+  if (prefix_len == 0)
+    return NULL;
+
+  /* The class name is everything up to and including PREFIX_LEN.  */
+  klass = savestring (lookup_name, prefix_len);
+  make_cleanup (xfree, klass);
+
+  /* The rest of the name is everything else past the initial scope
+     operator.  */
+  nested = xstrdup (lookup_name + prefix_len + 2);
+  make_cleanup (xfree, nested);
+
+  /* Lookup a class named KLASS.  If none is found, there is nothing
+     more that can be done.  */
+  klass_sym = lookup_symbol (klass, block, domain, NULL);
+  if (klass_sym == NULL)
+    return NULL;
+
+  /* Look for a symbol named NESTED in this class.  */
+  sym = cp_lookup_nested_symbol (SYMBOL_TYPE (klass_sym), nested, block);
+  do_cleanups (cleanup);
+  return sym;
 }
 
 /* Look up NAME in the C++ namespace NAMESPACE.  Other arguments are
@@ -660,6 +698,60 @@ lookup_symbol_file (const char *name,
   return sym;
 }
 
+/* Search through the base classes of PARENT_TYPE for a symbol named
+   NAME in block BLOCK.  */
+
+static struct symbol *
+find_symbol_in_baseclass (struct type *parent_type, const char *name,
+			   const struct block *block)
+{
+  int i;
+  struct symbol *sym;
+  char *concatenated_name;
+  struct cleanup *cleanup;
+
+  sym = NULL;
+  concatenated_name = NULL;
+  cleanup = make_cleanup (null_cleanup, NULL);
+  for (i = TYPE_N_BASECLASSES (parent_type) - 1; i >= 0; --i)
+    {
+      const char *base_name = TYPE_BASECLASS_NAME (parent_type, i);
+
+      /* Search this particular base class.  */
+      discard_cleanups (cleanup);
+      concatenated_name = xrealloc (concatenated_name,
+				    (strlen (base_name) + 2
+				     + strlen (name) + 1));
+      cleanup = make_cleanup (xfree, concatenated_name);
+      sprintf (concatenated_name, "%s::%s", base_name, name);
+      sym = lookup_symbol_static (concatenated_name, block, VAR_DOMAIN);
+      if (sym != NULL)
+	break;
+
+      /* If there is currently no BLOCK, e.g., the inferior hasn't yet
+	 been started, then try searching all STATIC_BLOCK symbols in
+	 all objfiles.  */
+      if (block == NULL)
+	{
+	  sym = lookup_static_symbol_aux (concatenated_name, VAR_DOMAIN);
+	  if (sym != NULL)
+	    break;
+	}
+
+      /* If this class has base classes, search them next.  */
+      if (TYPE_N_BASECLASSES (TYPE_BASECLASS (parent_type, i)) > 0)
+	{
+	  sym = find_symbol_in_baseclass (TYPE_BASECLASS (parent_type, i),
+					  name, block);
+	  if (sym != NULL)
+	    break;
+	}
+    }
+
+  do_cleanups (cleanup);
+  return sym;
+}
+
 /* Look up a symbol named NESTED_NAME that is nested inside the C++
    class or namespace given by PARENT_TYPE, from within the context
    given by BLOCK.  Return NULL if there is no such nested type.  */
@@ -698,10 +790,10 @@ cp_lookup_nested_symbol (struct type *parent_type,
 	  return sym;
 
 	/* Now search all static file-level symbols.  Not strictly
-	   correct, but more useful than an error.  We do not try to
+	   correct, but more useful than an error. We do not try to
 	   guess any imported namespace as even the fully specified
-	   namespace seach is is already not C++ compliant and more
-	   assumptions could make it too magic.  */
+	   namespace search is already not C++ compliant and more
+	   assumptions could make it too magical.  */
 
 	concatenated_name = alloca (strlen (parent_name) + 2
 				    + strlen (nested_name) + 1);
@@ -711,7 +803,9 @@ cp_lookup_nested_symbol (struct type *parent_type,
 	if (sym != NULL)
 	  return sym;
 
-	return NULL;
+	/* If no matching symbols were found, try searching any
+	   base classes.  */
+	return find_symbol_in_baseclass (parent_type, nested_name, block);
       }
     default:
       internal_error (__FILE__, __LINE__,
diff --git a/gdb/testsuite/gdb.cp/derivation.cc b/gdb/testsuite/gdb.cp/derivation.cc
index 942fcd2..81c9803 100644
--- a/gdb/testsuite/gdb.cp/derivation.cc
+++ b/gdb/testsuite/gdb.cp/derivation.cc
@@ -1,32 +1,32 @@
 class A {
 public:
-    int a;
-    int aa;
+    typedef int value_type;
+    value_type a;
+    value_type aa;
 
     A()
     {
         a=1;
         aa=2;
     }
-    int afoo();
-    int foo();
-    
+    value_type afoo();
+    value_type foo();
 };
 
 
 
 class B {
 public:
-    int b;
-    int bb;
+    A::value_type b;
+    A::value_type bb;
 
     B()
     {
         b=3;
         bb=4;
     }
-    int bfoo();
-    int foo();
+    A::value_type bfoo();
+    A::value_type foo();
     
 };
 
@@ -51,48 +51,48 @@ public:
 
 class D : private A, public B, protected C {
 public:
-    int d;
-    int dd;
+    value_type d;
+    value_type dd;
 
     D()
     {
         d =7;
         dd=8;
     }
-    int dfoo();
-    int foo();
+    value_type dfoo();
+    value_type foo();
     
 };
 
 
 class E : public A, B, protected C {
 public:
-    int e;
-    int ee;
+    value_type e;
+    value_type ee;
 
     E()
     {
         e =9;
         ee=10;
     }
-    int efoo();
-    int foo();
+    value_type efoo();
+    value_type foo();
     
 };
 
 
 class F : A, public B, C {
 public:
-    int f;
-    int ff;
+    value_type f;
+    value_type ff;
 
     F()
     {
         f =11;
         ff=12;
     }
-    int ffoo();
-    int foo();
+    value_type ffoo();
+    value_type foo();
     
 };
 
@@ -118,30 +118,40 @@ public:
     
 };
 
+class Z : public A
+{
+public:
+  typedef float value_type;
+  value_type z;
+};
 
+class ZZ : public Z
+{
+public:
+  value_type zz;
+};
 
-
-int A::afoo() {
+A::value_type A::afoo() {
     return 1;
 }
 
-int B::bfoo() {
+A::value_type B::bfoo() {
     return 2;
 }
 
-int C::cfoo() {
+A::value_type C::cfoo() {
     return 3;
 }
 
-int D::dfoo() {
+D::value_type D::dfoo() {
     return 4;
 }
 
-int E::efoo() {
+E::value_type E::efoo() {
     return 5;
 }
 
-int F::ffoo() {
+F::value_type F::ffoo() {
     return 6;
 }
 
@@ -149,37 +159,37 @@ int G::gfoo() {
     return 77;
 }
 
-int A::foo()
+A::value_type A::foo()
 {
     return 7;
     
 }
 
-int B::foo()
+A::value_type B::foo()
 {
     return 8;
     
 }
 
-int C::foo()
+A::value_type C::foo()
 {
     return 9;
     
 }
 
-int D::foo()
+D::value_type D::foo()
 {
     return 10;
     
 }
 
-int E::foo()
+E::value_type E::foo()
 {
     return 11;
     
 }
 
-int F::foo()
+F::value_type F::foo()
 {
     return 12;
     
@@ -207,7 +217,9 @@ int main(void)
     E e_instance;
     F f_instance;
     G g_instance;
-    
+    Z z_instance;
+    ZZ zz_instance;
+
     marker1(); // marker1-returns-here
     
     a_instance.a = 20; // marker1-returns-here
@@ -222,10 +234,10 @@ int main(void)
     e_instance.ee =29;
     f_instance.f =30;
     f_instance.ff =31;
-    
-    
-    
-
+    g_instance.g = 32;
+    g_instance.gg = 33;
+    z_instance.z = 34.0;
+    zz_instance.zz = 35.0;
     return 0;
     
 }
diff --git a/gdb/testsuite/gdb.cp/derivation.exp b/gdb/testsuite/gdb.cp/derivation.exp
index b752b52..26b1d12 100644
--- a/gdb/testsuite/gdb.cp/derivation.exp
+++ b/gdb/testsuite/gdb.cp/derivation.exp
@@ -38,6 +38,18 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
     return -1
 }
 
+# Check inheritance of typedefs.
+foreach klass {"A" "D" "E" "F"} {
+    gdb_test "ptype ${klass}::value_type" "type = int"
+    gdb_test "whatis ${klass}::value_type" "type = int"
+    gdb_test "p (${klass}::value_type) 0" " = 0"
+}
+foreach klass {"Z" "ZZ"} {
+    gdb_test "ptype ${klass}::value_type" "type = float"
+    gdb_test "whatis ${klass}::value_type" "type = float"
+    gdb_test "p (${klass}::value_type) 0" " = 0"
+}
+
 # Set it up at a breakpoint so we can play with the variable values.
 
 if ![runto 'marker1'] then {
@@ -56,11 +68,12 @@ gdb_test "print a_instance" "\\$\[0-9\]+ = \{a = 1, aa = 2\}" "print value of a_
 cp_test_ptype_class \
     "ptype a_instance" "" "class" "A" \
     {
-	{ field  public "int a;" }
-	{ field  public "int aa;" }
+	{ field  public "A::value_type a;" }
+	{ field  public "A::value_type aa;" }
 	{ method public "A();" }
-	{ method public "int afoo();" }
-	{ method public "int foo();" }
+	{ method public "A::value_type afoo();" }
+	{ method public "A::value_type foo();" }
+	{ typedef public "typedef int value_type;" }
     }
 
 # class D
@@ -77,11 +90,11 @@ cp_test_ptype_class \
 	{ base          "private A" }
 	{ base          "public B" }
 	{ base          "protected C" }
-	{ field  public "int d;" }
-	{ field  public "int dd;" }
+	{ field  public "A::value_type d;" }
+	{ field  public "A::value_type dd;" }
 	{ method public "D();" }
-	{ method public "int dfoo();" }
-	{ method public "int foo();" }
+	{ method public "A::value_type dfoo();" }
+	{ method public "A::value_type foo();" }
     } \
     "" \
     {
@@ -102,11 +115,11 @@ cp_test_ptype_class \
 	{ base          "public A" }
 	{ base          "private B" }
 	{ base          "protected C" }
-	{ field  public "int e;" }
-	{ field  public "int ee;" }
+	{ field  public "A::value_type e;" }
+	{ field  public "A::value_type ee;" }
 	{ method public "E();" }
-	{ method public "int efoo();" }
-	{ method public "int foo();" }
+	{ method public "A::value_type efoo();" }
+	{ method public "A::value_type foo();" }
     } \
     "" \
     {
@@ -127,10 +140,26 @@ cp_test_ptype_class \
 	{ base          "private A" }
 	{ base          "public B" }
 	{ base          "private C" }
-	{ field  public "int f;" }
-	{ field  public "int ff;" }
+	{ field  public "A::value_type f;" }
+	{ field  public "A::value_type ff;" }
 	{ method public "F();" }
-	{ method public "int ffoo();" }
+	{ method public "A::value_type ffoo();" }
+	{ method public "A::value_type foo();" }
+    }
+
+# class G
+cp_test_ptype_class \
+    "ptype g_instance" "" "class" "G" \
+    {
+	{ base          "private A" }
+	{ base          "public B" }
+	{ base          "protected C" }
+	{ field public "int g;" }
+	{ field public "int gg;" }
+	{ field public "int a;" }
+	{ field public "int b;" }
+	{ field public "int c;" }
+	{ method public "int gfoo();" }
 	{ method public "int foo();" }
     }
 
@@ -176,3 +205,32 @@ gdb_test_multiple "frame" "re-selected 'main' frame after inferior call" {
 
 gdb_test "print g_instance.bfoo()" "\\$\[0-9\]+ = 2" "print value of g_instance.bfoo()"
 gdb_test "print g_instance.cfoo()" "\\$\[0-9\]+ = 3" "print value of g_instance.cfoo()"
+
+# Check typedefs of fields
+foreach Klass {"C" "G"} {
+    set klass [string tolower $Klass]
+    set instance "${klass}_instance"
+    set var "${instance}.$klass"
+    gdb_test "whatis $var" "int"
+    gdb_test "ptype $var" "int"
+}
+
+foreach Klass {"A" "B" "D" "E" "F"} {
+    set klass [string tolower $Klass]
+    set instance "${klass}_instance"
+    set var "${instance}.$klass"
+    gdb_test "whatis $var" "A::value_type"
+    gdb_test "ptype $var" "int"
+    if {![string equal $Klass "B"]} {
+	gdb_test "p (${Klass}::value_type) 0" " = 0"
+    }
+}
+
+foreach Klass {"Z" "ZZ"} {
+    set klass [string tolower $Klass]
+    set instance "${klass}_instance"
+    set var "${instance}.$klass"
+    gdb_test "whatis $var" "Z::value_type"
+    gdb_test "ptype $var" "float"
+    gdb_test "p (${Klass}::value_type) 0" " = 0"
+}
\ No newline at end of file
diff --git a/gdb/testsuite/lib/cp-support.exp b/gdb/testsuite/lib/cp-support.exp
index 8829f97..467a25e 100644
--- a/gdb/testsuite/lib/cp-support.exp
+++ b/gdb/testsuite/lib/cp-support.exp
@@ -81,6 +81,11 @@ proc cp_check_errata { expected_string actual_string errata_table } {
 #      the class has a member function with the given access type
 #      and the given declaration.
 #
+#   { typedef "access" "declaration" }
+#
+#      the class has a typedef with the given access type and the
+#      given declaration.
+#
 # If you test the same class declaration more than once, you can specify
 # IN_CLASS_TABLE as "ibid".  "ibid" means: look for a previous class
 # table that had the same IN_KEY and IN_TAG, and re-use that table.
@@ -199,6 +204,7 @@ proc cp_test_ptype_class { in_command in_testname in_key in_tag in_class_table {
     set list_vbases  { }
     set list_fields  { }
     set list_methods { }
+    set list_typedefs { }
 
     foreach class_line $in_class_table {
 	switch [lindex $class_line 0] {
@@ -206,6 +212,7 @@ proc cp_test_ptype_class { in_command in_testname in_key in_tag in_class_table {
 	    "vbase"  { lappend list_vbases  [lindex $class_line 1] }
 	    "field"  { lappend list_fields  [lrange $class_line 1 2] }
 	    "method" { lappend list_methods [lrange $class_line 1 2] }
+	    "typedef" { lappend list_typedefs [lrange $class_line 1 2] }
 	    default  { fail "$in_testname // bad line in class table: $class_line"; return; }
 	}
     }
@@ -381,6 +388,22 @@ proc cp_test_ptype_class { in_command in_testname in_key in_tag in_class_table {
 	    }
 	}
 
+	# Typedef
+
+	if {[llength $list_typedefs] > 0} {
+	    set typedef_access [lindex [lindex $list_typedefs 0] 0]
+	    set typedef_decl [lindex [lindex $list_typedefs 0] 1]
+	    if {[string equal $actual_line $typedef_decl]} {
+		if {![string equal $access $typedef_access]} {
+		    cp_check_errata $typedef_access $access $in_errata_table
+		    fail "$in_testname // wrong access specifier for typedef: $access"
+		    return
+		}
+		set list_typedefs [lreplace $list_typedefs 0 0]
+		continue
+	    }
+	}
+
 	# Synthetic operators.  These are optional and can be mixed in
 	# with the methods in any order, but duplicates are wrong.
 	#
@@ -452,6 +475,11 @@ proc cp_test_ptype_class { in_command in_testname in_key in_tag in_class_table {
 	return
     }
 
+    if {[llength $list_typedefs] > 0} {
+	fail "$in_testname // missing typedefs"
+	return
+    }
+
     # Check the tail.
 
     set actual_tail [string trim $actual_tail]

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-11-16 20:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-12 23:24 [RFA] c++/13615 Keith Seitz
2012-09-26 20:52 ` Tom Tromey
2012-10-05 19:45   ` Keith Seitz
2012-11-08 21:57     ` Tom Tromey
2012-11-08 21:59       ` Tom Tromey
2012-11-14 21:48         ` Keith Seitz
2012-11-15 22:00           ` Tom Tromey
2012-11-16 18:47             ` Keith Seitz
2012-11-16 19:10               ` Tom Tromey
2012-11-16 20:56                 ` Keith Seitz
2012-11-08 22:27       ` Tom Tromey
2012-11-13 20:44       ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox