From patchwork Fri Jan 11 20:10:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1023763 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-493890-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="SvKKQeez"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="TX9sukQn"; dkim-atps=neutral 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 43bvBc3gLwz9s4s for ; Sat, 12 Jan 2019 07:11:03 +1100 (AEDT) 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=ZjwqiY53ZqpjNDy0RIiV9qmzYiO4hQWl7JDLkTlDLcUfy1Osrp DACcc9/EO5eo0VOuqUDugJAd0Ry29h1owLvfFvFHfTH7Hol5Q34xCS9HWZGRYQGV 32EQXHxi95mTow28d3SMYgij7tn6ST/B53DmQPEIBIsJfZ/S1zxoIMFgs= 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=V4DfBxUI/j+WCLpyXoXnA4oZvPw=; b=SvKKQeezCrUbET69VPnE +nPwAC9fP8Y2pgyKkIJqs8QEjh5B9AxrMkiZjLcwXi4xtYFXp/Y/R406CmpAhZ0S UGRgxtGpon/M4/BDGPba0SXev11w0hiRLOlEeYCYocYeVqbqIeiJlx5mA7l04smU wEg8hnMfW/cTM5nKP2wdY4s= Received: (qmail 103874 invoked by alias); 11 Jan 2019 20:10:56 -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 103857 invoked by uid 89); 11 Jan 2019 20:10:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=gathered, Clear, fes, dg-prune-output X-HELO: mail-qk1-f193.google.com Received: from mail-qk1-f193.google.com (HELO mail-qk1-f193.google.com) (209.85.222.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Jan 2019 20:10:50 +0000 Received: by mail-qk1-f193.google.com with SMTP id c21so7272072qkl.6 for ; Fri, 11 Jan 2019 12:10:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=lLBYeZ/kHASaPUrOV+JonwGJgFyvqMTKA+doEucLrnU=; b=TX9sukQnvAwKoXTIqzPw41OoBpQge+TOb84uhQ+NSua+fjWnefYMmr0gyMkmrKn14D Fbmxwxzev5Q6Isqpc4v7ZorJjL9ew/FV9ixq/OqeA86xsA2+mer4jW4R633MgLcNODLO rT0Zq7WzC/t+Wr6yfK5njaXusSrdI3Z0kCe+oOhoLIMEN7p37PzY7bQ36hoM+H22yGV5 B2Figfdxto5KQHpRVKCbLVyiSz5+LATcy/Jr9gN+qY+kqSxsPT+LorX7tM0Y65eyRgLW ZzLQZlO1L1RwBX/XhHOG/xroCwG/7iTkUa5SwiP8HXhZS7xx1hID23mB30YV80JolQf8 Y58A== Received: from [192.168.0.106] (174-16-101-177.hlrn.qwest.net. [174.16.101.177]) by smtp.gmail.com with ESMTPSA id t5sm15264788qkl.14.2019.01.11.12.10.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 11 Jan 2019 12:10:47 -0800 (PST) To: "gcc-patches@gcc.gnu.org" , "Joseph S. Myers" From: Martin Sebor Subject: [PATCH] detect references to statics in inline function signatures (PR 88718) Message-ID: <82358938-a70e-2734-3dfe-b7460d7e89d7@gmail.com> Date: Fri, 11 Jan 2019 13:10:45 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 X-IsSubscribed: yes The attached patch extends the detection of references to static variables in inline functions to the function signatures, including their return type. Since the declaration of a function need not be yet available at the time the static is referenced (e.g., when it's referenced in the functions return type), the patch introduces the concept of "tentative records" of such references and either completes the records when the full declaration of the function, including its definition, is known to be inline, or removes it when it isn't. Martin PR c/88718 - Strange inconsistency between old style and new style definitions of inline functions. gcc/c/ChangeLog: PR c/88718 * c-decl.c (reset_inline_statics): New function. (record_inline_static): Optimize. (check_inline_statics): Handle tentative records for inline declarations without definitions. Print static declaration location. (push_file_scope): Clear records of references to statics. (finish_decl): Add tentative records of references to statics. (finish_function): Same. * c-typeck.c (build_external_ref): Handle all references to statics. gcc/testsuite/ChangeLog: PR c/88718 * gcc.dg/inline-40.c: New test. * gcc.dg/inline-41.c: New test. Index: gcc/c/c-decl.c =================================================================== --- gcc/c/c-decl.c (revision 267616) +++ gcc/c/c-decl.c (working copy) @@ -826,14 +826,47 @@ c_finish_incomplete_decl (tree decl) } } -/* Record that inline function FUNC contains a reference (location - LOC) to static DECL (file-scope or function-local according to - TYPE). */ +/* Free records of references to static variables gathered so far. */ +static void +reset_inline_statics (void) +{ + if (!c_inline_statics) + return; + + for (c_inline_static *csi = c_inline_statics, *next = csi->next; + csi; csi = next) + ggc_free (csi); + + c_inline_statics = NULL; +} + +/* Record that inline function FUNC either does contain or may contain + a reference (location LOC) to static DECL (file-scope or function-local + according to TYPE). For a null FUNC, a tentative record is created that + reflects a reference in the function signature and that is either updated + or removed when the function declaration is complete. */ + void record_inline_static (location_t loc, tree func, tree decl, enum c_inline_static_type type) { + gcc_assert (decl); + + if (c_inline_statics) + { + /* Avoid adding another tentative record for this DECL if one + already exists. */ + for (c_inline_static *csi = c_inline_statics; csi; csi = csi->next) + { + if (c_inline_statics->function) + break; + + if (decl == c_inline_statics->static_decl) + return; + } + } + c_inline_static *csi = ggc_alloc (); csi->location = loc; csi->function = func; @@ -843,6 +876,50 @@ record_inline_static (location_t loc, tree func, t c_inline_statics = csi; } +/* Update tentative records of references to static declarations in + an inline declaration of function FUNC, or remove them if FUNC + isn't declared inline. A tentative record is one whose FUNCTION + is null. */ + +static void +update_tentative_inline_static (tree func) +{ + gcc_assert (func); + + c_inline_static *csi = c_inline_statics; + if (!csi) + return; + + /* True to associate FUNC with the tentative records, false to remove + them. */ + bool add = (DECL_DECLARED_INLINE_P (func) + && DECL_EXTERNAL (func) + && DECL_INITIAL (func)); + + for (c_inline_static *csi = c_inline_statics, *last = csi; + csi; csi = csi->next) + { + if (add) + { + if (csi->function == func + || csi->function) + continue; + + csi->function = func; + } + else + { + if (csi->function) + break; + + if (last == c_inline_statics) + c_inline_statics = last = csi->next; + else + last->next = csi->next; + } + } +} + /* Check for references to static declarations in inline functions at the end of the translation unit and diagnose them if the functions are still inline definitions. */ @@ -853,22 +930,36 @@ check_inline_statics (void) struct c_inline_static *csi; for (csi = c_inline_statics; csi; csi = csi->next) { - if (DECL_EXTERNAL (csi->function)) - switch (csi->type) - { - case csi_internal: - pedwarn (csi->location, 0, - "%qD is static but used in inline function %qD " - "which is not static", csi->static_decl, csi->function); - break; - case csi_modifiable: - pedwarn (csi->location, 0, - "%q+D is static but declared in inline function %qD " - "which is not static", csi->static_decl, csi->function); - break; - default: - gcc_unreachable (); - } + /* FUNCTION is unset for a declaration whose signature references + a static variable and for which a definition wasn't provided. */ + if (!csi->function) + continue; + + if (!DECL_EXTERNAL (csi->function)) + continue; + + bool warned; + switch (csi->type) + { + case csi_internal: + warned = pedwarn (csi->location, 0, + "%qD is static but used in inline function %qD " + "which is not static", + csi->static_decl, csi->function); + break; + case csi_modifiable: + warned = pedwarn (csi->location, 0, + "%q+D is static but declared in inline function %qD " + "which is not static", + csi->static_decl, csi->function); + break; + default: + gcc_unreachable (); + } + + if (warned) + inform (DECL_SOURCE_LOCATION (csi->static_decl), + "%qD declared here", csi->static_decl); } c_inline_statics = NULL; } @@ -1412,6 +1503,9 @@ push_file_scope (void) start_fname_decls (); + /* Free references to static declarations in inline functions. */ + reset_inline_statics (); + for (decl = visible_builtins; decl; decl = DECL_CHAIN (decl)) bind (DECL_NAME (decl), decl, file_scope, /*invisible=*/false, /*nested=*/true, DECL_SOURCE_LOCATION (decl)); @@ -5165,6 +5259,11 @@ finish_decl (tree decl, location_t init_loc, tree set_user_assembler_name (decl, asmspec); } + /* Remove any tentative record of a non-static inline function + referencing a static decl made a DECL that is not a definition. */ + if (TREE_CODE (decl) == FUNCTION_DECL) + update_tentative_inline_static (decl); + if (DECL_FILE_SCOPE_P (decl)) { if (DECL_INITIAL (decl) == NULL_TREE @@ -9636,6 +9735,11 @@ finish_function (void) } } + /* Associate any tentative records of references to static variables + with this function declaration if it is inline and not static, or + remove them otherwise. */ + update_tentative_inline_static (fndecl); + if (!decl_function_context (fndecl)) undef_nested_function = false; Index: gcc/c/c-typeck.c =================================================================== --- gcc/c/c-typeck.c (revision 267616) +++ gcc/c/c-typeck.c (working copy) @@ -52,6 +52,8 @@ along with GCC; see the file COPYING3. If not see #include "stringpool.h" #include "attribs.h" #include "asan.h" +#include "c-family/c-pragma.h" +#include "c-parser.h" /* Possible cases of implicit bad conversions. Used to select diagnostic messages in convert_for_assignment. */ @@ -2818,19 +2820,26 @@ build_external_ref (location_t loc, tree id, bool if (context != NULL_TREE && context != current_function_decl) DECL_NONLOCAL (ref) = 1; } - /* C99 6.7.4p3: An inline definition of a function with external - linkage ... shall not contain a reference to an identifier with - internal linkage. */ - else if (current_function_decl != NULL_TREE + else if (VAR_OR_FUNCTION_DECL_P (ref) + && (!VAR_P (ref) || TREE_STATIC (ref)) + && ! TREE_PUBLIC (ref)) + { + /* C99 6.7.4p3: An inline definition of a function with external + linkage ... shall not contain a reference to an identifier with + internal linkage. */ + if ((current_function_decl && DECL_DECLARED_INLINE_P (current_function_decl) && DECL_EXTERNAL (current_function_decl) - && VAR_OR_FUNCTION_DECL_P (ref) - && (!VAR_P (ref) || TREE_STATIC (ref)) - && ! TREE_PUBLIC (ref) && DECL_CONTEXT (ref) != current_function_decl) - record_inline_static (loc, current_function_decl, ref, - csi_internal); - + /* If the function declaration is not available yet at this + point make a record of the reference if it isn't associated + with any context yet and either fill in the details or + discard the record when the declaration has been completed. */ + || (!current_function_decl + && !DECL_CONTEXT (ref))) + record_inline_static (loc, current_function_decl, ref, + csi_internal); + } return ref; } Index: gcc/testsuite/gcc.dg/inline-40.c =================================================================== --- gcc/testsuite/gcc.dg/inline-40.c (nonexistent) +++ gcc/testsuite/gcc.dg/inline-40.c (working copy) @@ -0,0 +1,130 @@ +/* PR c/88718 - Strange inconsistency between old style and new style + definitions of inline functions + { dg-do compile } + { dg-options "-Wall -Wno-unused" } */ + +extern int global; +extern const int cst_global; + +static int local; +static const int cst_local; + +/* Verify that inline declarations that aren't definitions are accepted + without a warning even if their argument lists reference static + variables. */ + +inline void global_decl_arg_ref_global (int (*)[sizeof global]); +static inline void local_decl_arg_ref_global (int (*)[sizeof global]); + +inline void global_decl_arg_ref_cst_global (int (*)[sizeof cst_global]); +static inline void local_decl_arg_ref_cst_global (int (*)[sizeof cst_global]); + +inline void global_decl_arg_ref_local (int (*)[sizeof local]); +static inline void local_decl_arg_ref_local (int (*)[sizeof local]); + +inline void global_decl_arg_ref_cst_local (int (*)[sizeof cst_local]); +static inline void local_decl_arg_ref_cst_local (int (*)[sizeof cst_local]); + +/* Verify that implicitly extern inline definitions trigger a warning + when their argument lists reference static (but not extern) variables. */ + +inline void +global_decl_arg_ref_global (int (*p)[sizeof global]) { (void)&p; } + +inline void +global_decl_arg_ref_cst_global (int (*p)[sizeof cst_global]) { (void)&p; } + +inline void +global_decl_arg_ref_local (int (*p)[sizeof local]) +/* { dg-warning ".local. is static but used in inline function .global_decl_arg_ref_local." "" { target *-*-* } .-1 } */ +{ + (void)&p; +} + +inline void +global_decl_arg_ref_cst_local (int (*p)[sizeof cst_local]) +/* { dg-warning ".cst_local. is static but used in inline function .global_decl_arg_ref_cst_local." "" { target *-*-* } .-1 } */ +{ + (void)&p; +} + +/* Verify that static inline definitions don't trigger a warning + when their argument lists reference static or extern variables. */ + +static inline void +local_decl_arg_ref_global (int (*p)[sizeof global]) { (void)&p; } +static inline void +local_decl_arg_ref_cst_global (int (*p)[sizeof cst_global]) { (void)&p; } +static inline void +local_decl_arg_ref_local (int (*p)[sizeof local]) { (void)&p; } +static inline void +local_decl_arg_ref_cst_local (int (*p)[sizeof cst_local]) { (void)&p; } + + +/* Same as above but with return types. */ + +extern void abort (void); + +/* Verify that inline declarations that aren't definitions are accepted + without a warning even if their return types reference static + variables. */ + +inline struct { int a[sizeof global]; } + global_decl_ret_ref_global (void); + +static inline struct { int a[sizeof global]; } + local_decl_ret_ref_global (void); + +inline struct { int a[sizeof cst_global]; } + global_decl_ret_ref_cst_global (void); + +static inline struct { int a[sizeof cst_global]; } + local_decl_ret_ref_cst_global (void); + +inline struct { int a[sizeof local]; } + global_decl_ret_ref_local (void); +static inline struct { int a[sizeof local]; } + local_decl_ret_ref_local (void); + +inline struct { int a[sizeof cst_local]; } + global_decl_ret_ref_cst_local (void); + +static inline struct { int a[sizeof global]; } + local_decl_ret_ref_cst_local (void); + +/* Verify that implicitly extern inline definitions trigger a warning + when their return types reference static (but not extern) variables. */ + +inline struct { int a[sizeof global]; } + global_def_ret_ref_global (void) { abort (); } + +inline struct { int a[sizeof cst_global]; } + global_def_ret_ref_cst_global (void) { abort (); } + +inline struct { int a[sizeof local]; } + global_def_ret_ref_local (void) + /* { dg-warning ".local. is static but used in inline function .global_def_ret_ref_local." "" { target *-*-* } .-2 } */ + { + abort (); + } + +inline struct { int a[sizeof cst_local]; } + global_def_ret_ref_cst_local (void) + /* { dg-warning ".cst_local. is static but used in inline function .global_def_ret_ref_cst_local." "" { target *-*-* } .-2 } */ + { + abort (); + } + +/* Verify that static inline definitions don't trigger a warning + when their return types reference static or extern variables. */ + +static inline struct { int a[sizeof global]; } + local_def_ret_ref_global (void) { abort (); } +static inline struct { int a[sizeof cst_global]; } + local_def_ret_ref_cst_global (void) { abort (); } +static inline struct { int a[sizeof local]; } + local_def_ret_ref_local (void) { abort (); } +static inline struct { int a[sizeof cst_local]; } + local_def_ret_ref_cst_local (void) { abort (); } + +/* { dg-prune-output "declared but never defined" } */ Index: gcc/testsuite/gcc.dg/inline-41.c =================================================================== --- gcc/testsuite/gcc.dg/inline-41.c (nonexistent) +++ gcc/testsuite/gcc.dg/inline-41.c (working copy) @@ -0,0 +1,53 @@ +/* PR c/88718 - Strange inconsistency between old style and new style + definitions of inline functions + Verify that warnings for references to multiple static variables in + inline function signatures are all diagnosed and that the locations + of the diagnostics, including the notes, are correct. + { dg-do compile } + { dg-options "-Wall -Wno-unused" } */ + +extern const int e1; +extern const int e2; + +static const int s1 = 1; /* { dg-message ".s1. declared here" } */ +static const int s2 = 2; /* { dg-message ".s2. declared here" } */ + + +inline void fes (int (*)[sizeof e1][sizeof s2]); + +inline void +fes (int (*p) + [sizeof e1] + [sizeof s2]) /* { dg-warning ".s2. is static but used in inline function .fes. which is not static" } */ +{ } + + +inline void fse (int (*)[sizeof s1][sizeof e2]); + +inline void +fse (int (*p) + [sizeof s1] /* { dg-warning ".s1. is static but used in inline function .fse. which is not static" } */ + [sizeof e2]) +{ } + + +static int s3 = 3; /* { dg-message ".s3. declared here" } */ +static int s4 = 4; /* { dg-message ".s4. declared here" } */ + +/* Use s1 and s2 here and verify that the warnings mention s3 and s4 + referenced in the definition. */ +inline void fss (int (*)[sizeof s1][sizeof s2]); + +inline void +fss (int (*p) + [sizeof s3] /* { dg-warning ".s3. is static but used in inline function .fss. which is not static" } */ + [sizeof s4]) /* { dg-warning ".s4. is static but used in inline function .fss. which is not static" } */ +{ } + + +/* Use s1 and s2 in the declaration and verify that no warnings are + emitted for the definition that doesn't reference any statics. */ +inline void fee (int (*)[sizeof s1][sizeof s2]); + +inline void +fee (int (*p)[sizeof e1][sizeof e2]) { }