* Re: GDB crash due to endless recursion in namespace lookup
2010-06-24 18:10 GDB crash due to endless recursion in namespace lookup Ulrich Weigand
@ 2010-06-25 15:46 ` sami wagiaalla
2010-06-25 16:24 ` Ulrich Weigand
2010-06-25 16:20 ` [commit] Fix " Ulrich Weigand
1 sibling, 1 reply; 5+ messages in thread
From: sami wagiaalla @ 2010-06-25 15:46 UTC (permalink / raw)
To: gdb-patches
[-- 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.}
^ permalink raw reply [flat|nested] 5+ messages in thread
* [commit] Fix GDB crash due to endless recursion in namespace lookup
2010-06-24 18:10 GDB crash due to endless recursion in namespace lookup Ulrich Weigand
2010-06-25 15:46 ` sami wagiaalla
@ 2010-06-25 16:20 ` Ulrich Weigand
1 sibling, 0 replies; 5+ messages in thread
From: Ulrich Weigand @ 2010-06-25 16:20 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread