Skip to content

Commit

Permalink
c++: use after free during name lookup w/ modules [PR99479]
Browse files Browse the repository at this point in the history
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 gcc-mirror#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.
  • Loading branch information
Patrick Palka committed Apr 7, 2022
1 parent 68163cc commit 7e7a96f
Showing 1 changed file with 22 additions and 40 deletions.
62 changes: 22 additions & 40 deletions gcc/cp/name-lookup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -528,16 +528,8 @@ class name_lookup
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);
Expand Down Expand Up @@ -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.
Expand All @@ -1128,43 +1116,40 @@ 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))
{
gcc_assert (!DECL_NAMESPACE_ALIAS (scope));
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. */
if (SCOPE_DEPTH (pair.first) == depth)
pair.first = CP_DECL_CONTEXT (pair.first);
ix++;
}
while (ix < queue->length ());
while (ix < queue.length ());
done:;
if (scope == global_namespace)
break;
Expand All @@ -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;
}

Expand Down

0 comments on commit 7e7a96f

Please sign in to comment.