Re: Fix fir PR71696 in Libiberty Demangler (6)

2016-08-04 Thread Marcel Böhme
   is_proctypevec = 1;
+ push_processed_type (work, n);
+ remembered_type = work->typevec[n];
  mangled = &remembered_type;
}
  break;
@@ -3850,6 +3877,9 @@ do_type (struct work_stuff *work, const char **man
 string_delete (result);
   string_delete (&decl);
 
+  if (is_proctypevec)
+pop_processed_type (work); 
+
   if (success)
 /* Assume an integral type, if we're not sure.  */
 return (int) ((tk == tk_none) ? tk_integral : tk);
@@ -4263,6 +4293,41 @@ do_arg (struct work_stuff *work, const char **mang
 }
 
 static void
+push_processed_type (struct work_stuff *work, int typevec_index)
+{
+  if (work->nproctypes >= work->proctypevec_size)
+{
+  if (!work->proctypevec_size)
+   {
+ work->proctypevec_size = 4;
+ work->proctypevec = XNEWVEC (int, work->proctypevec_size);
+   }
+  else 
+   {
+ if (work->proctypevec_size < 16)
+   /* Double when small.  */
+   work->proctypevec_size *= 2;
+ else
+   {
+ /* Grow slower when large.  */
+ if (work->proctypevec_size > (INT_MAX / 3) * 2)
+xmalloc_failed (INT_MAX);
+  work->proctypevec_size = (work->proctypevec_size * 3 / 2);
+   }   
+  work->proctypevec
+= XRESIZEVEC (int, work->proctypevec, work->proctypevec_size);
+   }
+}
+work->proctypevec [work->nproctypes++] = typevec_index;
+}
+
+static void
+pop_processed_type (struct work_stuff *work)
+{
+  work->nproctypes--;
+}
+
+static void
 remember_type (struct work_stuff *work, const char *start, int len)
 {
   char *tem;
@@ -4526,10 +4591,13 @@ demangle_args (struct work_stuff *work, const char
{
  string_append (declp, ", ");
}
+ push_processed_type (work, t);  
  if (!do_arg (work, &tem, &arg))
{
+ pop_processed_type (work);
  return (0);
}
+ pop_processed_type (work);
  if (PRINT_ARG_TYPES)
{
  string_appends (declp, &arg);
Index: libiberty/testsuite/demangle-expected
===
--- libiberty/testsuite/demangle-expected   (revision 239112)
+++ libiberty/testsuite/demangle-expected   (working copy)
@@ -4587,3 +4587,8 @@ _Z808
 
 __t2m05B50_
 __t2m05B50_
+#
+# Tests stack overflow PR71696
+
+__10%0__S4_0T0T0
+%0<>::%0(%0<>)



Index: libiberty/ChangeLog
===
--- libiberty/ChangeLog (revision 239112)
+++ libiberty/ChangeLog (working copy)
@@ -1,3 +1,20 @@
+2016-08-04  Marcel Böhme  
+
+   PR c++/71696
+   * cplus-dem.c: Prevent infinite recursion when there is a cycle
+   in the referencing of remembered mangled types.
+   (work_stuff): New stack to keep track of the remembered mangled
+   types that are currently being processed.
+   (push_processed_type): New method to push currently processed
+   remembered type onto the stack.
+   (pop_processed_type): New method to pop currently processed
+   remembered type from the stack.
+   (work_stuff_copy_to_from): Copy values of new variables.
+   (delete_non_B_K_work_stuff): Free stack memory.
+   (demangle_args): Push/Pop currently processed remembered type.
+   (do_type): Do not demangle a cyclic reference and push/pop
+   referenced remembered type.
+
 2016-07-29  Aldy Hernandez  
 
* make-relative-prefix.c (make_relative_prefix_1): Fall back to
@@ -16,7 +33,7 @@
(d_template_args_1): Split out from d_template_args.
(d_args_length): New.
 
-2016-07-13  Marcel BÃhme  
+2016-07-13  Marcel Böhme  
 
PR c++/70926
* cplus-dem.c: Handle large values and overflow when demangling
Index: libiberty/cplus-dem.c
===
--- libiberty/cplus-dem.c   (revision 239112)
+++ libiberty/cplus-dem.c   (working copy)
@@ -144,6 +144,9 @@ struct work_stuff
   string* previous_argument; /* The last function argument demangled.  */
   int nrepeats; /* The number of times to repeat the previous
   argument.  */
+  int *proctypevec; /* Indices of currently processed remembered typevecs. 
 */
+  int proctypevec_size;
+  int nproctypes;
 };
 
 #define PRINT_ANSI_QUALIFIERS (work -> options & DMGL_ANSI)
@@ -436,6 +439,10 @@ iterate_demangle_function (struct work_stuff *,
 
 static void remember_type (struct work_stuff *, const char *, int);
 
+static void push_processed_type (struct work_stuff *, int);
+
+static void pop_processed_type (struct work_stuff *);
+
 static void remem

Re: Fix fir PR71696 in Libiberty Demangler (6)

2016-07-17 Thread Marcel Böhme
Hi,

This patch is still pending a full review.

Best regards,
- Marcel

> On 4 Jul 2016, at 8:54 PM, Bernd Schmidt  wrote:
> 
> On 06/30/2016 08:46 AM, Marcel Böhme wrote:
>> The attached patch fixes the stack overflow in the demangler due to
>> cycles in the references of “remembered” mangled types
>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71696).
>> 
>> The methods demangle_signature and do_arg in cplus-dem.c allow to
>> “remember” mangled type names that can later be referenced and will
>> also be demangled. The method demangle_args demangles those types
>> following any references. So, if there is a cycle in the referencing
>> (or in the simplest case a self-reference), the method enters
>> infinite recursion.
>> 
>> The patch tracks the mangled types that are currently being demangled
>> in a new variable called work->proctypevec. If a referenced type is
>> currently being demangled, the demangling is marked as not
>> successful.
> 
> I'll defer reviewing these to someone who understands demangling better. You 
> might want to Cc Jason.
> 
> 
> Bernd
> 
> 



Re: Fix for PR70909 in Libiberty Demangler (4)

2016-07-17 Thread Marcel Böhme
Hi,

This patch is still pending a full review.

Best regards,
- Marcel

> On 30 Jun 2016, at 12:09 AM, Pedro Alves  wrote:
> 
> On 06/29/2016 08:43 AM, Marcel Böhme wrote:
>> Hi Jason,
>> 
>> These test cases are generated by fuzzing which produces a lot of 
>> nonsensical input data. 
>> I think, "Garbage In, Garbage Out" is quite applicable here.
>> With the patch at least it doesn’t crash and fixes the vulnerability.
> 
> Note that demangling shows up high in gdb profiles when loading
> huge programs.  If we can avoid quadratic or worse complexity,
> it'd preferred.
> 
> Thanks,
> Pedro Alves
> 



Fix fir PR71696 in Libiberty Demangler (6)

2016-06-29 Thread Marcel Böhme
Hi,

The attached patch fixes the stack overflow in the demangler due to cycles in 
the references of “remembered” mangled types 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71696).

The methods demangle_signature and do_arg in cplus-dem.c allow to “remember” 
mangled type names that can later be referenced and will also be demangled. The 
method demangle_args demangles those types following any references. So, if 
there is a cycle in the referencing (or in the simplest case a self-reference), 
the method enters infinite recursion.

The patch tracks the mangled types that are currently being demangled in a new 
variable called work->proctypevec. If a referenced type is currently being 
demangled, the demangling is marked as not successful.

Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test case added to 
libiberty/testsuite/demangler-expected and checked PR71696 is resolved.


Index: libiberty/ChangeLog
===
--- libiberty/ChangeLog (revision 237852)
+++ libiberty/ChangeLog (working copy)
@@ -1,3 +1,21 @@
+2016-06-30  Marcel Böhme 
+
+   * cplus-dem.c: Prevent infinite recursion when there is a cycle
+   in the referencing of remembered mangled types.
+   (work_stuff): New stack to keep track of the remembered mangled
+   types that are currently being processed. 
+   (push_processed_type): New method to push currently processed
+   remembered type onto the stack.
+   (pop_processed_type): New method to pop currently processed
+   remembered type from the stack.
+   (work_stuff_copy_to_from): Copy values of new variables.
+   (delete_non_B_K_work_stuff): Free stack memory.
+   (demangle_args): Push/Pop currently processed remembered type.
+   (do_type): Do not demangle a cyclic reference and push/pop
+   referenced remembered type.
+
+   * testsuite/demangle-expected: Add tests.
+
 2016-05-31  Alan Modra  
 
* xmemdup.c (xmemdup): Use xmalloc rather than xcalloc.
Index: libiberty/cplus-dem.c
===
--- libiberty/cplus-dem.c   (revision 237852)
+++ libiberty/cplus-dem.c   (working copy)
@@ -144,6 +144,9 @@ struct work_stuff
   string* previous_argument; /* The last function argument demangled.  */
   int nrepeats; /* The number of times to repeat the previous
   argument.  */
+  int *proctypevec; /* Indices of currently processed remembered typevecs. 
 */
+  int proctypevec_size;
+  int nproctypes;
 };
 
 #define PRINT_ANSI_QUALIFIERS (work -> options & DMGL_ANSI)
@@ -436,6 +439,10 @@ iterate_demangle_function (struct work_stuff *,
 
 static void remember_type (struct work_stuff *, const char *, int);
 
+static void push_processed_type (struct work_stuff *, int);
+
+static void pop_processed_type (struct work_stuff *);
+
 static void remember_Btype (struct work_stuff *, const char *, int, int);
 
 static int register_Btype (struct work_stuff *);
@@ -1302,6 +1309,13 @@ work_stuff_copy_to_from (struct work_stuff *to, st
   memcpy (to->btypevec[i], from->btypevec[i], len);
 }
 
+  if (from->proctypevec)
+{
+  to->proctypevec = XNEWVEC (int, from->proctypevec_size);
+  memcpy (to->proctypevec, from->proctypevec, 
+ from->proctypevec_size * sizeof(int));
+}
+
   if (from->ntmpl_args)
 to->tmpl_argvec = XNEWVEC (char *, from->ntmpl_args);
 
@@ -1336,6 +1350,12 @@ delete_non_B_K_work_stuff (struct work_stuff *work
   work -> typevec = NULL;
   work -> typevec_size = 0;
 }
+  if (work -> proctypevec != NULL)
+{
+  free (work -> proctypevec);
+  work -> proctypevec = NULL;
+  work -> proctypevec_size = 0;
+}
   if (work->tmpl_argvec)
 {
   int i;
@@ -3554,6 +3574,8 @@ static int
 do_type (struct work_stuff *work, const char **mangled, string *result)
 {
   int n;
+  int i;
+  int is_proctypevec;
   int done;
   int success;
   string decl;
@@ -3566,6 +3588,7 @@ do_type (struct work_stuff *work, const char **man
 
   done = 0;
   success = 1;
+  is_proctypevec = 0;
   while (success && !done)
 {
   int member;
@@ -3626,7 +3649,14 @@ do_type (struct work_stuff *work, const char **man
  success = 0;
}
  else
-   {
+   for (i = 0; i < work -> nproctypes; i++)
+ if (work -> proctypevec [i] == n)
+   success = 0;
+ 
+ if (success)
+   {
+ is_proctypevec = 1;
+ push_processed_type (work, n);
  remembered_type = work -> typevec[n];
  mangled = &remembered_type;
}
@@ -3849,6 +3879,9 @@ do_type (struct work_stuff *work, const char **man
 string_delete (result);
   string_delete (&decl);
 
+  if (is_proctypevec)
+pop_processed_type (work);

Re: Fix for PR70909 in Libiberty Demangler (4)

2016-06-29 Thread Marcel Böhme
Hi Jason,

These test cases are generated by fuzzing which produces a lot of nonsensical 
input data. 
I think, "Garbage In, Garbage Out" is quite applicable here.
With the patch at least it doesn’t crash and fixes the vulnerability.

Best regards,
- Marcel

> On 26 May 2016, at 10:05 PM, Jason Merrill  wrote:
> 
> It seems like in cases of malformed input we should return the input
> again rather than produce garbage like "K> ".  Maybe catch this sort of situation in
> d_lookup_template_parameter?
> 
> Jason
> 
> 
> On Mon, May 2, 2016 at 11:21 AM, Marcel Böhme  wrote:
>> Hi,
>> 
>> This fixes several stack overflows due to infinite recursion in d_print_comp 
>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70909).
>> 
>> The method d_print_comp in cp-demangle.c recursively constructs the 
>> d_print_info dpi from the demangle_component dc. The method 
>> d_print_comp_inner traverses dc as a graph. Now, dc can be a graph with 
>> cycles leading to infinite recursion in several distinct cases. The patch 
>> uses the component stack to find whether the current node dc has itself as 
>> ancestor more than once.
>> 
>> Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test cases added 
>> to libiberty/testsuite/demangler-expected and checked PR70909 and related 
>> stack overflows are resolved.
>> 
>> Best regards,
>> - Marcel
>> 
>> 
>> 
>> Index: ChangeLog
>> ===
>> --- ChangeLog   (revision 235760)
>> +++ ChangeLog   (working copy)
>> @@ -1,3 +1,19 @@
>> +2016-05-02  Marcel Böhme  
>> +
>> +   PR c++/70909
>> +   PR c++/61460
>> +   PR c++/68700
>> +   PR c++/67738
>> +   PR c++/68383
>> +   PR c++/70517
>> +   PR c++/61805
>> +   PR c++/62279
>> +   PR c++/67264
>> +   * cp-demangle.c: Prevent infinite recursion when traversing cyclic
>> +   demangle component.
>> +   (d_print_comp): Return when demangle component has itself as ancistor
>> +   more than once.
>> +
>> 2016-04-30  Oleg Endo  
>> 
>>* configure: Remove SH5 support.
>> Index: cp-demangle.c
>> ===
>> --- cp-demangle.c   (revision 235760)
>> +++ cp-demangle.c   (working copy)
>> @@ -5436,6 +5436,24 @@ d_print_comp (struct d_print_info *dpi, int option
>> {
>>   struct d_component_stack self;
>> 
>> +  self.parent = dpi->component_stack;
>> +
>> +  while (self.parent)
>> +{
>> +  self.dc = self.parent->dc;
>> +  self.parent = self.parent->parent;
>> +  if (dc != NULL && self.dc == dc)
>> +   {
>> + while (self.parent)
>> +   {
>> + self.dc = self.parent->dc;
>> + self.parent = self.parent->parent;
>> + if (self.dc == dc)
>> +   return;
>> +   }
>> +   }
>> +}
>> +
>>   self.dc = dc;
>>   self.parent = dpi->component_stack;
>>   dpi->component_stack = &self;
>> Index: testsuite/demangle-expected
>> ===
>> --- testsuite/demangle-expected (revision 235760)
>> +++ testsuite/demangle-expected (working copy)
>> @@ -4431,3 +4431,69 @@ _Q.__0
>> 
>> _Q10-__9cafebabe.
>> cafebabe.::-(void)
>> +#
>> +# Test demangler crash PR62279
>> +
>> +_ZN5Utils9transformIPN15ProjectExplorer13BuildStepListEZNKS1_18BuildConfiguration14knownStepListsEvEUlS3_E_EE5QListIDTclfp0_cvT__RKS6_IS7_ET0_
>> +QList 
>> Utils::transform> ProjectExplorer::BuildConfiguration::knownStepLists() 
>> const::{lambda(ProjectExplorer::BuildStepList*)#1}>(ProjectExplorer::BuildConfiguration::knownStepLists()
>>  const::{lambda(ProjectExplorer::BuildStepList*)#1} const&, 
>> ProjectExplorer::BuildConfiguration::knownStepLists() 
>> const::{lambda(ProjectExplorer::BuildStepList*)#1})
>> +#
>> +
>> +_ZSt7forwardIKSaINSt6thread5_ImplISt12_Bind_simpleIFZN6WIM_DL5Utils9AsyncTaskC4IMNS3_8Hardware12FpgaWatchdogEKFvvEIPS8_EEEibOT_DpOT0_EUlvE_vEESD_RNSt16remove_referenceISC_E4typeE
>> +std::allocator>  (WIM_DL::Hardware::FpgaWatchdog::*)() const, 
>> WIM_DL::Hardware::FpgaWatchdog*>(int, bool, void 
>> (WIM_DL::Hardware::FpgaWatchdog::*&&)() const, 
>> WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > 

Re: Fix for PR70926 in Libiberty Demangler (5)

2016-06-26 Thread Marcel Böhme
Hi Jeff,

On 23 Jun 2016, at 4:21 AM, Jeff Law  wrote:
> 
> OK for the trunk.  Please install.
> 
> Sorry for the delays.
> 
> Jeff

I might not have the access rights to commit to trunk.

Best regards
- Marcel



Re: Fix for PR70909 in Libiberty Demangler (4)

2016-05-26 Thread Marcel Böhme
Hi,

This patch is pending a careful review.

Best regards,
- Marcel

> On 2 May 2016, at 11:21 PM, Marcel Böhme  wrote:
> 
> Hi,
> 
> This fixes several stack overflows due to infinite recursion in d_print_comp 
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70909).
> 
> The method d_print_comp in cp-demangle.c recursively constructs the 
> d_print_info dpi from the demangle_component dc. The method 
> d_print_comp_inner traverses dc as a graph. Now, dc can be a graph with 
> cycles leading to infinite recursion in several distinct cases. The patch 
> uses the component stack to find whether the current node dc has itself as 
> ancestor more than once. 
> 
> Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test cases added 
> to libiberty/testsuite/demangler-expected and checked PR70909 and related 
> stack overflows are resolved.
> 
> Best regards,
> - Marcel
> 
> 
> 
> Index: ChangeLog
> ===
> --- ChangeLog (revision 235760)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,19 @@
> +2016-05-02  Marcel Böhme  
> +
> + PR c++/70909
> + PR c++/61460
> + PR c++/68700
> + PR c++/67738
> + PR c++/68383
> + PR c++/70517
> + PR c++/61805
> + PR c++/62279
> + PR c++/67264
> + * cp-demangle.c: Prevent infinite recursion when traversing cyclic
> + demangle component.
> + (d_print_comp): Return when demangle component has itself as ancistor
> + more than once.
> +
> 2016-04-30  Oleg Endo  
> 
>   * configure: Remove SH5 support.
> Index: cp-demangle.c
> ===
> --- cp-demangle.c (revision 235760)
> +++ cp-demangle.c (working copy)
> @@ -5436,6 +5436,24 @@ d_print_comp (struct d_print_info *dpi, int option
> {
>   struct d_component_stack self;
> 
> +  self.parent = dpi->component_stack;
> +
> +  while (self.parent)
> +{
> +  self.dc = self.parent->dc;
> +  self.parent = self.parent->parent;
> +  if (dc != NULL && self.dc == dc)
> + {
> +   while (self.parent)
> + {
> +   self.dc = self.parent->dc;
> +   self.parent = self.parent->parent;
> +   if (self.dc == dc)
> + return;
> + }
> + }
> +}
> +
>   self.dc = dc;
>   self.parent = dpi->component_stack;
>   dpi->component_stack = &self;
> Index: testsuite/demangle-expected
> ===
> --- testsuite/demangle-expected   (revision 235760)
> +++ testsuite/demangle-expected   (working copy)
> @@ -4431,3 +4431,69 @@ _Q.__0
> 
> _Q10-__9cafebabe.
> cafebabe.::-(void)
> +#
> +# Test demangler crash PR62279
> +
> +_ZN5Utils9transformIPN15ProjectExplorer13BuildStepListEZNKS1_18BuildConfiguration14knownStepListsEvEUlS3_E_EE5QListIDTclfp0_cvT__RKS6_IS7_ET0_
> +QList 
> Utils::transform ProjectExplorer::BuildConfiguration::knownStepLists() 
> const::{lambda(ProjectExplorer::BuildStepList*)#1}>(ProjectExplorer::BuildConfiguration::knownStepLists()
>  const::{lambda(ProjectExplorer::BuildStepList*)#1} const&, 
> ProjectExplorer::BuildConfiguration::knownStepLists() 
> const::{lambda(ProjectExplorer::BuildStepList*)#1})
> +#
> +
> +_ZSt7forwardIKSaINSt6thread5_ImplISt12_Bind_simpleIFZN6WIM_DL5Utils9AsyncTaskC4IMNS3_8Hardware12FpgaWatchdogEKFvvEIPS8_EEEibOT_DpOT0_EUlvE_vEESD_RNSt16remove_referenceISC_E4typeE
> +std::allocator  (WIM_DL::Hardware::FpgaWatchdog::*)() const, 
> WIM_DL::Hardware::FpgaWatchdog*>(int, bool, void 
> (WIM_DL::Hardware::FpgaWatchdog::*&&)() const, 
> WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > const&& 
> std::forward  (WIM_DL::Hardware::FpgaWatchdog::*)() const, 
> WIM_DL::Hardware::FpgaWatchdog*>(int, bool, 
> std::allocator  (WIM_DL::Hardware::FpgaWatchdog::*)() const, 
> WIM_DL::Hardware::FpgaWatchdog*>(int, bool, void 
> (WIM_DL::Hardware::FpgaWatchdog::*&&)() const, 
> WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > const&&, 
> WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > 
> const>(std::remove_reference  (WIM_DL::Hardware::FpgaWatchdog::*)() const, 
> WIM_DL::Hardware::FpgaWatchdog*>(int, bool, void 
> (WIM_DL::Hardware::FpgaWatchdog::*&&)() const, 
> WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > const>::type&)
> +#
> +# Test demangler crash PR61805
> +
> +_ZNK5niven5ColorIfLi4EEdvIfEENSt9enable_ifIXsrSt13is_arithmeticIT_E5valueEKNS0_IDTmlcvS5__Ec

Re: Fix for PR70926 in Libiberty Demangler (5)

2016-05-26 Thread Marcel Böhme
Hi: Pending review.

Best - Marcel

> On 3 May 2016, at 10:40 PM, Marcel Böhme  wrote:
> 
> Hi,
> 
> This fixes four access violations 
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70926). 
> 
> Two of these first read the value of a length variable len from the mangled 
> string, then strncpy len characters from the mangled string; more than 
> necessary.
> The other two read the value of an array index n from the mangled string, 
> which can be negative due to an overflow.
> 
> Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test cases added 
> to libiberty/testsuite/demangler-expected and checked PR70926 is resolved.
> 
> Best regards,
> - Marcel
> 
> Index: libiberty/ChangeLog
> ===
> --- libiberty/ChangeLog   (revision 235801)
> +++ libiberty/ChangeLog   (working copy)
> @@ -1,3 +1,12 @@
> +2016-05-03  Marcel Böhme  
> +
> + PR c++/70926
> + * cplus-dem.c: Handle large values and overflow when demangling
> + length variables. 
> + (demangle_template_value_parm): Read only until end of mangled string.  
>  
> + (do_hpacc_template_literal): Likewise.
> + (do_type): Handle overflow when demangling array indices.
> +
> 2016-05-02  Marcel Böhme  
> 
>   PR c++/70498
> Index: libiberty/cplus-dem.c
> ===
> --- libiberty/cplus-dem.c (revision 235801)
> +++ libiberty/cplus-dem.c (working copy)
> @@ -2051,7 +2051,8 @@ demangle_template_value_parm (struct work_stuff *w
>   else
>   {
> int symbol_len  = consume_count (mangled);
> -   if (symbol_len == -1)
> +   if (symbol_len == -1 
> +   || symbol_len > (long) strlen (*mangled))
>   return -1;
> if (symbol_len == 0)
>   string_appendn (s, "0", 1);
> @@ -3611,7 +3612,7 @@ do_type (struct work_stuff *work, const char **man
>   /* A back reference to a previously seen type */
>   case 'T':
> (*mangled)++;
> -   if (!get_count (mangled, &n) || n >= work -> ntypes)
> +   if (!get_count (mangled, &n) || n < 0 || n >= work -> ntypes)
>   {
> success = 0;
>   }
> @@ -3789,7 +3790,7 @@ do_type (struct work_stuff *work, const char **man
> /* A back reference to a previously seen squangled type */
> case 'B':
>   (*mangled)++;
> -  if (!get_count (mangled, &n) || n >= work -> numb)
> +  if (!get_count (mangled, &n) || n < 0 || n >= work -> numb)
>   success = 0;
>   else
>   string_append (result, work->btypevec[n]);
> @@ -4130,7 +4131,8 @@ do_hpacc_template_literal (struct work_stuff *work
> 
>   literal_len = consume_count (mangled);
> 
> -  if (literal_len <= 0)
> +  if (literal_len <= 0
> +  || literal_len > (long) strlen (*mangled))
> return 0;
> 
>   /* Literal parameters are names of arrays, functions, etc.  and the
> Index: libiberty/testsuite/demangle-expected
> ===
> --- libiberty/testsuite/demangle-expected (revision 235801)
> +++ libiberty/testsuite/demangle-expected (working copy)
> @@ -4441,3 +4441,16 @@ __vt_900cafebabe
> 
> _Z808
> _Z808
> +#
> +# Tests write access violation PR70926
> +
> +0__Ot2m02R5T50
> +0__Ot2m02R5T50
> +#
> +
> +0__GT500_
> +0__GT500_
> +#
> +
> +__t2m05B50_
> +__t2m05B50_
> 



Re: Fix for PR68159 in Libiberty Demangler (6)

2016-05-06 Thread Marcel Böhme
Hi Ian,

Stack overflows are a security concern and must be addressed. The Libiberty 
demangler is part of several tools, including binutils, gdb, valgrind, and many 
other libbfd-based tools that are used by the security community for the 
analysis of program binaries. Without a patch, the reverse engineering of 
untrusted binaries as well as determining whether an untrusted binary is 
malicious could cause serious damage. More details here: 
http://www.openwall.com/lists/oss-security/2016/05/05/3

> On 7 May 2016, at 12:16 AM, Ian Lance Taylor  wrote:
> 
> The function cplus_demangle_v3_callback must not call malloc.  The
> whole point of that function is to work when nothing else works.  That
> is why d_demangle_callback does not, and must not, call malloc.

Point taken. In fact, I tracked down the patch submitted by Google's Simon 
Baldwin and the ensuing discussion from 2007: 
https://gcc.gnu.org/ml/gcc-patches/2007-01/msg01116.html (committed as revision 
121305).

In that thread, Mark Mitchell raised concerns about small stacks and large 
mangled names and suggested to focus on an allocation interface where the the 
caller provides "alloc" and "dealloc" functions (i.e., C++ allocators): 
https://gcc.gnu.org/ml/gcc-patches/2007-01/msg01904.html

In the later patch to libstdc++ which has vterminate use the malloc-less 
demangler, Benjamin Kosnik raised similar concerns: 
https://gcc.gnu.org/ml/libstdc++/2007-03/msg00181.html

Perhaps the allocation interface is the way to go?

Best regards,
- Marcel






Re: Fix for PR68159 in Libiberty Demangler (6)

2016-05-06 Thread Marcel Böhme
Hi,

This patch also removes the following part of the comment for method 
cplus_demangle_print_callback:
"It does not use heap memory to build an output string, so cannot encounter 
memory allocation failure”.

> On 6 May 2016, at 11:11 PM, Marcel Böhme  wrote:
> 
> 
>> If one malloc succeeds and the other fails, you leak memory.
>> 
>>  Jakub
> Nice catch. Thanks!
> 
> Bootstrapped and regression tested on x86_64-pc-linux-gnu.

Best - Marcel

Index: libiberty/ChangeLog
===
--- libiberty/ChangeLog (revision 235962)
+++ libiberty/ChangeLog (working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  
+
+   PR c++/68159
+   * cp-demangle.c: Allocate arrays of user-defined size on the heap,
+   not on the stack. Do not include .
+   (CP_DYNAMIC_ARRAYS): Remove definition.
+   (cplus_demangle_print_callback): Allocate memory for two arrays on
+   the heap. Free memory before return / exit.
+   (d_demangle_callback): Likewise.
+   (is_ctor_or_dtor): Likewise.
+   * testsuite/demangle-expected: Add regression test cases.
+
2016-05-02  Marcel Böhme  

PR c++/70498
Index: libiberty/cp-demangle.c
===
--- libiberty/cp-demangle.c (revision 235962)
+++ libiberty/cp-demangle.c (working copy)
@@ -116,18 +116,6 @@
 #include 
 #endif
 
-#ifdef HAVE_ALLOCA_H
-# include 
-#else
-# ifndef alloca
-#  ifdef __GNUC__
-#   define alloca __builtin_alloca
-#  else
-extern char *alloca ();
-#  endif /* __GNUC__ */
-# endif /* alloca */
-#endif /* HAVE_ALLOCA_H */
-
 #ifdef HAVE_LIMITS_H
 #include 
 #endif
@@ -186,20 +174,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
additional unresolved symbols when this code is used in a library.
FIXME: Is this really a valid reason?  This comes from the original
@@ -4112,9 +4086,7 @@ d_last_char (struct d_print_info *dpi)
CALLBACK is a function to call to flush demangled string segments
as they fill the intermediate buffer, and OPAQUE is a generalized
callback argument.  On success, this returns 1.  On failure,
-   it returns 0, indicating a bad parse.  It does not use heap
-   memory to build an output string, so cannot encounter memory
-   allocation failure.  */
+   it returns 0, indicating a bad parse.  */
 
 CP_STATIC_IF_GLIBCPP_V3
 int
@@ -4126,25 +4098,32 @@ cplus_demangle_print_callback (int options,
 
   d_print_init (&dpi, callback, opaque, dc);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-__extension__ struct d_print_template temps[dpi.num_copy_templates];
+  dpi.copy_templates
+= (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates) 
+ * sizeof (*dpi.copy_templates));
+  if (! dpi.copy_templates)
+{
+  d_print_error (&dpi);
+  return 0;
+}
 
-dpi.saved_scopes = scopes;
-dpi.copy_templates = temps;
-#else
-dpi.saved_scopes = alloca (dpi.num_saved_scopes
-  * sizeof (*dpi.saved_scopes));
-dpi.copy_templates = alloca (dpi.num_copy_templates
-* sizeof (*dpi.copy_templates));
-#endif
+  dpi.saved_scopes
+= (struct d_saved_scope *) malloc (((size_t) dpi.num_saved_scopes) 
+  * sizeof (*dpi.saved_scopes));  
+  if (! dpi.saved_scopes)
+{
+  free (dpi.copy_templates);
+  d_print_error (&dpi);
+  return 0;
+}
 
-d_print_comp (&dpi, options, dc);
-  }
+  d_print_comp (&dpi, options, dc);
 
   d_print_flush (&dpi);
 
+  free (dpi.copy_templates);
+  free (dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,57 +5924,61 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct demangle_component comps[di.num_comps];
-__extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (((size_t) di.num_comps) 
+  * sizeof (*di.comps));
+  if (! di.comps)
+return 0;
 
-di.comps = comps;
-di.subs = subs;
-#else
-di.comps = alloca (di.num_comps * sizeof (*di.comps));
-di.subs = alloca (di

Re: Fix for PR68159 in Libiberty Demangler (6)

2016-05-06 Thread Marcel Böhme

> If one malloc succeeds and the other fails, you leak memory.
> 
>   Jakub
Nice catch. Thanks!

Bootstrapped and regression tested on x86_64-pc-linux-gnu.

Best - Marcel

Index: libiberty/ChangeLog
===
--- libiberty/ChangeLog (revision 235962)
+++ libiberty/ChangeLog (working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  
+
+   PR c++/68159
+   * cp-demangle.c: Allocate arrays of user-defined size on the heap,
+   not on the stack. Do not include .
+   (CP_DYNAMIC_ARRAYS): Remove definition.
+   (cplus_demangle_print_callback): Allocate memory for two arrays on
+   the heap. Free memory before return / exit.
+   (d_demangle_callback): Likewise.
+   (is_ctor_or_dtor): Likewise.
+   * testsuite/demangle-expected: Add regression test cases.
+
2016-05-02  Marcel Böhme  

PR c++/70498
Index: libiberty/cp-demangle.c
===
--- libiberty/cp-demangle.c (revision 235962)
+++ libiberty/cp-demangle.c (working copy)
@@ -116,18 +116,6 @@
 #include 
 #endif
 
-#ifdef HAVE_ALLOCA_H
-# include 
-#else
-# ifndef alloca
-#  ifdef __GNUC__
-#   define alloca __builtin_alloca
-#  else
-extern char *alloca ();
-#  endif /* __GNUC__ */
-# endif /* alloca */
-#endif /* HAVE_ALLOCA_H */
-
 #ifdef HAVE_LIMITS_H
 #include 
 #endif
@@ -186,20 +174,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
additional unresolved symbols when this code is used in a library.
FIXME: Is this really a valid reason?  This comes from the original
@@ -4126,25 +4100,31 @@ cplus_demangle_print_callback (int options,
 
   d_print_init (&dpi, callback, opaque, dc);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-__extension__ struct d_print_template temps[dpi.num_copy_templates];
+  dpi.copy_templates
+= (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates) 
+ * sizeof (*dpi.copy_templates));
+  if (! dpi.copy_templates)
+{
+  d_print_error (&dpi);
+  return 0;
+}
 
-dpi.saved_scopes = scopes;
-dpi.copy_templates = temps;
-#else
-dpi.saved_scopes = alloca (dpi.num_saved_scopes
-  * sizeof (*dpi.saved_scopes));
-dpi.copy_templates = alloca (dpi.num_copy_templates
-* sizeof (*dpi.copy_templates));
-#endif
+  dpi.saved_scopes
+= (struct d_saved_scope *) malloc (((size_t) dpi.num_saved_scopes) 
+  * sizeof (*dpi.saved_scopes));  
+  if (! dpi.saved_scopes)
+{
+  d_print_error (&dpi);
+  return 0;
+}
 
-d_print_comp (&dpi, options, dc);
-  }
+  d_print_comp (&dpi, options, dc);
 
   d_print_flush (&dpi);
 
+  free (dpi.copy_templates);
+  free (dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,57 +5925,58 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct demangle_component comps[di.num_comps];
-__extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (((size_t) di.num_comps) 
+  * sizeof (*di.comps));
+  if (! di.comps)
+return 0;
 
-di.comps = comps;
-di.subs = subs;
-#else
-di.comps = alloca (di.num_comps * sizeof (*di.comps));
-di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  di.subs = (struct demangle_component **) malloc (((size_t) di.num_subs) 
+  * sizeof (*di.subs));  
+  if (! di.subs)
+return 0;
+
+  switch (type)
+{
+case DCT_TYPE:
+  dc = cplus_demangle_type (&di);
+  break;
+case DCT_MANGLED:
+  dc = cplus_demangle_mangled_name (&di, 1);
+  break;
+case DCT_GLOBAL_CTORS:
+case DCT_GLOBAL_DTORS:
+  d_advance (&di, 11);
+  dc = d_make_comp (&di,
+   (type == DCT_GLOBAL_CTORS
+? DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS
+: DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS),
+   d_make_demangle_mangled_name (&di, d_str (&di)),
+   NULL);
+

Re: Fix for PR68159 in Libiberty Demangler (6)

2016-05-06 Thread Marcel Böhme
Hi Jakub,

> On 6 May 2016, at 5:51 PM, Jakub Jelinek  wrote:
>> 
> 
> If you just want an array, restricting the size including the sizeof
> to fit into int makes no sense, you want to guard it against overflows
> during the multiplication.
Okay, done. (Someone might want to substitute size_t with unsigned int if the 
former causes any problems).

> Ian or Jason, can all the demangle users allocate heap memory or not?
This question remains.

> But much more importantly, you don't handle the allocation failure in
> anyway, so if malloc fails, you'll just segfault.
It is handled now. No abort. No overflow.

I also checked: Even if num_saved_scopes or num_copy_templates happen to 
overflow in d_count_templates_scopes, that integer overflow won’t lead to a 
buffer overflow because of the checks (and only their uses) in lines 4292 and 
4307.
4292:  if (dpi->next_saved_scope >= dpi->num_saved_scopes)
4293: {
4294:   d_print_error (dpi);
4295:   return;
4296: }

4307:  if (dpi->next_saved_scope >= dpi->num_saved_scopes)
4307:{
4307:  d_print_error (dpi);
4307:  return;
4307:}

As for your previous email:
> On 6 May 2016, at 3:09 PM, Jakub Jelinek  wrote:
> 
> Furthermore, if I apply your patch and rebuild libstdc++, it stops working
> altogether:
> ldd -d -r ./libstdc++.so.6.0.22 
>   linux-vdso.so.1 (0x7ffe6053c000)
>   libm.so.6 => /lib64/libm.so.6 (0x7f9de23fb000)
>   libc.so.6 => /lib64/libc.so.6 (0x7f9de203a000)
>   /lib64/ld-linux-x86-64.so.2 (0x5585ecc1d000)
>   libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x7f9de1e22000)
> undefined symbol: xmalloc (./libstdc++.so.6.0.22)
> undefined symbol: xmalloc_failed  (./libstdc++.so.6.0.22)

Hmm. Just checked, xmalloc should be available through libiberty.h which is 
imported by cp-demangle.c. 
That earlier patch was successfully bootstrapped and regression tested on on 
x86_64-pc-linux-gnu from the sources in trunk.

BTW: If I configure libstdc++-v3 directly, I receive an error message:
...
./config.status: creating include/Makefile
./config.status: line 2950: ./../../config-ml.in: No such file or directory 
  

Best regards,
- Marcel


Index: libiberty/ChangeLog
===
--- libiberty/ChangeLog (revision 235962)
+++ libiberty/ChangeLog (working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  
+
+   PR c++/68159
+   * cp-demangle.c: Allocate arrays of user-defined size on the heap,
+   not on the stack. Do not include .
+   (CP_DYNAMIC_ARRAYS): Remove definition.
+   (cplus_demangle_print_callback): Allocate memory for two arrays on
+   the heap. Free memory before return / exit.
+   (d_demangle_callback): Likewise.
+   (is_ctor_or_dtor): Likewise. 
+
 2016-05-02  Marcel Böhme  
 
PR c++/70498
Index: libiberty/cp-demangle.c
===
--- libiberty/cp-demangle.c (revision 235962)
+++ libiberty/cp-demangle.c (working copy)
@@ -116,18 +116,6 @@
 #include 
 #endif
 
-#ifdef HAVE_ALLOCA_H
-# include 
-#else
-# ifndef alloca
-#  ifdef __GNUC__
-#   define alloca __builtin_alloca
-#  else
-extern char *alloca ();
-#  endif /* __GNUC__ */
-# endif /* alloca */
-#endif /* HAVE_ALLOCA_H */
-
 #ifdef HAVE_LIMITS_H
 #include 
 #endif
@@ -186,20 +174,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
additional unresolved symbols when this code is used in a library.
FIXME: Is this really a valid reason?  This comes from the original
@@ -4126,25 +4100,26 @@ cplus_demangle_print_callback (int options,
 
   d_print_init (&dpi, callback, opaque, dc);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-__extension__ struct d_print_template temps[dpi.num_copy_templates];
+  dpi.copy_templates
+= (struct d_print_template *) malloc (((size_t) dpi.num_copy_templates) 
+ * sizeof (*dpi.copy_templates));
+  dpi.saved_scopes
+= (struct d_saved_scope *) malloc (((size_t) dpi.num_saved_scopes) 
+  * sizeof (*dpi.saved_scopes));
+  
+  if (! dpi.copy_templates || ! dpi.saved_scopes)
+{
+  d_print_error (&dpi);
+  return 0;
+}
 
-dp

Re: Fix for PR68159 in Libiberty Demangler (6)

2016-05-06 Thread Marcel Böhme
Hi Jakub,

The patch that is attached now is bootstrapped and regression tested on 
x86_64-pc-linux-gnu.

> 
> This file is used not just in the various tools like binutils or gdb, but
> also in libstdc++, where it used e.g. in the std::terminate handler,
> which I think can't just xmalloc_failed, that function can be called already
> in out of memory situation, where heap allocation is not possible.

Earlier, I was working on libiberty/cplus-dem.c where xmalloc was explicitly 
available. So, I assumed it would be in libiberty/cp-demangle.c as well.

> Why INT_MAX?
> I'd have thought that the allocation size is computed in size_t and
> thus it should be SIZE_MAX, (~(size_t) 0) or similar?

In two separate patches (the first in cplus-dem.c and the second in 
cp-demangle.c) it was decided that we should import limit.h and otherwise 
define INT_MAX, then check against INT_MAX.
However, I removed the overflow check since it is not clear what the behaviour 
should be when the integer actually overflows. Apparently, it can’t abort. 
Still, this remains an unresolved security concern if actually inputs can 
actually be generated that result in overflow.

I also fixed some indentation issues caused by the removal of 
in-the-middle-of-the-method variable declarations.

-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-__extension__ struct d_print_template temps[dpi.num_copy_templates];

Let me know if there are more concerns. There might be some more formatting 
issues lingering.

Best regards,
- Marcel


Index: ChangeLog
===
--- ChangeLog   (revision 235941)
+++ ChangeLog   (working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  
+
+   PR c++/68159
+   * cp-demangle.c: Allocate arrays of user-defined size on the heap,
+   not on the stack.
+   (CP_DYNAMIC_ARRAYS): Remove redundant definition.
+   (cplus_demangle_print_callback): Allocate memory for two arrays on
+   the heap. Free memory before return / exit.
+   (d_demangle_callback): Likewise.
+   (is_ctor_or_dtor): Likewise. 
+
 2016-05-02  Marcel Böhme  
 
PR c++/70498
Index: cp-demangle.c
===
--- cp-demangle.c   (revision 235941)
+++ cp-demangle.c   (working copy)
@@ -186,20 +186,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
additional unresolved symbols when this code is used in a library.
FIXME: Is this really a valid reason?  This comes from the original
@@ -4125,26 +4111,20 @@ cplus_demangle_print_callback (int options,
   struct d_print_info dpi;
 
   d_print_init (&dpi, callback, opaque, dc);
+  
+  dpi.copy_templates = (struct d_print_template *)
+  malloc (dpi.num_copy_templates * sizeof (*dpi.copy_templates));
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-__extension__ struct d_print_template temps[dpi.num_copy_templates];
+  dpi.saved_scopes = (struct d_saved_scope *) 
+  malloc (dpi.num_saved_scopes * sizeof (*dpi.saved_scopes));
 
-dpi.saved_scopes = scopes;
-dpi.copy_templates = temps;
-#else
-dpi.saved_scopes = alloca (dpi.num_saved_scopes
-  * sizeof (*dpi.saved_scopes));
-dpi.copy_templates = alloca (dpi.num_copy_templates
-* sizeof (*dpi.copy_templates));
-#endif
+  d_print_comp (&dpi, options, dc);
 
-d_print_comp (&dpi, options, dc);
-  }
-
   d_print_flush (&dpi);
 
+  free(dpi.copy_templates);
+  free(dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,57 +5925,54 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct demangle_component comps[di.num_comps];
-__extension__ struct demangle_component *subs[di.num_subs];
+  di.comps = (struct demangle_component *) malloc (di.num_comps 
+   * sizeof (*di.comps));
 
-di.comps = comps;
-di.subs = subs;
-#else
-di.comps = alloca (di.num_comps * sizeof (*di.comps));
-di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  di.subs = (struct demangle_component **) malloc (di.num_subs 
+   

Fix for PR68159 in Libiberty Demangler (6)

2016-05-05 Thread Marcel Böhme
Hi,

This patches fixes 
* the stack overflow reported in PR68159 in cplus_demangle_print_callback,
* a potential stack overflow in d_demangle_callback
* a potential stack overflow in is_ctor_or_dtor, and
* six potential buffer overflows (initialise less memory than needed due to 
integer overflow).

The stack overflow reported in PR68159 occurs due to assigning an array too 
much memory from the stack (447kb).
Similar stack overflows might occur in the remaining five dynamically allocated 
arrays in this and the other two functions.

Since the array size is controlled from the mangled string, we better safeguard 
from integer overflows and thus buffer overflows for these six arrays.

The patch allocates memory from the heap (xmalloc) instead of from the stack 
(dynamic arrays, alloca), checks for integer overflows, and frees the allocated 
memory before function return / abort.

Best regards,
- Marcel


Index: ChangeLog
===
--- ChangeLog   (revision 235941)
+++ ChangeLog   (working copy)
@@ -1,3 +1,14 @@
+2016-05-06  Marcel Böhme  
+
+   PR c++/68159
+   * cp-demangle.c: Check for overflow and allocate arrays of user-defined 
+   size on the heap, not on the stack.
+   (CP_DYNAMIC_ARRAYS): Remove redundant definition.
+   (cplus_demangle_print_callback): Check for overflow. Allocate memory 
+   for two arrays on the heap. Free memory before return / exit.
+   (d_demangle_callback): Likewise.
+   (is_ctor_or_dtor): Likewise. 
+
 2016-05-02  Marcel Böhme  
 
PR c++/70498
Index: cp-demangle.c
===
--- cp-demangle.c   (revision 235941)
+++ cp-demangle.c   (working copy)
@@ -186,20 +186,6 @@ static void d_init_info (const char *, int, size_t
 #define CP_STATIC_IF_GLIBCPP_V3
 #endif /* ! defined(IN_GLIBCPP_V3) */
 
-/* See if the compiler supports dynamic arrays.  */
-
-#ifdef __GNUC__
-#define CP_DYNAMIC_ARRAYS
-#else
-#ifdef __STDC__
-#ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
-#define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
-#endif /* defined (__STDC_VERSION__) */
-#endif /* defined (__STDC__) */
-#endif /* ! defined (__GNUC__) */
-
 /* We avoid pulling in the ctype tables, to prevent pulling in
additional unresolved symbols when this code is used in a library.
FIXME: Is this really a valid reason?  This comes from the original
@@ -4125,26 +4111,26 @@ cplus_demangle_print_callback (int options,
   struct d_print_info dpi;
 
   d_print_init (&dpi, callback, opaque, dc);
+  
+  if (dpi.num_copy_templates > INT_MAX / (int) sizeof (*dpi.copy_templates))
+xmalloc_failed(INT_MAX);
+  dpi.copy_templates = 
+  (struct d_print_template *) xmalloc (dpi.num_copy_templates 
+ * sizeof (*dpi.copy_templates));
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
-__extension__ struct d_print_template temps[dpi.num_copy_templates];
+  if (dpi.num_saved_scopes > INT_MAX / (int) sizeof (*dpi.saved_scopes))
+xmalloc_failed(INT_MAX);
+  dpi.saved_scopes = 
+  (struct d_saved_scope *) xmalloc (dpi.num_saved_scopes 
+   * sizeof (*dpi.saved_scopes));
 
-dpi.saved_scopes = scopes;
-dpi.copy_templates = temps;
-#else
-dpi.saved_scopes = alloca (dpi.num_saved_scopes
-  * sizeof (*dpi.saved_scopes));
-dpi.copy_templates = alloca (dpi.num_copy_templates
-* sizeof (*dpi.copy_templates));
-#endif
+  d_print_comp (&dpi, options, dc);
 
-d_print_comp (&dpi, options, dc);
-  }
-
   d_print_flush (&dpi);
 
+  free(dpi.copy_templates);
+  free(dpi.saved_scopes);
+
   return ! d_print_saw_error (&dpi);
 }
 
@@ -5945,18 +5931,17 @@ d_demangle_callback (const char *mangled, int opti
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
-  {
-#ifdef CP_DYNAMIC_ARRAYS
-__extension__ struct demangle_component comps[di.num_comps];
-__extension__ struct demangle_component *subs[di.num_subs];
+  if (di.num_comps > INT_MAX / (int) sizeof (*di.comps))
+xmalloc_failed(INT_MAX);
+  di.comps = (struct demangle_component *) xmalloc (di.num_comps 
+   * sizeof (*di.comps));
 
-di.comps = comps;
-di.subs = subs;
-#else
-di.comps = alloca (di.num_comps * sizeof (*di.comps));
-di.subs = alloca (di.num_subs * sizeof (*di.subs));
-#endif
+  if (di.num_subs > INT_MAX / (int) sizeof (*di.subs))
+xmalloc_failed(INT_MAX);
+  di.subs = (struct demangle_component **) xmalloc (di.num_subs 
+  * sizeof (*di.subs));
 
+  {
 switch (type)
   {
   case DCT_TYPE:
@@ -5977,6 +5962,8 @@ d_demangle_callback (const char *mangled, int opti

Fix for PR70926 in Libiberty Demangler (5)

2016-05-03 Thread Marcel Böhme
Hi,

This fixes four access violations 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70926). 

Two of these first read the value of a length variable len from the mangled 
string, then strncpy len characters from the mangled string; more than 
necessary.
The other two read the value of an array index n from the mangled string, which 
can be negative due to an overflow.

Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test cases added to 
libiberty/testsuite/demangler-expected and checked PR70926 is resolved.

Best regards,
- Marcel

Index: libiberty/ChangeLog
===
--- libiberty/ChangeLog (revision 235801)
+++ libiberty/ChangeLog (working copy)
@@ -1,3 +1,12 @@
+2016-05-03  Marcel Böhme  
+
+   PR c++/70926
+   * cplus-dem.c: Handle large values and overflow when demangling
+   length variables. 
+   (demangle_template_value_parm): Read only until end of mangled string.  
 
+   (do_hpacc_template_literal): Likewise.
+   (do_type): Handle overflow when demangling array indices.
+
 2016-05-02  Marcel Böhme  
 
PR c++/70498
Index: libiberty/cplus-dem.c
===
--- libiberty/cplus-dem.c   (revision 235801)
+++ libiberty/cplus-dem.c   (working copy)
@@ -2051,7 +2051,8 @@ demangle_template_value_parm (struct work_stuff *w
   else
{
  int symbol_len  = consume_count (mangled);
- if (symbol_len == -1)
+ if (symbol_len == -1 
+ || symbol_len > (long) strlen (*mangled))
return -1;
  if (symbol_len == 0)
string_appendn (s, "0", 1);
@@ -3611,7 +3612,7 @@ do_type (struct work_stuff *work, const char **man
/* A back reference to a previously seen type */
case 'T':
  (*mangled)++;
- if (!get_count (mangled, &n) || n >= work -> ntypes)
+ if (!get_count (mangled, &n) || n < 0 || n >= work -> ntypes)
{
  success = 0;
}
@@ -3789,7 +3790,7 @@ do_type (struct work_stuff *work, const char **man
 /* A back reference to a previously seen squangled type */
 case 'B':
   (*mangled)++;
-  if (!get_count (mangled, &n) || n >= work -> numb)
+  if (!get_count (mangled, &n) || n < 0 || n >= work -> numb)
success = 0;
   else
string_append (result, work->btypevec[n]);
@@ -4130,7 +4131,8 @@ do_hpacc_template_literal (struct work_stuff *work
 
   literal_len = consume_count (mangled);
 
-  if (literal_len <= 0)
+  if (literal_len <= 0
+  || literal_len > (long) strlen (*mangled))
 return 0;
 
   /* Literal parameters are names of arrays, functions, etc.  and the
Index: libiberty/testsuite/demangle-expected
===
--- libiberty/testsuite/demangle-expected   (revision 235801)
+++ libiberty/testsuite/demangle-expected   (working copy)
@@ -4441,3 +4441,16 @@ __vt_900cafebabe
 
 _Z808
 _Z808
+#
+# Tests write access violation PR70926
+
+0__Ot2m02R5T50
+0__Ot2m02R5T50
+#
+
+0__GT500_
+0__GT500_
+#
+
+__t2m05B50_
+__t2m05B50_



Fix for PR70909 in Libiberty Demangler (4)

2016-05-02 Thread Marcel Böhme
Hi,

This fixes several stack overflows due to infinite recursion in d_print_comp 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70909).

The method d_print_comp in cp-demangle.c recursively constructs the 
d_print_info dpi from the demangle_component dc. The method d_print_comp_inner 
traverses dc as a graph. Now, dc can be a graph with cycles leading to infinite 
recursion in several distinct cases. The patch uses the component stack to find 
whether the current node dc has itself as ancestor more than once. 

Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test cases added to 
libiberty/testsuite/demangler-expected and checked PR70909 and related stack 
overflows are resolved.

Best regards,
- Marcel



Index: ChangeLog
===
--- ChangeLog   (revision 235760)
+++ ChangeLog   (working copy)
@@ -1,3 +1,19 @@
+2016-05-02  Marcel Böhme  
+
+   PR c++/70909
+   PR c++/61460
+   PR c++/68700
+   PR c++/67738
+   PR c++/68383
+   PR c++/70517
+   PR c++/61805
+   PR c++/62279
+   PR c++/67264
+   * cp-demangle.c: Prevent infinite recursion when traversing cyclic
+   demangle component.
+   (d_print_comp): Return when demangle component has itself as ancistor
+   more than once.
+
 2016-04-30  Oleg Endo  
 
* configure: Remove SH5 support.
Index: cp-demangle.c
===
--- cp-demangle.c   (revision 235760)
+++ cp-demangle.c   (working copy)
@@ -5436,6 +5436,24 @@ d_print_comp (struct d_print_info *dpi, int option
 {
   struct d_component_stack self;
 
+  self.parent = dpi->component_stack;
+
+  while (self.parent)
+{
+  self.dc = self.parent->dc;
+  self.parent = self.parent->parent;
+  if (dc != NULL && self.dc == dc)
+   {
+ while (self.parent)
+   {
+ self.dc = self.parent->dc;
+ self.parent = self.parent->parent;
+ if (self.dc == dc)
+   return;
+   }
+   }
+}
+
   self.dc = dc;
   self.parent = dpi->component_stack;
   dpi->component_stack = &self;
Index: testsuite/demangle-expected
===
--- testsuite/demangle-expected (revision 235760)
+++ testsuite/demangle-expected (working copy)
@@ -4431,3 +4431,69 @@ _Q.__0
 
 _Q10-__9cafebabe.
 cafebabe.::-(void)
+#
+# Test demangler crash PR62279
+
+_ZN5Utils9transformIPN15ProjectExplorer13BuildStepListEZNKS1_18BuildConfiguration14knownStepListsEvEUlS3_E_EE5QListIDTclfp0_cvT__RKS6_IS7_ET0_
+QList 
Utils::transform(ProjectExplorer::BuildConfiguration::knownStepLists()
 const::{lambda(ProjectExplorer::BuildStepList*)#1} const&, 
ProjectExplorer::BuildConfiguration::knownStepLists() 
const::{lambda(ProjectExplorer::BuildStepList*)#1})
+#
+
+_ZSt7forwardIKSaINSt6thread5_ImplISt12_Bind_simpleIFZN6WIM_DL5Utils9AsyncTaskC4IMNS3_8Hardware12FpgaWatchdogEKFvvEIPS8_EEEibOT_DpOT0_EUlvE_vEESD_RNSt16remove_referenceISC_E4typeE
+std::allocator(int, bool, void 
(WIM_DL::Hardware::FpgaWatchdog::*&&)() const, 
WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > const&& 
std::forward(int, bool, 
std::allocator(int, bool, void 
(WIM_DL::Hardware::FpgaWatchdog::*&&)() const, 
WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > const&&, 
WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > 
const>(std::remove_reference(int, bool, void 
(WIM_DL::Hardware::FpgaWatchdog::*&&)() const, 
WIM_DL::Hardware::FpgaWatchdog*&&)::{lambda()#1} ()> > > const>::type&)
+#
+# Test demangler crash PR61805
+
+_ZNK5niven5ColorIfLi4EEdvIfEENSt9enable_ifIXsrSt13is_arithmeticIT_E5valueEKNS0_IDTmlcvS5__Ecvf_EELi44typeES5_
+std::enable_if::value, niven::Color const>::type niven::Color::operator/(float) const
+#
+# Test recursion PR70517
+
+_ZSt4moveIRZN11tconcurrent6futureIvE4thenIZ5awaitIS2_EDaOT_EUlRKS6_E_EENS1_INSt5decayIDTclfp_defpTEEE4typeEEES7_EUlvE_EONSt16remove_referenceIS6_E4typeES7_
+std::remove_reference::type> tconcurrent::future::then 
>(tconcurrent::future&&)::{lambda(tconcurrent::future::type> tconcurrent::future::then >()::{lambda( const&)#1}>( 
const)::{lambda()#1}& const&)#1}>(auto await 
>()::{lambda( const&)#1}&& const)::{lambda()#1}&>::type&& 
std::move::type> 
tconcurrent::future::then 
>(tconcurrent::future::type> 
tconcurrent::future::then 
>(tconcurrent::future&&)::{lambda(& const&)#1}>(auto 
await >()::{lambda( const&)#1}&& 
const)::{lambda()#1}&)::{lambda(tconcurrent::future::type> tconcurrent::future::then >(tconcurrent::future&&)::{lambda(& 
const&)#1}>(auto await >()::{lambda(&)#1}&&

Re: Fix for PR70498 in Libiberty Demangler

2016-05-01 Thread Marcel Böhme

> 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'))
 ret

Re: Fix for PR70498 in Libiberty Demangler

2016-04-15 Thread Marcel Böhme
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 *derive

Re: Fix for PR70498 in Libiberty Demangler

2016-04-13 Thread Marcel Böhme
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

2016-04-04 Thread Marcel Böhme

> 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

2016-04-02 Thread Marcel Böhme

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

Re: Proposed Patch for Bug 69687

2016-04-01 Thread Marcel Böhme

> 
> Forgot about this issue, sorry. At least this needs guarding with #ifdef 
> HAVE_LIMITS_H, as in the other files in libiberty. Several of them also go to 
> trouble to define the macros if limits.h is missing; not sure how much of an 
> issue that is nowadays, but you might want to adapt something like the code 
> from strtol.c:
> 
> #ifndef ULONG_MAX
> #define ULONG_MAX   ((unsigned long)(~0L))  /* 0x */
> #endif
> 
> #ifndef LONG_MAX
> #define LONG_MAX((long)(ULONG_MAX >> 1))/* 0x7FFF */
> #endif
> 
> Mind trying that and doing a full test run as described in my other mail?

Regression tested on x86_64-pc-linux-gnu (make check). Checked PR69687 is 
resolved (via binutils); even for our definition of INT_MAX.
No test cases added since the test would take up to 2 minutes and end in 
xmalloc_failed even in the successful case.

Thanks,
- Marcel

--

Index: libiberty/cplus-dem.c
===
--- libiberty/cplus-dem.c   (revision 234663)
+++ libiberty/cplus-dem.c   (working copy)
@@ -56,6 +56,13 @@ void * malloc ();
 void * realloc ();
 #endif
 
+#ifdef HAVE_LIMITS_H
+#include 
+#endif
+#ifndef INT_MAX
+# define INT_MAX   (int)(((unsigned int) ~0) >> 1)  /* 0x7FFF 
*/ 
+#endif
+
 #include 
 #undef CURRENT_DEMANGLING_STYLE
 #define CURRENT_DEMANGLING_STYLE work->options
@@ -4256,6 +4263,8 @@ remember_type (struct work_stuff *work,
}
   else
{
+  if (work -> typevec_size > INT_MAX / 2)
+   xmalloc_failed (INT_MAX);
  work -> typevec_size *= 2;
  work -> typevec
= XRESIZEVEC (char *, work->typevec, work->typevec_size);
@@ -4283,6 +4292,8 @@ remember_Ktype (struct work_stuff *work,
}
   else
{
+  if (work -> ksize > INT_MAX / 2)
+   xmalloc_failed (INT_MAX);
  work -> ksize *= 2;
  work -> ktypevec
= XRESIZEVEC (char *, work->ktypevec, work->ksize);
@@ -4312,6 +4323,8 @@ register_Btype (struct work_stuff *work)
}
   else
{
+  if (work -> bsize > INT_MAX / 2)
+   xmalloc_failed (INT_MAX);
  work -> bsize *= 2;
  work -> btypevec
= XRESIZEVEC (char *, work->btypevec, work->bsize);
@@ -4766,6 +4779,8 @@ string_need (string *s, int n)
   else if (s->e - s->p < n)
 {
   tem = s->p - s->b;
+  if (n > INT_MAX / 2 - tem)
+xmalloc_failed (INT_MAX); 
   n += tem;
   n *= 2;
   s->b = XRESIZEVEC (char, s->b, n);








Re: Fix for PR70498 in Libiberty Demangler

2016-04-01 Thread Marcel Böhme

> 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

2016-04-01 Thread Marcel Böhme
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 @@

Fix for PR70498 in Libiberty Demangler

2016-04-01 Thread Marcel Böhme
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);



Fix for PR70492

2016-03-31 Thread Marcel Böhme
Hi,

This fixes the invalid write of size 8 detailed in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70492

Handle the special case when consume_count returns -1 due to an integer 
overflow when parsing the length of the virtual table qualifier in 
cplus-dem.c:2994 (gnu_special).

Index: libiberty/cplus-dem.c
===
--- libiberty/cplus-dem.c   (revision 234663)
+++ libiberty/cplus-dem.c   (working copy)
@@ -3001,6 +3001,11 @@ gnu_special (work, mangled, declp)
  success = 1;
  break;
}
+  else if (n == -1)
+{
+  success = 0;
+  break;
+}
}
  else
{



Fix for PR70481 Libiberty Demangler

2016-03-31 Thread Marcel Böhme
Hi,

This fixes the use-after-free detailed in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70481

There is a variable ksize storing the amount of allocated memory for the array 
ktypevec. ksize being zero (0) indicates that some memory must be allocated 
upon the first write. When more memory is needed, both ksize and the memory are 
doubled during reallocation. At some point the memory for the array is freed 
(in squangle_mop_up) but the value of ksize remains. Since ksize is not 0, 
there is no indication that new memory must be allocated when there is another 
write to the array.

The same holds for bsize and btypevec.

Best regards,
- Marcel


Index: libiberty/cplus-dem.c
===
--- libiberty/cplus-dem.c   (revision 234607)
+++ libiberty/cplus-dem.c   (working copy)
@@ -1237,11 +1237,13 @@  squangle_mop_up (struct work_stuff *work)
 {
   free ((char *) work -> btypevec);
   work->btypevec = NULL;
+  work->bsize = 0;
 }
   if (work -> ktypevec != NULL)
 {
   free ((char *) work -> ktypevec);
   work->ktypevec = NULL;
+  work->ksize = 0;
 }
 }



Re: Proposed Patch for Bug 69687

2016-03-30 Thread Marcel Böhme
Hi Bernd,

> Are all the places being patched really problematic ones where an input file 
> could realistically cause an overflow, or just the string functions?
The loop in demangle_args allows to call the patched register*- and 
remember*-methods arbitrarily often. So, those should also overflow at some 
point.
Found a few other segmentation faults in libiberty that I’ll report and patch 
separately.

> I'm concerned about just returning without any kind of error indication. Not 
> sure what we should be calling from libiberty, but I was thinking maybe 
> xmalloc_failed.
Done. Now, clients of libiberty freeze for about 80 seconds and consume about 
3GB of memory before exiting with "out of memory allocating 2147483647 bytes 
after a total of 3221147648 bytes”.

> Might also want to guard against overflow from the first addition.
Done.

Index: libiberty/cplus-dem.c
===
--- libiberty/cplus-dem.c   (revision 234607)
+++ libiberty/cplus-dem.c   (working copy)
@@ -55,6 +55,7 @@ Boston, MA 02110-1301, USA.  */
 void * malloc ();
 void * realloc ();
 #endif
+#include 
 
 #include 
 #undef CURRENT_DEMANGLING_STYLE
@@ -4254,6 +4255,8 @@ remember_type (struct work_stuff *work, 
}
   else
{
+ if (work -> typevec_size > INT_MAX / 2)
+   xmalloc_failed (INT_MAX);
  work -> typevec_size *= 2;
  work -> typevec
= XRESIZEVEC (char *, work->typevec, work->typevec_size);
@@ -4281,6 +4284,8 @@ remember_Ktype (struct work_stuff *work,
}
   else
{
+ if (work -> ksize > INT_MAX / 2)
+   xmalloc_failed (INT_MAX);
  work -> ksize *= 2;
  work -> ktypevec
= XRESIZEVEC (char *, work->ktypevec, work->ksize);
@@ -4310,6 +4315,8 @@ register_Btype (struct work_stuff *work)
}
   else
{
+ if (work -> bsize > INT_MAX / 2)
+   xmalloc_failed (INT_MAX);
  work -> bsize *= 2;
  work -> btypevec
= XRESIZEVEC (char *, work->btypevec, work->bsize);
@@ -4764,6 +4771,8 @@ string_need (string *s, int n)
   else if (s->e - s->p < n)
 {
   tem = s->p - s->b;
+  if (n > INT_MAX / 2 - tem)
+xmalloc_failed (INT_MAX); 
   n += tem;
   n *= 2;
   s->b = XRESIZEVEC (char, s->b, n);

Re: Proposed Patch for Bug 69687

2016-03-28 Thread Marcel Böhme
Hi Bernd: I submitted the filled disclaimer form on March 4th. Now, a 
representative of FSF mentioned that the copyright assignment is ready on their 
end.

Can someone please go ahead and review the patch?

Best regards,
- Marcel


> On 4 Mar 2016, at 1:43 AM, Bernd Schmidt  wrote:
> 
> On 03/03/2016 04:18 PM, Mike Stump wrote:
>> On Mar 3, 2016, at 6:55 AM, Marcel Böhme  wrote:
>>> I have revised the patch and removed the limits.
>> 
>> I looked at the patch, I can find no more unreasonable limits!  Wonderful.  
>> Hope someone will finish off the review and approve.
> 
> Marcel, if you haven't contributed before, we'll need a copyright assignment 
> from you before we can accept the patch. Do you already have one on file?
> 
> 
> Bernd
> 



Re: Proposed Patch for Bug 69687

2016-03-03 Thread Marcel Böhme
On 4 Mar 2016, at 1:43 AM, Bernd Schmidt  wrote:
> 
> On 03/03/2016 04:18 PM, Mike Stump wrote:
>> On Mar 3, 2016, at 6:55 AM, Marcel Böhme  wrote:
>>> I have revised the patch and removed the limits.
>> 
>> I looked at the patch, I can find no more unreasonable limits!  Wonderful.  
>> Hope someone will finish off the review and approve.
> 
> Marcel, if you haven't contributed before, we'll need a copyright assignment 
> from you before we can accept the patch. Do you already have one on file?
> 
> 
> Bernd
> 


Hi Bernd, I have requested the disclaimer form as per 
/gd/gnuorg/Copyright/request-disclaim.changes

Thanks,
- Marcel

Re: Proposed Patch for Bug 69687

2016-03-03 Thread Marcel Böhme
Thanks Mike. I have revised the patch and removed the limits.

While perhaps less security critical, without the limits on the loop count (r) 
the test cases will still consume all your memory and effectively freeze GDB.

* Before any realloc, check for overflow.
* string_need now returns 1 if the allocation was successful.
* all clients of string_need refrain from extending the string anything if 
string_need was unsuccessful.

>  better use unsigned values that are large enough to never overflow. 
Throughout cplus-dem.c, the length of a string is measured as pointer 
difference. So, technically length is of type ptrdiff_t which is signed.

— a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -55,6 +55,7 @@
 void * malloc ();
 void * realloc ();
 #endif
+#include 
 
 #include 
 #undef CURRENT_DEMANGLING_STYLE
@@ -379,7 +380,7 @@
 
 static int arm_special (const char **, string *);
 
-static void string_need (string *, int);
+static int string_need (string *, int);
 
 static void string_delete (string *);
 
@@ -4254,7 +4255,9 @@
}
   else
{
- work -> typevec_size *= 2;
+ if (work -> typevec_size > INT_MAX / 2)
+return;
+  work -> typevec_size *= 2;
  work -> typevec
= XRESIZEVEC (char *, work->typevec, work->typevec_size);
}
@@ -4281,7 +4284,9 @@
}
   else
{
- work -> ksize *= 2;
+ if (work -> ksize > INT_MAX / 2)
+return; 
+  work -> ksize *= 2;
  work -> ktypevec
= XRESIZEVEC (char *, work->ktypevec, work->ksize);
}
@@ -4294,7 +4299,8 @@
 
 /* Register a B code, and get an index for it. B codes are registered
as they are seen, rather than as they are completed, so map >
-   registers map > as B0, and temp as B1 */
+   registers map > as B0, and temp as B1. Returns -1
+   if registration was unsuccessful. */
 
 static int
 register_Btype (struct work_stuff *work)
@@ -4310,7 +4316,9 @@
}
   else
{
- work -> bsize *= 2;
+ if (work -> bsize > INT_MAX / 2)
+return -1;
+  work -> bsize *= 2;
  work -> btypevec
= XRESIZEVEC (char *, work->btypevec, work->bsize);
}
@@ -4328,6 +4336,8 @@
 {
   char *tem;
 
+  if (index < 0)
+return;
   tem = XNEWVEC (char, len + 1);
   memcpy (tem, start, len);
   tem[len] = '\0';
@@ -4591,7 +4601,8 @@
   const char *tem;
 
   string_appendn (declp, (*mangled), scan - (*mangled));
-  string_need (declp, 1);
+  if (! string_need (declp, 1))
+ return 0;
   *(declp -> p) = '\0';
 
   /* Consume the function name, including the "__" separating the name
@@ -4747,7 +4758,7 @@
 
 /* a mini string-handling package */
 
-static void
+static int 
 string_need (string *s, int n)
 {
   int tem;
@@ -4765,11 +4776,14 @@
 {
   tem = s->p - s->b;
   n += tem;
+  if ( n > INT_MAX / 2) 
+return 0;
   n *= 2;
   s->b = XRESIZEVEC (char, s->b, n);
   s->p = s->b + tem;
   s->e = s->b + n;
 }
+return 1;
 }
 
 static void
@@ -4811,7 +4825,8 @@
   if (s == NULL || *s == '\0')
 return;
   n = strlen (s);
-  string_need (p, n);
+  if (! string_need (p, n))
+return;
   memcpy (p->p, s, n);
   p->p += n;
 }
@@ -4824,7 +4839,8 @@
   if (s->b != s->p)
 {
   n = s->p - s->b;
-  string_need (p, n);
+  if (! string_need (p, n))
+return;
   memcpy (p->p, s->b, n);
   p->p += n;
 }
@@ -4835,7 +4851,8 @@
 {
   if (n != 0)
 {
-  string_need (p, n);
+  if (! string_need (p, n))
+return;
   memcpy (p->p, s, n);
   p->p += n;
 }
@@ -4866,7 +4883,8 @@
 
   if (n != 0)
 {
-  string_need (p, n);
+  if (! string_need (p, n))
+return;
   for (q = p->p - 1; q >= p->b; q--)
{
  q[n] = q[0];

Proposed Patch for Bug 69687

2016-03-02 Thread Marcel Böhme
Hi,

Please find attached the proposed patch for Bug 69687: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69687

* Limiting the length of the mangled string to 264k characters.
* Limiting the loop iterations to 256 (max. of C++ function parameters).

--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -847,6 +847,13 @@ 
 char *
 cplus_demangle (const char *mangled, int options)
 {
+  /** Limit the maximum length of mangled.
+   * C++ Std, Annex B (Implementation quantities): 
+   * - Number of characters in an internal identifier or macro name [1 024]. 
+   * - Arguments in one function call [256]. */ 
+  if (mangled && strlen (mangled) > 263168) 
+return xstrdup (mangled);
+
   char *ret;
   struct work_stuff work[1];
@@ -4488,6 +4495,8 @@ 
{
  return (0);
}
+ /* C++ Standard Annex B: Parameters in one function definition 
[256].*/
+ if (r > 256) r = 256;
  while (work->nrepeats > 0 || --r >= 0)
{
  tem = work -> typevec[t];

Best regards,
- Marcel