diff mbox

[Cilk+,PR57541] Additional fix for issues witn array notations

Message ID 0EFAB2BDD0F67E4FB6CCC8B9F87D756956489DB3@IRSMSX101.ger.corp.intel.com
State New
Headers show

Commit Message

Zamyatin, Igor June 16, 2014, 8:13 p.m. UTC
Hi All!

The patch fixes ICE in array notation for the cases of incorrect arguments of Cilk+ builtins and undeclared initial index.

Is it ok for trunk and 4.9?

Thanks,
Igor

Comments

Jeff Law June 17, 2014, 6:30 p.m. UTC | #1
On 06/16/14 14:13, Zamyatin, Igor wrote:
> Hi All!
>
> The patch fixes ICE in array notation for the cases of incorrect arguments of Cilk+ builtins and undeclared initial index.
>
> Is it ok for trunk and 4.9?
>
> Thanks,
> Igor
>
> diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
> index 54d0de7..56e1b0b 100644
> --- a/gcc/c/ChangeLog
> +++ b/gcc/c/ChangeLog
> @@ -1,3 +1,12 @@
> +2014-06-16  Igor Zamyatin  <igor.zamyatin@intel.com>
> +
> +       PR middle-end/57541
> +       * c-array-notation.c (fix_builtin_array_notation_fn):
> +       Check for 0 arguments in builtin call. Check that bultin argument is
> +       correct.
> +       * c-parser.c (c_parser_array_notation): Check for incorrect initial
> +       index.
Shouldn't this have been caught earlier?  ISTM we should be catching any 
argument mix-ups during parsing?!?    Is there some reason we don't do that?

jeff
Zamyatin, Igor June 19, 2014, 12:30 p.m. UTC | #2
> On 06/16/14 14:13, Zamyatin, Igor wrote:
> > Hi All!
> >
> > The patch fixes ICE in array notation for the cases of incorrect arguments of
> Cilk+ builtins and undeclared initial index.
> >
> > Is it ok for trunk and 4.9?
> >
> > Thanks,
> > Igor
> >
> > diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog index 54d0de7..56e1b0b
> > 100644
> > --- a/gcc/c/ChangeLog
> > +++ b/gcc/c/ChangeLog
> > @@ -1,3 +1,12 @@
> > +2014-06-16  Igor Zamyatin  <igor.zamyatin@intel.com>
> > +
> > +       PR middle-end/57541
> > +       * c-array-notation.c (fix_builtin_array_notation_fn):
> > +       Check for 0 arguments in builtin call. Check that bultin argument is
> > +       correct.
> > +       * c-parser.c (c_parser_array_notation): Check for incorrect initial
> > +       index.
> Shouldn't this have been caught earlier?  ISTM we should be catching any
> argument mix-ups during parsing?!?    Is there some reason we don't do
> that?

But call stack for fix_builtin_array_notation_fn is from c-parser...

Thanks,
Igor

> 
> jeff
Jeff Law June 25, 2014, 9:23 p.m. UTC | #3
On 06/19/14 06:30, Zamyatin, Igor wrote:
>> On 06/16/14 14:13, Zamyatin, Igor wrote:
>>> Hi All!
>>>
>>> The patch fixes ICE in array notation for the cases of incorrect arguments of
>> Cilk+ builtins and undeclared initial index.
>>>
>>> Is it ok for trunk and 4.9?
>>>
>>> Thanks,
>>> Igor
>>>
>>> diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog index 54d0de7..56e1b0b
>>> 100644
>>> --- a/gcc/c/ChangeLog
>>> +++ b/gcc/c/ChangeLog
>>> @@ -1,3 +1,12 @@
>>> +2014-06-16  Igor Zamyatin  <igor.zamyatin@intel.com>
>>> +
>>> +       PR middle-end/57541
>>> +       * c-array-notation.c (fix_builtin_array_notation_fn):
>>> +       Check for 0 arguments in builtin call. Check that bultin argument is
>>> +       correct.
>>> +       * c-parser.c (c_parser_array_notation): Check for incorrect initial
>>> +       index.
>> Shouldn't this have been caught earlier?  ISTM we should be catching any
>> argument mix-ups during parsing?!?    Is there some reason we don't do
>> that?
>
> But call stack for fix_builtin_array_notation_fn is from c-parser...
Joys :(  In that case, approved for  the trunk & 4.9.


Jeff
diff mbox

Patch

diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 54d0de7..56e1b0b 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,12 @@ 
+2014-06-16  Igor Zamyatin  <igor.zamyatin@intel.com>
+
+       PR middle-end/57541
+       * c-array-notation.c (fix_builtin_array_notation_fn):
+       Check for 0 arguments in builtin call. Check that bultin argument is
+       correct.
+       * c-parser.c (c_parser_array_notation): Check for incorrect initial
+       index.
+
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 053155a..a2c66a8 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,10 @@ 
+2014-06-16  Igor Zamyatin  <igor.zamyatin@intel.com>
+
+       PR middle-end/57541
+       * cp-array-notation.c (expand_sec_reduce_builtin):
+       Check that bultin argument is correct.
+       * call.c (build_cxx_call): Check for 0 arguments in builtin call.
+
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index f9f03d9..f08600c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@ 
+2014-06-16  Igor Zamyatin  <igor.zamyatin@intel.com>
+
+       PR middle-end/57541
+       * c-c++-common/cilk-plus/AN/pr57541.c: New case added.
+       * c-c++-common/cilk-plus/AN/pr57541-2.c: New test.
+


diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
index b4015b8..67a8931 100644
--- a/gcc/c/c-array-notation.c
+++ b/gcc/c/c-array-notation.c
@@ -214,6 +214,13 @@  fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
   if (an_type == BUILT_IN_NONE)
     return NULL_TREE;

+  /* Builtin call should contain at least one argument.  */
+  if (call_expr_nargs (an_builtin_fn) == 0)
+    {
+      error_at (EXPR_LOCATION (an_builtin_fn), "Invalid builtin arguments");
+      return error_mark_node;
+    }
+
   if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE
       || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING)
     {
@@ -238,7 +245,10 @@  fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
     return error_mark_node;
  
   if (rank == 0)
-    return an_builtin_fn;
+    {
+      error_at (location, "Invalid builtin arguments");
+      return error_mark_node;
+    }
   else if (rank > 1 
           && (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND
               || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND))
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index abd636c..a04a23e 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -14097,7 +14097,7 @@  c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
   tree value_tree = NULL_TREE, type = NULL_TREE, array_type = NULL_TREE;
   tree array_type_domain = NULL_TREE; 
 
-  if (array_value == error_mark_node)
+  if (array_value == error_mark_node || initial_index == error_mark_node)
     {
       /* No need to continue.  If either of these 2 were true, then an error
         must be emitted already.  Thus, no need to emit them twice.  */

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 75a6a4a..e021a38 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7415,6 +7415,11 @@  build_cxx_call (tree fn, int nargs, tree *argarray,
          || bif == BUILT_IN_CILKPLUS_SEC_REDUCE
          || bif == BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING)
        { 
+         if (call_expr_nargs (fn) == 0)
+           {
+             error_at (EXPR_LOCATION (fn), "Invalid builtin arguments");
+             return error_mark_node;
+           }
          /* for bif == BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_ZERO or
             BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO or
             BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO or 
diff --git a/gcc/cp/cp-array-notation.c b/gcc/cp/cp-array-notation.c
index 0538e55..b45449b 100644
--- a/gcc/cp/cp-array-notation.c
+++ b/gcc/cp/cp-array-notation.c
@@ -250,7 +250,10 @@  expand_sec_reduce_builtin (tree an_builtin_fn, tree *new_var)
   if (!find_rank (location, an_builtin_fn, an_builtin_fn, true, &rank))
       return error_mark_node;
   if (rank == 0)
-    return an_builtin_fn;
+    {
+      error_at (location, "Invalid builtin arguments");
+      return error_mark_node;
+    }
   else if (rank > 1 
           && (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND
               || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND))

diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c
new file mode 100644
index 0000000..83325a7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541-2.c
@@ -0,0 +1,15 @@ 
+/* PR middle-end/57541 */
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+int foo1 ()
+{
+  int a;
+  a = __sec_reduce_add (1); /* { dg-error "Invalid builtin arguments" } */
+}
+
+int foo2 ()
+{
+  int a;
+  a = __sec_reduce_add (); /* { dg-error "Invalid builtin arguments" } */
+}
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541.c
index 9bff079..f379e46 100755
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541.c
@@ -1,9 +1,10 @@ 
+/* PR middle-end/57541 */
/* { dg-do compile } */
/* { dg-options "-fcilkplus" } */

 int A[10];

-int main () {
+int foo () {

   /* C compiler uses the term "undeclared" whereas C++ compiler uses
     "not declared".  Thus, grepping for declared seem to be the easiest.  */
@@ -13,5 +14,13 @@  int main () {
   A[l:s:c];
}

-/* { dg-message "note: each" "defined" { target c }  10 } */
+int foo1 (int N) {
+
+  char c = (char)N;
+  short s = (short)N;
+  A[l:s:c]; /* { dg-error "declared" } */
+}
+
+
+/* { dg-message "note: each" "defined" { target c }  11 } */