> -----Original Message----- > From: Jeff Law [mailto:l...@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-ow...@gcc.gnu.org > >> [mailto:gcc-patches- ow...@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 <balaji.v.i...@intel.com> > >>> > >>> * 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 > >>> <balaji.v.i...@intel.com> > >>> > >>> 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 <balaji.v.i...@intel.com> PR C/57457 * c-c++-common/cilk-plus/AN/pr57457.c: New test. gcc/c/ChangeLog 2013-06-04 Balaji V. Iyer <balaji.v.i...@intel.com> * 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<tree, va_gc> *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<tree, va_gc> *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); +}