From patchwork Tue Jun 4 17:37:59 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Iyer, Balaji V" X-Patchwork-Id: 248793 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id B7B332C009C for ; Wed, 5 Jun 2013 03:38:17 +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:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; q=dns; s=default; b=hHoyNlW2/FQ/djaD OcyDcCFh81KytnpRoLZgiCrm+O2yuO+5RJ50JoWwSPVRlGx/AmoinC7f08SM7g4W HZR7gNL1zamLm6UdECW3m5WaQP244N9LZwLYdMrhXzIYOGxEvu1YXwWEd58vSqSp R2Hh9fm92qoFUS2cl4GB0KXZAiY= 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:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; s=default; bh=WNi6HThDPmk0F4GwTBVz97 MQj7U=; b=QgiSQFeFTZqATXWEBFVKaWdvUBmkEFv1GWM+lmBwf6qFYSdsU/xSQ/ bvZJpNYPKBI17h4OaZHxX0ts5aibYlPAScwkm5VfE4qnc/NkhKLE3zNM9DXy15J1 rR4AcnWAT07r2GcSlI2YRkLoVFp7hNW1s8zb9LY0i6CjEbQZVspd4= Received: (qmail 22661 invoked by alias); 4 Jun 2013 17:38:11 -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 22648 invoked by uid 89); 4 Jun 2013 17:38:09 -0000 X-Spam-SWARE-Status: No, score=-7.4 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, SPF_PASS, TW_FN autolearn=ham version=3.3.1 Received: from mga11.intel.com (HELO mga11.intel.com) (192.55.52.93) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 04 Jun 2013 17:38:08 +0000 Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 04 Jun 2013 10:38:32 -0700 X-ExtLoop1: 1 Received: from fmsmsx104.amr.corp.intel.com ([10.19.9.35]) by fmsmga001.fm.intel.com with ESMTP; 04 Jun 2013 10:38:21 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by FMSMSX104.amr.corp.intel.com (10.19.9.35) with Microsoft SMTP Server (TLS) id 14.3.123.3; Tue, 4 Jun 2013 10:38:00 -0700 Received: from fmsmsx101.amr.corp.intel.com ([169.254.1.135]) by FMSMSX154.amr.corp.intel.com ([169.254.6.22]) with mapi id 14.03.0123.003; Tue, 4 Jun 2013 10:38:00 -0700 From: "Iyer, Balaji V" To: Jeff Law CC: "gcc-patches@gcc.gnu.org" , Steve Ellcey Subject: RE: [PATCH] pr57457 Date: Tue, 4 Jun 2013 17:37:59 +0000 Message-ID: References: <51A8C6B0.90302@redhat.com> <51ACE93A.6000307@redhat.com> In-Reply-To: <51ACE93A.6000307@redhat.com> MIME-Version: 1.0 X-Virus-Found: No > -----Original Message----- > From: Jeff Law [mailto:law@redhat.com] > Sent: Monday, June 03, 2013 3:07 PM > To: Iyer, Balaji V > Cc: gcc-patches@gcc.gnu.org; Steve Ellcey > Subject: Re: [PATCH] pr57457 > > On 05/31/2013 12:01 PM, Iyer, Balaji V wrote: > > > > > >> -----Original Message----- From: gcc-patches-owner@gcc.gnu.org > >> [mailto:gcc-patches- owner@gcc.gnu.org] On Behalf Of Jeff Law Sent: > >> Friday, May 31, 2013 11:50 AM To: Iyer, Balaji V Cc: > >> gcc-patches@gcc.gnu.org; Steve Ellcey Subject: Re: [PATCH] pr57457 > >> > >> On 05/31/2013 07:54 AM, Iyer, Balaji V wrote: > >>> Hello Everyone, This patch will fix a bug reported in PR57457. > >>> One of the array notation > >> function was not checking for NULL_TREE before accessing its fields. > >> This patch should fix that issue. A test case is also added. > >>> > >>> Is this OK for trunk? > >>> > >>> Here are the ChangeLog Entries: > >>> > >>> gcc/c/ChangeLog 2013-05-31 Balaji V. Iyer > >>> > >>> * c-array-notation.c (is_cilkplus_reduce_builtin): Added a check for > >>> NULL_TREE parameter input. > >>> > >>> gcc/testsuite/ChangeLog 2013-05-31 Balaji V. Iyer > >>> > >>> > >>> PR c/57457 * c-c++-common/cilk-plus/AN/pr57457.c: New testcase. > >> So what you need to do is explain how you got into this function with > >> a NULL fndecl and why that's OK. > > > > Hi Jeff, I looked into it, and there is another function call called > > inform_declaration, and that does exactly what I did (i.e. check for > > NULL fundecl before accessing its fields). From what I can tell, > > fundecl will be NULL_TREE if a function declaration is a function > > pointer. > The problematical calls are coming from convert_arguments which is a bit > inconsistent in how it handles a null FUNDECL. In some places it checks it > direction in convert_arguments and avoids certain actions. In other places the > callee checks. > > The code looks like it's screaming to be simplified. Neither flag_cilkplus nor > FUNDECL change inside the main loop in convert_arguments, so the first thing > I'd ponder is pulling that condition out of the loop. And after doing that we a > similar condition being used to suppress warnings just after the loop. I wonder if > we could have something like > > if (flag_enable_cilkplus > && fundecl > && is_cilkplus_reduction_builtin (fundecl)) > return vec_safe_length (values); > > before the loop, then eliminate the the test inside the loop and just after the > loop. > > That simplifies the code a bit and should fix this problem unless I'm missing > something? Yes, that does simplify the whole thing. Here is an updated ChangeLog and patch (with testcode) attached. So, is it Ok for trunk? gcc/testsuite/ChangeLog 2013-06-04 Balaji V. Iyer PR C/57457 * c-c++-common/cilk-plus/AN/pr57457.c: New test. gcc/c/ChangeLog 2013-06-04 Balaji V. Iyer * c-typeck.c (convert_arguments): Moved checking of builtin cilkplus reduction functions outside the for-loop. Also, added a check if the fundecl is non-NULL. Thanks, Balaji V. Iyer. > > > jeff > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index e5e1455..91ce67a 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -2942,6 +2942,8 @@ convert_arguments (tree typelist, vec *values, break; } } + if (flag_enable_cilkplus && fundecl && is_cilkplus_reduce_builtin (fundecl)) + return vec_safe_length (values); /* Scan the given expressions and types, producing individual converted arguments. */ @@ -2959,17 +2961,6 @@ convert_arguments (tree typelist, vec *values, bool npc; tree parmval; - // FIXME: I assume this code is here to handle the overloaded - // behavior of the __sec_reduce* builtins, and avoid giving - // argument mismatch warnings/errors. We should probably handle - // this with the resolve_overloaded_builtin infrastructure. - /* If the function call is a builtin function call, then we do not - worry about it since we break them up into its equivalent later and - we do the appropriate checks there. */ - if (flag_enable_cilkplus - && is_cilkplus_reduce_builtin (fundecl)) - continue; - if (type == void_type_node) { if (selector) diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c new file mode 100644 index 0000000..68a1fd8 --- /dev/null +++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c @@ -0,0 +1,39 @@ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus" } */ + +/* This test has no array notation components in it and thus should compile + fine without crashing. */ + +typedef unsigned int size_t; +typedef int (*__compar_fn_t) (const void *, const void *); +extern void *bsearch (const void *__key, const void *__base, + size_t __nmemb, size_t __size, __compar_fn_t + __compar) + __attribute__ ((__nonnull__ (1, 2, 5))) ; +extern __inline __attribute__ ((__gnu_inline__)) void * +bsearch (const void *__key, const void *__base, size_t __nmemb, size_t + __size, + __compar_fn_t __compar) +{ + size_t __l, __u, __idx; + const void *__p; + int __comparison; + __l = 0; + __u = __nmemb; + while (__l < __u) + { + __idx = (__l + __u) / 2; + __p = (void *) (((const char *) __base) + + (__idx * __size)); + __comparison = (*__compar) (__key, + __p); + if (__comparison < 0) + __u = __idx; + else if (__comparison > 0) + __l = __idx + 1; + else + return (void *) + __p; + } + return ((void *)0); +}