public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 3/3] mangler/demangler dogfooding
  2014-05-27 11:57 [PATCH 0/3] mangler/demangler dogfooding Pedro Alves
  2014-05-27 11:57 ` [PATCH 1/3] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined Pedro Alves
  2014-05-27 11:57 ` [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters Pedro Alves
@ 2014-05-27 11:57 ` Pedro Alves
  2014-05-27 18:16   ` Mike Stump
  2014-05-28 22:31   ` [PATCH v2 " Pedro Alves
  2 siblings, 2 replies; 18+ messages in thread
From: Pedro Alves @ 2014-05-27 11:57 UTC (permalink / raw)
  To: gcc-patches

This patch teaches g++ to try to demangle any symbol it
generates/mangles, when checking is enabled in the build, as soon as
the symbol is mangled.  The idea here is validate the demangling as
close as possible to the generator as possible.

This allows catching demangler bugs/crashes much earlier in the cycle,
when building libstdc++ and friends, when running g++'s testsuite, and
even potentially earlier than that, when developers work on new
C++11/14-and-beyond features, which influence mangling, validating
against some random test that's not in the testsuite yet (and
sometimes doesn't make it there either), rather than much later in
production when the user is trying to debug the code, or the program
tries to generate a backtrace.  Both libstdc++ and the testsuite
include a good set of tricky symbols to demangle, and the latter
naturally includes all sort of random, weird, code full of corner
cases.  And, I assume g++ maintainers once in a while run WIP g++
through some piles of very complicated C++ code.

It seems clear to me that ideally we should be able to do

 assert (demangle (mangle (symbol));

But, unfortunately, we can't yet.  I built g++ and ran the testsuite
with a variant of this patch that would print the mangled symbol if
the demangling fails, but would otherwise continue without aborting,
and I found out that:

- we can't demangle ~40 symbols generated while building libstdc++
  and friends.  E.g.:

  _ZN9__gnu_cxx13new_allocatorINSt13__future_base13_State_baseV2EE9constructIS2_IEEEvPT_DpOT0_
  _ZNSt15_Sp_counted_ptrIDnLN9__gnu_cxx12_Lock_policyE1EEC1ERKS3_

- we can't demangle around ~2600 (!) symbols generated while running the g++ testsuite.  E.g.,

  _Z1f1SIXadL3FooEEE
  _ZZ4mainENKUlRT_E_clI1SEEDaS0_

Of course, these all may well be mangler bugs rather than demangler
bugs.  It's possible that these are already known mangler bugs even,
that have been fixed, but require a higher -fabi-version=X.  I didn't
investigate that.

Bootstrapped and regtested on x86_64 Fedora 20.

gcc/
2014-05-27  Pedro Alves  <palves@redhat.com>

	* cp/mangle.c: Include demangle.h.
	(finish_mangling_internal) [ENABLE_CHECKING]: Demangle the just
	mangled symbol.
---
 gcc/cp/mangle.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 4205fec..c4eb5dc 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "target.h"
 #include "cgraph.h"
 #include "wide-int.h"
+#include "demangle.h"
 
 /* Debugging support.  */
 
@@ -3367,6 +3368,35 @@ finish_mangling_internal (const bool warn)
 
   /* Null-terminate the string.  */
   write_char ('\0');
+
+#if ENABLE_CHECKING
+  /* Make sure we can demangle what we just generated.  */
+  {
+    const char *str ATTRIBUTE_UNUSED;
+    const char *mangled_str;
+    int dmgl_opts;
+
+    dmgl_opts = (DMGL_VERBOSE
+		 | DMGL_ANSI
+		 | DMGL_GNU_V3
+		 | DMGL_RET_POSTFIX
+		 | DMGL_PARAMS);
+
+    mangled_str = (const char *) obstack_base (mangle_obstack);
+    str = cplus_demangle_v3 (mangled_str, dmgl_opts);
+#if 0
+    /* XXX The above catches potential demangler crashes.  And,
+       ideally, we'd also abort if demangling fails.  However, we
+       can't do that because the demangler isn't able to demangle all
+       symbols we generate by default.  Some failures might be
+       demangler bugs, others unknown mangler bugs, and others known
+       mangler bugs fixed with a higher -fabi-version, which the
+       demangler doesn't have a workaround for.  */
+    if ((str != NULL) != (mangled_str[0] == '_' && mangled_str[1] == 'Z'))
+      internal_error ("demangling failed for: %s", mangled_str);
+#endif
+  }
+#endif
 }
 
 
-- 
1.9.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-05-27 11:57 [PATCH 0/3] mangler/demangler dogfooding Pedro Alves
  2014-05-27 11:57 ` [PATCH 1/3] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined Pedro Alves
@ 2014-05-27 11:57 ` Pedro Alves
  2014-05-30 17:37   ` Cary Coutant
  2014-06-02 13:42   ` Jason Merrill
  2014-05-27 11:57 ` [PATCH 3/3] mangler/demangler dogfooding Pedro Alves
  2 siblings, 2 replies; 18+ messages in thread
From: Pedro Alves @ 2014-05-27 11:57 UTC (permalink / raw)
  To: gcc-patches

The fix for bug 59195:

 [C++ demangler handles conversion operator incorrectly]
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59195

unfortunately makes the demangler crash due to infinite recursion, in
case of casts in template parameters.

For example, with:

 template<int> struct A {};
 template <typename Y> void function_temp(A<sizeof ((Y)(999))>) {}
 template void function_temp<int>(A<sizeof (int)>);

The 'function_temp<int>' instantiation above mangles to:

  _Z13function_tempIiEv1AIXszcvT_Li999EEE

The demangler parses this as:

typed name
  template
    name 'function_temp'
    template argument list
      builtin type int
  function type
    builtin type void
    argument list
      template                          (*)
        name 'A'
        template argument list
          unary operator
            operator sizeof
            unary operator
              cast
                template parameter 0    (**)
              literal
                builtin type int
                name '999'

And after the fix for 59195, due to:

 static void
 d_print_cast (struct d_print_info *dpi, int options,
	       const struct demangle_component *dc)
 {
 ...
   /* For a cast operator, we need the template parameters from
      the enclosing template in scope for processing the type.  */
   if (dpi->current_template != NULL)
     {
       dpt.next = dpi->templates;
       dpi->templates = &dpt;
       dpt.template_decl = dpi->current_template;
     }

when printing the template argument list of A (what should be "<sizeof
(int)>"), the template parameter 0 (that is, "T_", the '**' above) now
refers to the first parameter of the the template argument list of the
'A' template (the '*' above), exactly what we were already trying to
print.  This leads to infinite recursion, and stack exaustion.  The
template parameter 0 should actually refer to the first parameter of
the 'function_temp' template.

Where it reads "for the cast operator" in the comment in d_print_cast
(above), it's really talking about a conversion operator, like:

  struct A { template <typename U> explicit operator U(); };

We don't want to inject the template parameters from the enclosing
template in scope when processing a cast _expression_, only when
handling a conversion operator.

The problem is that DEMANGLE_COMPONENT_CAST is currently ambiguous,
and means _both_ 'conversion operator' and 'cast expression'.

Fix this by adding a new DEMANGLE_COMPONENT_CONVERSION component type,
which does what DEMANGLE_COMPONENT_CAST does today, and making
DEMANGLE_COMPONENT_CAST just simply print its component subtree.

I think we could instead reuse DEMANGLE_COMPONENT_CAST and in
d_print_comp_inner still do:

 @@ -5001,9 +5013,9 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
        d_print_comp (dpi, options, dc->u.s_extended_operator.name);
        return;

     case DEMANGLE_COMPONENT_CAST:
       d_append_string (dpi, "operator ");
 -     d_print_cast (dpi, options, dc);
 +     d_print_conversion (dpi, options, dc);
       return;

leaving the unary cast case below calling d_print_cast, but seems to
me that spliting the component types makes it easier to reason about
the code.

g++'s testsuite actually generates three symbols that crash the
demangler in the same way.  I've added those as tests in the demangler
testsuite as well.

And then this fixes PR other/61233 too, which happens to be a
demangler crash originally reported to GDB, at:
https://sourceware.org/bugzilla/show_bug.cgi?id=16957

Bootstrapped and regtested on x86_64 Fedora 20.

Also ran this through GDB's testsuite.  GDB will require a small
update to use DEMANGLE_COMPONENT_CONVERSION in one place it's using
DEMANGLE_COMPONENT_CAST in its sources.

libiberty/
2014-05-27  Pedro Alves <palves@redhat.com>

	PR other/61321
	PR other/61233
	* demangle.h (enum demangle_component_type)
	<DEMANGLE_COMPONENT_CONVERSION>: New value.
	* cp-demangle.c (d_demangle_callback, d_make_comp): Handle
	DEMANGLE_COMPONENT_CONVERSION.
	(is_ctor_dtor_or_conversion): Handle DEMANGLE_COMPONENT_CONVERSION
	instead of DEMANGLE_COMPONENT_CAST.
	(d_operator_name): Return a DEMANGLE_COMPONENT_CONVERSION
	component if handling a conversion.
	(d_count_templates_scopes, d_print_comp_inner): Handle
	DEMANGLE_COMPONENT_CONVERSION.
	(d_print_comp_inner): Handle DEMANGLE_COMPONENT_CONVERSION instead
	of DEMANGLE_COMPONENT_CAST.
	(d_print_cast): Rename as ...
	(d_print_conversion): ... this.  Adjust comments.
	(d_print_cast): Rewrite - simply print the left subcomponent.
	* cp-demint.c (cplus_demangle_fill_component): Handle
	DEMANGLE_COMPONENT_CONVERSION.

	* testsuite/demangle-expected: Add tests.
---
 include/demangle.h                    |  4 ++++
 libiberty/cp-demangle.c               | 37 +++++++++++++++++++++++++++--------
 libiberty/cp-demint.c                 |  1 +
 libiberty/testsuite/demangle-expected | 23 ++++++++++++++++++++++
 4 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/include/demangle.h b/include/demangle.h
index bbad71b..7dc2648 100644
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -373,6 +373,10 @@ enum demangle_component_type
   /* A typecast, represented as a unary operator.  The one subtree is
      the type to which the argument should be cast.  */
   DEMANGLE_COMPONENT_CAST,
+  /* A conversion operator, represented as a unary operator.  The one
+     subtree is the type to which the argument should be converted
+     to.  */
+  DEMANGLE_COMPONENT_CONVERSION,
   /* A nullary expression.  The left subtree is the operator.  */
   DEMANGLE_COMPONENT_NULLARY,
   /* A unary expression.  The left subtree is the operator, and the
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index c0d2ffe..a86e8aa 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -538,8 +538,10 @@ d_print_array_type (struct d_print_info *, int,
 static void
 d_print_expr_op (struct d_print_info *, int, const struct demangle_component *);
 
-static void
-d_print_cast (struct d_print_info *, int, const struct demangle_component *);
+static void d_print_cast (struct d_print_info *, int,
+			  const struct demangle_component *);
+static void d_print_conversion (struct d_print_info *, int,
+				const struct demangle_component *);
 
 static int d_demangle_callback (const char *, int,
                                 demangle_callbackref, void *);
@@ -727,6 +729,9 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_CAST:
       printf ("cast\n");
       break;
+    case DEMANGLE_COMPONENT_CONVERSION:
+      printf ("conversion operator\n");
+      break;
     case DEMANGLE_COMPONENT_NULLARY:
       printf ("nullary operator\n");
       break;
@@ -936,6 +941,7 @@ d_make_comp (struct d_info *di, enum demangle_component_type type,
     case DEMANGLE_COMPONENT_IMAGINARY:
     case DEMANGLE_COMPONENT_VENDOR_TYPE:
     case DEMANGLE_COMPONENT_CAST:
+    case DEMANGLE_COMPONENT_CONVERSION:
     case DEMANGLE_COMPONENT_JAVA_RESOURCE:
     case DEMANGLE_COMPONENT_DECLTYPE:
     case DEMANGLE_COMPONENT_PACK_EXPANSION:
@@ -1227,7 +1233,7 @@ is_ctor_dtor_or_conversion (struct demangle_component *dc)
       return is_ctor_dtor_or_conversion (d_right (dc));
     case DEMANGLE_COMPONENT_CTOR:
     case DEMANGLE_COMPONENT_DTOR:
-    case DEMANGLE_COMPONENT_CAST:
+    case DEMANGLE_COMPONENT_CONVERSION:
       return 1;
     }
 }
@@ -1783,11 +1789,16 @@ d_operator_name (struct d_info *di)
     {
       struct demangle_component *type;
       int was_conversion = di->is_conversion;
+      struct demangle_component *res;
 
       di->is_conversion = ! di->is_expression;
       type = cplus_demangle_type (di);
+      if (di->is_conversion)
+	res = d_make_comp (di, DEMANGLE_COMPONENT_CONVERSION, type, NULL);
+      else
+	res = d_make_comp (di, DEMANGLE_COMPONENT_CAST, type, NULL);
       di->is_conversion = was_conversion;
-      return d_make_comp (di, DEMANGLE_COMPONENT_CAST, type, NULL);
+      return res;
     }
   else
     {
@@ -3881,6 +3892,7 @@ d_count_templates_scopes (int *num_templates, int *num_scopes,
     case DEMANGLE_COMPONENT_TEMPLATE_ARGLIST:
     case DEMANGLE_COMPONENT_INITIALIZER_LIST:
     case DEMANGLE_COMPONENT_CAST:
+    case DEMANGLE_COMPONENT_CONVERSION:
     case DEMANGLE_COMPONENT_NULLARY:
     case DEMANGLE_COMPONENT_UNARY:
     case DEMANGLE_COMPONENT_BINARY:
@@ -5001,9 +5013,9 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
       d_print_comp (dpi, options, dc->u.s_extended_operator.name);
       return;
 
-    case DEMANGLE_COMPONENT_CAST:
+    case DEMANGLE_COMPONENT_CONVERSION:
       d_append_string (dpi, "operator ");
-      d_print_cast (dpi, options, dc);
+      d_print_conversion (dpi, options, dc);
       return;
 
     case DEMANGLE_COMPONENT_NULLARY:
@@ -5736,11 +5748,20 @@ d_print_expr_op (struct d_print_info *dpi, int options,
 
 static void
 d_print_cast (struct d_print_info *dpi, int options,
-              const struct demangle_component *dc)
+		    const struct demangle_component *dc)
+{
+  d_print_comp (dpi, options, d_left (dc));
+}
+
+/* Print a conversion operator.  */
+
+static void
+d_print_conversion (struct d_print_info *dpi, int options,
+		    const struct demangle_component *dc)
 {
   struct d_print_template dpt;
 
-  /* For a cast operator, we need the template parameters from
+  /* For a conversion operator, we need the template parameters from
      the enclosing template in scope for processing the type.  */
   if (dpi->current_template != NULL)
     {
diff --git a/libiberty/cp-demint.c b/libiberty/cp-demint.c
index 1d1a77a..efcc5b7 100644
--- a/libiberty/cp-demint.c
+++ b/libiberty/cp-demint.c
@@ -110,6 +110,7 @@ cplus_demangle_fill_component (struct demangle_component *p,
     case DEMANGLE_COMPONENT_IMAGINARY:
     case DEMANGLE_COMPONENT_VENDOR_TYPE:
     case DEMANGLE_COMPONENT_CAST:
+    case DEMANGLE_COMPONENT_CONVERSION:
       if (right != NULL)
 	return 0;
       break;
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 864ee7e..723c632 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4348,3 +4348,26 @@ void post<std::function<void ()> >(std::function<void ()>&&)::{lambda()#1}*& std
 _QueueNotification_QueueController__$4PPPPPPPM_A_INotice___Z
 _QueueNotification_QueueController__$4PPPPPPPM_A_INotice___Z
 _QueueNotification_QueueController__$4PPPPPPPM_A_INotice___Z
+#
+# These two are from gcc PR61321, and gcc PR61233 / gdb PR16957
+#
+--format=gnu-v3
+_Z13function_tempIiEv1AIXszcvT_Li999EEE
+void function_temp<int>(A<sizeof ((int)(999))>)
+#
+--format=gnu-v3
+_Z7ZipWithI7QStringS0_5QListZN4oral6detail16AdaptCreateTableI7AccountEES0_RKNS3_16CachedFieldsDataEEUlRKS0_SA_E_ET1_IDTclfp1_cvT__EcvT0__EEEERKT1_ISC_ERKT1_ISD_ET2_
+QList<decltype ({parm#3}((QString)(), (QString)()))> ZipWith<QString, QString, QList, QString oral::detail::AdaptCreateTable<Account>(oral::detail::CachedFieldsData const&)::{lambda(QString const&, QString const&)#1}>(QList<QString oral::detail::AdaptCreateTable<Account>(oral::detail::CachedFieldsData const&)::{lambda(QString const&, QString const&)#1}> const&, QList<QList> const&, QString oral::detail::AdaptCreateTable<Account>(oral::detail::CachedFieldsData const&)::{lambda(QString const&, QString const&)#1})
+#
+# These three are symbols generated by g++'s testsuite, which triggered the same bug as above.
+--format=gnu-v3
+_Z14int_if_addableI1YERiP1AIXszpldecvPT_Li0EdecvS4_Li0EEE
+int& int_if_addable<Y>(A<sizeof ((*((Y*)(0)))+(*((Y*)(0))))>*)
+#
+--format=gnu-v3
+_Z3bazIiEvP1AIXszcl3foocvT__ELCf00000000_00000000EEEE
+void baz<int>(A<sizeof (foo((int)(), (floatcomplex )00000000_00000000))>*)
+#
+--format=gnu-v3
+_Z3fooI1FEN1XIXszdtcl1PclcvT__EEE5arrayEE4TypeEv
+X<sizeof ((P(((F)())())).array)>::Type foo<F>()
-- 
1.9.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/3] mangler/demangler dogfooding
@ 2014-05-27 11:57 Pedro Alves
  2014-05-27 11:57 ` [PATCH 1/3] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined Pedro Alves
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Pedro Alves @ 2014-05-27 11:57 UTC (permalink / raw)
  To: gcc-patches

There's been discussion on the GDB lists about demangler crashes
bringing down GDB.

Such demangler bugs should be fixed, of course.  Patch 2 in this
series fixes the one that is currently known.

But while GDB demangles all symbols in every file it loads, always, in
libstdc++, the demangler is only called in error situations, and then
only to demangle the few symbols that appear in the backtrace.  This
means that mangler/demangler bugs can easily escape undetected until
much later, when the user is trying to debug the program.

So we had a discussion in the gdb lists about whether there is
something that could be done to demangler testing to make it more
effective in catching bugs that GDB would catch.  Ideas in progress so
far:

- Just throw more symbols at it once in a while.

We'd like to try "demangling the world" once in a while.  That is,
demangle all symbols in a distro.  Florian Weimer did this with the
ELF symbols in Fedora 20 and rawhide, and didn't get any crashes,
which is reassuring.  We haven't yet tried extracting and demangling
all DWARF symbols, but we'd like to get there too.

- Throw pseudo-random symbols at the demangler.

That is, make it more robust wrt to invalid input.  Gary wrote a fuzzy
tester, and it already caught a few crashes.

- Teach g++ itself to try to demangle any symbol it generates/mangles

... when checking is enabled in the build.

The idea here is validate the mangling/demangling as close as possible
to the generator as possible.  Patch 3 in this series implements this.
This is my favorite approach, as this catches demangler crashes much
earlier in the cycle, when building libstdc++ and friends, and when
running g++'s testsuite, rather than much later in production when the
user is trying to debug the code, or the program tries to generate a
backtrace.  Both libstdc++ and the testsuite include a good set of
tricky symbols to demangle, and the latter naturally includes all sort
of random, weird, code full of corner cases.  And, I assume g++
maintainers once in a while run WIP g++ through some piles of very
complicated C++ code.

Patch 2 fixes the demangler crashes I got when running the testsuite
with patch 3 installed.

And then patch 1 fixes crashes in the demangler debug that I got while
I was writing patch 2.

Pedro Alves (3):
  Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined
  PR other/61321 - demangler crash on casts in template parameters
  mangler/demangler dogfooding

 gcc/cp/mangle.c                       | 30 ++++++++++++++++++++++++
 include/demangle.h                    |  4 ++++
 libiberty/cp-demangle.c               | 43 ++++++++++++++++++++++++++++-------
 libiberty/cp-demint.c                 |  1 +
 libiberty/testsuite/demangle-expected | 23 +++++++++++++++++++
 5 files changed, 93 insertions(+), 8 deletions(-)

-- 
1.9.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined
  2014-05-27 11:57 [PATCH 0/3] mangler/demangler dogfooding Pedro Alves
@ 2014-05-27 11:57 ` Pedro Alves
  2014-05-27 13:56   ` Ian Lance Taylor
  2014-05-27 11:57 ` [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters Pedro Alves
  2014-05-27 11:57 ` [PATCH 3/3] mangler/demangler dogfooding Pedro Alves
  2 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2014-05-27 11:57 UTC (permalink / raw)
  To: gcc-patches

I tried running the demangler's testsuite with CP_DEMANGLE_DEBUG
defined, but it crashed, with:

 Program received signal SIGSEGV, Segmentation fault.
 0x000000000040a8c3 in d_dump (dc=0x1, indent=12) at ../../src/libiberty/cp-demangle.c:567
 567       switch (dc->type)

 (gdb) bt 3
 #0  0x000000000040a8c3 in d_dump (dc=0x1, indent=12) at ../../src/libiberty/cp-demangle.c:567
 #1  0x000000000040ae47 in d_dump (dc=0x7fffffffd098, indent=10) at ../../src/libiberty/cp-demangle.c:787
 #2  0x000000000040ae47 in d_dump (dc=0x7fffffffd0c8, indent=8) at ../../src/libiberty/cp-demangle.c:787

Note dc=0x1, which is obviously a bogus pointer.  This is the end of
d_dump recursing for a component type that that doesn't actually have
subtrees:

 787       d_dump (d_left (dc), indent + 2);
 788       d_dump (d_right (dc), indent + 2);

This fixes the two cases the testsuite trips on.

libiberty/
2014-05-26  Pedro Alves  <palves@redhat.com>

	* cp-demangle.c (d_dump): Handle DEMANGLE_COMPONENT_FUNCTION_PARAM
	and DEMANGLE_COMPONENT_NUMBER.
---
 libiberty/cp-demangle.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 68d8ee1..c0d2ffe 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -575,6 +575,9 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_TEMPLATE_PARAM:
       printf ("template parameter %ld\n", dc->u.s_number.number);
       return;
+    case DEMANGLE_COMPONENT_FUNCTION_PARAM:
+      printf ("function parameter %ld\n", dc->u.s_number.number);
+      return;
     case DEMANGLE_COMPONENT_CTOR:
       printf ("constructor %d\n", (int) dc->u.s_ctor.kind);
       d_dump (dc->u.s_ctor.name, indent + 2);
@@ -760,6 +763,9 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_CHARACTER:
       printf ("character '%c'\n",  dc->u.s_character.character);
       return;
+    case DEMANGLE_COMPONENT_NUMBER:
+      printf ("number %ld\n", dc->u.s_number.number);
+      return;
     case DEMANGLE_COMPONENT_DECLTYPE:
       printf ("decltype\n");
       break;
-- 
1.9.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined
  2014-05-27 11:57 ` [PATCH 1/3] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined Pedro Alves
@ 2014-05-27 13:56   ` Ian Lance Taylor
  2014-05-28 22:20     ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Lance Taylor @ 2014-05-27 13:56 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gcc-patches

On Tue, May 27, 2014 at 4:57 AM, Pedro Alves <palves@redhat.com> wrote:
>
> libiberty/
> 2014-05-26  Pedro Alves  <palves@redhat.com>
>
>         * cp-demangle.c (d_dump): Handle DEMANGLE_COMPONENT_FUNCTION_PARAM
>         and DEMANGLE_COMPONENT_NUMBER.

This is OK.

Thanks.

Ian

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] mangler/demangler dogfooding
  2014-05-27 11:57 ` [PATCH 3/3] mangler/demangler dogfooding Pedro Alves
@ 2014-05-27 18:16   ` Mike Stump
  2014-05-28 22:31   ` [PATCH v2 " Pedro Alves
  1 sibling, 0 replies; 18+ messages in thread
From: Mike Stump @ 2014-05-27 18:16 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gcc-patches

On May 27, 2014, at 4:57 AM, Pedro Alves <palves@redhat.com> wrote:
> This patch teaches g++ to try to demangle any symbol it
> generates/mangles, when checking is enabled in the build

I like this strategy.  :-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined
  2014-05-27 13:56   ` Ian Lance Taylor
@ 2014-05-28 22:20     ` Pedro Alves
  0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2014-05-28 22:20 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc-patches, GDB Patches

+gdb-patches

On 05/27/2014 02:56 PM, Ian Lance Taylor wrote:
> On Tue, May 27, 2014 at 4:57 AM, Pedro Alves <palves@redhat.com> wrote:
>>
>> libiberty/
>> 2014-05-26  Pedro Alves  <palves@redhat.com>
>>
>>         * cp-demangle.c (d_dump): Handle DEMANGLE_COMPONENT_FUNCTION_PARAM
>>         and DEMANGLE_COMPONENT_NUMBER.
> 
> This is OK.

Thank you.  I've committed this to gcc svn, and pushed it to binutils-gdb
as well.

8<-------------
From: Pedro Alves <palves@redhat.com>
Subject: [PATCH] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG
 defined

Running the demangler's testsuite with CP_DEMANGLE_DEBUG defined
crashes, with:

 Program received signal SIGSEGV, Segmentation fault.
 0x000000000040a8c3 in d_dump (dc=0x1, indent=12) at ../../src/libiberty/cp-demangle.c:567
 567       switch (dc->type)

 (gdb) bt 3
 #0  0x000000000040a8c3 in d_dump (dc=0x1, indent=12) at ../../src/libiberty/cp-demangle.c:567
 #1  0x000000000040ae47 in d_dump (dc=0x7fffffffd098, indent=10) at ../../src/libiberty/cp-demangle.c:787
 #2  0x000000000040ae47 in d_dump (dc=0x7fffffffd0c8, indent=8) at ../../src/libiberty/cp-demangle.c:787

Note dc=0x1, which is obviously a bogus pointer.  This is the end of
d_dump recursing for a component type that that doesn't actually have
subtrees:

 787       d_dump (d_left (dc), indent + 2);
 788       d_dump (d_right (dc), indent + 2);

This fixes the two cases the testsuite currently trips on.

libiberty/
2014-05-28  Pedro Alves  <palves@redhat.com>

	* cp-demangle.c (d_dump): Handle DEMANGLE_COMPONENT_FUNCTION_PARAM
	and DEMANGLE_COMPONENT_NUMBER.
---
 libiberty/ChangeLog     | 5 +++++
 libiberty/cp-demangle.c | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 7b25c7e..16eb6f7 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,8 @@
+2014-05-28  Pedro Alves  <palves@redhat.com>
+
+	* cp-demangle.c (d_dump): Handle DEMANGLE_COMPONENT_FUNCTION_PARAM
+	and DEMANGLE_COMPONENT_NUMBER.
+
 2014-05-22  Thomas Schwinge  <thomas@codesourcery.com>
 
 	* testsuite/demangle-expected: Fix last commit.
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 68d8ee1..c0d2ffe 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -575,6 +575,9 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_TEMPLATE_PARAM:
       printf ("template parameter %ld\n", dc->u.s_number.number);
       return;
+    case DEMANGLE_COMPONENT_FUNCTION_PARAM:
+      printf ("function parameter %ld\n", dc->u.s_number.number);
+      return;
     case DEMANGLE_COMPONENT_CTOR:
       printf ("constructor %d\n", (int) dc->u.s_ctor.kind);
       d_dump (dc->u.s_ctor.name, indent + 2);
@@ -760,6 +763,9 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_CHARACTER:
       printf ("character '%c'\n",  dc->u.s_character.character);
       return;
+    case DEMANGLE_COMPONENT_NUMBER:
+      printf ("number %ld\n", dc->u.s_number.number);
+      return;
     case DEMANGLE_COMPONENT_DECLTYPE:
       printf ("decltype\n");
       break;
-- 
1.9.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 3/3] mangler/demangler dogfooding
  2014-05-27 11:57 ` [PATCH 3/3] mangler/demangler dogfooding Pedro Alves
  2014-05-27 18:16   ` Mike Stump
@ 2014-05-28 22:31   ` Pedro Alves
  2014-06-02 13:38     ` Jason Merrill
  1 sibling, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2014-05-28 22:31 UTC (permalink / raw)
  To: gcc-patches

On 05/27/2014 12:57 PM, Pedro Alves wrote:

> +    dmgl_opts = (DMGL_VERBOSE
> +		 | DMGL_ANSI
> +		 | DMGL_GNU_V3
> +		 | DMGL_RET_POSTFIX
> +		 | DMGL_PARAMS);

Hmm, don't know why I had put DMGL_RET_POSTFIX there in the
first place.  That's unintended... GDB only uses that for java.
Without that, the set of symbols we can't demangle when running
the testsuite drops from 2600 to 300.  Clearly that shows 
DMGL_RET_POSTFIX is buggy, but that's not something we really
about...

And related, I've now added type demangling coverage too.

> +    mangled_str = (const char *) obstack_base (mangle_obstack);
> +    str = cplus_demangle_v3 (mangled_str, dmgl_opts);

Err, I forgot to free the demangled string.  Fixed now.

> +#if 0

I've now added a #define for this, instead of #if 0.

> +    /* XXX The above catches potential demangler crashes.  And,
> +       ideally, we'd also abort if demangling fails.  However, we
> +       can't do that because the demangler isn't able to demangle all
> +       symbols we generate by default.  Some failures might be
> +       demangler bugs, others unknown mangler bugs, and others known
> +       mangler bugs fixed with a higher -fabi-version, which the
> +       demangler doesn't have a workaround for.  */
> +    if ((str != NULL) != (mangled_str[0] == '_' && mangled_str[1] == 'Z'))
> +      internal_error ("demangling failed for: %s", mangled_str);

I've now made this check complete in accordance to cp-demangle.c.

Here's v2.  Okay to apply, once patch #2 is in?

8<--------
Subject: [PATCH] mangler/demangler dogfooding

This patch teaches g++ to try to demangle any symbol it
generates/mangles, when checking is enabled in the build, as soon as
the symbol is mangled.  The idea here is validate the demangling as
close as possible to the generator as possible.

This allows catching demangler bugs/crashes much earlier in the cycle,
when building libstdc++ and friends, when running g++'s testsuite, and
even potentially earlier than that, when developers work on new
C++11/14-and-beyond features, which influence mangling, validating
against some random test that's not in the testsuite yet (and
sometimes doesn't make it there either), rather than much later in
production when the user is trying to debug the code, or the program
tries to generate a backtrace.  Both libstdc++ and the testsuite
include a good set of tricky symbols to demangle, and the latter
naturally includes all sort of random, weird, code full of corner
cases.  And, I assume g++ maintainers once in a while run WIP g++
through some piles of very complicated C++ code.

It seems clear to me that ideally we should be able to do

 assert (demangle (mangle (symbol));

But, unfortunately, we can't yet.  I built g++ and ran the testsuite
with a variant of this patch that would print the mangled symbol if
the demangling fails, but would otherwise continue without aborting,
and I found out that:

- we can't demangle ~40 symbols generated while building libstdc++
  and friends.  E.g.:

  _ZN9__gnu_cxx13new_allocatorINSt13__future_base13_State_baseV2EE9constructIS2_IEEEvPT_DpOT0_
  _ZNSt15_Sp_counted_ptrIDnLN9__gnu_cxx12_Lock_policyE1EEC1ERKS3_

- we can't demangle around ~300 symbols generated while running the
  g++ testsuite.  E.g.,

  _Z1f1SIXadL3FooEEE
  _ZZ4mainENKUlRT_E_clI1SEEDaS0_

Of course, these all may well be mangler bugs rather than demangler
bugs.  It's possible that these are already known mangler bugs even,
that have been fixed, but require a higher -fabi-version=X.  I didn't
investigate that.

Bootstrapped and regtested on x86_64 Fedora 20.

gcc/
2014-05-28  Pedro Alves  <palves@redhat.com>

	* cp/mangle.c: Include demangle.h.
	(EXTRA_DEMANGLE_CHECKING): Define if not defined.
	(globals) <type>: New field.
	(start_mangling): Clear it.
	(finish_mangling_internal)
	[ENABLE_CHECKING || EXTRA_DEMANGLE_CHECKING]: Demangle the just
	mangled symbol.
	(mangle_decl_string): Set G.type if mangling a type.
	(mangle_type_string): Set G.type.
---
 gcc/cp/mangle.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 4205fec..b323c9c 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "target.h"
 #include "cgraph.h"
 #include "wide-int.h"
+#include "demangle.h"
 
 /* Debugging support.  */
 
@@ -66,6 +67,14 @@ along with GCC; see the file COPYING3.  If not see
 #define DEBUG_MANGLE 0
 #endif
 
+/* If ENABLE_CHECKING, we try to demangle what we mangle in order to
+   catch nasty demangler crashes early.  But by default, we don't
+   actually make sure the result is sane.  Define this to enable such
+   extra validation.  */
+#ifndef EXTRA_DEMANGLE_CHECKING
+#define EXTRA_DEMANGLE_CHECKING 1
+#endif
+
 /* Macros for tracing the write_* functions.  */
 #if DEBUG_MANGLE
 # define MANGLE_TRACE(FN, INPUT) \
@@ -104,6 +113,9 @@ typedef struct GTY(()) globals {
   /* True if the mangling will be different in a future version of the
      ABI.  */
   bool need_abi_warning;
+
+  /* True if we mangled a type.  */
+  bool type;
 } globals;
 
 static GTY (()) globals G;
@@ -3345,6 +3357,7 @@ start_mangling (const tree entity)
 {
   G.entity = entity;
   G.need_abi_warning = false;
+  G.type = false;
   obstack_free (&name_obstack, name_base);
   mangle_obstack = &name_obstack;
   name_base = obstack_alloc (&name_obstack, 0);
@@ -3367,6 +3380,48 @@ finish_mangling_internal (const bool warn)
 
   /* Null-terminate the string.  */
   write_char ('\0');
+
+#if ENABLE_CHECKING || EXTRA_DEMANGLE_CHECKING
+  /* Make sure we can demangle what we just generated.  */
+  {
+    const char *mangled_str;
+    char *str;
+    int dmgl_opts;
+    int mangled_p ATTRIBUTE_UNUSED;
+
+    dmgl_opts = (G.type
+		 ? DMGL_TYPES
+		 : DMGL_VERBOSE | DMGL_ANSI | DMGL_PARAMS);
+
+    mangled_str = (const char *) obstack_base (mangle_obstack);
+    str = cplus_demangle_v3 (mangled_str, dmgl_opts);
+
+#if EXTRA_DEMANGLE_CHECKING
+    /* XXX The above catches potential demangler crashes.  And,
+       ideally, we'd also abort if demangling fails.  However, we
+       can't do that by default because the demangler isn't able to
+       demangle all symbols we generate.  Some failures might be
+       demangler bugs, others unknown mangler bugs, and others known
+       mangler bugs fixed with a higher -fabi-version that the
+       demangler doesn't have a workaround for.  */
+    mangled_p = (G.type
+		 || (mangled_str[0] == '_' && mangled_str[1] == 'Z')
+		 || (strncmp (mangled_str, "_GLOBAL_", 8) == 0
+		     && (mangled_str[8] == '.'
+			 || mangled_str[8] == '_'
+			 || mangled_str[8] == '$')
+		     && (mangled_str[9] == 'D' || mangled_str[9] == 'I')
+		     && mangled_str[10] == '_'));
+    if ((str != NULL) != mangled_p)
+      internal_error ("mangler/demangler failure for %s: %s",
+		      G.type ? "type" :"fn",
+		      mangled_str);
+
+#endif
+
+    free (str);
+  }
+#endif
 }
 
 
@@ -3438,7 +3493,10 @@ mangle_decl_string (const tree decl)
   start_mangling (decl);
 
   if (TREE_CODE (decl) == TYPE_DECL)
-    write_type (TREE_TYPE (decl));
+    {
+      G.type = true;
+      write_type (TREE_TYPE (decl));
+    }
   else
     write_mangled_name (decl, true);
 
@@ -3535,6 +3593,7 @@ mangle_type_string (const tree type)
   const char *result;
 
   start_mangling (type);
+  G.type = true;
   write_type (type);
   result = finish_mangling (/*warn=*/false);
   if (DEBUG_MANGLE)
-- 
1.9.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-05-27 11:57 ` [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters Pedro Alves
@ 2014-05-30 17:37   ` Cary Coutant
  2014-05-30 18:05     ` Ian Lance Taylor
  2014-06-02 13:42   ` Jason Merrill
  1 sibling, 1 reply; 18+ messages in thread
From: Cary Coutant @ 2014-05-30 17:37 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gcc-patches, Ian Lance Taylor

> Fix this by adding a new DEMANGLE_COMPONENT_CONVERSION component type,
> which does what DEMANGLE_COMPONENT_CAST does today, and making
> DEMANGLE_COMPONENT_CAST just simply print its component subtree.
>
> I think we could instead reuse DEMANGLE_COMPONENT_CAST and in
> d_print_comp_inner still do:
>
>  @@ -5001,9 +5013,9 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
>         d_print_comp (dpi, options, dc->u.s_extended_operator.name);
>         return;
>
>      case DEMANGLE_COMPONENT_CAST:
>        d_append_string (dpi, "operator ");
>  -     d_print_cast (dpi, options, dc);
>  +     d_print_conversion (dpi, options, dc);
>        return;
>
> leaving the unary cast case below calling d_print_cast, but seems to
> me that spliting the component types makes it easier to reason about
> the code.

I agree.

> libiberty/
> 2014-05-27  Pedro Alves <palves@redhat.com>
>
>         PR other/61321
>         PR other/61233
>         * demangle.h (enum demangle_component_type)
>         <DEMANGLE_COMPONENT_CONVERSION>: New value.
>         * cp-demangle.c (d_demangle_callback, d_make_comp): Handle
>         DEMANGLE_COMPONENT_CONVERSION.
>         (is_ctor_dtor_or_conversion): Handle DEMANGLE_COMPONENT_CONVERSION
>         instead of DEMANGLE_COMPONENT_CAST.
>         (d_operator_name): Return a DEMANGLE_COMPONENT_CONVERSION
>         component if handling a conversion.
>         (d_count_templates_scopes, d_print_comp_inner): Handle
>         DEMANGLE_COMPONENT_CONVERSION.
>         (d_print_comp_inner): Handle DEMANGLE_COMPONENT_CONVERSION instead
>         of DEMANGLE_COMPONENT_CAST.
>         (d_print_cast): Rename as ...
>         (d_print_conversion): ... this.  Adjust comments.
>         (d_print_cast): Rewrite - simply print the left subcomponent.
>         * cp-demint.c (cplus_demangle_fill_component): Handle
>         DEMANGLE_COMPONENT_CONVERSION.
>
>         * testsuite/demangle-expected: Add tests.

Looks good to me. Thanks!

Ian, does this look good to you?

-cary

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-05-30 17:37   ` Cary Coutant
@ 2014-05-30 18:05     ` Ian Lance Taylor
  2014-05-30 19:00       ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Lance Taylor @ 2014-05-30 18:05 UTC (permalink / raw)
  To: Cary Coutant, Jason Merrill; +Cc: Pedro Alves, gcc-patches

On Fri, May 30, 2014 at 10:37 AM, Cary Coutant <ccoutant@google.com> wrote:
>> Fix this by adding a new DEMANGLE_COMPONENT_CONVERSION component type,
>> which does what DEMANGLE_COMPONENT_CAST does today, and making
>> DEMANGLE_COMPONENT_CAST just simply print its component subtree.
>>
>> I think we could instead reuse DEMANGLE_COMPONENT_CAST and in
>> d_print_comp_inner still do:
>>
>>  @@ -5001,9 +5013,9 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
>>         d_print_comp (dpi, options, dc->u.s_extended_operator.name);
>>         return;
>>
>>      case DEMANGLE_COMPONENT_CAST:
>>        d_append_string (dpi, "operator ");
>>  -     d_print_cast (dpi, options, dc);
>>  +     d_print_conversion (dpi, options, dc);
>>        return;
>>
>> leaving the unary cast case below calling d_print_cast, but seems to
>> me that spliting the component types makes it easier to reason about
>> the code.
>
> I agree.
>
>> libiberty/
>> 2014-05-27  Pedro Alves <palves@redhat.com>
>>
>>         PR other/61321
>>         PR other/61233
>>         * demangle.h (enum demangle_component_type)
>>         <DEMANGLE_COMPONENT_CONVERSION>: New value.
>>         * cp-demangle.c (d_demangle_callback, d_make_comp): Handle
>>         DEMANGLE_COMPONENT_CONVERSION.
>>         (is_ctor_dtor_or_conversion): Handle DEMANGLE_COMPONENT_CONVERSION
>>         instead of DEMANGLE_COMPONENT_CAST.
>>         (d_operator_name): Return a DEMANGLE_COMPONENT_CONVERSION
>>         component if handling a conversion.
>>         (d_count_templates_scopes, d_print_comp_inner): Handle
>>         DEMANGLE_COMPONENT_CONVERSION.
>>         (d_print_comp_inner): Handle DEMANGLE_COMPONENT_CONVERSION instead
>>         of DEMANGLE_COMPONENT_CAST.
>>         (d_print_cast): Rename as ...
>>         (d_print_conversion): ... this.  Adjust comments.
>>         (d_print_cast): Rewrite - simply print the left subcomponent.
>>         * cp-demint.c (cplus_demangle_fill_component): Handle
>>         DEMANGLE_COMPONENT_CONVERSION.
>>
>>         * testsuite/demangle-expected: Add tests.
>
> Looks good to me. Thanks!
>
> Ian, does this look good to you?

I tend to defer to Jason on this sort of newfangled mangling
nuttiness, but I can take a look if he doesn't have time.

Ian

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-05-30 18:05     ` Ian Lance Taylor
@ 2014-05-30 19:00       ` Pedro Alves
  0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2014-05-30 19:00 UTC (permalink / raw)
  To: Ian Lance Taylor, Cary Coutant, Jason Merrill; +Cc: gcc-patches

On 05/30/2014 07:05 PM, Ian Lance Taylor wrote:
>> > Looks good to me. Thanks!

Thanks Cary!

>> > Ian, does this look good to you?
> I tend to defer to Jason on this sort of newfangled mangling
> nuttiness, but I can take a look if he doesn't have time.

Thanks!

FWIW, it'd be great to have this approved/rejected, as I'd
like to get this fixed in gdb 7.8, and we're just about
to branch.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 3/3] mangler/demangler dogfooding
  2014-05-28 22:31   ` [PATCH v2 " Pedro Alves
@ 2014-06-02 13:38     ` Jason Merrill
  0 siblings, 0 replies; 18+ messages in thread
From: Jason Merrill @ 2014-06-02 13:38 UTC (permalink / raw)
  To: Pedro Alves, gcc-patches

On 05/28/2014 06:31 PM, Pedro Alves wrote:
> Some failures might be
> +       demangler bugs, others unknown mangler bugs, and others known
> +       mangler bugs fixed with a higher -fabi-version that the
> +       demangler doesn't have a workaround for.

You can avoid the latter by skipping demangling if G.need_abi_warning is 
set.  If you do that, are there still any failures?

Jason

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-05-27 11:57 ` [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters Pedro Alves
  2014-05-30 17:37   ` Cary Coutant
@ 2014-06-02 13:42   ` Jason Merrill
  2014-07-24 22:44     ` Cary Coutant
  1 sibling, 1 reply; 18+ messages in thread
From: Jason Merrill @ 2014-06-02 13:42 UTC (permalink / raw)
  To: Pedro Alves, gcc-patches

On 05/27/2014 07:57 AM, Pedro Alves wrote:
> And after the fix for 59195, due to:
>
>   static void
>   d_print_cast (struct d_print_info *dpi, int options,
> 	       const struct demangle_component *dc)
>   {
>   ...
>     /* For a cast operator, we need the template parameters from
>        the enclosing template in scope for processing the type.  */
>     if (dpi->current_template != NULL)
>       {
>         dpt.next = dpi->templates;
>         dpi->templates = &dpt;
>         dpt.template_decl = dpi->current_template;
>       }
>
> when printing the template argument list of A (what should be "<sizeof
> (int)>"), the template parameter 0 (that is, "T_", the '**' above) now
> refers to the first parameter of the the template argument list of the
> 'A' template (the '*' above), exactly what we were already trying to
> print.  This leads to infinite recursion, and stack exaustion.  The
> template parameter 0 should actually refer to the first parameter of
> the 'function_temp' template.

It seems that the problem here is more general; a template argument list 
is not in scope within that same template argument list.  Can't we fix 
that without special-casing conversion ops?

Jason

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-06-02 13:42   ` Jason Merrill
@ 2014-07-24 22:44     ` Cary Coutant
  2014-07-25  9:55       ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Cary Coutant @ 2014-07-24 22:44 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Pedro Alves, gcc-patches

> It seems that the problem here is more general; a template argument list is
> not in scope within that same template argument list.  Can't we fix that
> without special-casing conversion ops?

I think conversion ops really are a special case. It's the only case
where the template parameters refer to the template argument list from
the cast operator's enclosing template. In a cast expression, like
anywhere else you might have a template parameter, the template
parameter refers to the template argument list of the immediately
enclosing template.

I think this note from Section 5.1.3 (Operator Encodings) of the ABI
is what makes this a special case (it's an informative comment in the
document, but seems to me to be normative):

"For a user-defined conversion operator the result type (i.e., the
type to which the operator converts) is part of the mangled name of
the function. If the conversion operator is a member template, the
result type will appear before the template parameters. There may be
forward references in the result type to the template parameters."

-cary

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-07-24 22:44     ` Cary Coutant
@ 2014-07-25  9:55       ` Pedro Alves
  2014-10-13 18:46         ` Cary Coutant
  0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2014-07-25  9:55 UTC (permalink / raw)
  To: Cary Coutant, Jason Merrill; +Cc: gcc-patches

On 07/24/2014 11:35 PM, Cary Coutant wrote:
>> It seems that the problem here is more general; a template argument list is
>> not in scope within that same template argument list.  Can't we fix that
>> without special-casing conversion ops?
> 
> I think conversion ops really are a special case. 

Thanks Cary.  FWIW, I agree.

(GDB 7.8 hasn't been released yet, though it's close.  If this
patch is approved as is, we'll be able to have the crash
fixed there.  If this requires a significant rewrite though,
I'm afraid I might not be able to do it myself anytime soon.)

> It's the only case
> where the template parameters refer to the template argument list from
> the cast operator's enclosing template. In a cast expression, like
> anywhere else you might have a template parameter, the template
> parameter refers to the template argument list of the immediately
> enclosing template.
> 
> I think this note from Section 5.1.3 (Operator Encodings) of the ABI
> is what makes this a special case (it's an informative comment in the
> document, but seems to me to be normative):
> 
> "For a user-defined conversion operator the result type (i.e., the
> type to which the operator converts) is part of the mangled name of
> the function. If the conversion operator is a member template, the
> result type will appear before the template parameters. There may be
> forward references in the result type to the template parameters."
> 

-- 
Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-07-25  9:55       ` Pedro Alves
@ 2014-10-13 18:46         ` Cary Coutant
  2014-11-10 21:45           ` Cary Coutant
  0 siblings, 1 reply; 18+ messages in thread
From: Cary Coutant @ 2014-10-13 18:46 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Jason Merrill, gcc-patches

Ping. Jason, do you still think the special-case for conversion ops is
inappropriate?

-cary


On Fri, Jul 25, 2014 at 2:16 AM, Pedro Alves <palves@redhat.com> wrote:
> On 07/24/2014 11:35 PM, Cary Coutant wrote:
>>> It seems that the problem here is more general; a template argument list is
>>> not in scope within that same template argument list.  Can't we fix that
>>> without special-casing conversion ops?
>>
>> I think conversion ops really are a special case.
>
> Thanks Cary.  FWIW, I agree.
>
> (GDB 7.8 hasn't been released yet, though it's close.  If this
> patch is approved as is, we'll be able to have the crash
> fixed there.  If this requires a significant rewrite though,
> I'm afraid I might not be able to do it myself anytime soon.)
>
>> It's the only case
>> where the template parameters refer to the template argument list from
>> the cast operator's enclosing template. In a cast expression, like
>> anywhere else you might have a template parameter, the template
>> parameter refers to the template argument list of the immediately
>> enclosing template.
>>
>> I think this note from Section 5.1.3 (Operator Encodings) of the ABI
>> is what makes this a special case (it's an informative comment in the
>> document, but seems to me to be normative):
>>
>> "For a user-defined conversion operator the result type (i.e., the
>> type to which the operator converts) is part of the mangled name of
>> the function. If the conversion operator is a member template, the
>> result type will appear before the template parameters. There may be
>> forward references in the result type to the template parameters."
>>
>
> --
> Thanks,
> Pedro Alves
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-10-13 18:46         ` Cary Coutant
@ 2014-11-10 21:45           ` Cary Coutant
  2014-11-12 15:16             ` Jason Merrill
  0 siblings, 1 reply; 18+ messages in thread
From: Cary Coutant @ 2014-11-10 21:45 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Pedro Alves

Ping. I'm getting more reports of this bug internally, and it would be
nice to have the fix upstream.

-cary


On Mon, Oct 13, 2014 at 11:43 AM, Cary Coutant <ccoutant@google.com> wrote:
> Ping. Jason, do you still think the special-case for conversion ops is
> inappropriate?
>
> -cary
>
>
> On Fri, Jul 25, 2014 at 2:16 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 07/24/2014 11:35 PM, Cary Coutant wrote:
>>>> It seems that the problem here is more general; a template argument list is
>>>> not in scope within that same template argument list.  Can't we fix that
>>>> without special-casing conversion ops?
>>>
>>> I think conversion ops really are a special case.
>>
>> Thanks Cary.  FWIW, I agree.
>>
>> (GDB 7.8 hasn't been released yet, though it's close.  If this
>> patch is approved as is, we'll be able to have the crash
>> fixed there.  If this requires a significant rewrite though,
>> I'm afraid I might not be able to do it myself anytime soon.)
>>
>>> It's the only case
>>> where the template parameters refer to the template argument list from
>>> the cast operator's enclosing template. In a cast expression, like
>>> anywhere else you might have a template parameter, the template
>>> parameter refers to the template argument list of the immediately
>>> enclosing template.
>>>
>>> I think this note from Section 5.1.3 (Operator Encodings) of the ABI
>>> is what makes this a special case (it's an informative comment in the
>>> document, but seems to me to be normative):
>>>
>>> "For a user-defined conversion operator the result type (i.e., the
>>> type to which the operator converts) is part of the mangled name of
>>> the function. If the conversion operator is a member template, the
>>> result type will appear before the template parameters. There may be
>>> forward references in the result type to the template parameters."
>>>
>>
>> --
>> Thanks,
>> Pedro Alves
>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters
  2014-11-10 21:45           ` Cary Coutant
@ 2014-11-12 15:16             ` Jason Merrill
  0 siblings, 0 replies; 18+ messages in thread
From: Jason Merrill @ 2014-11-12 15:16 UTC (permalink / raw)
  To: Cary Coutant; +Cc: gcc-patches, Pedro Alves

Sorry I forgot about this for so long.  The patch is OK.

Jason

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-11-12 15:13 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-27 11:57 [PATCH 0/3] mangler/demangler dogfooding Pedro Alves
2014-05-27 11:57 ` [PATCH 1/3] Fix demangler testsuite crashes with CP_DEMANGLE_DEBUG defined Pedro Alves
2014-05-27 13:56   ` Ian Lance Taylor
2014-05-28 22:20     ` Pedro Alves
2014-05-27 11:57 ` [PATCH 2/3] PR other/61321 - demangler crash on casts in template parameters Pedro Alves
2014-05-30 17:37   ` Cary Coutant
2014-05-30 18:05     ` Ian Lance Taylor
2014-05-30 19:00       ` Pedro Alves
2014-06-02 13:42   ` Jason Merrill
2014-07-24 22:44     ` Cary Coutant
2014-07-25  9:55       ` Pedro Alves
2014-10-13 18:46         ` Cary Coutant
2014-11-10 21:45           ` Cary Coutant
2014-11-12 15:16             ` Jason Merrill
2014-05-27 11:57 ` [PATCH 3/3] mangler/demangler dogfooding Pedro Alves
2014-05-27 18:16   ` Mike Stump
2014-05-28 22:31   ` [PATCH v2 " Pedro Alves
2014-06-02 13:38     ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).