> -----Original Message-----
> From: Jeff Law [mailto:[email protected]]
> Sent: Monday, June 03, 2013 3:07 PM
> To: Iyer, Balaji V
> Cc: [email protected]; Steve Ellcey
> Subject: Re: [PATCH] pr57457
>
> On 05/31/2013 12:01 PM, Iyer, Balaji V wrote:
> >
> >
> >> -----Original Message----- From: [email protected]
> >> [mailto:gcc-patches- [email protected]] On Behalf Of Jeff Law Sent:
> >> Friday, May 31, 2013 11:50 AM To: Iyer, Balaji V Cc:
> >> [email protected]; 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 <[email protected]>
> >>>
> >>> * 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
> >>> <[email protected]>
> >>>
> >>> 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 <[email protected]>
PR C/57457
* c-c++-common/cilk-plus/AN/pr57457.c: New test.
gcc/c/ChangeLog
2013-06-04 Balaji V. Iyer <[email protected]>
* 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);
+}
