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

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. > >

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

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

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

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

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

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)

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

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

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

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

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