[PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Liu, Lei]

Hi,

This patch is trying to fix a bug in gdb.  The problem is found in following
C++ test case.

#include <cstdio>

class A {
public:
   enum E {X,Y,Z};
};

class B : A {
public:
   void test(E e);
};

void B::test(E e) {
   if (e == X) {        //b 14 if e==X
     printf("%d\n",e);
   }
}


int main() {
   B b;
   b.test(A::X);
   return 0;
}

Compiled by gcc-4.1.2 with -O0 -g.

I tried to plant a conditional breakpoint in line 14 as shown in comment but
got a error shows 'No symbol "X" in current context'.  The symbol 'X' is
accessible in that scope.

The bug is that gdb did not look up enum constant symbols derived from base
classes.  My patch adds some code in cp_lookup_symbol_namespace to call 
itself
recursively, so that to make gdb search symbols from all base classes.

Is this patch OK?

Thanks.
Lei


2010-11-02  Lei Liu <lei.liu2@windriver.com>

     * cp-namespace.c (cp_lookup_symbol_namespace): Recursively call
     itself to search C++ base classes.
---
  gdb/cp-namespace.c |   27 +++++++++++++++++++++++++++
  1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 16f58ca..822423e 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -513,6 +513,8 @@ cp_lookup_symbol_namespace (const char *scope,
                              const domain_enum domain)
  {
    struct symbol *sym;
+  struct symbol *scope_sym;
+  struct type *scope_type;

    /* First, try to find the symbol in the given namespace.  */
    sym = cp_lookup_symbol_in_namespace (scope, name, block, domain);
@@ -530,6 +532,31 @@ cp_lookup_symbol_namespace (const char *scope,
        block = BLOCK_SUPERBLOCK (block);
      }

+  /* If scope is a C++ class, we need to search all its base classes.  */
+  if (scope == NULL || scope[0] == '\0')
+    return NULL;
+
+  scope_sym = lookup_symbol (scope, NULL, VAR_DOMAIN, NULL);
+  if (scope_sym == NULL)
+    return NULL;
+
+  scope_type = SYMBOL_TYPE(scope_sym);
+  if (scope_type == NULL)
+    return NULL;
+
+  if (TYPE_CODE (scope_type) == TYPE_CODE_STRUCT)
+    {
+      int nbases = TYPE_N_BASECLASSES (scope_type);
+      int i;
+      for (i = 0; i < nbases; i++)
+        {
+          const char *base_name = TYPE_BASECLASS_NAME (scope_type, i);
+          sym = cp_lookup_symbol_namespace (base_name, name, block, 
domain);
+          if (sym != NULL)
+            return sym;
+        }
+    }
+
    return NULL;
  }

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Yao Qi]

On Tue, 2010-11-02 at 11:48 +0800, Liu, Lei wrote:

Here are my two cents on coding conventions.

> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
> index 16f58ca..822423e 100644
> --- a/gdb/cp-namespace.c
> +++ b/gdb/cp-namespace.c
> @@ -513,6 +513,8 @@ cp_lookup_symbol_namespace (const char *scope,
>                               const domain_enum domain)
>   {
>     struct symbol *sym;
> +  struct symbol *scope_sym;
> +  struct type *scope_type;
> 

Add one more space before "struct".

>     /* First, try to find the symbol in the given namespace.  */
>     sym = cp_lookup_symbol_in_namespace (scope, name, block, domain);
> @@ -530,6 +532,31 @@ cp_lookup_symbol_namespace (const char *scope,
>         block = BLOCK_SUPERBLOCK (block);
>       }
> 
> +  /* If scope is a C++ class, we need to search all its base classes.  */
> +  if (scope == NULL || scope[0] == '\0')
> +    return NULL;
> +
> +  scope_sym = lookup_symbol (scope, NULL, VAR_DOMAIN, NULL);
> +  if (scope_sym == NULL)
> +    return NULL;
> +
> +  scope_type = SYMBOL_TYPE(scope_sym);

Add a space between SYMBOL_TYPE and "(".

> +  if (scope_type == NULL)
> +    return NULL;
> +
> +  if (TYPE_CODE (scope_type) == TYPE_CODE_STRUCT)
> +    {
> +      int nbases = TYPE_N_BASECLASSES (scope_type);
> +      int i;
> +      for (i = 0; i < nbases; i++)
> +        {
> +          const char *base_name = TYPE_BASECLASS_NAME (scope_type, i);
> +          sym = cp_lookup_symbol_namespace (base_name, name, block, 
> domain);
> +          if (sym != NULL)
> +            return sym;
> +        }
> +    }
> +
>     return NULL;
>   }

-- 
Yao Qi <yao@codesourcery.com>
CodeSourcery

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Tom Tromey]

>>>>> ">" == Liu, Lei <lei.liu2@windriver.com> writes:

>> This patch is trying to fix a bug in gdb.  The problem is found in
>> following C++ test case.

Thanks.

This looks pretty reasonable overall.  In order for it to go in, we
would need a couple things:

* Copyright assignment forms filed with the FSF.  If you don't have this
  already, one of us can get you started.

* A test case -- the one you have is fine, it just needs to be in the
  form used by our test suite.

>> 2010-11-02  Lei Liu <lei.liu2@windriver.com>

>>     * cp-namespace.c (cp_lookup_symbol_namespace): Recursively call
>>     itself to search C++ base classes.

It isn't 100% clear to me that cp_lookup_symbol_namespace is the right
function.  That file is kind of spaghetti-ish right now :(

>> +  scope_sym = lookup_symbol (scope, NULL, VAR_DOMAIN, NULL);
>> +  if (scope_sym == NULL)
>> +    return NULL;
>> +
>> +  scope_type = SYMBOL_TYPE(scope_sym);
>> +  if (scope_type == NULL)
>> +    return NULL;

I think lookup_typename is better here.

>> +  if (TYPE_CODE (scope_type) == TYPE_CODE_STRUCT)

I think you should call check_typedef on scope_type before this check.

thanks,
Tom

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Pedro Alves]

Hi!

I'm looking at this exact problem as well.  :-)

> 2010-11-02  Lei Liu <lei.liu2@windriver.com>
> 
>      * cp-namespace.c (cp_lookup_symbol_namespace): Recursively call
>      itself to search C++ base classes.

> @@ -513,6 +513,8 @@ cp_lookup_symbol_namespace (const char *scope,
>                               const domain_enum domain)
>   {
...
> @@ -530,6 +532,31 @@ cp_lookup_symbol_namespace (const char *scope,
>         block = BLOCK_SUPERBLOCK (block);
>       }
> 
> +  /* If scope is a C++ class, we need to search all its base classes.  */
> +  if (scope == NULL || scope[0] == '\0')
> +    return NULL;

Your testcase is not hitting this empty scope check, due to your
"#include <cstdio>", which does namespace things, and
ends up emitting DW_TAG_namespace in the debug info, and
so dwarf2read.c sets the processing_has_namespace_info
global, and so a "namespace" for you test's class
is installed by:

  /* For C++, set the block's scope.  */
  if (cu->language == language_cplus || cu->language == language_fortran)
    cp_set_block_scope (new->name, block, &objfile->objfile_obstack,
			determine_prefix (die, cu),
			processing_has_namespace_info);

If I remove that include from your test (and the printf), then
the patch no longer works.  We need to do something here.

A related issue that should be addressed as well, I think, is:

 class A {
 public:
   enum E { X, Y, Z };
 };

 A a;

Both "print a.X" and "ptype a.X" should work.  I've got a
patch that fixes this part, and a dejagnuified testcase,
but haven't sorted out the processing_has_namespace_info
part yet.

-- 
Pedro Alves

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Liu, Lei]

On 2010年11月02日 16:05, Yao Qi wrote:
> On Tue, 2010-11-02 at 11:48 +0800, Liu, Lei wrote:
>
> Here are my two cents on coding conventions.
>
>    
>> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
>> index 16f58ca..822423e 100644
>> --- a/gdb/cp-namespace.c
>> +++ b/gdb/cp-namespace.c
>> @@ -513,6 +513,8 @@ cp_lookup_symbol_namespace (const char *scope,
>>                                const domain_enum domain)
>>    {
>>      struct symbol *sym;
>> +  struct symbol *scope_sym;
>> +  struct type *scope_type;
>>
>>      
> Add one more space before "struct".
>
>    
>>      /* First, try to find the symbol in the given namespace.  */
>>      sym = cp_lookup_symbol_in_namespace (scope, name, block, domain);
>> @@ -530,6 +532,31 @@ cp_lookup_symbol_namespace (const char *scope,
>>          block = BLOCK_SUPERBLOCK (block);
>>        }
>>
>> +  /* If scope is a C++ class, we need to search all its base classes.  */
>> +  if (scope == NULL || scope[0] == '\0')
>> +    return NULL;
>> +
>> +  scope_sym = lookup_symbol (scope, NULL, VAR_DOMAIN, NULL);
>> +  if (scope_sym == NULL)
>> +    return NULL;
>> +
>> +  scope_type = SYMBOL_TYPE(scope_sym);
>>      
> Add a space between SYMBOL_TYPE and "(".
>
>    
>> +  if (scope_type == NULL)
>> +    return NULL;
>> +
>> +  if (TYPE_CODE (scope_type) == TYPE_CODE_STRUCT)
>> +    {
>> +      int nbases = TYPE_N_BASECLASSES (scope_type);
>> +      int i;
>> +      for (i = 0; i<  nbases; i++)
>> +        {
>> +          const char *base_name = TYPE_BASECLASS_NAME (scope_type, i);
>> +          sym = cp_lookup_symbol_namespace (base_name, name, block,
>> domain);
>> +          if (sym != NULL)
>> +            return sym;
>> +        }
>> +    }
>> +
>>      return NULL;
>>    }
>>      
>    

Thanks for the corrections.

Lei

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Liu, Lei]

On 2010年11月03日 03:48, Tom Tromey wrote:
>>>>>> ">" == Liu, Lei<lei.liu2@windriver.com>  writes:
>
>>> This patch is trying to fix a bug in gdb.  The problem is found in
>>> following C++ test case.
>
> Thanks.
>
> This looks pretty reasonable overall.  In order for it to go in, we
> would need a couple things:
>
> * Copyright assignment forms filed with the FSF.  If you don't have this
>    already, one of us can get you started.

I don't have this yet. We can get start to do it.

>
> * A test case -- the one you have is fine, it just needs to be in the
>    form used by our test suite.

Right. I'll reform it.

>
>>> 2010-11-02  Lei Liu<lei.liu2@windriver.com>
>
>>>      * cp-namespace.c (cp_lookup_symbol_namespace): Recursively call
>>>      itself to search C++ base classes.
>
> It isn't 100% clear to me that cp_lookup_symbol_namespace is the right
> function.  That file is kind of spaghetti-ish right now :(

It seems to me that all the C++ specific symbol looking up code are located
in this file. I don't know where else I can put my code in.

>
>>> +  scope_sym = lookup_symbol (scope, NULL, VAR_DOMAIN, NULL);
>>> +  if (scope_sym == NULL)
>>> +    return NULL;
>>> +
>>> +  scope_type = SYMBOL_TYPE(scope_sym);
>>> +  if (scope_type == NULL)
>>> +    return NULL;
>
> I think lookup_typename is better here.

Well, I tried that first, but we must have a GDBARCH passed to it. I'm not
sure where can I get this.

>
>>> +  if (TYPE_CODE (scope_type) == TYPE_CODE_STRUCT)
>
> I think you should call check_typedef on scope_type before this check.

OK. I missed that.

>
> thanks,
> Tom
>

Thanks.
Lei

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Liu, Lei]

On 2010年11月03日 08:09, Pedro Alves wrote:
> Hi!
>
> I'm looking at this exact problem as well.  :-)
>
>> 2010-11-02  Lei Liu<lei.liu2@windriver.com>
>>
>>       * cp-namespace.c (cp_lookup_symbol_namespace): Recursively call
>>       itself to search C++ base classes.
>
>> @@ -513,6 +513,8 @@ cp_lookup_symbol_namespace (const char *scope,
>>                                const domain_enum domain)
>>    {
> ...
>> @@ -530,6 +532,31 @@ cp_lookup_symbol_namespace (const char *scope,
>>          block = BLOCK_SUPERBLOCK (block);
>>        }
>>
>> +  /* If scope is a C++ class, we need to search all its base classes.  */
>> +  if (scope == NULL || scope[0] == '\0')
>> +    return NULL;
>
> Your testcase is not hitting this empty scope check, due to your
> "#include<cstdio>", which does namespace things, and
> ends up emitting DW_TAG_namespace in the debug info, and
> so dwarf2read.c sets the processing_has_namespace_info
> global, and so a "namespace" for you test's class
> is installed by:
>
>    /* For C++, set the block's scope.  */
>    if (cu->language == language_cplus || cu->language == language_fortran)
>      cp_set_block_scope (new->name, block,&objfile->objfile_obstack,
> 			determine_prefix (die, cu),
> 			processing_has_namespace_info);
>
> If I remove that include from your test (and the printf), then
> the patch no longer works.  We need to do something here.

Right. There is a problem here. It seems that we can't rely on the
scope name as C++ class name since it may not be set in some cases.
Is there any other way to get the class name from a block? If not,
I think we need to set the scope regardless of value of
processing_has_namespace_info.

>
> A related issue that should be addressed as well, I think, is:
>
>   class A {
>   public:
>     enum E { X, Y, Z };
>   };
>
>   A a;
>
> Both "print a.X" and "ptype a.X" should work.  I've got a
> patch that fixes this part, and a dejagnuified testcase,
> but haven't sorted out the processing_has_namespace_info
> part yet.
>
Thanks.
Lei

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Pedro Alves]

FYI, I'm still looking at this.   I found out that in the case
there's global symbol with the same name of the class symbol that
we actually want, gdb finds _that_:

enum E { X, Y, Z } ee;

class A
{
public:
  enum E { X, Y, Z };
};

class B : public A
{
public:
  void test (E e);
};

void B::test(E e)
{
  if (e == X)  /* set breakpoint here */
    test (Z);
}

Meaning, when stopped at the breapoint line, we'd get:

(gdb) p X
$1 = X

Instead of the correct:
(gdb) p X
$1 = A::X

Fixing...

-- 
Pedro Alves

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Liu, Lei]

On 2010年11月03日 08:09, Pedro Alves wrote:
> Your testcase is not hitting this empty scope check, due to your
> "#include<cstdio>", which does namespace things, and
> ends up emitting DW_TAG_namespace in the debug info, and
> so dwarf2read.c sets the processing_has_namespace_info
> global, and so a "namespace" for you test's class
> is installed by:
>
>    /* For C++, set the block's scope.  */
>    if (cu->language == language_cplus || cu->language == language_fortran)
>      cp_set_block_scope (new->name, block,&objfile->objfile_obstack,
> 			determine_prefix (die, cu),
> 			processing_has_namespace_info);
>
> If I remove that include from your test (and the printf), then
> the patch no longer works.  We need to do something here.

I updated my patch to deal with this as long as the suggestions from
Tom and Yao.

I think in cp_set_block_scope, we should set scope name on a block
regardless of whether processing_has_namespace_info has been set.
For the cases that we only have classes but no namespaces, we still
need scope information to tell us which symbols are valid in a block.

How about this?

Thanks.
Lei


2010-11-04 Lei Liu <lei.liu2@windriver.com>

gdb/
* cp-namespace.c (cp_lookup_symbol_namespace): Recursively call
itself.
(cp_set_block_scope): Call block_set_scope if a valid scope name is
given.
gdb/testsuite/
* gdb.cp/enum.exp: New test case.
* gdb.cp/enum1.cc: Ditto.
* gdb.cp/enum2.cc: Ditto.
---
gdb/cp-namespace.c | 38 +++++++++++++++++++++
gdb/testsuite/gdb.cp/enum.exp | 74 +++++++++++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.cp/enum1.cc | 34 +++++++++++++++++++
gdb/testsuite/gdb.cp/enum2.cc | 37 ++++++++++++++++++++
4 files changed, 183 insertions(+), 0 deletions(-)
create mode 100644 gdb/testsuite/gdb.cp/enum.exp
create mode 100644 gdb/testsuite/gdb.cp/enum1.cc
create mode 100644 gdb/testsuite/gdb.cp/enum2.cc

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 16f58ca..bbce580 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -214,6 +214,16 @@ cp_set_block_scope (const struct symbol *symbol,
obsavestring (name, prefix_len, obstack),
obstack);
}
+ else if (processing_current_prefix != NULL
+ && processing_current_prefix[0] != '\0')
+ {
+ /* Just set the scope name if we get a valid one. */
+ block_set_scope
+ (block, obsavestring (processing_current_prefix,
+ strlen (processing_current_prefix),
+ obstack),
+ obstack);
+ }
}

/* Test whether or not NAMESPACE looks like it mentions an anonymous
@@ -513,6 +523,8 @@ cp_lookup_symbol_namespace (const char *scope,
const domain_enum domain)
{
struct symbol *sym;
+ struct symbol *scope_sym;
+ struct type *scope_type;

/* First, try to find the symbol in the given namespace. */
sym = cp_lookup_symbol_in_namespace (scope, name, block, domain);
@@ -530,6 +542,32 @@ cp_lookup_symbol_namespace (const char *scope,
block = BLOCK_SUPERBLOCK (block);
}

+ /* If scope is a C++ class, we need to search all its base classes. */
+ if (scope == NULL || scope[0] == '\0')
+ return NULL;
+
+ scope_sym = lookup_symbol (scope, NULL, VAR_DOMAIN, NULL);
+ if (scope_sym == NULL)
+ return NULL;
+
+ scope_type = SYMBOL_TYPE (scope_sym);
+ if (scope_type == NULL)
+ return NULL;
+
+ scope_type = check_typedef (scope_type);
+ if (TYPE_CODE (scope_type) == TYPE_CODE_STRUCT)
+ {
+ int nbases = TYPE_N_BASECLASSES (scope_type);
+ int i;
+ for (i = 0; i < nbases; i++)
+ {
+ const char *base_name = TYPE_BASECLASS_NAME (scope_type, i);
+ sym = cp_lookup_symbol_namespace (base_name, name, block, domain);
+ if (sym != NULL)
+ return sym;
+ }
+ }
+
return NULL;
}

diff --git a/gdb/testsuite/gdb.cp/enum.exp b/gdb/testsuite/gdb.cp/enum.exp
new file mode 100644
index 0000000..3d5e0f6
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/enum.exp
@@ -0,0 +1,74 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test searching enum constant symbols derived from base classes.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set testfile1 enum1
+set testfile2 enum2
+set srcfile1 ${testfile1}.cc
+set srcfile2 ${testfile2}.cc
+set binfile1 ${objdir}/${subdir}/${testfile1}
+set binfile2 ${objdir}/${subdir}/${testfile2}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile1}" 
executable {debug c++}] != "" } {
+ untested "Couldn't compile test program"
+ return -1
+}
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" 
executable {debug c++}] != "" } {
+ untested "Couldn't compile test program"
+ return -1
+}
+
+if [get_compiler_info ${binfile1}] {
+ return -1
+}
+if [get_compiler_info ${binfile2}] {
+ return -1
+}
+
+# Get things started.
+
+############################################
+# Test printing an enum constant symbol derived from base
+# class without namespaces
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile1}
+
+gdb_breakpoint [gdb_get_line_number "breakpoint" $srcfile1]
+gdb_run_cmd
+gdb_test "print X" "= A::X" "Print enum constant X of class A"
+
+
+############################################
+# Test printing an enum constant symbol derived from base
+# class in a namespace
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile2}
+
+gdb_breakpoint [gdb_get_line_number "breakpoint" $srcfile2]
+gdb_run_cmd
+gdb_test "print X" "= N::A::X" \
+ "Print enum constant X of class A in namespace N"
+
diff --git a/gdb/testsuite/gdb.cp/enum1.cc b/gdb/testsuite/gdb.cp/enum1.cc
new file mode 100644
index 0000000..6211cea
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/enum1.cc
@@ -0,0 +1,34 @@
+class A
+{
+public:
+ enum E {X,Y,Z};
+};
+
+class B1 : public A
+{
+};
+
+class B2 : public A
+{
+};
+
+class C : public B1, public B2
+{
+public:
+ void test(E e);
+};
+
+void C::test(E e)
+{
+ if (e == X) //breakpoint
+ {
+ }
+}
+
+int main()
+{
+ C c;
+ c.test(A::X);
+ return 0;
+}
+
diff --git a/gdb/testsuite/gdb.cp/enum2.cc b/gdb/testsuite/gdb.cp/enum2.cc
new file mode 100644
index 0000000..c864c1c
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/enum2.cc
@@ -0,0 +1,37 @@
+namespace N
+{
+ class A
+ {
+ public:
+ enum E {X,Y,Z};
+ };
+
+ class B1 : public A
+ {
+ };
+
+ class B2 : public A
+ {
+ };
+
+ class C : public B1, public B2
+ {
+ public:
+ void test(E e);
+ };
+
+ void C::test(E e)
+ {
+ if (e == X) //breakpoint
+ {
+ }
+ }
+};
+
+int main()
+{
+ N::C c;
+ c.test(N::A::X);
+ return 0;
+}
+
-- 
1.6.0.4

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Yao Qi]

On 11/04/2010 04:52 PM, Liu, Lei wrote:
>
> I updated my patch to deal with this as long as the suggestions from
> Tom and Yao.
>
> I think in cp_set_block_scope, we should set scope name on a block
> regardless of whether processing_has_namespace_info has been set.
> For the cases that we only have classes but no namespaces, we still
> need scope information to tell us which symbols are valid in a block.
>
> How about this?

Lei,
It looks like your mail client eats spaces and tabs when you paste your 
patch in your mail client.  Please re-submit your patch again with 
following steps,

1.  Run 'git diff' to generate patch and dump it to a file, named 
"foo.patch" for example,
2.  Add your ChangeLog entry either in patch header or replied mail.
3.  Reply mail in this thread, and *attach* your patch in your mail, and 
send it out.

> + /* Just set the scope name if we get a valid one. */

Should be two spaces between "." and "*/".  Please have a look at 
"16.1.3 Comments" in
http://sourceware.org/gdb/current/onlinedocs/gdbint/Coding-Standards.html#Coding-Standards

-- 
Yao Qi
CodeSourcery
yao@codesourcery.com
(650) 331-3385 x739

Re: [PATCH] call cp_lookup_symbol_namespace recursively to search symbols in C++ base classes

[Liu, Lei]

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

On 2010年11月07日 18:03, Yao Qi wrote:
> On 11/04/2010 04:52 PM, Liu, Lei wrote:
>>
>> I updated my patch to deal with this as long as the suggestions from
>> Tom and Yao.
>>
>> I think in cp_set_block_scope, we should set scope name on a block
>> regardless of whether processing_has_namespace_info has been set.
>> For the cases that we only have classes but no namespaces, we still
>> need scope information to tell us which symbols are valid in a block.
>>
>> How about this?
>
> Lei,
> It looks like your mail client eats spaces and tabs when you paste 
> your patch in your mail client.  Please re-submit your patch again 
> with following steps,
>
> 1.  Run 'git diff' to generate patch and dump it to a file, named 
> "foo.patch" for example,
> 2.  Add your ChangeLog entry either in patch header or replied mail.
> 3.  Reply mail in this thread, and *attach* your patch in your mail, 
> and send it out.
>
>> + /* Just set the scope name if we get a valid one. */
>
> Should be two spaces between "." and "*/".  Please have a look at 
> "16.1.3 Comments" in
> http://sourceware.org/gdb/current/onlinedocs/gdbint/Coding-Standards.html#Coding-Standards 
>
>

Thanks Yao.  I resubmit it.

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

2010-11-02  Lei Liu  <lei.liu2@windriver.com>

gdb/
	* cp-namespace.c (cp_lookup_symbol_namespace): Recursively call
	itself.
	(cp_set_block_scope): Call block_set_scope if a valid scope name is
	given.
gdb/testsuite/
	* gdb.cp/enum.exp: New test case.
	* gdb.cp/enum1.cc: Ditto.
	* gdb.cp/enum2.cc: Ditto.
---
---
 gdb/cp-namespace.c            |   38 +++++++++++++++++++++
 gdb/testsuite/gdb.cp/enum.exp |   74 +++++++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/enum1.cc |   34 +++++++++++++++++++
 gdb/testsuite/gdb.cp/enum2.cc |   37 ++++++++++++++++++++
 4 files changed, 183 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/enum.exp
 create mode 100644 gdb/testsuite/gdb.cp/enum1.cc
 create mode 100644 gdb/testsuite/gdb.cp/enum2.cc

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 16f58ca..bbce580 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -214,6 +214,16 @@ cp_set_block_scope (const struct symbol *symbol,
 		       obsavestring (name, prefix_len, obstack),
 		       obstack);
     }
+  else if (processing_current_prefix != NULL
+           && processing_current_prefix[0] != '\0')
+    {
+      /* Just set the scope name if we get a valid one.  */
+      block_set_scope
+	(block, obsavestring (processing_current_prefix,
+			      strlen (processing_current_prefix),
+			      obstack),
+	 obstack);
+    }
 }
 
 /* Test whether or not NAMESPACE looks like it mentions an anonymous
@@ -513,6 +523,8 @@ cp_lookup_symbol_namespace (const char *scope,
                             const domain_enum domain)
 {
   struct symbol *sym;
+  struct symbol *scope_sym;
+  struct type *scope_type;
   
   /* First, try to find the symbol in the given namespace.  */
   sym = cp_lookup_symbol_in_namespace (scope, name, block, domain);
@@ -530,6 +542,32 @@ cp_lookup_symbol_namespace (const char *scope,
       block = BLOCK_SUPERBLOCK (block);
     }
 
+  /* If scope is a C++ class, we need to search all its base classes.  */
+  if (scope == NULL || scope[0] == '\0')
+    return NULL;
+
+  scope_sym = lookup_symbol (scope, NULL, VAR_DOMAIN, NULL);
+  if (scope_sym == NULL)
+    return NULL;
+
+  scope_type = SYMBOL_TYPE (scope_sym);
+  if (scope_type == NULL)
+    return NULL;
+
+  scope_type = check_typedef (scope_type);
+  if (TYPE_CODE (scope_type) == TYPE_CODE_STRUCT)
+    {
+      int nbases = TYPE_N_BASECLASSES (scope_type);
+      int i;
+      for (i = 0; i < nbases; i++)
+        {
+          const char *base_name = TYPE_BASECLASS_NAME (scope_type, i);
+          sym = cp_lookup_symbol_namespace (base_name, name, block, domain);
+          if (sym != NULL)
+            return sym;
+        }
+    }
+
   return NULL;
 }
 
diff --git a/gdb/testsuite/gdb.cp/enum.exp b/gdb/testsuite/gdb.cp/enum.exp
new file mode 100644
index 0000000..3d5e0f6
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/enum.exp
@@ -0,0 +1,74 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test searching enum constant symbols derived from base classes.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile1 enum1
+set testfile2 enum2
+set srcfile1 ${testfile1}.cc
+set srcfile2 ${testfile2}.cc
+set binfile1 ${objdir}/${subdir}/${testfile1}
+set binfile2 ${objdir}/${subdir}/${testfile2}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile1}" executable {debug c++}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug c++}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+
+if [get_compiler_info ${binfile1}] {
+    return -1
+}
+if [get_compiler_info ${binfile2}] {
+    return -1
+}
+
+# Get things started.
+
+############################################
+# Test printing an enum constant symbol derived from base
+# class without namespaces
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile1}
+
+gdb_breakpoint [gdb_get_line_number "breakpoint" $srcfile1]
+gdb_run_cmd
+gdb_test "print X" "= A::X" "Print enum constant X of class A"
+
+
+############################################
+# Test printing an enum constant symbol derived from base 
+# class in a namespace
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile2}
+
+gdb_breakpoint [gdb_get_line_number "breakpoint" $srcfile2]
+gdb_run_cmd
+gdb_test "print X" "= N::A::X" \
+         "Print enum constant X of class A in namespace N"
+
diff --git a/gdb/testsuite/gdb.cp/enum1.cc b/gdb/testsuite/gdb.cp/enum1.cc
new file mode 100644
index 0000000..6211cea
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/enum1.cc
@@ -0,0 +1,34 @@
+class A 
+{
+public:
+  enum E {X,Y,Z};
+};
+
+class B1 : public A 
+{
+};
+
+class B2 : public A 
+{
+};
+
+class C : public B1, public B2
+{
+public:
+  void test(E e);
+};
+
+void C::test(E e)
+{
+  if (e == X)  //breakpoint
+    {
+    }
+}
+
+int main()
+{
+  C c;
+  c.test(A::X);
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.cp/enum2.cc b/gdb/testsuite/gdb.cp/enum2.cc
new file mode 100644
index 0000000..c864c1c
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/enum2.cc
@@ -0,0 +1,37 @@
+namespace N
+{
+  class A
+  {
+  public:
+    enum E {X,Y,Z};
+  };
+
+  class B1 : public A
+  {
+  };
+
+  class B2 : public A
+  {
+  };
+
+  class C : public B1, public B2
+  {
+  public:
+    void test(E e);
+  };
+
+  void C::test(E e)
+  {
+    if (e == X)  //breakpoint
+      {
+      }
+  }
+};
+
+int main()
+{
+  N::C c;
+  c.test(N::A::X);
+  return 0;
+}
+
-- 
1.6.0.4