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)
- From: Marek Polacek <polacek at redhat dot com>
- To: Tom de Vries <Tom_deVries at mentor dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>, Richard Biener <rguenther at suse dot de>
- Date: Mon, 12 Dec 2016 11:56:36 +0100
- Subject: Re: Fix ICE with __builtin_va_list on i?86 (PR middle-end/78716)
- Authentication-results: sourceware.org; auth=none
- References: <20161208210759.GL2337@redhat.com> <aefbe68c-be7f-2044-8238-62d445854859@mentor.com>
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?
> [ 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