On 08/12/16 22:07, Marek Polacek wrote:
This test ICEs since r240072 where Tom disallowed using va_list * as a first
argument to va_arg.  I think the problem is in the ADDR_EXPR check in
gimplify_va_arg_expr.  It's meant to handle Case 1, but the valist doesn't
always have to be ADDR_EXPR.  E.g. for this testcase build_va_arg creates
VA_ARG_EXPR <&*s>
but during clone_body this is transformed into
VA_ARG_EXPR <s>
so we have a valid valist, but it's not an ADDR_EXPR, so we don't call
targetm.canonical_va_list_type and crash on the subsequent assert.

Tom, does this make sense to you?

Hi,

duing the compilation resulting in the ICE, the "Case 1" is triggered in build_va_arg, so we need the targetm.canonical_va_list_type retry in gimplify_va_arg_expr. Hence, the patch looks good to me.

[ FTR, it would be cleaner to add an encoding of the case to VA_ARG_EXPR and call either:
...
    have_va_type
      = targetm.canonical_va_list_type (TREE_TYPE (TREE_TYPE (valist)));
...

or:
...
    have_va_type
      = targetm.canonical_va_list_type (TREE_TYPE (valist));
...
in gimplify_va_arg_expr based on that encoding, but that's probably overkill. ]

Thanks,
- Tom

Reply via email to