GDB crash due to endless recursion in namespace lookup

GDB crash due to endless recursion in namespace lookup

[Ulrich Weigand]

Hello,

since one of this year's C++ namespace search related changes,
GDB will go into an endless recursion and crash due to stack
overflow when looking up symbols in the presence of a cycle
in the "using" directive graph.

For example, with the following test:

namespace A
{
  namespace B
    {
      using namespace ::A;
    }
  using namespace B;
}

using namespace A;

class test { };
test x;

int main() { }

I'm seeing:

(gdb) start
Temporary breakpoint 1 at 0x80000626: file xxx.cc, line 16.
Starting program: /home7/uweigand/fsf/gdb-head-build/gdb/a.out

Temporary breakpoint 1, main () at xxx.cc:16
16      int main() { }
(gdb) print x[0]
Segmentation fault (core dumped)

(The lookup that happens here is for operator[] -- which doesn't
exist, but in trying to find it we run into the endless loop.)

This is particularly annyoing as the SLES10 system library on
s390 and ppc contains exactly this construct in the header
/usr/include/c++/4.1.2/bits/localefwd.h:

namespace std
{
  namespace __gnu_cxx_ldbl128
    {
      using namespace ::std;
    }
  using namespace __gnu_cxx_ldbl128 __attribute__((__strong__));

which means it is present in just about every C++ executable.

(This seems specific to the SLES backport of the long-double
compatibility support.  The upstream implementation does not
have this cycle ...  But still, the general construct looks
to be valid C++ as far as I can tell.)

Any thoughts how to fix this?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: GDB crash due to endless recursion in namespace lookup

[sami wagiaalla]

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

Hi Ulrich,

Thanks for reporting things and the test case. I have attached a fix and 
added your test case to the test suite.

Thoughts ?
   Sami

On 06/24/2010 02:09 PM, Ulrich Weigand wrote:
> Hello,
>
> since one of this year's C++ namespace search related changes,
> GDB will go into an endless recursion and crash due to stack
> overflow when looking up symbols in the presence of a cycle
> in the "using" directive graph.
>
> For example, with the following test:
>
> namespace A
> {
>    namespace B
>      {
>        using namespace ::A;
>      }
>    using namespace B;
> }
>
> using namespace A;
>
> class test { };
> test x;
>
> int main() { }
>
> I'm seeing:
>
> (gdb) start
> Temporary breakpoint 1 at 0x80000626: file xxx.cc, line 16.
> Starting program: /home7/uweigand/fsf/gdb-head-build/gdb/a.out
>
> Temporary breakpoint 1, main () at xxx.cc:16
> 16      int main() { }
> (gdb) print x[0]
> Segmentation fault (core dumped)
>
> (The lookup that happens here is for operator[] -- which doesn't
> exist, but in trying to find it we run into the endless loop.)
>
> This is particularly annyoing as the SLES10 system library on
> s390 and ppc contains exactly this construct in the header
> /usr/include/c++/4.1.2/bits/localefwd.h:
>
> namespace std
> {
>    namespace __gnu_cxx_ldbl128
>      {
>        using namespace ::std;
>      }
>    using namespace __gnu_cxx_ldbl128 __attribute__((__strong__));
>
> which means it is present in just about every C++ executable.
>
> (This seems specific to the SLES backport of the long-double
> compatibility support.  The upstream implementation does not
> have this cycle ...  But still, the general construct looks
> to be valid C++ as far as I can tell.)
>
> Any thoughts how to fix this?
>
> Bye,
> Ulrich
>


[-- Attachment #2: namespace-loop.patch --]
[-- Type: text/plain, Size: 4490 bytes --]

2010-06-24  Sami Wagiaalla  <swagiaal@redhat.com>

	* cp-namespace.c (reset_directive_searched): moved from here...
	* cp-support.c (reset_directive_searched): ... to here.
	(make_symbol_overload_list_using): Added check for search flag.

2010-06-24  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdb.cp/operator.cc: Created an import loop.
	* gdb.cp/operator.exp: Added testcase for import loop.

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 0daf732..525a90d 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -273,16 +273,6 @@ cp_lookup_symbol_in_namespace (const char *namespace,
     }
 }
 
-/* Used for cleanups to reset the "searched" flag incase
-   of an error.  */
-
-static void
-reset_directive_searched (void *data)
-{
-  struct using_direct *direct = data;
-  direct->searched = 0;
-}
-
 /* Search for NAME by applying all import statements belonging
    to BLOCK which are applicable in SCOPE.  If DECLARATION_ONLY the search
    is restricted to using declarations.
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 41af7ae..6e403ed 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -838,6 +838,16 @@ make_symbol_overload_list_adl (struct type **arg_types, int nargs,
   return sym_return_val;
 }
 
+/* Used for cleanups to reset the "searched" flag incase
+   of an error.  */
+
+void
+reset_directive_searched (void *data)
+{
+  struct using_direct *direct = data;
+  direct->searched = 0;
+}
+
 /* This applies the using directives to add namespaces to search in,
    and then searches for overloads in all of those namespaces.  It
    adds the symbols found to sym_return_val.  Arguments are as in
@@ -847,8 +857,9 @@ static void
 make_symbol_overload_list_using (const char *func_name,
 				 const char *namespace)
 {
-  const struct using_direct *current;
+  struct using_direct *current;
   const struct block *block;
+  struct cleanup *searched_cleanup;
 
   /* First, go through the using directives.  If any of them apply,
      look in the appropriate namespaces for new functions to match
@@ -861,12 +872,23 @@ make_symbol_overload_list_using (const char *func_name,
 	current != NULL;
 	current = current->next)
       {
-        /* If this is a namespace alias or imported declaration ignore it.  */
-        if (current->alias != NULL || current->declaration != NULL)
+        /* If this import statement has been explored before, or if this is a
+           namespace alias or imported declaration ignore it.  */
+        if (current->searched
+            || current->alias != NULL
+            || current->declaration != NULL)
           continue;
 
         if (strcmp (namespace, current->import_dest) == 0)
-          make_symbol_overload_list_using (func_name, current->import_src);
+          {
+            current->searched = 1;
+            searched_cleanup = make_cleanup (reset_directive_searched, current);
+
+            make_symbol_overload_list_using (func_name, current->import_src);
+
+            discard_cleanups (searched_cleanup);
+            current->searched = 0;
+          }
       }
 
   /* Now, add names for this namespace.  */
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index ddc4c93..5e4e1e9 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -179,4 +179,9 @@ extern char *cp_comp_to_string (struct demangle_component *result,
 
 extern struct cmd_list_element *maint_cplus_cmd_list;
 
+/* Used for cleanups to reset the "searched" flag incase
+   of an error.  */
+
+extern void reset_directive_searched (void *data);
+
 #endif /* CP_SUPPORT_H */
diff --git a/gdb/testsuite/gdb.cp/operator.cc b/gdb/testsuite/gdb.cp/operator.cc
index cc925a0..8431376 100644
--- a/gdb/testsuite/gdb.cp/operator.cc
+++ b/gdb/testsuite/gdb.cp/operator.cc
@@ -157,6 +157,22 @@ using namespace N;
 
 //------------------
 
+namespace O
+{
+  namespace P
+    {
+      using namespace ::O;
+    }
+  using namespace P;
+}
+
+using namespace O;
+
+class test { };
+test x;
+
+//------------------
+
 int main ()
 {
   A a;
diff --git a/gdb/testsuite/gdb.cp/operator.exp b/gdb/testsuite/gdb.cp/operator.exp
index ac89d2b..0e36e4c 100644
--- a/gdb/testsuite/gdb.cp/operator.exp
+++ b/gdb/testsuite/gdb.cp/operator.exp
@@ -56,3 +56,6 @@ gdb_test "p j == 1" "Cannot resolve function operator== to any overloaded instan
 
 # Test that indirectly imported operators work
 gdb_test "p l == 1" "= 88"
+
+# Test that we don't fall into an import loop
+gdb_test {p x[0]} {No symbol "operator\[\]" in current context.}

[commit] Fix GDB crash due to endless recursion in namespace lookup

[Ulrich Weigand]

I wrote:

> since one of this year's C++ namespace search related changes,
> GDB will go into an endless recursion and crash due to stack
> overflow when looking up symbols in the presence of a cycle
> in the "using" directive graph.

Found it.  There is a mechanism to prevent infinite recursion, the
"searched" flag in struct using_directive, which is used in 
cp-namespace.c:cp_lookup_symbol_imports.  However, for some reason,
a similar recursive loop in cp-support.c:make_symbol_overload_list_using
did not use this flag, causing the problem.

Fixed in the obvious way by the patch below.
Tested on s390x-ibm-linux, committed to mainline.

Bye,
Ulrich


ChangeLog:

	* cp-support.c (reset_directive_searched): New function.
	(make_symbol_overload_list_using): Prevent recursive calls.


Index: gdb/cp-support.c
===================================================================
RCS file: /cvs/src/src/gdb/cp-support.c,v
retrieving revision 1.43
diff -u -p -r1.43 cp-support.c
--- gdb/cp-support.c	7 Jun 2010 17:51:03 -0000	1.43
+++ gdb/cp-support.c	25 Jun 2010 15:11:43 -0000
@@ -838,6 +838,15 @@ make_symbol_overload_list_adl (struct ty
   return sym_return_val;
 }
 
+/* Used for cleanups to reset the "searched" flag in case of an error.  */
+
+static void
+reset_directive_searched (void *data)
+{
+  struct using_direct *direct = data;
+  direct->searched = 0;
+}
+
 /* This applies the using directives to add namespaces to search in,
    and then searches for overloads in all of those namespaces.  It
    adds the symbols found to sym_return_val.  Arguments are as in
@@ -847,7 +856,7 @@ static void
 make_symbol_overload_list_using (const char *func_name,
 				 const char *namespace)
 {
-  const struct using_direct *current;
+  struct using_direct *current;
   const struct block *block;
 
   /* First, go through the using directives.  If any of them apply,
@@ -861,12 +870,27 @@ make_symbol_overload_list_using (const c
 	current != NULL;
 	current = current->next)
       {
+	/* Prevent recursive calls.  */
+	if (current->searched)
+	  continue;
+
         /* If this is a namespace alias or imported declaration ignore it.  */
         if (current->alias != NULL || current->declaration != NULL)
           continue;
 
         if (strcmp (namespace, current->import_dest) == 0)
-          make_symbol_overload_list_using (func_name, current->import_src);
+	  {
+	    /* Mark this import as searched so that the recursive call does
+	       not search it again.  */
+	    struct cleanup *old_chain;
+	    current->searched = 1;
+	    old_chain = make_cleanup (reset_directive_searched, current);
+
+	    make_symbol_overload_list_using (func_name, current->import_src);
+
+	    current->searched = 0;
+	    discard_cleanups (old_chain);
+	  }
       }
 
   /* Now, add names for this namespace.  */

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: GDB crash due to endless recursion in namespace lookup

[Ulrich Weigand]

Sami Wagiaalla wrote:

> Thanks for reporting things and the test case. I have attached a fix and 
> added your test case to the test suite.

Oops, I didn't see your email and just checked in a fix myself (which
seems more or less the same as yours) ...  Sorry for the confusion!

> 2010-06-24  Sami Wagiaalla  <swagiaal@redhat.com>
> 
> 	* gdb.cp/operator.cc: Created an import loop.
> 	* gdb.cp/operator.exp: Added testcase for import loop.

Except I didn't add the test case since on my machine, I already had
several other tests failing due to the bug.  But I guess you're right,
we should add the test anyway, for machines without the loop in the
system headers ...  Please feel free to check in the test!

Thanks for the quick fix!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: GDB crash due to endless recursion in namespace lookup

[Sami Wagiaalla]

> Oops, I didn't see your email and just checked in a fix myself (which
> seems more or less the same as yours) ...  Sorry for the confusion!
>

Cool. Thanks :) I had the fix ready yesterday but the testsuite takes a 
while to run on my machine, but I should have replied to your email :).

>> 2010-06-24  Sami Wagiaalla<swagiaal@redhat.com>
>>
>> 	* gdb.cp/operator.cc: Created an import loop.
>> 	* gdb.cp/operator.exp: Added testcase for import loop.
>
> Except I didn't add the test case since on my machine, I already had
> several other tests failing due to the bug.  But I guess you're right,
> we should add the test anyway, for machines without the loop in the
> system headers ...  Please feel free to check in the test!
>

will do.

> Thanks for the quick fix!

No problem

Sami