On 03/03/2016 09:15 AM, Marek Polacek wrote:
On Thu, Mar 03, 2016 at 03:28:01PM +0100, Jakub Jelinek wrote:
On Thu, Mar 03, 2016 at 03:15:41PM +0100, Marek Polacek wrote:
This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so e.g.
_Cilk_spawn (void) is invalid.  The function call after the cilk_spawn keyword
is parsed using recursive call in c_parser_postfix_expression (case
RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
typename, so it thinks we're inside a compound literal, which means it expects
'{', but that isn't there, so we crash on the assert in c_parser_braced_init:
gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
But as the comment in c_parser_postfix_expression says, the code for parsing
a compound literal here is likely dead.  I made an experiment and added
gcc_unreachable () in that block, ran regtest, and there were no failures.
Thus it should be safe to just remove the code, which also fixes this ICE; with
the patch we just give a proper error and don't crash anymore.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
nervous about the change, so maybe better table until gcc7?

This reminds me of PR67495.  Perhaps the answer here is also during the
_Cilk* stuff parsing don't call c_parser_postfix_expression, but call
c_parser_cast_expression instead and then verify what it got?

Alternatively this one works as well.  I don't know if any verification of the
result should be done (in the second call, the first one is invalid anyway).

2016-03-03  Marek Polacek  <pola...@redhat.com>

        PR c/69798
        * c-parser.c (c_parser_postfix_expression): Call
        c_parser_cast_expression instead of c_parser_postfix_expression.

        * gcc.dg/cilk-plus/pr69798-1.c: New test.
        * gcc.dg/cilk-plus/pr69798-2.c: New test.
Do we need to do anything for the call into c_parser_postfix_expression that occurs between the two you changed in this patch.

I think you can get into that code with something like

_Cilk_spawn _Cilk_spawn (<whatever>));


For the non-error case (the second one you changed), I think we do want to verify that the expression we got back was a function call and issue an appropriate error otherwise. You could make an argument that we should issue a diagnostic for the other cases as well, but it's less obviously correct.

jeff


Reply via email to