From patchwork Wed Jun 13 22:34:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 929115 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-479681-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="m7FVK6Pg"; 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 415hQl3H5sz9ryk for ; Thu, 14 Jun 2018 08:35:09 +1000 (AEST) 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=ILJF6eNJ+ZaYtbAkkydZAOhk5i7Kn5NjjQxyDf6U9SOCzxNLxH OLkySh3mQnl+pc26Cwp9ul5wdRLKzM/qyLXj6VhM1pVPai2kd9l3FNZL2Ujp4ZSl ewTN0UzlxM0OBtlvRN7Kf48Nmg66tkZTtwynwlYFGhOVPdL4Ho28qqggE= 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=BAmjOD3jXiv0v3FCUJ9FT9RRrRA=; b=m7FVK6PgZkq8pM0JI/xC lKDt7T0FtiI4Rq+0jlXRubJvlQAMxIItj/WRW4aZ9O37vYu1EIkL66z0XLpdwauc /GRhD6Z1dooqP3LIvEBjRNnIFxv3eKzp3FxW8Jj9yBFpYaUyB+B6OzliBe1o3ac9 j6XtucF20h3eEzYdZ3g/tyQ= Received: (qmail 61652 invoked by alias); 13 Jun 2018 22:35:01 -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 61622 invoked by uid 89); 13 Jun 2018 22:35:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.7 required=5.0 tests=AWL, 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=diagnosing, diagnose, Hx-languages-length:7328 X-HELO: mail-ot0-f170.google.com Received: from mail-ot0-f170.google.com (HELO mail-ot0-f170.google.com) (74.125.82.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Jun 2018 22:34:58 +0000 Received: by mail-ot0-f170.google.com with SMTP id q17-v6so4862621otg.2 for ; Wed, 13 Jun 2018 15:34:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version; bh=0A52qiaIZkhB3LaqBmco7n10Q5p9fyXflG5CiLoOX4o=; b=bLnyubmUeVKwbleY3ZzVyfw5ac9kLzktghikeXjkD+ZmtY6YbOze8XpH1QC4ImcyrN 41zJUwlDGnGS/LFghFjfCzKxJonpCsrba5NwYBCh3JYcTXO37KJsOI9y6UZy0dLBH7C6 vbUDNjZ+D5gcV9nGlXL5biHS3d/+mzz/67QMJa8PhGaeG7nPowP82Xi9lkjtG/3M9vi1 GtaDn50KmkaWSvevkKxLp4mC1jVORFeARSe0xqq5V1zVU+TP2+85mhmmHerqcMhd+pe7 fo88haNyvs/QVdntpz/SXF9HXtohOGIDGT8MIOGoq9Q5MkAjyua5jpvzkx32PVLMhx5n fDTQ== X-Gm-Message-State: APt69E0EzR0eTrN+tWndcXND0J6hETSILT6nfQFStHTOuapHz8zqnvIT xeQApqqk+YZs0oxcEHjNPQ4ycw== X-Google-Smtp-Source: ADUXVKJIB1OhcEXB44ai0ScFFeUOY7lAVSyh+UiFqhW3FqQh8Hn5AYXXktVLj9L+K2UQycXbfwt+hQ== X-Received: by 2002:a9d:5912:: with SMTP id t18-v6mr26112oth.217.1528929296099; Wed, 13 Jun 2018 15:34:56 -0700 (PDT) Received: from localhost.localdomain (75-166-107-65.hlrn.qwest.net. [75.166.107.65]) by smtp.gmail.com with ESMTPSA id t4-v6sm4385707otd.40.2018.06.13.15.34.54 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 13 Jun 2018 15:34:54 -0700 (PDT) To: Gcc Patch List From: Martin Sebor Subject: [PATCH] tighten up -Wbuiltin-declaration-mismatch (PR 86125) Message-ID: Date: Wed, 13 Jun 2018 16:34:53 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 X-IsSubscribed: yes The C implementation of the -Wbuiltin-declaration-mismatch warning relies on TYPE_MODE to detect incompatibilities between return and argument types in user declarations of built-in functions. That prevents mistakes like char* strlen (const char*); from being detected (where sizeof (char*) == sizeof (size_t)), while at the same diagnosing similar bugs such as char* strcmp (const char*, const char*); where sizeof (char*) != sizeof (int), and always diagnosing benign declarations like: void strcpy (char*, const char*) The attached patch tightens up the detection of incompatible types so that when -Wextra is set it diagnoses more of these kinds of problems, including mismatches in qualifiers. (I added this under -Wextra mainly to avoid the warning with just -Wall for some of the more benign incompatibilities like those in const-qualifiers). This enhancement was prompted by bug 86114. As it is, it would not have prevented the ICE in that bug because it does not reject the incompatible redeclaration (I did that to keep compatibility with prior GCC versions). If there is support for it, though, I think rejecting all incompatible declarations would be a better solution. Partly because the middle-end doesn't seem to fully consider incompatible return types and so might end up introducing transformations that don't make sense. And partly because I think at least the C and POSIX standard built-in functions should be declared with the types they are specified. Martin PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched return type gcc/c/ChangeLog: PR c/86125 * c-decl.c (match_builtin_function_types): Add arguments. (diagnose_mismatched_decls): Diagnose mismatched declarations of built-ins more strictly. gcc/testsuite/ChangeLog: PR c/86125 * gcc.dg/Wbuiltin-declaration-mismatch.c: New test. Index: gcc/c/c-decl.c =================================================================== --- gcc/c/c-decl.c (revision 261551) +++ gcc/c/c-decl.c (working copy) @@ -1630,41 +1630,54 @@ c_bind (location_t loc, tree decl, bool is_global) /* Subroutine of compare_decls. Allow harmless mismatches in return and argument types provided that the type modes match. This function - return a unified type given a suitable match, and 0 otherwise. */ + returns a unified type given a suitable match, and 0 otherwise. */ static tree -match_builtin_function_types (tree newtype, tree oldtype) +match_builtin_function_types (tree newtype, tree oldtype, + tree *strict, unsigned *argno) { - tree newrettype, oldrettype; - tree newargs, oldargs; - tree trytype, tryargs; - /* Accept the return type of the new declaration if same modes. */ - oldrettype = TREE_TYPE (oldtype); - newrettype = TREE_TYPE (newtype); + tree oldrettype = TREE_TYPE (oldtype); + tree newrettype = TREE_TYPE (newtype); + *argno = 0; + *strict = NULL_TREE; + if (TYPE_MODE (oldrettype) != TYPE_MODE (newrettype)) return NULL_TREE; - oldargs = TYPE_ARG_TYPES (oldtype); - newargs = TYPE_ARG_TYPES (newtype); - tryargs = newargs; + if (!comptypes (oldrettype, newrettype)) + *strict = oldrettype; - while (oldargs || newargs) + tree oldargs = TYPE_ARG_TYPES (oldtype); + tree newargs = TYPE_ARG_TYPES (newtype); + tree tryargs = newargs; + + for (unsigned i = 1; oldargs || newargs; ++i) { if (!oldargs || !newargs || !TREE_VALUE (oldargs) - || !TREE_VALUE (newargs) - || TYPE_MODE (TREE_VALUE (oldargs)) - != TYPE_MODE (TREE_VALUE (newargs))) + || !TREE_VALUE (newargs)) return NULL_TREE; + tree oldtype = TREE_VALUE (oldargs); + tree newtype = TREE_VALUE (newargs); + if (TYPE_MODE (TREE_VALUE (oldargs)) + != TYPE_MODE (TREE_VALUE (newargs))) + return NULL_TREE; + + if (!*strict && !comptypes (oldtype, newtype)) + { + *argno = i; + *strict = oldtype; + } + oldargs = TREE_CHAIN (oldargs); newargs = TREE_CHAIN (newargs); } - trytype = build_function_type (newrettype, tryargs); + tree trytype = build_function_type (newrettype, tryargs); /* Allow declaration to change transaction_safe attribute. */ tree oldattrs = TYPE_ATTRIBUTES (oldtype); @@ -1874,10 +1887,19 @@ diagnose_mismatched_decls (tree newdecl, tree oldd if (TREE_CODE (olddecl) == FUNCTION_DECL && DECL_BUILT_IN (olddecl) && !C_DECL_DECLARED_BUILTIN (olddecl)) { - /* Accept harmless mismatch in function types. - This is for the ffs and fprintf builtins. */ - tree trytype = match_builtin_function_types (newtype, oldtype); + /* Accept "harmless" mismatch in function types such as + missing qualifiers or pointer vs same size integer + mismatches. This is for the ffs and fprintf builtins. + However, with -Wextra in effect, diagnose return and + argument types that are incompatible according to + language rules. */ + tree mismatch_expect; + unsigned mismatch_argno; + tree trytype = match_builtin_function_types (newtype, oldtype, + &mismatch_expect, + &mismatch_argno); + if (trytype && comptypes (newtype, trytype)) *oldtypep = oldtype = trytype; else @@ -1885,11 +1907,30 @@ diagnose_mismatched_decls (tree newdecl, tree oldd /* If types don't match for a built-in, throw away the built-in. No point in calling locate_old_decl here, it won't print anything. */ - warning (OPT_Wbuiltin_declaration_mismatch, - "conflicting types for built-in function %q+D", - newdecl); + warning_at (DECL_SOURCE_LOCATION (newdecl), + OPT_Wbuiltin_declaration_mismatch, + "conflicting types for built-in function %qD; " + "expected %qT", + newdecl, oldtype); return false; } + + if (mismatch_expect && extra_warnings) + { + /* If types match only loosely, print a warning but accept + the redeclaration. */ + location_t newloc = DECL_SOURCE_LOCATION (newdecl); + if (mismatch_argno) + warning_at (newloc, OPT_Wbuiltin_declaration_mismatch, + "mismatch in argument %u type of built-in " + "function %qD; expected %qT", + mismatch_argno, newdecl, mismatch_expect); + else + warning_at (newloc, OPT_Wbuiltin_declaration_mismatch, + "mismatch in return type of built-in " + "function %qD; expected %qT", + newdecl, mismatch_expect); + } } else if (TREE_CODE (olddecl) == FUNCTION_DECL && DECL_IS_BUILTIN (olddecl)) Index: gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c =================================================================== --- gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c (nonexistent) +++ gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c (working copy) @@ -0,0 +1,15 @@ +/* PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched + return type + { dg-do compile } + { dg-options "-Wbuiltin-declaration-mismatch -Wextra" } */ + +char* strlen (const char*); /* { dg-warning "mismatch in return type of built-in function .strlen.; expected .(long )?unsigned int." } */ + +enum E { e }; +enum E strcmp (const char*, const char*); /* { dg-warning "mismatch in return type of built-in function .strcmp.; expected .int." } */ + +char* strcpy (char*, char*); /* { dg-warning "mismatch in argument 2 type of built-in function .strcpy.; expected .const char \\*." } */ + +char* strcat (const char*, char*); /* { dg-warning "mismatch in argument 1 type of built-in function .strcat.; expected .char \\*." } */ + +void strchr (const char*, int); /* { dg-warning "conflicting types for built-in function .strchr.; expected .char \\*\\(const char \\*, int\\)." } */