[ was: Re: [PATCH, PR71602] Give error for invalid va_list argument to
va_arg ]
On 23/06/16 23:21, Jason Merrill wrote:
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:>> 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
...
__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?
This patch fixes PR71602 by making canonical_va_list_type more strict.
Bootstrapped and reg-tested on x86_64.
OK for trunk, 6-branch?
Thanks,
- Tom
