From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 125959 invoked by alias); 22 Jun 2015 08:05:40 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 125944 invoked by uid 89); 22 Jun 2015 08:05:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: fencepost.gnu.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (208.118.235.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 22 Jun 2015 08:05:38 +0000 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60842) by fencepost.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1Z6wjY-0004YS-0o for gcc-patches@gnu.org; Mon, 22 Jun 2015 04:05:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z6wjU-0006lm-63 for gcc-patches@gnu.org; Mon, 22 Jun 2015 04:05:35 -0400 Received: from relay1.mentorg.com ([192.94.38.131]:41862) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z6wjT-0006dm-Vi for gcc-patches@gnu.org; Mon, 22 Jun 2015 04:05:32 -0400 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1Z6wjP-0001pp-UV from Tom_deVries@mentor.com for gcc-patches@gnu.org; Mon, 22 Jun 2015 01:05:28 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Mon, 22 Jun 2015 09:04:31 +0100 Message-ID: <5587C18A.9050304@mentor.com> Date: Mon, 22 Jun 2015 08:20:00 -0000 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Subject: [PATCH] Check dominator info in compute_dominance_frontiers Content-Type: multipart/mixed; boundary="------------050609000708090002040701" X-detected-operating-system: by eggs.gnu.org: Windows NT kernel [generic] [fuzzy] X-Received-From: 192.94.38.131 X-SW-Source: 2015-06/txt/msg01408.txt.bz2 --------------050609000708090002040701 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1986 Hi, during development of a patch I ran into a case where compute_dominance_frontiers was called with incorrect dominance info. The result was a segmentation violation somewhere in the bitmap code while executing this bitmap_set_bit in compute_dominance_frontiers_1: ... if (!bitmap_set_bit (&frontiers[runner->index], b->index)) break; ... The segmentation violation happens because runner->index is 0, and frontiers[0] is uninitialized. [ The initialization in update_ssa looks like this: ... dfs = XNEWVEC (bitmap_head, last_basic_block_for_fn (cfun)); FOR_EACH_BB_FN (bb, cfun) bitmap_initialize (&dfs[bb->index], &bitmap_default_obstack); compute_dominance_frontiers (dfs); ... FOR_EACH_BB_FN skips over the entry-block and the exit-block, so dfs[0] (frontiers[0] in compute_dominance_frontiers_1) is not initialized. We could add initialization by making the entry/exit-block bitmap_heads empty and setting the obstack to a reserved obstack bitmap_no_obstack for which allocation results in an assert. ] AFAIU, the immediate problem is not that frontiers[0] is uninitialized, but that the loop reaches the state of runner->index == 0, due to the incorrect dominance info. The patch adds an assert to the loop in compute_dominance_frontiers_1, to make the failure mode cleaner and easier to understand. I think we wouldn't catch all errors in dominance info with this assert. So the patch also contains an ENABLE_CHECKING-enabled verify_dominators call at the start of compute_dominance_frontiers. I'm not sure if: - adding the verify_dominators call is too costly in runtime. - the verify_dominators call should be inside or outside the TV_DOM_FRONTIERS measurement. - there is a level of ENABLE_CHECKING that is more appropriate for the verify_dominators call. Is this ok for trunk if bootstrap and reg-test on x86_64 succeeds? Thanks, - Tom --------------050609000708090002040701 Content-Type: text/x-patch; name="0002-Check-dominator-info-in-compute_dominance_frontiers.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0002-Check-dominator-info-in-compute_dominance_frontiers.pat"; filename*1="ch" Content-length: 1227 Check dominator info in compute_dominance_frontiers 2015-06-22 Tom de Vries * cfganal.c (compute_dominance_frontiers_1): Add assert. (compute_dominance_frontiers): Verify dominators if ENABLE_CHECKING. --- gcc/cfganal.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/gcc/cfganal.c b/gcc/cfganal.c index b8d67bc..0e0e2bb 100644 --- a/gcc/cfganal.c +++ b/gcc/cfganal.c @@ -1261,6 +1261,11 @@ compute_dominance_frontiers_1 (bitmap_head *frontiers) domsb = get_immediate_dominator (CDI_DOMINATORS, b); while (runner != domsb) { + /* If you're running into this assert, the dominator info is + incorrect. Try enabling the verify_dominators call at the + start of compute_dominance_frontiers. */ + gcc_assert (runner != ENTRY_BLOCK_PTR_FOR_FN (cfun)); + if (!bitmap_set_bit (&frontiers[runner->index], b->index)) break; @@ -1276,6 +1281,10 @@ compute_dominance_frontiers_1 (bitmap_head *frontiers) void compute_dominance_frontiers (bitmap_head *frontiers) { +#if ENABLE_CHECKING + verify_dominators (CDI_DOMINATORS); +#endif + timevar_push (TV_DOM_FRONTIERS); compute_dominance_frontiers_1 (frontiers); -- 1.9.1 --------------050609000708090002040701--