Re: Fix for PR70498 in Libiberty Demangler
On 05/01/2016 06:24 PM, Marcel Böhme wrote: Please attach it (text/plain) instead. Done. That still seemed to be inlined, but I managed to apply this version. Committed. Bernd
Re: Fix for PR70498 in Libiberty Demangler
> On 28 Apr 2016, at 12:28 AM, Bernd Schmidt wrote: > > Please attach it (text/plain) instead. Done. Index: libiberty/ChangeLog === --- libiberty/ChangeLog (revision 235691) +++ libiberty/ChangeLog (working copy) @@ -1,3 +1,22 @@ +2016-04-16 Marcel Böhme + + PR c++/70498 + * cp-demangle.c: Parse numbers as integer instead of long to avoid + overflow after sanity checks. Include if available. + (INT_MAX): Define if necessary. + (d_make_template_param): Takes integer argument instead of long. + (d_make_function_param): Likewise. + (d_append_num): Likewise. + (d_identifier): Likewise. + (d_number): Parse as and return integer. + (d_compact_number): Handle overflow. + (d_source_name): Change variable type to integer for parsed number. + (d_java_resource): Likewise. + (d_special_name): Likewise. + (d_discriminator): Likewise. + (d_unnamed_type): Likewise. + * testsuite/demangle-expected: Add regression test cases. + 2016-04-30 Oleg Endo * configure: Remove SH5 support. Index: libiberty/cp-demangle.c === --- libiberty/cp-demangle.c (revision 235691) +++ libiberty/cp-demangle.c (working copy) @@ -128,6 +128,13 @@ # endif /* alloca */ #endif /* HAVE_ALLOCA_H */ +#ifdef HAVE_LIMITS_H +#include +#endif +#ifndef INT_MAX +# define INT_MAX (int)(((unsigned int) ~0) >> 1) /* 0x7FFF */ +#endif + #include "ansidecl.h" #include "libiberty.h" #include "demangle.h" @@ -398,7 +405,7 @@ struct demangle_component *); static struct demangle_component * -d_make_template_param (struct d_info *, long); +d_make_template_param (struct d_info *, int); static struct demangle_component * d_make_sub (struct d_info *, const char *, int); @@ -421,9 +428,9 @@ static struct demangle_component *d_source_name (struct d_info *); -static long d_number (struct d_info *); +static int d_number (struct d_info *); -static struct demangle_component *d_identifier (struct d_info *, long); +static struct demangle_component *d_identifier (struct d_info *, int); static struct demangle_component *d_operator_name (struct d_info *); @@ -1119,7 +1126,7 @@ /* Add a new template parameter. */ static struct demangle_component * -d_make_template_param (struct d_info *di, long i) +d_make_template_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1135,7 +1142,7 @@ /* Add a new function parameter. */ static struct demangle_component * -d_make_function_param (struct d_info *di, long i) +d_make_function_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1620,7 +1627,7 @@ static struct demangle_component * d_source_name (struct d_info *di) { - long len; + int len; struct demangle_component *ret; len = d_number (di); @@ -1633,12 +1640,12 @@ /* number ::= [n] <(non-negative decimal integer)> */ -static long +static int d_number (struct d_info *di) { int negative; char peek; - long ret; + int ret; negative = 0; peek = d_peek_char (di); @@ -1681,7 +1688,7 @@ /* identifier ::= <(unqualified source code identifier)> */ static struct demangle_component * -d_identifier (struct d_info *di, long len) +d_identifier (struct d_info *di, int len) { const char *name; @@ -1702,7 +1709,7 @@ /* Look for something which looks like a gcc encoding of an anonymous namespace, and replace it with a more user friendly name. */ - if (len >= (long) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 + if (len >= (int) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 && memcmp (name, ANONYMOUS_NAMESPACE_PREFIX, ANONYMOUS_NAMESPACE_PREFIX_LEN) == 0) { @@ -1870,7 +1877,7 @@ { struct demangle_component *p = NULL; struct demangle_component *next = NULL; - long len, i; + int len, i; char c; const char *str; @@ -2012,7 +2019,7 @@ case 'C': { struct demangle_component *derived_type; - long offset; + int offset; struct demangle_component *base_type; derived_type = cplus_demangle_type (di); @@ -2946,10 +2953,10 @@ /* _ */ -static long +static int d_compact_number (struct d_info *di) { - long num; + int num; if (d_peek_char (di) == '_') num = 0; else if (d_peek_char (di) == 'n') @@ -2957,7 +2964,7 @@ else num = d_number (di) + 1; - if (! d_check_char (di, '_')) + if (num < 0 || ! d_check_char (di, '_')) return -1; return num; } @@ -2969,7 +2976,7 @@ static struct demangle_component * d_template_param (struct d_info *di) { - long param; + int param; if (! d_check_char (di, 'T')) return NULL; @@ -3171,9 +3178,10 @@ } else { - index = d_compact_number (di) + 1; - if (index == 0)
Re: Fix for PR70498 in Libiberty Demangler
On 04/15/2016 07:39 PM, Marcel Böhme wrote: Sure. The updated patch, including Changelog and more test cases. Regression tested. This patch seems seriously damaged by sending it through the email body. Please attach it (text/plain) instead. Bernd
Re: Fix for PR70498 in Libiberty Demangler
Hi Bernd, >>> index = d_compact_number (di) + 1; if (index == 0) return NULL; >>> >>> which probably ought to have the same kind of check (I'll note that >>> at this point we've accumulated two "+1"s, I'll assume that's what >>> we want). >> Yes. There should be an overflow check here. > > Could you update the patch for that? Sure. The updated patch, including Changelog and more test cases. Regression tested. Best regards, - Marcel Index: ChangeLog === --- ChangeLog (revision 235032) +++ ChangeLog (working copy) @@ -1,3 +1,22 @@ +2016-04-16 Marcel Böhme + + PR c++/70498 + * cp-demangle.c: Parse/handle numbers as integer instead of long. + Include if available. + (INT_MAX): Define if necessary. + (d_make_template_param): Takes integer argument instead of long. + (d_make_function_param): Likewise. + (d_append_num): Likewise. + (d_identifier): Likewise. + (d_number): Parse as and return as integer. + (d_compact_number): Handle overflow. + (d_expression_1): Likewise. + (d_source_name): Change variable type to integer for parsed number. + (d_java_resource): Likewise. + (d_special_name): Likewise. + (d_discriminator): Likewise. + (d_unnamed_type): Likewise. + * testsuite/demangle-expected: Add regression test cases. + 2016-04-08 Marcel Böhme PR c++/69687 Index: cp-demangle.c === --- cp-demangle.c (revision 235032) +++ cp-demangle.c (working copy) @@ -128,6 +128,13 @@ extern char *alloca (); # endif /* alloca */ #endif /* HAVE_ALLOCA_H */ +#ifdef HAVE_LIMITS_H +#include +#endif +#ifndef INT_MAX +# define INT_MAX (int)(((unsigned int) ~0) >> 1) /* 0x7FFF */ +#endif + #include "ansidecl.h" #include "libiberty.h" #include "demangle.h" @@ -398,7 +405,7 @@ d_make_dtor (struct d_info *, enum gnu_v struct demangle_component *); static struct demangle_component * -d_make_template_param (struct d_info *, long); +d_make_template_param (struct d_info *, int); static struct demangle_component * d_make_sub (struct d_info *, const char *, int); @@ -421,9 +428,9 @@ static struct demangle_component *d_unqu static struct demangle_component *d_source_name (struct d_info *); -static long d_number (struct d_info *); +static int d_number (struct d_info *); -static struct demangle_component *d_identifier (struct d_info *, long); +static struct demangle_component *d_identifier (struct d_info *, int); static struct demangle_component *d_operator_name (struct d_info *); @@ -1119,7 +1126,7 @@ d_make_dtor (struct d_info *di, enum gnu /* Add a new template parameter. */ static struct demangle_component * -d_make_template_param (struct d_info *di, long i) +d_make_template_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1135,7 +1142,7 @@ d_make_template_param (struct d_info *di /* Add a new function parameter. */ static struct demangle_component * -d_make_function_param (struct d_info *di, long i) +d_make_function_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1620,7 +1627,7 @@ d_unqualified_name (struct d_info *di) static struct demangle_component * d_source_name (struct d_info *di) { - long len; + int len; struct demangle_component *ret; len = d_number (di); @@ -1633,12 +1640,12 @@ d_source_name (struct d_info *di) /* number ::= [n] <(non-negative decimal integer)> */ -static long +static int d_number (struct d_info *di) { int negative; char peek; - long ret; + int ret; negative = 0; peek = d_peek_char (di); @@ -1681,7 +1688,7 @@ d_number_component (struct d_info *di) /* identifier ::= <(unqualified source code identifier)> */ static struct demangle_component * -d_identifier (struct d_info *di, long len) +d_identifier (struct d_info *di, int len) { const char *name; @@ -1702,7 +1709,7 @@ d_identifier (struct d_info *di, long le /* Look for something which looks like a gcc encoding of an anonymous namespace, and replace it with a more user friendly name. */ - if (len >= (long) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 + if (len >= (int) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 && memcmp (name, ANONYMOUS_NAMESPACE_PREFIX, ANONYMOUS_NAMESPACE_PREFIX_LEN) == 0) { @@ -1870,7 +1877,7 @@ d_java_resource (struct d_info *di) { struct demangle_component *p = NULL; struct demangle_component *next = NULL; - long len, i; + int len, i; char c; const char *str; @@ -2012,7 +2019,7 @@ d_special_name (struct d_info *di) case 'C': { struct demangle_component *derived_type; - long offset; + int offset; struct demangle_component *base_type; derived_type = cplus_demangle_type (di); @@
Re: Fix for PR70498 in Libiberty Demangler
On 04/13/2016 03:04 PM, Marcel Böhme wrote: Hi Bernd, Shouldn't we check for overflows before performing the +1 addition (i.e. 0 <= num < INT_MAX)? Ideally we'd also have a way to signal from d_number if we had an overflow while parsing that number. Without an overflow signal, d_number will already be prone to return a negative number for supposedly non-negative numbers (those not preceded with ’n’). In that case an overflow check would be unnecessary in d_compact_number which is supposed to always return a positive number or a negative one (-1). If you decide in favour of an overflow signal, it must be handled by the call-sites. Not sure what the “default behaviour” should be then. Otherwise, we can simply assume that the call sites for d_number can handle negative numbers. Shouldn't we look into fixing d_number eventually so it can signal error? index = d_compact_number (di) + 1; if (index == 0) return NULL; which probably ought to have the same kind of check (I'll note that at this point we've accumulated two "+1"s, I'll assume that's what we want). Yes. There should be an overflow check here. Could you update the patch for that? Bernd
Re: Fix for PR70498 in Libiberty Demangler
Hi Bernd, > -static long > +static int > d_compact_number (struct d_info *di) > { > - long num; > + int num; >if (d_peek_char (di) == '_') > num = 0; >else if (d_peek_char (di) == 'n') > @@ -2957,7 +2957,7 @@ d_compact_number (struct d_info *di) >else > num = d_number (di) + 1; > > - if (! d_check_char (di, '_')) > + if (num < 0 || ! d_check_char (di, '_')) > return -1; >return num; > } > Shouldn't we check for overflows before performing the +1 addition (i.e. 0 <= > num < INT_MAX)? Ideally we'd also have a way to signal from d_number if we > had an overflow while parsing that number. Without an overflow signal, d_number will already be prone to return a negative number for supposedly non-negative numbers (those not preceded with ’n’). In that case an overflow check would be unnecessary in d_compact_number which is supposed to always return a positive number or a negative one (-1). If you decide in favour of an overflow signal, it must be handled by the call-sites. Not sure what the “default behaviour” should be then. Otherwise, we can simply assume that the call sites for d_number can handle negative numbers. Kindly advice. > > There's also this, in d_expression_1: > > index = d_compact_number (di) + 1; > if (index == 0) > return NULL; > > which probably ought to have the same kind of check (I'll note that at this > point we've accumulated two "+1"s, I'll assume that's what we want). Yes. There should be an overflow check here. Thanks! - Marcel
Re: Fix for PR70498 in Libiberty Demangler
I've been looking through this patch. I had intended to commit it, but after looking through it a little more carefully I think there are a few things left to solve. So, d_number/d_compact_number now return ints rather than longs, which makes sense since the lengths in things like struct demangle_component's s_name are integers. However, s_number there is defined as a long, so this does mean a tighter limit for things like d_template_param/d_make_template_param. Cc'ing Jason for an opinion on whether that's a problem or not (I suspect it isn't - t). -static long +static int d_compact_number (struct d_info *di) { - long num; + int num; if (d_peek_char (di) == '_') num = 0; else if (d_peek_char (di) == 'n') @@ -2957,7 +2957,7 @@ d_compact_number (struct d_info *di) else num = d_number (di) + 1; - if (! d_check_char (di, '_')) + if (num < 0 || ! d_check_char (di, '_')) return -1; return num; } Shouldn't we check for overflows before performing the +1 addition (i.e. 0 <= num < INT_MAX)? Ideally we'd also have a way to signal from d_number if we had an overflow while parsing that number. There's also this, in d_expression_1: index = d_compact_number (di) + 1; if (index == 0) return NULL; which probably ought to have the same kind of check (I'll note that at this point we've accumulated two "+1"s, I'll assume that's what we want). Please include a ChangeLog entry with the next patch. Bernd
Re: Fix for PR70498 in Libiberty Demangler
> On 4 Apr 2016, at 9:24 PM, Bernd Schmidt wrote: > >> >> The patch now also accounts for overflows in d_compact_number which >> is supposed to return -1 in case of negative numbers. > > I take it this isn't for the normal 'n' case, but for instances where we > encounter overflows in d_number? Yes. Best regards, - Marcel
Re: Fix for PR70498 in Libiberty Demangler
On 04/02/2016 11:49 AM, Marcel Böhme wrote: Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test cases added + checked PR70498 is resolved. Richard - any objections from an RM perspective to check in such potentially security-related fixes now even if not regressions? The patch now also accounts for overflows in d_compact_number which is supposed to return -1 in case of negative numbers. I take it this isn't for the normal 'n' case, but for instances where we encounter overflows in d_number? Bernd
Re: Fix for PR70498 in Libiberty Demangler
> On 2 Apr 2016, at 1:44 AM, Bernd Schmidt wrote: > > On 04/01/2016 07:41 PM, Pedro Alves wrote: >> On 04/01/2016 11:21 AM, Marcel Böhme wrote: >>> static inline void >>> -d_append_num (struct d_print_info *dpi, long l) >>> +d_append_num (struct d_print_info *dpi, int l) >>> { >>>char buf[25]; >>>sprintf (buf,"%ld", l); >> >> Doesn't this warn about %ld format vs int type mismatch? > > Well spotted. Marcel, please correct this issue and check for other warnings. > Unless libiberty is somehow exempt from -Werror, this should have shown up in > a bootstrap. Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test cases added + checked PR70498 is resolved. The patch now also accounts for overflows in d_compact_number which is supposed to return -1 in case of negative numbers. Thanks, - Marcel -- Index: libiberty/cp-demangle.c === --- libiberty/cp-demangle.c (revision 234663) +++ libiberty/cp-demangle.c (working copy) @@ -398,7 +398,7 @@ d_make_dtor (struct d_info *, enum gnu_v struct demangle_component *); static struct demangle_component * -d_make_template_param (struct d_info *, long); +d_make_template_param (struct d_info *, int); static struct demangle_component * d_make_sub (struct d_info *, const char *, int); @@ -421,9 +421,9 @@ static struct demangle_component *d_unqu static struct demangle_component *d_source_name (struct d_info *); -static long d_number (struct d_info *); +static int d_number (struct d_info *); -static struct demangle_component *d_identifier (struct d_info *, long); +static struct demangle_component *d_identifier (struct d_info *, int); static struct demangle_component *d_operator_name (struct d_info *); @@ -1119,7 +1119,7 @@ d_make_dtor (struct d_info *di, enum gnu /* Add a new template parameter. */ static struct demangle_component * -d_make_template_param (struct d_info *di, long i) +d_make_template_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1135,7 +1135,7 @@ d_make_template_param (struct d_info *di /* Add a new function parameter. */ static struct demangle_component * -d_make_function_param (struct d_info *di, long i) +d_make_function_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1620,7 +1620,7 @@ d_unqualified_name (struct d_info *di) static struct demangle_component * d_source_name (struct d_info *di) { - long len; + int len; struct demangle_component *ret; len = d_number (di); @@ -1633,12 +1633,12 @@ d_source_name (struct d_info *di) /* number ::= [n] <(non-negative decimal integer)> */ -static long +static int d_number (struct d_info *di) { int negative; char peek; - long ret; + int ret; negative = 0; peek = d_peek_char (di); @@ -1681,7 +1681,7 @@ d_number_component (struct d_info *di) /* identifier ::= <(unqualified source code identifier)> */ static struct demangle_component * -d_identifier (struct d_info *di, long len) +d_identifier (struct d_info *di, int len) { const char *name; @@ -1702,7 +1702,7 @@ d_identifier (struct d_info *di, long le /* Look for something which looks like a gcc encoding of an anonymous namespace, and replace it with a more user friendly name. */ - if (len >= (long) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 + if (len >= (int) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 && memcmp (name, ANONYMOUS_NAMESPACE_PREFIX, ANONYMOUS_NAMESPACE_PREFIX_LEN) == 0) { @@ -1870,7 +1870,7 @@ d_java_resource (struct d_info *di) { struct demangle_component *p = NULL; struct demangle_component *next = NULL; - long len, i; + int len, i; char c; const char *str; @@ -2012,7 +2012,7 @@ d_special_name (struct d_info *di) case 'C': { struct demangle_component *derived_type; - long offset; + int offset; struct demangle_component *base_type; derived_type = cplus_demangle_type (di); @@ -2946,10 +2946,10 @@ d_pointer_to_member_type (struct d_info /* _ */ -static long +static int d_compact_number (struct d_info *di) { - long num; + int num; if (d_peek_char (di) == '_') num = 0; else if (d_peek_char (di) == 'n') @@ -2957,7 +2957,7 @@ d_compact_number (struct d_info *di) else num = d_number (di) + 1; - if (! d_check_char (di, '_')) + if (num < 0 || ! d_check_char (di, '_')) return -1; return num; } @@ -2969,7 +2969,7 @@ d_compact_number (struct d_info *di) static struct demangle_component * d_template_param (struct d_info *di) { - long param; + int param; if (! d_check_char (di, 'T')) return NULL; @@ -3502,7 +3502,7 @@ d_local_name (struct d_info *di) static int d_discriminator (struct d_info *di) { - long discrim; + int discrim; if (d_peek_char (di) != '_') return 1; @@ -3558,7 +3558,7 @@ static struct demangle_component * d_unnamed_typ
Re: Fix for PR70498 in Libiberty Demangler
On 04/01/2016 07:41 PM, Pedro Alves wrote: On 04/01/2016 11:21 AM, Marcel Böhme wrote: static inline void -d_append_num (struct d_print_info *dpi, long l) +d_append_num (struct d_print_info *dpi, int l) { char buf[25]; sprintf (buf,"%ld", l); Doesn't this warn about %ld format vs int type mismatch? Well spotted. Marcel, please correct this issue and check for other warnings. Unless libiberty is somehow exempt from -Werror, this should have shown up in a bootstrap. Bernd
Re: Fix for PR70498 in Libiberty Demangler
On 04/01/2016 11:21 AM, Marcel Böhme wrote: > static inline void > -d_append_num (struct d_print_info *dpi, long l) > +d_append_num (struct d_print_info *dpi, int l) > { >char buf[25]; >sprintf (buf,"%ld", l); Doesn't this warn about %ld format vs int type mismatch? Thanks, Pedro Alves
Re: Fix for PR70498 in Libiberty Demangler
On 04/01/2016 03:39 PM, Marcel Böhme wrote: Hi Bernd, Thanks for the feedback! Patches need to be bootstrapped and regression tested, and patch submissions should include which target this was done on. Ideally you'd also want to include testcases along with your patches, although I'm not entirely sure how we can arrange for this type of problem to be tested. Regression tested on x86_64-pc-linux-gnu (make check). Test cases added to libiberty/testsuite/demangler-expected and checked PR70498 is resolved. Not sure how to bootstrap the patch. You configure gcc normally and build it - that should automatically bootstrap, unless you're cross-compiling. You'll have stage1-* and stage2-* directories at the end if that worked. You should then run the testsuite on the bootstrapped compiler. Lastly, for this specific patch, I have trouble seeing how it fixes anything. I'd need a more detailed explanation of how the problem happens in the first place. In the patched version, the values wrap around when they are parsed in d_number. Since the mangled string may contain negative numbers, there is usually proper handling of negative numbers in the clients of d_number. Without the patch a value can become negative when cast from long to int *after* these checks. For instance, in d_source_name the length of the identifier is parsed as long from the mangled string and checked whether it is negative. Since d_identifier takes an int as length, d_identifier is called with a negative length after the implicit cast: Ok, I think I see it. Guess I'll queue this up and commit it for you in the next few days. Bernd
Re: Fix for PR70498 in Libiberty Demangler
> Since d_identifier takes an int as length, d_identifier is called with a > negative length after the implicit cast: Sorry, d_make_name called from d_identifier in cp_demangle.c:1721 takes an int as length. Best regards - Marcel
Re: Fix for PR70498 in Libiberty Demangler
Hi Bernd, Thanks for the feedback! > Patches need to be bootstrapped and regression tested, and patch submissions > should include which target this was done on. > > Ideally you'd also want to include testcases along with your patches, > although I'm not entirely sure how we can arrange for this type of problem to > be tested. Regression tested on x86_64-pc-linux-gnu (make check). Test cases added to libiberty/testsuite/demangler-expected and checked PR70498 is resolved. Not sure how to bootstrap the patch. > Lastly, for this specific patch, I have trouble seeing how it fixes anything. > I'd need a more detailed explanation of how the problem happens in the first > place. In the patched version, the values wrap around when they are parsed in d_number. Since the mangled string may contain negative numbers, there is usually proper handling of negative numbers in the clients of d_number. Without the patch a value can become negative when cast from long to int *after* these checks. For instance, in d_source_name the length of the identifier is parsed as long from the mangled string and checked whether it is negative. Since d_identifier takes an int as length, d_identifier is called with a negative length after the implicit cast: static struct demangle_component * d_source_name (struct d_info *di) { int len; struct demangle_component *ret; len = d_number (di); if (len <= 0) return NULL; ret = d_identifier (di, len); di->last_name = ret; return ret; } -- Index: libiberty/cp-demangle.c === --- libiberty/cp-demangle.c (revision 234663) +++ libiberty/cp-demangle.c (working copy) @@ -398,7 +398,7 @@ struct demangle_component *); static struct demangle_component * -d_make_template_param (struct d_info *, long); +d_make_template_param (struct d_info *, int); static struct demangle_component * d_make_sub (struct d_info *, const char *, int); @@ -421,9 +421,9 @@ static struct demangle_component *d_source_name (struct d_info *); -static long d_number (struct d_info *); +static int d_number (struct d_info *); -static struct demangle_component *d_identifier (struct d_info *, long); +static struct demangle_component *d_identifier (struct d_info *, int); static struct demangle_component *d_operator_name (struct d_info *); @@ -1119,7 +1119,7 @@ /* Add a new template parameter. */ static struct demangle_component * -d_make_template_param (struct d_info *di, long i) +d_make_template_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1135,7 +1135,7 @@ /* Add a new function parameter. */ static struct demangle_component * -d_make_function_param (struct d_info *di, long i) +d_make_function_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1620,7 +1620,7 @@ static struct demangle_component * d_source_name (struct d_info *di) { - long len; + int len; struct demangle_component *ret; len = d_number (di); @@ -1633,12 +1633,12 @@ /* number ::= [n] <(non-negative decimal integer)> */ -static long +static int d_number (struct d_info *di) { int negative; char peek; - long ret; + int ret; negative = 0; peek = d_peek_char (di); @@ -1681,7 +1681,7 @@ /* identifier ::= <(unqualified source code identifier)> */ static struct demangle_component * -d_identifier (struct d_info *di, long len) +d_identifier (struct d_info *di, int len) { const char *name; @@ -1702,7 +1702,7 @@ /* Look for something which looks like a gcc encoding of an anonymous namespace, and replace it with a more user friendly name. */ - if (len >= (long) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 + if (len >= ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 && memcmp (name, ANONYMOUS_NAMESPACE_PREFIX, ANONYMOUS_NAMESPACE_PREFIX_LEN) == 0) { @@ -1870,7 +1870,7 @@ { struct demangle_component *p = NULL; struct demangle_component *next = NULL; - long len, i; + int len, i; char c; const char *str; @@ -2012,7 +2012,7 @@ case 'C': { struct demangle_component *derived_type; - long offset; + int offset; struct demangle_component *base_type; derived_type = cplus_demangle_type (di); @@ -2946,10 +2946,10 @@ /* _ */ -static long +static int d_compact_number (struct d_info *di) { - long num; + int num; if (d_peek_char (di) == '_') num = 0; else if (d_peek_char (di) == 'n') @@ -2969,7 +2969,7 @@ static struct demangle_component * d_template_param (struct d_info *di) { - long param; + int param; if (! d_check_char (di, 'T')) return NULL; @@ -3502,7 +3502,7 @@ static int d_discriminator (struct d_info *di) { - long discrim; + int discrim; if (d_peek_char (di) != '_') return 1; @@ -3558,7 +3558,7 @@ d_unnamed_type (struct d_info *di) { struct demangle_component *ret; - long num; + int num; if (! d_check_char (di, 'U')) return NULL; @@ -4086,7 +4086,7 @@
Re: Fix for PR70498 in Libiberty Demangler
On 04/01/2016 12:21 PM, Marcel Böhme wrote: This fixes the write access violation detailed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70498 (and a few other unreported cases). Sometimes length-variables for strings and arrays are of type long other times of type int. Since cp-demangle.h exports structs and methods with length-variables of type int, this has been made consistent in cp-demangle.c. Patches need to be bootstrapped and regression tested, and patch submissions should include which target this was done on. Ideally you'd also want to include testcases along with your patches, although I'm not entirely sure how we can arrange for this type of problem to be tested. Lastly, for this specific patch, I have trouble seeing how it fixes anything. I'd need a more detailed explanation of how the problem happens in the first place. Bernd
Fix for PR70498 in Libiberty Demangler
Hi, This fixes the write access violation detailed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70498 (and a few other unreported cases). Sometimes length-variables for strings and arrays are of type long other times of type int. Since cp-demangle.h exports structs and methods with length-variables of type int, this has been made consistent in cp-demangle.c. Best regards, - Marcel Index: libiberty/cp-demangle.c === --- libiberty/cp-demangle.c (revision 234663) +++ libiberty/cp-demangle.c (working copy) @@ -398,7 +398,7 @@ struct demangle_component *); static struct demangle_component * -d_make_template_param (struct d_info *, long); +d_make_template_param (struct d_info *, int); static struct demangle_component * d_make_sub (struct d_info *, const char *, int); @@ -421,9 +421,9 @@ static struct demangle_component *d_source_name (struct d_info *); -static long d_number (struct d_info *); +static int d_number (struct d_info *); -static struct demangle_component *d_identifier (struct d_info *, long); +static struct demangle_component *d_identifier (struct d_info *, int); static struct demangle_component *d_operator_name (struct d_info *); @@ -1119,7 +1119,7 @@ /* Add a new template parameter. */ static struct demangle_component * -d_make_template_param (struct d_info *di, long i) +d_make_template_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1135,7 +1135,7 @@ /* Add a new function parameter. */ static struct demangle_component * -d_make_function_param (struct d_info *di, long i) +d_make_function_param (struct d_info *di, int i) { struct demangle_component *p; @@ -1620,7 +1620,7 @@ static struct demangle_component * d_source_name (struct d_info *di) { - long len; + int len; struct demangle_component *ret; len = d_number (di); @@ -1633,12 +1633,12 @@ /* number ::= [n] <(non-negative decimal integer)> */ -static long +static int d_number (struct d_info *di) { int negative; char peek; - long ret; + int ret; negative = 0; peek = d_peek_char (di); @@ -1681,7 +1681,7 @@ /* identifier ::= <(unqualified source code identifier)> */ static struct demangle_component * -d_identifier (struct d_info *di, long len) +d_identifier (struct d_info *di, int len) { const char *name; @@ -1702,7 +1702,7 @@ /* Look for something which looks like a gcc encoding of an anonymous namespace, and replace it with a more user friendly name. */ - if (len >= (long) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 + if (len >= ANONYMOUS_NAMESPACE_PREFIX_LEN + 2 && memcmp (name, ANONYMOUS_NAMESPACE_PREFIX, ANONYMOUS_NAMESPACE_PREFIX_LEN) == 0) { @@ -1870,7 +1870,7 @@ { struct demangle_component *p = NULL; struct demangle_component *next = NULL; - long len, i; + int len, i; char c; const char *str; @@ -2012,7 +2012,7 @@ case 'C': { struct demangle_component *derived_type; - long offset; + int offset; struct demangle_component *base_type; derived_type = cplus_demangle_type (di); @@ -2946,10 +2946,10 @@ /* _ */ -static long +static int d_compact_number (struct d_info *di) { - long num; + int num; if (d_peek_char (di) == '_') num = 0; else if (d_peek_char (di) == 'n') @@ -2969,7 +2969,7 @@ static struct demangle_component * d_template_param (struct d_info *di) { - long param; + int param; if (! d_check_char (di, 'T')) return NULL; @@ -3502,7 +3502,7 @@ static int d_discriminator (struct d_info *di) { - long discrim; + int discrim; if (d_peek_char (di) != '_') return 1; @@ -3558,7 +3558,7 @@ d_unnamed_type (struct d_info *di) { struct demangle_component *ret; - long num; + int num; if (! d_check_char (di, 'U')) return NULL; @@ -4086,7 +4086,7 @@ } static inline void -d_append_num (struct d_print_info *dpi, long l) +d_append_num (struct d_print_info *dpi, int l) { char buf[25]; sprintf (buf,"%ld", l);