[PATCH 1/3] Fix logic bug in Cilk Plus array expansion

[Patrick Palka]

On Sat, Jan 2, 2016 at 3:21 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Jan 01, 2016 at 10:06:34PM -0700, Jeff Law wrote:
>> >gcc/cp/ChangeLog:
>> >
>> >     * cp-array-notation.c (cp_expand_cond_array_notations): Return
>> >     error_mark_node only if find_rank failed, not if it was
>> >     successful.
>> Can you use -fdump-tree-original in the testcase and verify there's no <<<
>> error >>> expressions in the resulting dump file?
>>
>> With that change, this is OK.
>
> I think the patch is incomplete.  Because, find_rank does not always emit
> an error if it returns false, so we again have cases where we can get
> error_mark_node in the code without error being emitted.
>       else if (*rank != current_rank)
>         {
>           /* In this case, find rank is being recursed through a set of
>              expression of the form A <OPERATION> B, where A and B both have
>              array notations in them and the rank of A is not equal to rank of B.
>              A simple example of such case is the following: X[:] + Y[:][:] */
>           *rank = current_rank;
>           return false;
>         }
> and other spots.  E.g.
>                   if (prev_arg && EXPR_HAS_LOCATION (prev_arg))
>                     error_at (EXPR_LOCATION (prev_arg),
>                               "rank mismatch between %qE and %qE", prev_arg,
>                               TREE_OPERAND (expr, ii));
> looks very suspicious.

Hmm, good point. Here's a contrived test case that causes find_rank to
return false without emitting an error message thus we again end up
with an error_mark_node in the gimplifier:

/* { dg-do compile } */
/* { dg-options "-fcilkplus" } */

void foo() {}

#define ALEN 1024

int main(int argc, char* argv[])
{
  typedef void (*f) (void *);
  f b[ALEN], c[ALEN][ALEN];
  (b[:]) ((void *)c[:][:]);
  _Cilk_spawn foo();
  return 0;
}

But this patch was intended to only fix the testsuite fallout that
patch 3 would have otherwise caused, and not to e.g. fix all the bugs
with find_rank.

(BTW patch 3 also makes this test case trigger an ICE, instead of
being silently miscompiled.)