From patchwork Wed May 17 15:47:37 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Sidwell X-Patchwork-Id: 763677 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wSdxn5cPBz9s2s for ; Thu, 18 May 2017 01:47:57 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="lo4VDoUh"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=xY1mtoO4AK2QGV2QpKWnOHUHORe3HYH5prZ5/y4/cIEo4ooiVz Iq9U3cVgSvNPWQnyeyD1O+DUr6jkDHGZ68ITlJGTJZuTeTBCE8jEM3PkYOc/bE7J U2pqKL8xTKWyN1kInRhJYgTolq/ZBoy+5VLbdp6Cv1Lal6LkmFhtAXeEE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=twqMlFKc+uEZHnthUvgcwoN35Jc=; b=lo4VDoUhDRRFG4X1calx r6JUVrpVPM+1Ae+YHXW9sVzJZlB5em4JtFaYlGlye8qQ+f44L7vhKH32cJ5+mhZI CZPYjPoT8x9uAjbe9iouz2Ndhq0YWuEpH8YCCu7DFzcicSdLvoJxtTIdpFBHhiKT D+AJl7QpWTjf5RcvMfIuMk8= Received: (qmail 7404 invoked by alias); 17 May 2017 15:47:44 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 1059 invoked by uid 89); 17 May 2017 15:47:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.6 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=massage, 7.5 X-HELO: mail-yw0-f178.google.com Received: from mail-yw0-f178.google.com (HELO mail-yw0-f178.google.com) (209.85.161.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 17 May 2017 15:47:38 +0000 Received: by mail-yw0-f178.google.com with SMTP id b68so7912091ywe.3 for ; Wed, 17 May 2017 08:47:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:to:from:subject:message-id:date :user-agent:mime-version:content-language; bh=pnG6rmhSg8vQi3HEsVGoUfOHrxtSOCONVP7GonqIKic=; b=EeLqya/eHqEEO/oNjS2m4mYUX/T0RjygdZyLExLzxjX5pRpPOnPGpdGswsIY5ipm3N 7xjZN6HQvdsXej4SzcVpiYBa2TIV4ZsWIXK73/LNJrpCRvHjb+dBvnxPTOhon5MHg04S oFYWmNGNmg+nO3zdDWRF4qGF5DOxi1Lh0APr7lZWA3PL8QfG0PE5BlZNN3Ny8UwXuRZH r2LjunLtIUxcAmA3CbnbnBTF/NhVDqj2Vdbu2yaaNQrjBrhU3KtxBanlbyz/eBgk5vxv 5sz9W38nJsGnMRVtwynWL+N+AlHhMV3RdhtapNsFBXtUHepn7NLLqq6NzBc2QNhiXxF1 cTZg== X-Gm-Message-State: AODbwcBmvIhMAcCMzv3IHfG0rM32jWZjQgTE0AKtC4+iFbA8Wv5vsEPP riXJ/J9cua4WWQ== X-Received: by 10.129.45.134 with SMTP id t128mr3040767ywt.17.1495036060104; Wed, 17 May 2017 08:47:40 -0700 (PDT) Received: from ?IPv6:2620:10d:c0a3:20fb:f6d0:5ac5:64cd:f102? ([2620:10d:c091:200::7:4a1a]) by smtp.googlemail.com with ESMTPSA id a16sm1099648ywh.21.2017.05.17.08.47.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 17 May 2017 08:47:39 -0700 (PDT) To: GCC Patches From: Nathan Sidwell Subject: [C++ PATCH] extern "C" checking Message-ID: Date: Wed, 17 May 2017 11:47:37 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 This patch breaks a bit more out of pushdecl. When pushing an extern "C" function, we have to go check for other instances of it in different namespaces. We had lookup_extern_c_fun_in_all_ns to go find it in a different namespace. Looking for a name across all namespaces is the one thing our current data representation could do well. Pity we only need it for this case, (and the other use cases suck). This patch adds a new hash table to record such extern C lists -- and commonly of course there are no other instances. We have to have a new hash trait specialization for identifier_node, so that we can use the identifier's hash value and not the raw pointer -- because of PCH remapping possibilities. I'll be using this hash table type more later on, so this isn't a one-off just for this use. If it wasn't for redefine_extname pragma we'd only ever need to record a single decl per hash slot. Later patches will use check_extern_c_conflict on another code path, where we fail to check properly at the moment. nathan 2017-05-17 Nathan Sidwell * cp-tree.h (default_hash_traits ): New specialization. * name-lookup.c (lookup_extern_c_fun_in_all_ns): Delete. (extern_c_fns): New hash table. (check_extern_c_conflict): New, broken out of ... (pushdecl_maybe_friend_1): ... here. Call it. (c_linkage_bindings): Just look in hash table. Index: cp-tree.h =================================================================== --- cp-tree.h (revision 248151) +++ cp-tree.h (working copy) @@ -535,6 +535,26 @@ identifier_p (tree t) return NULL; } +/* Hash trait specialization for lang_identifiers. This allows + PCH-safe maps keyed by DECL_NAME. If it wasn't for PCH, we could + just use a regular tree key. */ + +template <> +struct default_hash_traits + : pointer_hash , ggc_remove +{ + /* Use a regular tree as the type, to make using the hash table + simpler. We'll get dynamic type checking with the hash function + itself. */ + GTY((skip)) typedef tree value_type; + GTY((skip)) typedef tree compare_type; + + static hashval_t hash (const value_type &id) + { + return IDENTIFIER_HASH_VALUE (id); + } +}; + /* In an IDENTIFIER_NODE, nonzero if this identifier is actually a keyword. C_RID_CODE (node) is then the RID_* value of the keyword. */ Index: name-lookup.c =================================================================== --- name-lookup.c (revision 248151) +++ name-lookup.c (working copy) @@ -60,7 +60,6 @@ static void consider_binding_level (tree enum lookup_name_fuzzy_kind kind); static tree lookup_type_current_level (tree); static tree push_using_directive (tree); -static tree lookup_extern_c_fun_in_all_ns (tree); static void diagnose_name_conflict (tree, tree); /* Add DECL to the list of things declared in B. */ @@ -1184,6 +1183,75 @@ supplement_binding (cxx_binding *binding return ret; } +/* Map of identifiers to extern C functions (or LISTS thereof). */ + +static GTY(()) hash_map *extern_c_fns; + +/* DECL has C linkage. If we have an existing instance, make sure it + has the same exception specification [7.5, 7.6]. If there's no + instance, add DECL to the map. */ + +static void +check_extern_c_conflict (tree decl) +{ + /* Ignore artificial or system header decls. */ + if (DECL_ARTIFICIAL (decl) || DECL_IN_SYSTEM_HEADER (decl)) + return; + + if (!extern_c_fns) + extern_c_fns = hash_map::create_ggc (127); + + bool existed; + tree *slot = &extern_c_fns->get_or_insert (DECL_NAME (decl), &existed); + if (!existed) + *slot = decl; + else + { + tree old = *slot; + if (TREE_CODE (old) == TREE_LIST) + old = TREE_VALUE (old); + + int mismatch = 0; + if (DECL_CONTEXT (old) == DECL_CONTEXT (decl)) + ; /* If they're in the same context, we'll have already complained + about a (possible) mismatch, when inserting the decl. */ + else if (!decls_match (decl, old)) + mismatch = 1; + else if (!comp_except_specs (TYPE_RAISES_EXCEPTIONS (TREE_TYPE (old)), + TYPE_RAISES_EXCEPTIONS (TREE_TYPE (decl)), + ce_normal)) + mismatch = -1; + else if (DECL_ASSEMBLER_NAME_SET_P (old)) + SET_DECL_ASSEMBLER_NAME (decl, DECL_ASSEMBLER_NAME (old)); + + if (mismatch) + { + pedwarn (input_location, 0, + "declaration of %q#D with C language linkage", decl); + pedwarn (DECL_SOURCE_LOCATION (old), 0, + "conflicts with previous declaration %q#D", old); + if (mismatch < 0) + pedwarn (input_location, 0, + "due to different exception specifications"); + } + else + /* Chain it on for c_linkage_binding's use. */ + *slot = tree_cons (NULL_TREE, decl, *slot); + } +} + +/* Returns a list of C-linkage decls with the name NAME. Used in + c-family/c-pragma.c to implement redefine_extname pragma. */ + +tree +c_linkage_bindings (tree name) +{ + if (extern_c_fns) + if (tree *slot = extern_c_fns->get (name)) + return *slot; + return NULL_TREE; +} + /* DECL is being declared at a local scope. Emit suitable shadow warnings. */ @@ -1591,63 +1659,6 @@ pushdecl_maybe_friend_1 (tree x, bool is } } - /* If x has C linkage-specification, (extern "C"), - lookup its binding, in case it's already bound to an object. - The lookup is done in all namespaces. - If we find an existing binding, make sure it has the same - exception specification as x, otherwise, bail in error [7.5, 7.6]. */ - if ((TREE_CODE (x) == FUNCTION_DECL) - && DECL_EXTERN_C_P (x) - /* We should ignore declarations happening in system headers. */ - && !DECL_ARTIFICIAL (x) - && !DECL_IN_SYSTEM_HEADER (x)) - { - tree previous = lookup_extern_c_fun_in_all_ns (x); - if (previous - && !DECL_ARTIFICIAL (previous) - && !DECL_IN_SYSTEM_HEADER (previous) - && DECL_CONTEXT (previous) != DECL_CONTEXT (x)) - { - /* In case either x or previous is declared to throw an exception, - make sure both exception specifications are equal. */ - if (decls_match (x, previous)) - { - tree x_exception_spec = NULL_TREE; - tree previous_exception_spec = NULL_TREE; - - x_exception_spec = - TYPE_RAISES_EXCEPTIONS (TREE_TYPE (x)); - previous_exception_spec = - TYPE_RAISES_EXCEPTIONS (TREE_TYPE (previous)); - if (!comp_except_specs (previous_exception_spec, - x_exception_spec, - ce_normal)) - { - pedwarn (input_location, 0, - "declaration of %q#D with C language linkage", - x); - pedwarn (DECL_SOURCE_LOCATION (previous), 0, - "conflicts with previous declaration %q#D", - previous); - pedwarn (input_location, 0, - "due to different exception specifications"); - return error_mark_node; - } - if (DECL_ASSEMBLER_NAME_SET_P (previous)) - SET_DECL_ASSEMBLER_NAME (x, - DECL_ASSEMBLER_NAME (previous)); - } - else - { - pedwarn (input_location, 0, - "declaration of %q#D with C language linkage", x); - pedwarn (DECL_SOURCE_LOCATION (previous), 0, - "conflicts with previous declaration %q#D", - previous); - } - } - } - check_template_shadow (x); /* If this is a function conjured up by the back end, massage it @@ -1848,6 +1859,9 @@ pushdecl_maybe_friend_1 (tree x, bool is if (VAR_P (x)) maybe_register_incomplete_var (x); + if (TREE_CODE (x) == FUNCTION_DECL && DECL_EXTERN_C_P (x)) + /* We need to check and register the fn now. */ + check_extern_c_conflict (x); } if (need_new_binding) @@ -2722,74 +2736,6 @@ binding_for_name (cp_binding_level *scop return result; } -/* Walk through the bindings associated to the name of FUNCTION, - and return the first declaration of a function with a - "C" linkage specification, a.k.a 'extern "C"'. - This function looks for the binding, regardless of which scope it - has been defined in. It basically looks in all the known scopes. - Note that this function does not lookup for bindings of builtin functions - or for functions declared in system headers. */ -static tree -lookup_extern_c_fun_in_all_ns (tree function) -{ - tree name; - cxx_binding *iter; - - gcc_assert (function && TREE_CODE (function) == FUNCTION_DECL); - - name = DECL_NAME (function); - gcc_assert (name && identifier_p (name)); - - for (iter = IDENTIFIER_NAMESPACE_BINDINGS (name); - iter; - iter = iter->previous) - { - tree ovl; - for (ovl = iter->value; ovl; ovl = OVL_NEXT (ovl)) - { - tree decl = OVL_CURRENT (ovl); - if (decl - && TREE_CODE (decl) == FUNCTION_DECL - && DECL_EXTERN_C_P (decl) - && !DECL_ARTIFICIAL (decl)) - { - return decl; - } - } - } - return NULL; -} - -/* Returns a list of C-linkage decls with the name NAME. */ - -tree -c_linkage_bindings (tree name) -{ - tree decls = NULL_TREE; - cxx_binding *iter; - - for (iter = IDENTIFIER_NAMESPACE_BINDINGS (name); - iter; - iter = iter->previous) - { - tree ovl; - for (ovl = iter->value; ovl; ovl = OVL_NEXT (ovl)) - { - tree decl = OVL_CURRENT (ovl); - if (decl - && DECL_EXTERN_C_P (decl) - && !DECL_ARTIFICIAL (decl)) - { - if (decls == NULL_TREE) - decls = decl; - else - decls = tree_cons (NULL_TREE, decl, decls); - } - } - } - return decls; -} - /* Insert another USING_DECL into the current binding level, returning this declaration. If this is a redeclaration, do nothing, and return NULL_TREE if this not in namespace scope (in namespace