On 06/30/2016 12:46 AM, Marcel Böhme wrote:
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 <boehme.mar...@gmail.com>
+
+       * 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.
+
@@ -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));
+    }
Isn't this

 to->proctypevec
   = XDUPVEC (int, from->proctypevec, from->proctypevec_size);


@@ -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;
+    }
Whitespace nits. No whitespace around the "->". Can you fix the one immediately above your new code as well? (there's many others in cplus-dem.c, feel free to fix those as an independent patch).

I see similar whitespace nits in your changes to do_type.


@@ -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;
So presumably this doesn't happen all that often or this could get expensive and we'd want something more efficient for searching, right?

 static void
+push_processed_type (struct work_stuff *work, int typevec_index)
+{
+  if (work -> nproctypes >= work -> proctypevec_size)
+    {
+      if (work -> proctypevec_size == 0)
+       {
+         work -> proctypevec_size = 3;
+         work -> proctypevec = XNEWVEC (int, work -> proctypevec_size);
+       }
+      else
+       {
+         if (work -> proctypevec_size > INT_MAX / 2)
+            xmalloc_failed (INT_MAX);
+          work -> proctypevec_size *= 2;
+          work -> proctypevec
+            = XRESIZEVEC (int, work->proctypevec, work->proctypevec_size);
+        }
+    }
+    work -> proctypevec [work -> nproctypes ++] = typevec_index;
"->" whitespace nits all over the place here. I thought we had a better growing heuristic than just double every time.. Hmm, in gcc/vec.c we have:

  /* Exponential growth. */
  if (!alloc)
    alloc = 4;
  else if (alloc < 16)
    /* Double when small.  */
    alloc = alloc * 2;
  else
    /* Grow slower when large.  */
    alloc = (alloc * 3 / 2);

  /* If this is still too small, set it to the right size. */
  if (alloc < desired)
    alloc = desired;

Let's go with something similar here.



+}
+
+static void
+pop_processed_type (struct work_stuff *work)
+{
+  work -> nproctypes --;
No whitespace around the "->" or "--".

Can you take care of the minor issues above, retest & repost?

THanks,
Jeff

Reply via email to