[PATCH 0/7] Fix bugs found during demangler fuzz-testing

[PATCH 0/7] Fix bugs found during demangler fuzz-testing

[Mikhail Maltsev]

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

Hi, all!
I performed some fuzz-testing of C++ demangler
(libiberty/cp-demangle.c). The test revealed several bugs, which are
addressed by this series of patches.

Here is a brief description of them:
First one adds a macro CHECK_DEMANGLER. When this macro is defined,
d_peek_next_char and d_advance macros are replaced by inline functions
which perform additional sanity checks and call __builtin_abort when
out-of-bound access is detected.
The second patch fixes a syntax error in debug dump code (it is normally
disabled, unless CP_DEMANGLE_DEBUG is defined).
All other parts fix some errors on invalid input. The attached files
contain a cumulative patch and changelog record.
Bootstrapped and regtested on x86_64-linux.

Some notes:
* Patch 4 adds "#include <limits.h>" to demangler (because of INT_MAX).
I noticed that this file is checked by configury scripts. Do we have
hosts, which do not provide this header? If so, what is the appropriate
replacement for it?
* Testcase "_ZDTtl" (fixed in patch 5) did not actually cause segfault,
but it still invoked undefined behavior (read 1 byte past buffer end).
* Testcase "DpDv1_c" (from patch 7) is now demangled as "(char
__vector(1))..." (it used to segfault). I'm not sure, whether it is
correct or should be rejected.

I have some more test cases, lots of them cause infinite recursion,
because of conversion operator being used as template parameter. Some
are fixed in PR61321, some are not. For example, there are cases when
conversion operator is used as a nested (qualified) name:

_Z1fIN1CcvT_EEvv -> segfault
Presumably this means:
template<typename T>
void f<C::operator T>()

I wonder, if it is possible in valid C++ code?

Notice that the following template instantiation is demangled correctly:
void f<C::operator int>()
_Z1fIN1CcviEEvv -> OK

-- 
Regards,
    Mikhail Maltsev

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

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 2988b6b..4ca285e 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -103,6 +103,7 @@
 #include "config.h"
 #endif
 
+#include <limits.h>
 #include <stdio.h>
 
 #ifdef HAVE_STDLIB_H
@@ -715,7 +716,7 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_FIXED_TYPE:
       printf ("fixed-point type, accum? %d, sat? %d\n",
               dc->u.s_fixed.accum, dc->u.s_fixed.sat);
-      d_dump (dc->u.s_fixed.length, indent + 2)
+      d_dump (dc->u.s_fixed.length, indent + 2);
       break;
     case DEMANGLE_COMPONENT_ARGLIST:
       printf ("argument list\n");
@@ -1599,7 +1600,7 @@ d_source_name (struct d_info *di)
   struct demangle_component *ret;
 
   len = d_number (di);
-  if (len <= 0)
+  if (len <= 0 || len > INT_MAX)
     return NULL;
   ret = d_identifier (di, len);
   di->last_name = ret;
@@ -3166,6 +3167,8 @@ d_expression_1 (struct d_info *di)
       struct demangle_component *type = NULL;
       if (peek == 't')
 	type = cplus_demangle_type (di);
+      if (!d_peek_next_char (di))
+	return NULL;
       d_advance (di, 2);
       return d_make_comp (di, DEMANGLE_COMPONENT_INITIALIZER_LIST,
 			  type, d_exprlist (di, 'E'));
@@ -3240,6 +3243,8 @@ d_expression_1 (struct d_info *di)
 	    struct demangle_component *left;
 	    struct demangle_component *right;
 
+	    if (code == NULL)
+	      return NULL;
 	    if (op_is_new_cast (op))
 	      left = cplus_demangle_type (di);
 	    else
@@ -3267,7 +3272,9 @@ d_expression_1 (struct d_info *di)
 	    struct demangle_component *second;
 	    struct demangle_component *third;
 
-	    if (!strcmp (code, "qu"))
+	    if (code == NULL)
+	      return NULL;
+	    else if (!strcmp (code, "qu"))
 	      {
 		/* ?: expression.  */
 		first = d_expression_1 (di);
@@ -4196,6 +4203,9 @@ d_find_pack (struct d_print_info *dpi,
     case DEMANGLE_COMPONENT_CHARACTER:
     case DEMANGLE_COMPONENT_FUNCTION_PARAM:
     case DEMANGLE_COMPONENT_UNNAMED_TYPE:
+    case DEMANGLE_COMPONENT_FIXED_TYPE:
+    case DEMANGLE_COMPONENT_DEFAULT_ARG:
+    case DEMANGLE_COMPONENT_NUMBER:
       return NULL;
 
     case DEMANGLE_COMPONENT_EXTENDED_OPERATOR:
@@ -4431,6 +4441,11 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
 	    local_name = d_right (typed_name);
 	    if (local_name->type == DEMANGLE_COMPONENT_DEFAULT_ARG)
 	      local_name = local_name->u.s_unary_num.sub;
+	    if (local_name == NULL)
+	      {
+		d_print_error (dpi);
+		return;
+	      }
 	    while (local_name->type == DEMANGLE_COMPONENT_RESTRICT_THIS
 		   || local_name->type == DEMANGLE_COMPONENT_VOLATILE_THIS
 		   || local_name->type == DEMANGLE_COMPONENT_CONST_THIS
diff --git a/libiberty/cp-demangle.h b/libiberty/cp-demangle.h
index 6fce025..c37a91f 100644
--- a/libiberty/cp-demangle.h
+++ b/libiberty/cp-demangle.h
@@ -135,12 +135,41 @@ struct d_info
    - call d_check_char(di, '\0')
    Everything else is safe.  */
 #define d_peek_char(di) (*((di)->n))
-#define d_peek_next_char(di) ((di)->n[1])
-#define d_advance(di, i) ((di)->n += (i))
+#ifndef CHECK_DEMANGLER
+#  define d_peek_next_char(di) ((di)->n[1])
+#  define d_advance(di, i) ((di)->n += (i))
+#endif
 #define d_check_char(di, c) (d_peek_char(di) == c ? ((di)->n++, 1) : 0)
 #define d_next_char(di) (d_peek_char(di) == '\0' ? '\0' : *((di)->n++))
 #define d_str(di) ((di)->n)
 
+/* Define CHECK_DEMANGLER to perform additional sanity checks (i.e., when
+   debugging the demangler).  It will cause some slowdown, but will allow to
+   catch out-of-bound access errors earlier.
+   Note: CHECK_DEMANGLER is not compatible with compilers other that GCC.  */
+#ifdef CHECK_DEMANGLER
+static inline char
+d_peek_next_char (const struct d_info *di)
+{
+  if (!di->n[0])
+    __builtin_abort ();
+  return di->n[1];
+}
+
+static inline void
+d_advance (struct d_info *di, int i)
+{
+  if (i < 0)
+    __builtin_abort ();
+  while (i--)
+    {
+      if (!di->n[0])
+	__builtin_abort ();
+      di->n++;
+    }
+}
+#endif
+
 /* Functions and arrays in cp-demangle.c which are referenced by
    functions in cp-demint.c.  */
 #ifdef IN_GLIBCPP_V3
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 6ea64ae..b58cea2 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4091,6 +4091,36 @@ void g<1>(A<1>&, B<static_cast<bool>(1)>&)
 _ZNKSt7complexIiE4realB5cxx11Ev
 std::complex<int>::real[abi:cxx11]() const
 #
+# Some more crashes revealed by fuzz-testing:
+# Check for NULL pointer when demangling trinary operators
+--format=gnu-v3
+Av32_f
+Av32_f
+# Check range when converting from long to int
+--format=gnu-v3
+_Z11111111111
+_Z11111111111
+# Check out-of-bounds access when decoding braced initializer list
+--format=gnu-v3
+_ZDTtl
+_ZDTtl
+# Check for NULL pointer when demangling DEMANGLE_COMPONENT_LOCAL_NAME
+--format=gnu-v3
+_ZZN1fEEd_lEv
+_ZZN1fEEd_lEv
+# Handle DEMANGLE_COMPONENT_FIXED_TYPE in d_find_pack
+--format=gnu-v3
+DpDFT_
+DpDFT_
+# Likewise, DEMANGLE_COMPONENT_DEFAULT_ARG
+--format=gnu-v3
+DpZ1fEd_
+DpZ1fEd_
+# Likewise, DEMANGLE_COMPONENT_NUMBER (??? result is probably still wrong)
+--format=gnu-v3
+DpDv1_c
+(char __vector(1))...
+#
 # Ada (GNAT) tests.
 #
 # Simple test.

[-- Attachment #3: demangler.clog --]
[-- Type: text/plain, Size: 552 bytes --]

libiberty/ChangeLog:

2015-07-06  Mikhail Maltsev  <maltsevm@gmail.com>

	* cp-demangle.c (d_dump): Fix syntax error.
	(d_source_name): Check range of name length.
	(d_expression_1): Fix out-of-bounds access.  Check code variable for
	NULL before dereferencing it.
	(d_find_pack): Do not recurse for FIXED_TYPE, DEFAULT_ARG and NUMBER.
	(d_print_comp_inner): Add NULL pointer check.
	* cp-demangle.h (d_peek_next_char): Define as inline function when
	CHECK_DEMANGLER is defined.
	(d_advance): Likewise.
	* testsuite/demangle-expected: New testcases.

[PATCH 1/7] Add CHECK_DEMANGLER

[Mikhail Maltsev]

---
 libiberty/cp-demangle.h | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/libiberty/cp-demangle.h b/libiberty/cp-demangle.h
index 6fce025..c37a91f 100644
--- a/libiberty/cp-demangle.h
+++ b/libiberty/cp-demangle.h
@@ -135,12 +135,41 @@ struct d_info
    - call d_check_char(di, '\0')
    Everything else is safe.  */
 #define d_peek_char(di) (*((di)->n))
-#define d_peek_next_char(di) ((di)->n[1])
-#define d_advance(di, i) ((di)->n += (i))
+#ifndef CHECK_DEMANGLER
+#  define d_peek_next_char(di) ((di)->n[1])
+#  define d_advance(di, i) ((di)->n += (i))
+#endif
 #define d_check_char(di, c) (d_peek_char(di) == c ? ((di)->n++, 1) : 0)
 #define d_next_char(di) (d_peek_char(di) == '\0' ? '\0' : *((di)->n++))
 #define d_str(di) ((di)->n)

+/* Define CHECK_DEMANGLER to perform additional sanity checks (i.e., when
+   debugging the demangler).  It will cause some slowdown, but will allow to
+   catch out-of-bound access errors earlier.
+   Note: CHECK_DEMANGLER is not compatible with compilers other than GCC.  */
+#ifdef CHECK_DEMANGLER
+static inline char
+d_peek_next_char (const struct d_info *di)
+{
+  if (!di->n[0])
+    __builtin_abort ();
+  return di->n[1];
+}
+
+static inline void
+d_advance (struct d_info *di, int i)
+{
+  if (i < 0)
+    __builtin_abort ();
+  while (i--)
+    {
+      if (!di->n[0])
+	__builtin_abort ();
+      di->n++;
+    }
+}
+#endif
+
 /* Functions and arrays in cp-demangle.c which are referenced by
    functions in cp-demint.c.  */
 #ifdef IN_GLIBCPP_V3
-- 
1.8.3.1

[PATCH 2/7] Fix build with CP_DEMANGLE_DEBUG

[Mikhail Maltsev]

---
 libiberty/cp-demangle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 2988b6b..12093cc 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -715,7 +715,7 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_FIXED_TYPE:
       printf ("fixed-point type, accum? %d, sat? %d\n",
               dc->u.s_fixed.accum, dc->u.s_fixed.sat);
-      d_dump (dc->u.s_fixed.length, indent + 2)
+      d_dump (dc->u.s_fixed.length, indent + 2);
       break;
     case DEMANGLE_COMPONENT_ARGLIST:
       printf ("argument list\n");
-- 
1.8.3.1

[PATCH 3/7] Fix trinary op

[Mikhail Maltsev]

---
 libiberty/cp-demangle.c               | 4 +++-
 libiberty/testsuite/demangle-expected | 6 ++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 12093cc..44a0a9b 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -3267,7 +3267,9 @@ d_expression_1 (struct d_info *di)
 	    struct demangle_component *second;
 	    struct demangle_component *third;

-	    if (!strcmp (code, "qu"))
+	    if (code == NULL)
+	      return NULL;
+	    else if (!strcmp (code, "qu"))
 	      {
 		/* ?: expression.  */
 		first = d_expression_1 (di);
diff --git a/libiberty/testsuite/demangle-expected
b/libiberty/testsuite/demangle-expected
index 6ea64ae..47ca8f5 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4091,6 +4091,12 @@ void g<1>(A<1>&, B<static_cast<bool>(1)>&)
 _ZNKSt7complexIiE4realB5cxx11Ev
 std::complex<int>::real[abi:cxx11]() const
 #
+# Some more crashes revealed by fuzz-testing:
+# Check for NULL pointer when demangling trinary operators
+--format=gnu-v3
+Av32_f
+Av32_f
+#
 # Ada (GNAT) tests.
 #
 # Simple test.
-- 
1.8.3.1

[PATCH 4/7] Fix int overflow

[Mikhail Maltsev]

---
 libiberty/cp-demangle.c               | 3 ++-
 libiberty/testsuite/demangle-expected | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 44a0a9b..befa6b6 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -103,6 +103,7 @@
 #include "config.h"
 #endif

+#include <limits.h>
 #include <stdio.h>

 #ifdef HAVE_STDLIB_H
@@ -1599,7 +1600,7 @@ d_source_name (struct d_info *di)
   struct demangle_component *ret;

   len = d_number (di);
-  if (len <= 0)
+  if (len <= 0 || len > INT_MAX)
     return NULL;
   ret = d_identifier (di, len);
   di->last_name = ret;
diff --git a/libiberty/testsuite/demangle-expected
b/libiberty/testsuite/demangle-expected
index 47ca8f5..9a8d3db 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4096,6 +4096,10 @@ std::complex<int>::real[abi:cxx11]() const
 --format=gnu-v3
 Av32_f
 Av32_f
+# Check range when converting from long to int
+--format=gnu-v3
+_Z11111111111
+_Z11111111111
 #
 # Ada (GNAT) tests.
 #
-- 
1.8.3.1

[PATCH 5/7] Fix braced-init-list demangling

[Mikhail Maltsev]

---
 libiberty/cp-demangle.c               | 2 ++
 libiberty/testsuite/demangle-expected | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index befa6b6..424b1c5 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -3167,6 +3167,8 @@ d_expression_1 (struct d_info *di)
       struct demangle_component *type = NULL;
       if (peek == 't')
 	type = cplus_demangle_type (di);
+      if (!d_peek_next_char (di))
+	return NULL;
       d_advance (di, 2);
       return d_make_comp (di, DEMANGLE_COMPONENT_INITIALIZER_LIST,
 			  type, d_exprlist (di, 'E'));
diff --git a/libiberty/testsuite/demangle-expected
b/libiberty/testsuite/demangle-expected
index 9a8d3db..2dbab14 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4100,6 +4100,10 @@ Av32_f
 --format=gnu-v3
 _Z11111111111
 _Z11111111111
+# Check out-of-bounds access when decoding braced initializer list
+--format=gnu-v3
+_ZDTtl
+_ZDTtl
 #
 # Ada (GNAT) tests.
 #
-- 
1.8.3.1

[PATCH 6/7] Fix DEMANGLE_COMPONENT_LOCAL_NAME

[Mikhail Maltsev]

---
 libiberty/cp-demangle.c               | 7 +++++++
 libiberty/testsuite/demangle-expected | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 424b1c5..289a704 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -3243,6 +3243,8 @@ d_expression_1 (struct d_info *di)
 	    struct demangle_component *left;
 	    struct demangle_component *right;

+	    if (code == NULL)
+	      return NULL;
 	    if (op_is_new_cast (op))
 	      left = cplus_demangle_type (di);
 	    else
@@ -4436,6 +4438,11 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
 	    local_name = d_right (typed_name);
 	    if (local_name->type == DEMANGLE_COMPONENT_DEFAULT_ARG)
 	      local_name = local_name->u.s_unary_num.sub;
+	    if (local_name == NULL)
+	      {
+		d_print_error (dpi);
+		return;
+	      }
 	    while (local_name->type == DEMANGLE_COMPONENT_RESTRICT_THIS
 		   || local_name->type == DEMANGLE_COMPONENT_VOLATILE_THIS
 		   || local_name->type == DEMANGLE_COMPONENT_CONST_THIS
diff --git a/libiberty/testsuite/demangle-expected
b/libiberty/testsuite/demangle-expected
index 2dbab14..cfa2691 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4104,6 +4104,10 @@ _Z11111111111
 --format=gnu-v3
 _ZDTtl
 _ZDTtl
+# Check for NULL pointer when demangling DEMANGLE_COMPONENT_LOCAL_NAME
+--format=gnu-v3
+_ZZN1fEEd_lEv
+_ZZN1fEEd_lEv
 #
 # Ada (GNAT) tests.
 #
-- 
1.8.3.1

[PATCH 7/7] Fix several crashes in d_find_pack

[Mikhail Maltsev]

---
 libiberty/cp-demangle.c               |  3 +++
 libiberty/testsuite/demangle-expected | 12 ++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 289a704..4ca285e 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -4203,6 +4203,9 @@ d_find_pack (struct d_print_info *dpi,
     case DEMANGLE_COMPONENT_CHARACTER:
     case DEMANGLE_COMPONENT_FUNCTION_PARAM:
     case DEMANGLE_COMPONENT_UNNAMED_TYPE:
+    case DEMANGLE_COMPONENT_FIXED_TYPE:
+    case DEMANGLE_COMPONENT_DEFAULT_ARG:
+    case DEMANGLE_COMPONENT_NUMBER:
       return NULL;

     case DEMANGLE_COMPONENT_EXTENDED_OPERATOR:
diff --git a/libiberty/testsuite/demangle-expected
b/libiberty/testsuite/demangle-expected
index cfa2691..b58cea2 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4108,6 +4108,18 @@ _ZDTtl
 --format=gnu-v3
 _ZZN1fEEd_lEv
 _ZZN1fEEd_lEv
+# Handle DEMANGLE_COMPONENT_FIXED_TYPE in d_find_pack
+--format=gnu-v3
+DpDFT_
+DpDFT_
+# Likewise, DEMANGLE_COMPONENT_DEFAULT_ARG
+--format=gnu-v3
+DpZ1fEd_
+DpZ1fEd_
+# Likewise, DEMANGLE_COMPONENT_NUMBER (??? result is probably still wrong)
+--format=gnu-v3
+DpDv1_c
+(char __vector(1))...
 #
 # Ada (GNAT) tests.
 #
-- 
1.8.3.1

Re: [PATCH 2/7] Fix build with CP_DEMANGLE_DEBUG

[Jeff Law]

On 07/06/2015 01:33 PM, Mikhail Maltsev wrote:
> ---
>   libiberty/cp-demangle.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index 2988b6b..12093cc 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -715,7 +715,7 @@ d_dump (struct demangle_component *dc, int indent)
>       case DEMANGLE_COMPONENT_FIXED_TYPE:
>         printf ("fixed-point type, accum? %d, sat? %d\n",
>                 dc->u.s_fixed.accum, dc->u.s_fixed.sat);
> -      d_dump (dc->u.s_fixed.length, indent + 2)
> +      d_dump (dc->u.s_fixed.length, indent + 2);
>         break;
>       case DEMANGLE_COMPONENT_ARGLIST:
>         printf ("argument list\n");
With a ChangeLog entry, this will obviously be OK.

jeff

Re: [PATCH 1/7] Add CHECK_DEMANGLER

[Jeff Law]

On 07/06/2015 01:31 PM, Mikhail Maltsev wrote:
> ---
>   libiberty/cp-demangle.h | 33 +++++++++++++++++++++++++++++++--
>   1 file changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/libiberty/cp-demangle.h b/libiberty/cp-demangle.h
> index 6fce025..c37a91f 100644
> --- a/libiberty/cp-demangle.h
> +++ b/libiberty/cp-demangle.h
This part is OK with a ChangeLog entry as well.

jeff

Re: [PATCH 4/7] Fix int overflow

[Jeff Law]

On 07/06/2015 01:36 PM, Mikhail Maltsev wrote:
> ---
>   libiberty/cp-demangle.c               | 3 ++-
>   libiberty/testsuite/demangle-expected | 4 ++++
>   2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index 44a0a9b..befa6b6 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -103,6 +103,7 @@
>   #include "config.h"
>   #endif
>
> +#include <limits.h>
>   #include <stdio.h>
>
>   #ifdef HAVE_STDLIB_H
> @@ -1599,7 +1600,7 @@ d_source_name (struct d_info *di)
>     struct demangle_component *ret;
>
>     len = d_number (di);
> -  if (len <= 0)
> +  if (len <= 0 || len > INT_MAX)
>       return NULL;
>     ret = d_identifier (di, len);
>     di->last_name = ret;
Isn't this only helpful if sizeof (long) > sizeof (int)?  Otherwise the 
compiler is going to eliminate that newly added test, right?

So with that in mind, what happens on i686-unknown-linux with this test?


Jeff

Re: [PATCH 5/7] Fix braced-init-list demangling

[Jeff Law]

On 07/06/2015 01:37 PM, Mikhail Maltsev wrote:
> ---
>   libiberty/cp-demangle.c               | 2 ++
>   libiberty/testsuite/demangle-expected | 4 ++++
>   2 files changed, 6 insertions(+)
>
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index befa6b6..424b1c5 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -3167,6 +3167,8 @@ d_expression_1 (struct d_info *di)
>         struct demangle_component *type = NULL;
>         if (peek == 't')
>   	type = cplus_demangle_type (di);
> +      if (!d_peek_next_char (di))
> +	return NULL;
>         d_advance (di, 2);
>         return d_make_comp (di, DEMANGLE_COMPONENT_INITIALIZER_LIST,
>   			  type, d_exprlist (di, 'E'));
> diff --git a/libiberty/testsuite/demangle-expected
> b/libiberty/testsuite/demangle-expected
> index 9a8d3db..2dbab14 100644
> --- a/libiberty/testsuite/demangle-expected
> +++ b/libiberty/testsuite/demangle-expected
> @@ -4100,6 +4100,10 @@ Av32_f
>   --format=gnu-v3
>   _Z11111111111
>   _Z11111111111
> +# Check out-of-bounds access when decoding braced initializer list
> +--format=gnu-v3
> +_ZDTtl
> +_ZDTtl
>   #
>   # Ada (GNAT) tests.
>   #
>
This one is OK.
jeff

Re: [PATCH 4/7] Fix int overflow

[Mikhail Maltsev]

On 07.07.2015 1:55, Jeff Law wrote:

>>     len = d_number (di);
>> -  if (len <= 0)
>> +  if (len <= 0 || len > INT_MAX)
>>       return NULL;
>>     ret = d_identifier (di, len);
>>     di->last_name = ret;
> Isn't this only helpful if sizeof (long) > sizeof (int)?  Otherwise the
> compiler is going to eliminate that newly added test, right?
> 
> So with that in mind, what happens on i686-unknown-linux with this test?
> 
> 
> Jeff
> 

Probably it should be fine, because the problem occurred when len became
negative after implicit conversion to int (d_identifier does not check
for negative length, but it does check that length does not exceed total
string length). In this case (i.e. on ILP32 targets) len will not change
sign after conversion to int (because it's a no-op).
I'm not completely sure about compiler warnings, but AFAIR, in multilib
build libiberty is also built for 32-bit target, and I did not get any
additional warnings.

-- 
Regards,
    Mikhail Maltsev

Re: [PATCH 3/7] Fix trinary op

[Jeff Law]

On 07/06/2015 01:34 PM, Mikhail Maltsev wrote:
> ---
>   libiberty/cp-demangle.c               | 4 +++-
>   libiberty/testsuite/demangle-expected | 6 ++++++
>   2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index 12093cc..44a0a9b 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -3267,7 +3267,9 @@ d_expression_1 (struct d_info *di)
>   	    struct demangle_component *second;
>   	    struct demangle_component *third;
>
> -	    if (!strcmp (code, "qu"))
> +	    if (code == NULL)
> +	      return NULL;
> +	    else if (!strcmp (code, "qu"))
>   	      {
>   		/* ?: expression.  */
>   		first = d_expression_1 (di);
> diff --git a/libiberty/testsuite/demangle-expected
> b/libiberty/testsuite/demangle-expected
> index 6ea64ae..47ca8f5 100644
> --- a/libiberty/testsuite/demangle-expected
> +++ b/libiberty/testsuite/demangle-expected
> @@ -4091,6 +4091,12 @@ void g<1>(A<1>&, B<static_cast<bool>(1)>&)
>   _ZNKSt7complexIiE4realB5cxx11Ev
>   std::complex<int>::real[abi:cxx11]() const
>   #
> +# Some more crashes revealed by fuzz-testing:
> +# Check for NULL pointer when demangling trinary operators
> +--format=gnu-v3
> +Av32_f
> +Av32_f
> +#
>   # Ada (GNAT) tests.
>   #
>   # Simple test.
>
OK with a suitable ChangeLog entry.

And a generic question on the testsuite -- presumably it turns on type 
demangling?    I wanted to verify the flow through d_expression_1 was 
what I expected it to be and it took a while to realize that c++filt 
doesn't demangle types by default, thus Av32_f would demangle to Av32_f 
without ever getting into d_expression_1.

jeff

Re: [PATCH 6/7] Fix DEMANGLE_COMPONENT_LOCAL_NAME

[Jeff Law]

On 07/06/2015 01:39 PM, Mikhail Maltsev wrote:
> ---
>   libiberty/cp-demangle.c               | 7 +++++++
>   libiberty/testsuite/demangle-expected | 4 ++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index 424b1c5..289a704 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -3243,6 +3243,8 @@ d_expression_1 (struct d_info *di)
>   	    struct demangle_component *left;
>   	    struct demangle_component *right;
>
> +	    if (code == NULL)
> +	      return NULL;
>   	    if (op_is_new_cast (op))
>   	      left = cplus_demangle_type (di);
>   	    else
> @@ -4436,6 +4438,11 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
>   	    local_name = d_right (typed_name);
>   	    if (local_name->type == DEMANGLE_COMPONENT_DEFAULT_ARG)
>   	      local_name = local_name->u.s_unary_num.sub;
> +	    if (local_name == NULL)
> +	      {
> +		d_print_error (dpi);
> +		return;
> +	      }
>   	    while (local_name->type == DEMANGLE_COMPONENT_RESTRICT_THIS
>   		   || local_name->type == DEMANGLE_COMPONENT_VOLATILE_THIS
>   		   || local_name->type == DEMANGLE_COMPONENT_CONST_THIS
> diff --git a/libiberty/testsuite/demangle-expected
> b/libiberty/testsuite/demangle-expected
> index 2dbab14..cfa2691 100644
> --- a/libiberty/testsuite/demangle-expected
> +++ b/libiberty/testsuite/demangle-expected
> @@ -4104,6 +4104,10 @@ _Z11111111111
>   --format=gnu-v3
>   _ZDTtl
>   _ZDTtl
> +# Check for NULL pointer when demangling DEMANGLE_COMPONENT_LOCAL_NAME
> +--format=gnu-v3
> +_ZZN1fEEd_lEv
> +_ZZN1fEEd_lEv
>   #
>   # Ada (GNAT) tests.
>   #
>
Also OK with a suitable ChangeLog entry.

jeff

Re: [PATCH 4/7] Fix int overflow

[Jeff Law]

On 07/06/2015 06:04 PM, Mikhail Maltsev wrote:
> On 07.07.2015 1:55, Jeff Law wrote:
>
>>>      len = d_number (di);
>>> -  if (len <= 0)
>>> +  if (len <= 0 || len > INT_MAX)
>>>        return NULL;
>>>      ret = d_identifier (di, len);
>>>      di->last_name = ret;
>> Isn't this only helpful if sizeof (long) > sizeof (int)?  Otherwise the
>> compiler is going to eliminate that newly added test, right?
>>
>> So with that in mind, what happens on i686-unknown-linux with this test?
>>
>>
>> Jeff
>>
>
> Probably it should be fine, because the problem occurred when len became
> negative after implicit conversion to int (d_identifier does not check
> for negative length, but it does check that length does not exceed total
> string length). In this case (i.e. on ILP32 targets) len will not change
> sign after conversion to int (because it's a no-op).
> I'm not completely sure about compiler warnings, but AFAIR, in multilib
> build libiberty is also built for 32-bit target, and I did not get any
> additional warnings.
You may need -Wtype-limits to see the warning.

I'm not questioning whether or not the test will cause a problem, but 
instead questioning if the test does what you expect it to do on a 32bit 
host.

On a host where sizeof (int) == sizeof (long), that len > INT_MAX test 
is always going to be false.

If you want to do overflow testing, you have to compute len in a wider 
type.  You might consider using "long long" or "int64_t" depending on 
the outcome of a configure test.  Falling back to a simple "long" if the 
host compiler doesn't have "long long" or "int64_t".

Interesting exercise feeding those tests into demangle.com  :-0  A 
suitably interested party might be able to exploit that overflow.


jeff

Re: [PATCH 7/7] Fix several crashes in d_find_pack

[Jeff Law]

On 07/06/2015 01:40 PM, Mikhail Maltsev wrote:
> ---
>   libiberty/cp-demangle.c               |  3 +++
>   libiberty/testsuite/demangle-expected | 12 ++++++++++++
>   2 files changed, 15 insertions(+)
>
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index 289a704..4ca285e 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -4203,6 +4203,9 @@ d_find_pack (struct d_print_info *dpi,
>       case DEMANGLE_COMPONENT_CHARACTER:
>       case DEMANGLE_COMPONENT_FUNCTION_PARAM:
>       case DEMANGLE_COMPONENT_UNNAMED_TYPE:
> +    case DEMANGLE_COMPONENT_FIXED_TYPE:
> +    case DEMANGLE_COMPONENT_DEFAULT_ARG:
> +    case DEMANGLE_COMPONENT_NUMBER:
>         return NULL;
>
>       case DEMANGLE_COMPONENT_EXTENDED_OPERATOR:
> diff --git a/libiberty/testsuite/demangle-expected
> b/libiberty/testsuite/demangle-expected
> index cfa2691..b58cea2 100644
> --- a/libiberty/testsuite/demangle-expected
> +++ b/libiberty/testsuite/demangle-expected
> @@ -4108,6 +4108,18 @@ _ZDTtl
>   --format=gnu-v3
>   _ZZN1fEEd_lEv
>   _ZZN1fEEd_lEv
> +# Handle DEMANGLE_COMPONENT_FIXED_TYPE in d_find_pack
> +--format=gnu-v3
> +DpDFT_
> +DpDFT_
> +# Likewise, DEMANGLE_COMPONENT_DEFAULT_ARG
> +--format=gnu-v3
> +DpZ1fEd_
> +DpZ1fEd_
> +# Likewise, DEMANGLE_COMPONENT_NUMBER (??? result is probably still wrong)
> +--format=gnu-v3
> +DpDv1_c
> +(char __vector(1))...
>   #
>   # Ada (GNAT) tests.
>   #
>
OK with a suitable ChangeLog entry.

FWIW, demangler.com doesn't give any results for that case.  It just 
returns DpDv1_c

Jeff

Re: [PATCH 4/7] Fix int overflow

[Mikhail Maltsev]

On 08.07.2015 1:48, Jeff Law wrote:
> I'm not questioning whether or not the test will cause a problem, but
> instead questioning if the test does what you expect it to do on a 32bit
> host.
> 
> On a host where sizeof (int) == sizeof (long), that len > INT_MAX test
> is always going to be false.
Yes, but in this case the call "d_identifier (di, len)" is also safe,
because implicit conversion from long (the type of len variable) to int
(the type of d_identifier's second argument) will never cause overflow.
I first wanted to change one of those types to match the other, but it
turned out that they are used rather consistently: all the code that
works with demangle_component.u.s_number.number uses type long (because
strtol was used for string-to-integer conversion before r73788), and
ints are used for lengths of various identifiers. Should I try to
refactor it somehow?

> If you want to do overflow testing, you have to compute len in a wider
> type.  You might consider using "long long" or "int64_t" depending on
> the outcome of a configure test.  Falling back to a simple "long" if the
> host compiler doesn't have "long long" or "int64_t".
So, it's probably vice-versa: the test is only needed if long is wider
than int.

> And a generic question on the testsuite -- presumably it turns on type demangling?
> I wanted to verify the flow through d_expression_1 was what I expected
> it to be and it took a while to realize that c++filt doesn't demangle types by
> default, thus Av32_f would demangle to Av32_f without ever getting into d_expression_1. 
Yes, the testsuite is based on libiberty/testsuite/test-demangle.c (OMG,
there are still K&R-style definitions there), and it uses (DMGL_PARAMS |
DMGL_ANSI | DMGL_TYPES) by default.

(snip)
> FWIW, demangler.com doesn't give any results for that case.  It just returns DpDv1_c 
But "_Z1fDpDv1_c" makes it crash :)

-- 
Regards,
    Mikhail Maltsev

Re: [PATCH 4/7] Fix int overflow

[Ian Lance Taylor]

On Mon, Jul 6, 2015 at 12:36 PM, Mikhail Maltsev <maltsevm@gmail.com> wrote:
>
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index 44a0a9b..befa6b6 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -103,6 +103,7 @@
>  #include "config.h"
>  #endif
>
> +#include <limits.h>

All existing uses of limits.h in libiberty are inside #ifdef
HAVE_LIMITS_H.  See other files in the directory.


> @@ -1599,7 +1600,7 @@ d_source_name (struct d_info *di)
>    struct demangle_component *ret;
>
>    len = d_number (di);
> -  if (len <= 0)
> +  if (len <= 0 || len > INT_MAX)
>      return NULL;

This is not, in my opinion, the best way to write this kind of thing.
Instead, write something like

    int ilen;


    ilen = (int) len:
    if ((long) ilen != len)
      return NULL;


But better still is to consider the larger context.  We want the
demangler to work the same on all hosts, if at all possible.
d_identifier is called exactly once.  Change it to take a parameter of
type long.  Don't worry about changing d_source_name.

Then look at the fact that d_number does not check for overflow.  We
should consider changing d_number to limit itself to 32-bit integers,
and to return an error indication on overflow.  From a quick glance I
don't see any need for the demangler to support numbers larger than 32
bits.  I think it's OK if we fail to demangle symbol names that are
more than 2 billion characters long.

Ian

Re: [PATCH 3/7] Fix trinary op

[Ian Lance Taylor]

On Tue, Jul 7, 2015 at 3:40 PM, Jeff Law <law@redhat.com> wrote:
>
> And a generic question on the testsuite -- presumably it turns on type
> demangling?    I wanted to verify the flow through d_expression_1 was what I
> expected it to be and it took a while to realize that c++filt doesn't
> demangle types by default, thus Av32_f would demangle to Av32_f without ever
> getting into d_expression_1.

The testsuite passes DMGL_TYPES to the demangler (see
libiberty/testsuite/test-demangle.c).  The c++filt program does not
use DMGL_TYPES by defaut (you can turn it on with the -t option).

I don't know of anybody who actually uses the DMGL_TYPES support.  I
don't know why anybody would.

Ian

Re: [PATCH 3/7] Fix trinary op

[Tom Tromey]

>>>>> "Ian" == Ian Lance Taylor <iant@google.com> writes:

Ian> I don't know of anybody who actually uses the DMGL_TYPES support.  I
Ian> don't know why anybody would.

It's used in gdb's DWARF reader, though I no longer remember why.

Tom

Re: [PATCH 3/7] Fix trinary op

[Ian Lance Taylor]

On Wed, Jul 8, 2015 at 6:42 AM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Ian" == Ian Lance Taylor <iant@google.com> writes:
>
> Ian> I don't know of anybody who actually uses the DMGL_TYPES support.  I
> Ian> don't know why anybody would.
>
> It's used in gdb's DWARF reader, though I no longer remember why.

Looking at the code briefly, I bet everything would keep working if
the DMGL_TYPES were simply removed.

Ian

Re: [PATCH 3/7] Fix trinary op

[Mikhail Maltsev]

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

On 08.07.2015 13:55, Ian Lance Taylor wrote:
> I don't know of anybody who actually uses the DMGL_TYPES support.  I
> don't know why anybody would.
> 
> Ian
Thanks for pointing that out. I updated the testcases, so that now they
don't depend on DMGL_TYPES being used.

> But better still is to consider the larger context.  We want the
> demangler to work the same on all hosts, if at all possible.
> d_identifier is called exactly once.  Change it to take a parameter of
> type long.  Don't worry about changing d_source_name.
Fixed.

> Then look at the fact that d_number does not check for overflow.  We
> should consider changing d_number to limit itself to 32-bit integers,
> and to return an error indication on overflow.  From a quick glance I
> don't see any need for the demangler to support numbers larger than 32
> bits.  I think it's OK if we fail to demangle symbol names that are
> more than 2 billion characters long.
OK, but I think it'll be better to fix that in a separate patch.

The attached patch includes the changes mentioned above, there is also a
small change: I moved the comment for CHECK_DEMANGLER macro to
cp-demangle.c (it already contains a comment for other similar macros)
and replaced __builtin_abort() with abort(). For some reason I thought
that it might need an additional #include, but in reality libiberty (and
the demangler too) already use abort().
The changelog is also attached. OK for trunk after regtest?

-- 
Regards,
    Mikhail Maltsev

[-- Attachment #2: demangler.clog --]
[-- Type: text/plain, Size: 570 bytes --]

libiberty/ChangeLog:

2015-07-10  Mikhail Maltsev  <maltsevm@gmail.com>

	* cp-demangle.c (d_dump): Fix syntax error.
	(d_identifier): Adjust type of len to match d_source_name.
	(d_expression_1): Fix out-of-bounds access.  Check code variable for
	NULL before dereferencing it.
	(d_find_pack): Do not recurse for FIXED_TYPE, DEFAULT_ARG and NUMBER.
	(d_print_comp_inner): Add NULL pointer check.
	* cp-demangle.h (d_peek_next_char): Define as inline function when
	CHECK_DEMANGLER is defined.
	(d_advance): Likewise.
	* testsuite/demangle-expected: Add new testcases.


[-- Attachment #3: cumulative2.patch --]
[-- Type: text/plain, Size: 6204 bytes --]

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 2988b6b..8254100 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -93,7 +93,11 @@
    CP_DEMANGLE_DEBUG
       If defined, turns on debugging mode, which prints information on
       stdout about the mangled string.  This is not generally useful.
-*/
+
+   CHECK_DEMANGLER
+      If defined, additional sanity checks will be performed.  It will
+      cause some slowdown, but will allow to catch out-of-bound access
+      errors earlier.  This macro is intended for testing and debugging.  */
 
 #if defined (_AIX) && !defined (__GNUC__)
  #pragma alloca
@@ -419,7 +423,7 @@ static struct demangle_component *d_source_name (struct d_info *);
 
 static long d_number (struct d_info *);
 
-static struct demangle_component *d_identifier (struct d_info *, int);
+static struct demangle_component *d_identifier (struct d_info *, long);
 
 static struct demangle_component *d_operator_name (struct d_info *);
 
@@ -715,7 +719,7 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_FIXED_TYPE:
       printf ("fixed-point type, accum? %d, sat? %d\n",
               dc->u.s_fixed.accum, dc->u.s_fixed.sat);
-      d_dump (dc->u.s_fixed.length, indent + 2)
+      d_dump (dc->u.s_fixed.length, indent + 2);
       break;
     case DEMANGLE_COMPONENT_ARGLIST:
       printf ("argument list\n");
@@ -1656,7 +1660,7 @@ d_number_component (struct d_info *di)
 /* identifier ::= <(unqualified source code identifier)>  */
 
 static struct demangle_component *
-d_identifier (struct d_info *di, int len)
+d_identifier (struct d_info *di, long len)
 {
   const char *name;
 
@@ -1677,7 +1681,7 @@ d_identifier (struct d_info *di, int len)
   /* Look for something which looks like a gcc encoding of an
      anonymous namespace, and replace it with a more user friendly
      name.  */
-  if (len >= (int) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2
+  if (len >= (long) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2
       && memcmp (name, ANONYMOUS_NAMESPACE_PREFIX,
 		 ANONYMOUS_NAMESPACE_PREFIX_LEN) == 0)
     {
@@ -3166,6 +3170,8 @@ d_expression_1 (struct d_info *di)
       struct demangle_component *type = NULL;
       if (peek == 't')
 	type = cplus_demangle_type (di);
+      if (!d_peek_next_char (di))
+	return NULL;
       d_advance (di, 2);
       return d_make_comp (di, DEMANGLE_COMPONENT_INITIALIZER_LIST,
 			  type, d_exprlist (di, 'E'));
@@ -3240,6 +3246,8 @@ d_expression_1 (struct d_info *di)
 	    struct demangle_component *left;
 	    struct demangle_component *right;
 
+	    if (code == NULL)
+	      return NULL;
 	    if (op_is_new_cast (op))
 	      left = cplus_demangle_type (di);
 	    else
@@ -3267,7 +3275,9 @@ d_expression_1 (struct d_info *di)
 	    struct demangle_component *second;
 	    struct demangle_component *third;
 
-	    if (!strcmp (code, "qu"))
+	    if (code == NULL)
+	      return NULL;
+	    else if (!strcmp (code, "qu"))
 	      {
 		/* ?: expression.  */
 		first = d_expression_1 (di);
@@ -4196,6 +4206,9 @@ d_find_pack (struct d_print_info *dpi,
     case DEMANGLE_COMPONENT_CHARACTER:
     case DEMANGLE_COMPONENT_FUNCTION_PARAM:
     case DEMANGLE_COMPONENT_UNNAMED_TYPE:
+    case DEMANGLE_COMPONENT_FIXED_TYPE:
+    case DEMANGLE_COMPONENT_DEFAULT_ARG:
+    case DEMANGLE_COMPONENT_NUMBER:
       return NULL;
 
     case DEMANGLE_COMPONENT_EXTENDED_OPERATOR:
@@ -4431,6 +4444,11 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
 	    local_name = d_right (typed_name);
 	    if (local_name->type == DEMANGLE_COMPONENT_DEFAULT_ARG)
 	      local_name = local_name->u.s_unary_num.sub;
+	    if (local_name == NULL)
+	      {
+		d_print_error (dpi);
+		return;
+	      }
 	    while (local_name->type == DEMANGLE_COMPONENT_RESTRICT_THIS
 		   || local_name->type == DEMANGLE_COMPONENT_VOLATILE_THIS
 		   || local_name->type == DEMANGLE_COMPONENT_CONST_THIS
diff --git a/libiberty/cp-demangle.h b/libiberty/cp-demangle.h
index 6fce025..197883e 100644
--- a/libiberty/cp-demangle.h
+++ b/libiberty/cp-demangle.h
@@ -135,12 +135,37 @@ struct d_info
    - call d_check_char(di, '\0')
    Everything else is safe.  */
 #define d_peek_char(di) (*((di)->n))
-#define d_peek_next_char(di) ((di)->n[1])
-#define d_advance(di, i) ((di)->n += (i))
+#ifndef CHECK_DEMANGLER
+#  define d_peek_next_char(di) ((di)->n[1])
+#  define d_advance(di, i) ((di)->n += (i))
+#endif
 #define d_check_char(di, c) (d_peek_char(di) == c ? ((di)->n++, 1) : 0)
 #define d_next_char(di) (d_peek_char(di) == '\0' ? '\0' : *((di)->n++))
 #define d_str(di) ((di)->n)
 
+#ifdef CHECK_DEMANGLER
+static inline char
+d_peek_next_char (const struct d_info *di)
+{
+  if (!di->n[0])
+    abort ();
+  return di->n[1];
+}
+
+static inline void
+d_advance (struct d_info *di, int i)
+{
+  if (i < 0)
+    abort ();
+  while (i--)
+    {
+      if (!di->n[0])
+	abort ();
+      di->n++;
+    }
+}
+#endif
+
 /* Functions and arrays in cp-demangle.c which are referenced by
    functions in cp-demint.c.  */
 #ifdef IN_GLIBCPP_V3
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 6ea64ae..4c6359e 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4091,6 +4091,36 @@ void g<1>(A<1>&, B<static_cast<bool>(1)>&)
 _ZNKSt7complexIiE4realB5cxx11Ev
 std::complex<int>::real[abi:cxx11]() const
 #
+# Some more crashes revealed by fuzz-testing:
+# Check for NULL pointer when demangling trinary operators
+--format=gnu-v3
+_Z1fAv32_f
+_Z1fAv32_f
+# Do not overflow when decoding identifier length
+--format=gnu-v3
+_Z11111111111
+_Z11111111111
+# Check out-of-bounds access when decoding braced initializer list
+--format=gnu-v3
+_ZDTtl
+_ZDTtl
+# Check for NULL pointer when demangling DEMANGLE_COMPONENT_LOCAL_NAME
+--format=gnu-v3
+_ZZN1fEEd_lEv
+_ZZN1fEEd_lEv
+# Handle DEMANGLE_COMPONENT_FIXED_TYPE in d_find_pack
+--format=gnu-v3
+_Z1fDpDFT_
+_Z1fDpDFT_
+# Likewise, DEMANGLE_COMPONENT_DEFAULT_ARG
+--format=gnu-v3
+_Z1fIDpZ1fEd_E
+_Z1fIDpZ1fEd_E
+# Likewise, DEMANGLE_COMPONENT_NUMBER
+--format=gnu-v3
+_Z1fDpDv1_c
+f((char __vector(1))...)
+#
 # Ada (GNAT) tests.
 #
 # Simple test.

Re: [PATCH 3/7] Fix trinary op

[Jeff Law]

On 07/09/2015 10:48 PM, Mikhail Maltsev wrote:
> On 08.07.2015 13:55, Ian Lance Taylor wrote:
>> I don't know of anybody who actually uses the DMGL_TYPES support.  I
>> don't know why anybody would.
>>
>> Ian
> Thanks for pointing that out. I updated the testcases, so that now they
> don't depend on DMGL_TYPES being used.
>
>> But better still is to consider the larger context.  We want the
>> demangler to work the same on all hosts, if at all possible.
>> d_identifier is called exactly once.  Change it to take a parameter of
>> type long.  Don't worry about changing d_source_name.
> Fixed.
>
>> Then look at the fact that d_number does not check for overflow.  We
>> should consider changing d_number to limit itself to 32-bit integers,
>> and to return an error indication on overflow.  From a quick glance I
>> don't see any need for the demangler to support numbers larger than 32
>> bits.  I think it's OK if we fail to demangle symbol names that are
>> more than 2 billion characters long.
> OK, but I think it'll be better to fix that in a separate patch.
>
> The attached patch includes the changes mentioned above, there is also a
> small change: I moved the comment for CHECK_DEMANGLER macro to
> cp-demangle.c (it already contains a comment for other similar macros)
> and replaced __builtin_abort() with abort(). For some reason I thought
> that it might need an additional #include, but in reality libiberty (and
> the demangler too) already use abort().
> The changelog is also attached. OK for trunk after regtest?

OK after regression testing.

jeff

Re: [PATCH 3/7] Fix trinary op

[Mikhail Maltsev]

On 07/10/2015 11:44 PM, Jeff Law wrote:
> 
> OK after regression testing.
> 
> jeff
> 
Bootstrapped and regtested on x86_64-unknown-linux-gnu. Applied as r225727.

-- 
Regards,
    Mikhail Maltsev