On Thu, Jun 23, 2016 at 1:27 PM, Tom de Vries <tom_devr...@mentor.com> wrote:
> Hi,
>
> this patch fixes PR71602, a 6/7 regression.
>
> Consider this test-case:
> ...
> __builtin_va_list *pap;
>
> void
> fn1 (void)
> {
> __builtin_va_arg(pap, double);
> }
> ...
>
> The testcase is invalid, because we're not passing a va_list as first
> argument of va_arg, but a va_list*.
>
> When compiling for x86_64 -m64, we run into the second assert in this
> snippet from build_va_arg:
> ...
> {
> /* Case 2b: va_list is pointer to array elem type. */
> gcc_assert (POINTER_TYPE_P (va_type));
> gcc_assert (TREE_TYPE (va_type) == TREE_TYPE (canon_va_type));
>
> /* Don't take the address. We've already got '&ap'. */
> ;
> }
> ...
>
> At that point, va_type and canon_va_type are:
> ...
> (gdb) call debug_generic_expr (va_type)
> struct [1] *
> (gdb) call debug_generic_expr (canon_va_type)
> struct [1]
> ...
>
> so TREE_TYPE (va_type) and TREE_TYPE (canon_va_type) are not equal:
> ...
> (gdb) call debug_generic_expr (va_type.typed.type)
> struct [1]
> (gdb) call debug_generic_expr (canon_va_type.typed.type)
> struct
> ...
>
> Given the semantics of the target hook:
> ...
> Target Hook: tree TARGET_CANONICAL_VA_LIST_TYPE (tree type)
>
> This hook returns the va_list type of the calling convention specified
> by the type of type. If type is not a valid va_list type, it returns
> NULL_TREE.
> ...
> one could argue that canonical_va_list_type should return NULL_TREE for a
> va_list*, which would fix the ICE. But the current implementation seems to
> rely on canonical_va_list_type to return va_list for a va_list* argument.
It does seem like it's the job of canonical_va_list_type to detect an
invalid argument. Why not fix that?
> The patch fixes the ICE by making the valid va_list check in build_va_arg
> more precise, by taking into account the non-strict behavior of
> canonical_va_list_type.
If you do need to check this here, is there a reason you need to pass
in a callback rather than use lang_hooks.types_compatible_p?
Jason
