From patchwork Thu Jun 5 11:18:33 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Enkovich X-Patchwork-Id: 356304 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 63A1B14007F for ; Thu, 5 Jun 2014 21:18:54 +1000 (EST) 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:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=Fc1DgQ25fIeGZ/qD+ HUSKhjs4uuObYJdoz4AETNRSAKLPfI24Q1cm29HlM4xZpc9UFPbEkp34FSdbJvLl 1SxIdbFcuTGrFbSg0TIv4mcaQwQIYyXBuEBLKsbCEIllQ2abWqKVLiAdgmbuBukO H0jWJMPxTHH04SkvePhGdOGK4Y= 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:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=uhPnmkAcwt2MB7So5zR5kUF m13s=; b=hyFVb6mTcqhGttPAhTDzP7fiCHQvcxtT2hltu3Fvg68rrpDaTIS97jn cUuci05cuRc9CURN9oUnvD2qyiAmLmf3ZqBNAihvAW3IhepwWr3Budj0UZzSpElJ EG4/gj0tXPoTe/gnheSZ/8LfZMBACCtO/wQsx3kVP5jitGCqSjEM= Received: (qmail 8362 invoked by alias); 5 Jun 2014 11:18:48 -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 8343 invoked by uid 89); 5 Jun 2014 11:18:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pb0-f48.google.com Received: from mail-pb0-f48.google.com (HELO mail-pb0-f48.google.com) (209.85.160.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 05 Jun 2014 11:18:46 +0000 Received: by mail-pb0-f48.google.com with SMTP id rr13so974727pbb.35 for ; Thu, 05 Jun 2014 04:18:44 -0700 (PDT) X-Received: by 10.68.197.39 with SMTP id ir7mr76612275pbc.78.1401967124555; Thu, 05 Jun 2014 04:18:44 -0700 (PDT) Received: from msticlxl57.ims.intel.com ([192.55.54.40]) by mx.google.com with ESMTPSA id ue3sm21645966pbc.49.2014.06.05.04.18.42 for (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 05 Jun 2014 04:18:43 -0700 (PDT) Date: Thu, 5 Jun 2014 15:18:33 +0400 From: Ilya Enkovich To: Richard Biener Cc: Jeff Law , gcc-patches Subject: Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning Message-ID: <20140605111833.GA7275@msticlxl57.ims.intel.com> References: <20140529110526.GB30323@msticlxl57.ims.intel.com> <5388B8F2.3050509@redhat.com> <20140602104806.GC24161@msticlxl57.ims.intel.com> <538CB407.30504@redhat.com> <538EC0BB.6050307@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes On 04 Jun 11:59, Richard Biener wrote: > On Wed, Jun 4, 2014 at 8:46 AM, Jeff Law wrote: > > On 06/03/14 03:29, Richard Biener wrote: > >> > >> On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich > >> wrote: > >>> > >>> 2014-06-02 21:27 GMT+04:00 Jeff Law : > >>>> > >>>> On 06/02/14 04:48, Ilya Enkovich wrote: > >>>>>> > >>>>>> > >>>>>> Hmm, so if I understand things correctly, src_fun has no loop > >>>>>> structures attached, thus there's nothing to copy. Presumably at > >>>>>> some later point we build loop structures for the copy from scratch? > >>>>> > >>>>> > >>>>> I suppose it is just a simple bug with absent NULL pointer check. Here > >>>>> is > >>>>> original code: > >>>>> > >>>>> /* Duplicate the loop tree, if available and wanted. */ > >>>>> if (loops_for_fn (src_cfun) != NULL > >>>>> && current_loops != NULL) > >>>>> { > >>>>> copy_loops (id, entry_block_map->loop_father, > >>>>> get_loop (src_cfun, 0)); > >>>>> /* Defer to cfgcleanup to update loop-father fields of > >>>>> basic-blocks. */ > >>>>> loops_state_set (LOOPS_NEED_FIXUP); > >>>>> } > >>>>> > >>>>> /* If the loop tree in the source function needed fixup, mark the > >>>>> destination loop tree for fixup, too. */ > >>>>> if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP) > >>>>> loops_state_set (LOOPS_NEED_FIXUP); > >>>>> > >>>>> As you may see we have check for absent loops structure in the first > >>>>> if-statement and no check in the second one. I hit segfault and added > >>>>> the > >>>>> check. > >>>> > >>>> > >>>> Downthread you indicated you're not in SSA form which might explain the > >>>> inconsistency here. If so, then we need to make sure that the loop & df > >>>> structures do get set up properly later. > >>> > >>> > >>> That is what init_data_structures pass will do for us as Richard pointed. > >>> Right? > >> > >> > >> loops are set up during the CFG construction and thus are available > >> everywhere. > > > > Which would argue that the hunk that checks for the loop tree's existence > > before accessing it should not be needed. Ilya -- is it possible you hit > > this prior to Richi's work to build the loop structures as part of CFG > > construction and maintain them throughout compilation. > > That's likely. It's still on my list of janitor things to do to remove all > those if (current_loops) checks ... I tried to remove this loops check and got no failures this time. So, here is a new patch version. Bootstrapped and tested on linux-x86_64. Thanks, Ilya --- gcc/ 2014-06-05 Ilya Enkovich * tree-inline.c (tree_function_versioning): Check DF info existence before accessing it. diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 4293241..2972346 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -5350,8 +5350,9 @@ tree_function_versioning (tree old_decl, tree new_decl, DECL_ARGUMENTS (new_decl) = DECL_ARGUMENTS (old_decl); initialize_cfun (new_decl, old_decl, old_entry_block->count); - DECL_STRUCT_FUNCTION (new_decl)->gimple_df->ipa_pta - = id.src_cfun->gimple_df->ipa_pta; + if (DECL_STRUCT_FUNCTION (new_decl)->gimple_df) + DECL_STRUCT_FUNCTION (new_decl)->gimple_df->ipa_pta + = id.src_cfun->gimple_df->ipa_pta; /* Copy the function's static chain. */ p = DECL_STRUCT_FUNCTION (old_decl)->static_chain_decl;