[gcc r12-8051] c++: use after free during name lookup w/ modules [PR99479]

[gcc r12-8051] c++: use after free during name lookup w/ modules [PR99479]

[Patrick Palka]

https://gcc.gnu.org/g:7e7a96f774ed892e5cef53fcb68297cd0d513820

commit r12-8051-g7e7a96f774ed892e5cef53fcb68297cd0d513820
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Apr 7 16:09:52 2022 -0400

    c++: use after free during name lookup w/ modules [PR99479]
    
    name_lookup::search_unqualified uses a statically allocated vector
    in order to avoid repeated reallocation, under the assumption that
    the function can't be called recursively.  With modules however,
    this assumption turns out to be false, and search_unqualified can
    be called recursively as demonstrated by the testcase in comment #19
    of PR99479[1] where the recursive call causes the vector to get
    reallocated which invalidates the reference to queue[ix] held by the
    parent call.
    
    This patch makes search_unqualified instead use an auto_vec with 16
    elements of internal storage.  In turn we can simplify the API of some
    member functions to take the vector by reference and return void.
    
    [1]: https://gcc.gnu.org/PR99479#c19
    
            PR c++/99479
    
    gcc/cp/ChangeLog:
    
            * name-lookup.cc (name_lookup::using_queue): Change to an
            auto_vec (with 16 elements of internal storage).
            (name_lookup::queue_namespace): Change return type to void,
            take queue parameter by reference and adjust function body
            accordingly.
            (name_lookup::do_queue_usings): Inline into ...
            (name_lookup::queue_usings): ... here.  As in queue_namespace.
            (name_lookup::search_unqualified): Don't make queue static,
            remove length variable, and adjust function body accordingly.

Diff:
---
 gcc/cp/name-lookup.cc | 62 ++++++++++++++++++---------------------------------
 1 file changed, 22 insertions(+), 40 deletions(-)

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index d16c577c029..b7fc2781d53 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -429,7 +429,7 @@ class name_lookup
 {
 public:
   typedef std::pair<tree, tree> using_pair;
-  typedef vec<using_pair, va_heap, vl_embed> using_queue;
+  typedef auto_vec<using_pair, 16> using_queue;
 
 public:
   tree name;	/* The identifier being looked for.  */
@@ -528,16 +528,8 @@ private:
   bool search_usings (tree scope);
 
 private:
-  using_queue *queue_namespace (using_queue *queue, int depth, tree scope);
-  using_queue *do_queue_usings (using_queue *queue, int depth,
-				vec<tree, va_gc> *usings);
-  using_queue *queue_usings (using_queue *queue, int depth,
-			     vec<tree, va_gc> *usings)
-  {
-    if (usings)
-      queue = do_queue_usings (queue, depth, usings);
-    return queue;
-  }
+  void queue_namespace (using_queue& queue, int depth, tree scope);
+  void queue_usings (using_queue& queue, int depth, vec<tree, va_gc> *usings);
 
 private:
   void add_fns (tree);
@@ -1084,39 +1076,35 @@ name_lookup::search_qualified (tree scope, bool usings)
 /* Add SCOPE to the unqualified search queue, recursively add its
    inlines and those via using directives.  */
 
-name_lookup::using_queue *
-name_lookup::queue_namespace (using_queue *queue, int depth, tree scope)
+void
+name_lookup::queue_namespace (using_queue& queue, int depth, tree scope)
 {
   if (see_and_mark (scope))
-    return queue;
+    return;
 
   /* Record it.  */
   tree common = scope;
   while (SCOPE_DEPTH (common) > depth)
     common = CP_DECL_CONTEXT (common);
-  vec_safe_push (queue, using_pair (common, scope));
+  queue.safe_push (using_pair (common, scope));
 
   /* Queue its inline children.  */
   if (vec<tree, va_gc> *inlinees = DECL_NAMESPACE_INLINEES (scope))
     for (unsigned ix = inlinees->length (); ix--;)
-      queue = queue_namespace (queue, depth, (*inlinees)[ix]);
+      queue_namespace (queue, depth, (*inlinees)[ix]);
 
   /* Queue its using targets.  */
-  queue = queue_usings (queue, depth, NAMESPACE_LEVEL (scope)->using_directives);
-
-  return queue;
+  queue_usings (queue, depth, NAMESPACE_LEVEL (scope)->using_directives);
 }
 
 /* Add the namespaces in USINGS to the unqualified search queue.  */
 
-name_lookup::using_queue *
-name_lookup::do_queue_usings (using_queue *queue, int depth,
-			      vec<tree, va_gc> *usings)
+void
+name_lookup::queue_usings (using_queue& queue, int depth, vec<tree, va_gc> *usings)
 {
-  for (unsigned ix = usings->length (); ix--;)
-    queue = queue_namespace (queue, depth, (*usings)[ix]);
-
-  return queue;
+  if (usings)
+    for (unsigned ix = usings->length (); ix--;)
+      queue_namespace (queue, depth, (*usings)[ix]);
 }
 
 /* Unqualified namespace lookup in SCOPE.
@@ -1128,15 +1116,12 @@ name_lookup::do_queue_usings (using_queue *queue, int depth,
 bool
 name_lookup::search_unqualified (tree scope, cp_binding_level *level)
 {
-  /* Make static to avoid continual reallocation.  We're not
-     recursive.  */
-  static using_queue *queue = NULL;
+  using_queue queue;
   bool found = false;
-  int length = vec_safe_length (queue);
 
   /* Queue local using-directives.  */
   for (; level->kind != sk_namespace; level = level->level_chain)
-    queue = queue_usings (queue, SCOPE_DEPTH (scope), level->using_directives);
+    queue_usings (queue, SCOPE_DEPTH (scope), level->using_directives);
 
   for (; !found; scope = CP_DECL_CONTEXT (scope))
     {
@@ -1144,19 +1129,19 @@ name_lookup::search_unqualified (tree scope, cp_binding_level *level)
       int depth = SCOPE_DEPTH (scope);
 
       /* Queue namespaces reachable from SCOPE. */
-      queue = queue_namespace (queue, depth, scope);
+      queue_namespace (queue, depth, scope);
 
       /* Search every queued namespace where SCOPE is the common
 	 ancestor.  Adjust the others.  */
-      unsigned ix = length;
+      unsigned ix = 0;
       do
 	{
-	  using_pair &pair = (*queue)[ix];
+	  using_pair &pair = queue[ix];
 	  while (pair.first == scope)
 	    {
 	      found |= search_namespace_only (pair.second);
-	      pair = queue->pop ();
-	      if (ix == queue->length ())
+	      pair = queue.pop ();
+	      if (ix == queue.length ())
 		goto done;
 	    }
 	  /* The depth is the same as SCOPE, find the parent scope.  */
@@ -1164,7 +1149,7 @@ name_lookup::search_unqualified (tree scope, cp_binding_level *level)
 	    pair.first = CP_DECL_CONTEXT (pair.first);
 	  ix++;
 	}
-      while (ix < queue->length ());
+      while (ix < queue.length ());
     done:;
       if (scope == global_namespace)
 	break;
@@ -1181,9 +1166,6 @@ name_lookup::search_unqualified (tree scope, cp_binding_level *level)
 
   dedup (false);
 
-  /* Restore to incoming length.  */
-  vec_safe_truncate (queue, length);
-
   return found;
 }