Re: [2nd PING] [C++ PATCH] demangler fix (take 2)
On 10/08/2013 05:54 AM, Gary Benson wrote: diff --git a/test.c b/test.c I don't think we want to add this to the top level directory. :) Other than that, the patch is OK. Jason
[2nd PING] [C++ PATCH] demangler fix (take 2)
Hi all,
This is a resubmission of my previous demangler fix [1] rewritten
to avoid using hashtables and other libiberty features.
From the above referenced email:
d_print_comp maintains a certain amount of scope across calls (namely
a stack of templates) which is used when evaluating references in
template argument lists. If such a reference is later used from a
subtitution then the scope in force at the time of the substitution is
used. This appears to be wrong (I say appears because I couldn't find
anything in the API [2] to clarify this).
The attached patch causes the demangler to capture the scope the first
time such a reference is traversed, and to use that captured scope on
subsequent traversals. This fixes GDB PR 14963 [3] whereby a
reference is resolved against the wrong template, causing an infinite
loop and eventual stack overflow and segmentation fault.
I've added the result to the demangler test suite, but I know of no
way to check the validity of the demangled symbol other than by
inspection (and I am no expert here!) If anybody knows a way to
check this then please let me know! Otherwise, I hope this
not-really-checked demangled version is acceptable.
Thanks,
Gary
[1] http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00215.html
[2] http://mentorembedded.github.io/cxx-abi/abi.html#mangling
[3] http://sourceware.org/bugzilla/show_bug.cgi?id=14963
--
http://gbenson.net/
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 89e108a..2ff8216 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,20 @@
+2013-09-17 Gary Benson gben...@redhat.com
+
+ * cp-demangle.c (struct d_saved_scope): New structure.
+ (struct d_print_info): New fields saved_scopes and
+ num_saved_scopes.
+ (d_print_init): Initialize the above.
+ (d_print_free): New function.
+ (cplus_demangle_print_callback): Call the above.
+ (d_copy_templates): New function.
+ (d_print_comp): New variables saved_templates and
+ need_template_restore.
+ [DEMANGLE_COMPONENT_REFERENCE,
+ DEMANGLE_COMPONENT_RVALUE_REFERENCE]: Capture scope the first
+ time the component is traversed, and use the captured scope for
+ subsequent traversals.
+ * testsuite/demangle-expected: Add regression test.
+
2013-09-10 Paolo Carlini paolo.carl...@oracle.com
PR bootstrap/58386
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 70f5438..a199f6d 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -275,6 +275,18 @@ struct d_growable_string
int allocation_failure;
};
+/* A demangle component and some scope captured when it was first
+ traversed. */
+
+struct d_saved_scope
+{
+ /* The component whose scope this is. */
+ const struct demangle_component *container;
+ /* The list of templates, if any, that was current when this
+ scope was captured. */
+ struct d_print_template *templates;
+};
+
enum { D_PRINT_BUFFER_LENGTH = 256 };
struct d_print_info
{
@@ -302,6 +314,10 @@ struct d_print_info
int pack_index;
/* Number of d_print_flush calls so far. */
unsigned long int flush_count;
+ /* Array of saved scopes for evaluating substitutions. */
+ struct d_saved_scope *saved_scopes;
+ /* Number of saved scopes in the above array. */
+ int num_saved_scopes;
};
#ifdef CP_DEMANGLE_DEBUG
@@ -3665,6 +3681,30 @@ d_print_init (struct d_print_info *dpi,
demangle_callbackref callback,
dpi-opaque = opaque;
dpi-demangle_failure = 0;
+
+ dpi-saved_scopes = NULL;
+ dpi-num_saved_scopes = 0;
+}
+
+/* Free a print information structure. */
+
+static void
+d_print_free (struct d_print_info *dpi)
+{
+ int i;
+
+ for (i = 0; i dpi-num_saved_scopes; i++)
+{
+ struct d_print_template *ts, *tn;
+
+ for (ts = dpi-saved_scopes[i].templates; ts != NULL; ts = tn)
+ {
+ tn = ts-next;
+ free (ts);
+ }
+}
+
+ free (dpi-saved_scopes);
}
/* Indicate that an error occurred during printing, and test for error. */
@@ -3749,6 +3789,7 @@ cplus_demangle_print_callback (int options,
demangle_callbackref callback, void *opaque)
{
struct d_print_info dpi;
+ int success;
d_print_init (dpi, callback, opaque);
@@ -3756,7 +3797,9 @@ cplus_demangle_print_callback (int options,
d_print_flush (dpi);
- return ! d_print_saw_error (dpi);
+ success = ! d_print_saw_error (dpi);
+ d_print_free (dpi);
+ return success;
}
/* Turn components into a human readable string. OPTIONS is the
@@ -3913,6 +3956,36 @@ d_print_subexpr (struct d_print_info *dpi, int options,
d_append_char (dpi, ')');
}
+/* Return a shallow copy of the current list of templates.
+ On error d_print_error is called and a partial list may
+ be returned. Whatever is returned must be freed. */
+
+static struct d_print_template *
+d_copy_templates (struct d_print_info *dpi)
+{
+ struct d_print_template *src, *result,
[PING] [C++ PATCH] demangler fix (take 2)
Gary Benson wrote:
Hi all,
This is a resubmission of my previous demangler fix [1] rewritten
to avoid using hashtables and other libiberty features.
From the above referenced email:
d_print_comp maintains a certain amount of scope across calls (namely
a stack of templates) which is used when evaluating references in
template argument lists. If such a reference is later used from a
subtitution then the scope in force at the time of the substitution is
used. This appears to be wrong (I say appears because I couldn't find
anything in the API [2] to clarify this).
The attached patch causes the demangler to capture the scope the first
time such a reference is traversed, and to use that captured scope on
subsequent traversals. This fixes GDB PR 14963 [3] whereby a
reference is resolved against the wrong template, causing an infinite
loop and eventual stack overflow and segmentation fault.
I've added the result to the demangler test suite, but I know of no
way to check the validity of the demangled symbol other than by
inspection (and I am no expert here!) If anybody knows a way to
check this then please let me know! Otherwise, I hope this
not-really-checked demangled version is acceptable.
Thanks,
Gary
[1] http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00215.html
[2] http://mentorembedded.github.io/cxx-abi/abi.html#mangling
[3] http://sourceware.org/bugzilla/show_bug.cgi?id=14963
--
http://gbenson.net/
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 89e108a..2ff8216 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,20 @@
+2013-09-17 Gary Benson gben...@redhat.com
+
+ * cp-demangle.c (struct d_saved_scope): New structure.
+ (struct d_print_info): New fields saved_scopes and
+ num_saved_scopes.
+ (d_print_init): Initialize the above.
+ (d_print_free): New function.
+ (cplus_demangle_print_callback): Call the above.
+ (d_copy_templates): New function.
+ (d_print_comp): New variables saved_templates and
+ need_template_restore.
+ [DEMANGLE_COMPONENT_REFERENCE,
+ DEMANGLE_COMPONENT_RVALUE_REFERENCE]: Capture scope the first
+ time the component is traversed, and use the captured scope for
+ subsequent traversals.
+ * testsuite/demangle-expected: Add regression test.
+
2013-09-10 Paolo Carlini paolo.carl...@oracle.com
PR bootstrap/58386
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 70f5438..a199f6d 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -275,6 +275,18 @@ struct d_growable_string
int allocation_failure;
};
+/* A demangle component and some scope captured when it was first
+ traversed. */
+
+struct d_saved_scope
+{
+ /* The component whose scope this is. */
+ const struct demangle_component *container;
+ /* The list of templates, if any, that was current when this
+ scope was captured. */
+ struct d_print_template *templates;
+};
+
enum { D_PRINT_BUFFER_LENGTH = 256 };
struct d_print_info
{
@@ -302,6 +314,10 @@ struct d_print_info
int pack_index;
/* Number of d_print_flush calls so far. */
unsigned long int flush_count;
+ /* Array of saved scopes for evaluating substitutions. */
+ struct d_saved_scope *saved_scopes;
+ /* Number of saved scopes in the above array. */
+ int num_saved_scopes;
};
#ifdef CP_DEMANGLE_DEBUG
@@ -3665,6 +3681,30 @@ d_print_init (struct d_print_info *dpi,
demangle_callbackref callback,
dpi-opaque = opaque;
dpi-demangle_failure = 0;
+
+ dpi-saved_scopes = NULL;
+ dpi-num_saved_scopes = 0;
+}
+
+/* Free a print information structure. */
+
+static void
+d_print_free (struct d_print_info *dpi)
+{
+ int i;
+
+ for (i = 0; i dpi-num_saved_scopes; i++)
+{
+ struct d_print_template *ts, *tn;
+
+ for (ts = dpi-saved_scopes[i].templates; ts != NULL; ts = tn)
+ {
+ tn = ts-next;
+ free (ts);
+ }
+}
+
+ free (dpi-saved_scopes);
}
/* Indicate that an error occurred during printing, and test for error. */
@@ -3749,6 +3789,7 @@ cplus_demangle_print_callback (int options,
demangle_callbackref callback, void *opaque)
{
struct d_print_info dpi;
+ int success;
d_print_init (dpi, callback, opaque);
@@ -3756,7 +3797,9 @@ cplus_demangle_print_callback (int options,
d_print_flush (dpi);
- return ! d_print_saw_error (dpi);
+ success = ! d_print_saw_error (dpi);
+ d_print_free (dpi);
+ return success;
}
/* Turn components into a human readable string. OPTIONS is the
@@ -3913,6 +3956,36 @@ d_print_subexpr (struct d_print_info *dpi, int options,
d_append_char (dpi, ')');
}
+/* Return a shallow copy of the current list of templates.
+ On error d_print_error is called and a partial list may
+ be returned. Whatever is returned must be freed. */
+
+static struct d_print_template *
+d_copy_templates (struct
