From patchwork Mon Jun 22 08:04:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 487123 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 92A61140082 for ; Mon, 22 Jun 2015 18:05:47 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=w34WmCcG; 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 :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=psxhmwWW2LV67ESjF2UTkY6M5pQjo2ibmq45PPCoP5d03p fr/XUYPaB16i4cmhObY1/CKDkvj0lvwFkQ4uKhOzA6ls4HvnEvrw9H/DVmnMX0FN ybS4UjUc8opBWKqIMzI004S4/kYglciuYqkmQ8m+1zsmJ+2FZymokLtLIaFcA= 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 :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=TbSvEuzvTb2MPWb1NhQsLcxiU6g=; b=w34WmCcGUToscusZiRMN ig6pHc2AO+tmUnT9BsB1QKNv4bivN7d6rq2q3ZFP21yv4TZJHhjoqRHKLpn+OL8L BdMU9WKkplnM552JgzfnDb65Qim+Vi/eYoRiOhjFyUR9P9e9A/HBXlqKWN+TX64r P0HzYsaYXmD1O4LtZxAyDQU= 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-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 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 10:04:26 +0200 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 X-detected-operating-system: by eggs.gnu.org: Windows NT kernel [generic] [fuzzy] X-Received-From: 192.94.38.131 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 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