This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix ICE with __builtin_va_list on i?86 (PR middle-end/78716)
On December 12, 2016 11:56:36 AM GMT+01:00, Marek Polacek <polacek@redhat.com> wrote:
>On Mon, Dec 12, 2016 at 11:10:31AM +0100, Tom de Vries wrote:
>> 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.
>
>Thanks.
>
>Ok to commit, then?
>
OK.
Richard.
>> [ 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. ]
>
> Marek