From patchwork Thu Feb 17 20:24:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 1594499 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=vTnRO5yC; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4K05tW5nHLz9sFt for ; Fri, 18 Feb 2022 07:25:38 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 793BE385B83E for ; Thu, 17 Feb 2022 20:25:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 793BE385B83E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1645129534; bh=XsI1asnW8fpSqY3KxbHz1ovnXDdmSnOnpZsnhzLZ4TE=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=vTnRO5yCtmjrey+NCfEkWp5Fn7Hlj0rb//LOZCkaDZnpFTrrteWqvlwpO9FpiVeh/ YFsBFUD0lAqdJfGKpCqefALQtK2Nu0rmgdnFhefJC6NWL+AUnTO7BlzHHUjAzYKG7/ 23wmhRw28zYLgQph1IziMWS3OLJZHJpo1Gn8AtGE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id E9F3A3858D20 for ; Thu, 17 Feb 2022 20:24:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E9F3A3858D20 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-599-TfZZW_1EPjiWXE12fZCYDw-1; Thu, 17 Feb 2022 15:24:45 -0500 X-MC-Unique: TfZZW_1EPjiWXE12fZCYDw-1 Received: by mail-qk1-f200.google.com with SMTP id m22-20020a05620a221600b005f180383baeso5229402qkh.15 for ; Thu, 17 Feb 2022 12:24:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=XsI1asnW8fpSqY3KxbHz1ovnXDdmSnOnpZsnhzLZ4TE=; b=uSLJyH8UOKh/ndRbs/diN6XXb+55YVYF9zM5AtNhbbnbubdQtySZHx/zXDrZiTQltg y7+btIORg+20bcT4h7ClwZ0VL7+mPLMQAVeMEVngU6W27VS2OLxXKQVq3ejj8BCBO2uL 4EdyCAjZVM4mHAMNoLWxjUmcACskTDrhFbVGlfMFMA1N6DWSzEUK07Yx8D/nzaYZuNhu AKYp34cKH2mKu9BvyOT7pDTxgtO17OHys2o6OpBYGn3o3DHuRMpuu6IMWQQGhAgkKffd U0oNARn2d4Lle7CA0ROHEUPEwIPGieOj01M8M2B2zD96dzyj97dZoaJ4pwMfX5CFki0t yKYw== X-Gm-Message-State: AOAM5322hb9u9DGnoX17k6Mkxe8MDKAw80NvHdSC1HivtCcMU+kucjHf y/8yQIQif+JDot8hVP12S61IMMx19dyhP8ghjgxbIN7amm8FhHnKa+j5zlv/MTVWFmtQYQmZ+nr zpQu4axOIpaJ8b+gZKZwb+qfhEl+Qdibj2fSDHM7MyXzk+mSjSEJ3C5CEf3js4UpPFIg= X-Received: by 2002:a05:622a:1647:b0:2dd:1bb2:6c00 with SMTP id y7-20020a05622a164700b002dd1bb26c00mr3907728qtj.531.1645129484321; Thu, 17 Feb 2022 12:24:44 -0800 (PST) X-Google-Smtp-Source: ABdhPJypmVXktfgqc9zYbzPCYH7rmohiQ0fJT4wZAyulAzX36NiHVaHHM4Fy3zX/UBMRlBn+Ps/qpw== X-Received: by 2002:a05:622a:1647:b0:2dd:1bb2:6c00 with SMTP id y7-20020a05622a164700b002dd1bb26c00mr3907705qtj.531.1645129483880; Thu, 17 Feb 2022 12:24:43 -0800 (PST) Received: from localhost.localdomain (ool-18e40894.dyn.optonline.net. [24.228.8.148]) by smtp.gmail.com with ESMTPSA id w14sm690811qkf.102.2022.02.17.12.24.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Feb 2022 12:24:43 -0800 (PST) To: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: memory corruption during name lookup w/ modules [PR99479] Date: Thu, 17 Feb 2022 15:24:41 -0500 Message-Id: <20220217202441.3518973-1-ppalka@redhat.com> X-Mailer: git-send-email 2.35.1.193.g45fe28c951 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-14.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Patrick Palka via Gcc-patches From: Patrick Palka Reply-To: Patrick Palka Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" 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 testcase in comment #19 of PR99479[1] where the recursive call causes the vector to get reallocated which invalidates the reference held by the parent call. This patch makes search_unqualified instead use an auto_vec with 16 elements of internal storage (since with the various libraries I tested, the size of the vector never exceeded 12). In turn we can simplify the API of subroutines to take the vector by reference and return void. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? [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, assume its incoming length is 0, and adjust function body accordingly. --- 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 93c4eb7193b..5c965d6fba1 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -429,7 +429,7 @@ class name_lookup { public: typedef std::pair using_pair; - typedef vec using_queue; + typedef auto_vec 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 *usings); - using_queue *queue_usings (using_queue *queue, int depth, - vec *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 *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 *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 *usings) +void +name_lookup::queue_usings (using_queue& queue, int depth, vec *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; }