From patchwork Thu Jul 27 14:44:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 794456 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-459178-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="VpeKkcmN"; 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 3xJFB12321z9s1h for ; Fri, 28 Jul 2017 00:44:40 +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:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=hNyFEV20dX4Ec+s2xxE4CDKgSX55saPY+S5syy38S1C+ruCKVJ7Mf xbLAp1zQz/daEf6b9WMdQzuAvnXDk56QfX4A+ssebPbf8sP63yoRkN3+Vm5KGdZE zPcGIakZAWL+j/Ja8Ehh7UyAZOsCCuRAO+OYVZxiPdCHnDgoCGZwbw= 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:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=02H9cVg/3oLv2ZMObnMYZTOesa0=; b=VpeKkcmN4DTltZ7r7E9X f97Bc4B3+nNUL53NC4ljgQz5jn+XMm91FQA6P+BeqaxA4Oh7epaFKeNZp7Ls6Iu2 GsvnBQSWFZo8NMAyKyMQ6VD3QboOQuwz+Tgy7g3Q8W9OYXPiaTmjOG7CxdT3gUbh u3Jxf1QZvViFpveaagkxqYU= Received: (qmail 46659 invoked by alias); 27 Jul 2017 14:44:29 -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 46636 invoked by uid 89); 27 Jul 2017 14:44:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=BODY X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Jul 2017 14:44:25 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B1724C081F4B; Thu, 27 Jul 2017 14:44:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B1724C081F4B Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=polacek@redhat.com Received: from redhat.com (ovpn-204-38.brq.redhat.com [10.40.204.38]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9D3ED6F9FF; Thu, 27 Jul 2017 14:44:23 +0000 (UTC) Date: Thu, 27 Jul 2017 16:44:20 +0200 From: Marek Polacek To: GCC Patches , David Malcolm , Joseph Myers Subject: c-family PATCH to improve and simplify -Wmultistatement-macros (PR c/81448, c/81306) Message-ID: <20170727144420.GK3397@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.8.3 (2017-05-23) To recap, -Wmultistatement-macros warns for code like #define FOO y++; z++ if (i) FOO; if FOO expands to multiple statements not wrapped in {}. It tracks the location of the guard (if), the location of the body of the conditional (y++) and the location of the following statement (z++). This warning only warns if BODY and NEXT come from the same macro expansion, and the guard doesn't come from that expansion. But it can be fooled with code like #define IF if(1) #define BAR \ void bar (void) \ { \ IF \ baz (); \ return; \ } where BODY and NEXT come from the same expansion (BAR), the guard doesn't, yet we shouldn't warn. To stave off the bogus warnings, my fix is to keep unwinding the IF macro, and if any expansion is the same as the expansion of BODY and LOC, don't warn. So basically avoid the warning in this scenario: BAR | +------------------+ | | IF baz (); return; | if While working on the fix, I noticed I can simplify the code a lot, the MACRO_MAP_LOCATIONS walk is not necessary -- which should also fix the ugly PR81306 - yay! Bootstrapped/regtested on x86_64-linux and ppc64le-linux, ok for trunk? 2017-07-27 Marek Polacek PR c/81448 PR c/81306 * c-warn.c (warn_for_multistatement_macros): Prevent bogus warnings. Avoid walking MACRO_MAP_LOCATIONS. * c-c++-common/Wmultistatement-macros-13.c: New test. Marek diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c index a8b38c1b98d..0bfb24994d9 100644 --- gcc/c-family/c-warn.c +++ gcc/c-family/c-warn.c @@ -2456,34 +2456,44 @@ warn_for_multistatement_macros (location_t body_loc, location_t next_loc, || body_loc_exp == next_loc_exp) return; - /* Find the macro map for the macro expansion BODY_LOC. */ - const line_map *map = linemap_lookup (line_table, body_loc); - const line_map_macro *macro_map = linemap_check_macro (map); - - /* Now see if the following token is coming from the same macro - expansion. If it is, it's a problem, because it should've been - parsed at this point. We only look at odd-numbered indexes - within the MACRO_MAP_LOCATIONS array, i.e. the spelling locations - of the tokens. */ - bool found_guard = false; - bool found_next = false; - for (unsigned int i = 1; - i < 2 * MACRO_MAP_NUM_MACRO_TOKENS (macro_map); - i += 2) - { - if (MACRO_MAP_LOCATIONS (macro_map)[i] == next_loc_exp) - found_next = true; - if (MACRO_MAP_LOCATIONS (macro_map)[i] == guard_loc_exp) - found_guard = true; - } + /* Find the macro maps for the macro expansions. */ + const line_map *body_map = linemap_lookup (line_table, body_loc); + const line_map *next_map = linemap_lookup (line_table, next_loc); + const line_map *guard_map = linemap_lookup (line_table, guard_loc); + + /* Now see if the following token (after the body) is coming from the + same macro expansion. If it is, it might be a problem. */ + if (body_map != next_map) + return; /* The conditional itself must not come from the same expansion, because we don't want to warn about #define IF if (x) x++; y++ and similar. */ - if (!found_next || found_guard) + if (guard_map == body_map) return; + /* Handle the case where NEXT and BODY come from the same expansion while + GUARD doesn't, yet we shouldn't warn. E.g. + + #define GUARD if (...) + #define GUARD2 GUARD + + and in the definition of another macro: + + GUARD2 + foo (); + return 1; + */ + while (linemap_macro_expansion_map_p (guard_map)) + { + const line_map_macro *mm = linemap_check_macro (guard_map); + guard_loc_exp = MACRO_MAP_EXPANSION_POINT_LOCATION (mm); + guard_map = linemap_lookup (line_table, guard_loc_exp); + if (guard_map == body_map) + return; + } + if (warning_at (body_loc, OPT_Wmultistatement_macros, "macro expands to multiple statements")) inform (guard_loc, "some parts of macro expansion are not guarded by " diff --git gcc/testsuite/c-c++-common/Wmultistatement-macros-13.c gcc/testsuite/c-c++-common/Wmultistatement-macros-13.c index e69de29bb2d..9f42e268d9f 100644 --- gcc/testsuite/c-c++-common/Wmultistatement-macros-13.c +++ gcc/testsuite/c-c++-common/Wmultistatement-macros-13.c @@ -0,0 +1,104 @@ +/* PR c/81448 */ +/* { dg-do compile } */ +/* { dg-options "-Wmultistatement-macros" } */ + +extern int i; + +#define BAD4 i++; i++ /* { dg-warning "macro expands to multiple statements" } */ +#define BAD5 i++; i++ /* { dg-warning "macro expands to multiple statements" } */ +#define BAD6 i++; i++ /* { dg-warning "macro expands to multiple statements" } */ +#define BAD7 i++; i++ /* { dg-warning "macro expands to multiple statements" } */ +#define BAD8 i++; i++ /* { dg-warning "macro expands to multiple statements" } */ +#define BAD9 i++; i++ /* { dg-warning "macro expands to multiple statements" } */ +#define IF if (1) /* { dg-message "not guarded by this 'if' clause" } */ +#define IF2 IF /* { dg-message "in expansion of macro .IF." } */ +#define BADB7 BAD7 /* { dg-message "in expansion of macro .BAD7." } */ +#define BADB8 BAD8 /* { dg-message "in expansion of macro .BAD8." } */ +#define BADB9 BAD9 /* { dg-message "in expansion of macro .BAD9." } */ + +#define FN0 \ +void fn0 (void) \ +{ \ + IF \ + i++; \ + return; \ +} + +#define FN1 \ +void fn1 (void) \ +{ \ + IF2 \ + i++; \ + return; \ +} + +#define FN2 \ +void fn2 (void) \ +{ \ + if (1) \ + i++; \ + return; \ +} + +#define TOP FN3 +#define FN3 \ +void fn3 (void) \ +{ \ + IF \ + i++; \ + return; \ +} + +#define TOP2 FN4 /* { dg-message "in expansion of macro .FN4." } */ +#define FN4 \ +void fn4 (void) \ +{ \ + IF2 /* { dg-message "in expansion of macro .IF2." } */ \ + BAD4; /* { dg-message "in expansion of macro .BAD4." } */ \ +} + +#define FN5 \ +void fn5 (void) \ +{ \ + IF /* { dg-message "in expansion of macro .IF." } */ \ + BAD5; /* { dg-message "in expansion of macro .BAD5." } */ \ +} + +#define FN6 \ +void fn6 (void) \ +{ \ + if (1) /* { dg-message "not guarded by this 'if' clause" } */ \ + BAD6; /* { dg-message "in expansion of macro .BAD6." } */ \ +} + +#define FN7 \ +void fn7 (void) \ +{ \ + if (1) /* { dg-message "not guarded by this 'if' clause" } */ \ + BADB7; /* { dg-message "in expansion of macro .BADB7." } */ \ +} + +#define FN8 \ +void fn8 (void) \ +{ \ + IF2 /* { dg-message "in expansion of macro .IF2." } */ \ + BADB8; /* { dg-message "in expansion of macro .BADB8." } */ \ +} + +#define FN9 \ +void fn9 (void) \ +{ \ + IF /* { dg-message "in expansion of macro .IF." } */ \ + BADB9; /* { dg-message "in expansion of macro .BADB9." } */ \ +} + +FN0 +FN1 +FN2 +TOP +TOP2 /* { dg-message "in expansion of macro .TOP2." } */ +FN5 /* { dg-message "in expansion of macro .FN5." } */ +FN6 /* { dg-message "in expansion of macro .FN6." } */ +FN7 /* { dg-message "in expansion of macro .FN7." } */ +FN8 /* { dg-message "in expansion of macro .FN8." } */ +FN9 /* { dg-message "in expansion of macro .FN9." } */