[PATCH, PR71602] Give error for invalid va_list argument to va_arg

[Tom de Vries <Tom_deVries at mentor dot com>]

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.

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.

Bootstrapped and reg-tested on x86_64 (-m64 and -m32).

OK for trunk?

Thanks,
- Tom

Give error for invalid va_list argument to va_arg

2016-06-22  Tom de Vries  <tom@codesourcery.com>

	PR c/71602
	* c-common.c (build_va_arg): Add comp_types parameter.  Give error for
	invalid va_list argument.
	* c-common.h (build_va_arg): Add comp_types parameter.

	* c-typeck.c (va_list_comptypes): New function.
	(c_build_va_arg): Add argument to build_va_arg call.

	* call.c (build_x_va_arg): Add argument to build_va_arg call.

	* c-c++-common/va-arg-va-list-type.c: New test.

---
 gcc/c-family/c-common.c                          | 34 ++++++++++++++----------
 gcc/c-family/c-common.h                          |  2 +-
 gcc/c/c-typeck.c                                 | 11 +++++++-
 gcc/cp/call.c                                    |  6 +++--
 gcc/testsuite/c-c++-common/va-arg-va-list-type.c |  9 +++++++
 5 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 85f3a03..8d0f335 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5696,7 +5696,8 @@ build_va_arg_1 (location_t loc, tree type, tree op)
    va_arg (EXPR, TYPE) at source location LOC.  */
 
 tree
-build_va_arg (location_t loc, tree expr, tree type)
+build_va_arg (location_t loc, tree expr, tree type,
+	      bool (*comp_types) (tree, tree))
 {
   tree va_type = TREE_TYPE (expr);
   tree canon_va_type = (va_type == error_mark_node
@@ -5712,6 +5713,14 @@ build_va_arg (location_t loc, tree expr, tree type)
       return build_va_arg_1 (loc, type, expr);
     }
 
+  bool valid_va_list
+    = ((TREE_CODE (canon_va_type) == ARRAY_TYPE
+	&& TREE_CODE (va_type) == POINTER_TYPE)
+       ? comp_types (TREE_TYPE (canon_va_type), TREE_TYPE (va_type))
+       : comp_types (canon_va_type, va_type));
+  if (!valid_va_list)
+    goto fail_first_arg_type;
+
   if (TREE_CODE (canon_va_type) != ARRAY_TYPE)
     {
       /* Case 1: Not an array type.  */
@@ -5724,11 +5733,7 @@ build_va_arg (location_t loc, tree expr, tree type)
       tree canon_expr_type
 	= targetm.canonical_va_list_type (TREE_TYPE (expr));
       if (canon_expr_type == NULL_TREE)
-	{
-	  error_at (loc,
-		    "first argument to %<va_arg%> not of type %<va_list%>");
-	  return error_mark_node;
-	}
+	goto fail_first_arg_type;
 
       return build_va_arg_1 (loc, type, expr);
     }
@@ -5797,23 +5802,24 @@ build_va_arg (location_t loc, tree expr, tree type)
       tree canon_expr_type
 	= targetm.canonical_va_list_type (TREE_TYPE (expr));
       if (canon_expr_type == NULL_TREE)
-	{
-	  error_at (loc,
-		    "first argument to %<va_arg%> not of type %<va_list%>");
-	  return error_mark_node;
-	}
+	goto fail_first_arg_type;
     }
-  else
+  else if (TREE_CODE (va_type) == POINTER_TYPE)
     {
       /* 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'.  */
       ;
     }
+  else
+    goto fail_first_arg_type;
 
   return build_va_arg_1 (loc, type, expr);
+
+ fail_first_arg_type:
+  error_at (loc,
+	    "first argument to %<va_arg%> not of type %<va_list%>");
+  return error_mark_node;
 }
 
 
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 4e6aa00..37bfd07 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -878,7 +878,7 @@ extern void disable_builtin_function (const char *);
 
 extern void set_compound_literal_name (tree decl);
 
-extern tree build_va_arg (location_t, tree, tree);
+extern tree build_va_arg (location_t, tree, tree, bool (*) (tree, tree));
 
 extern const unsigned int c_family_lang_mask;
 extern unsigned int c_common_option_lang_mask (void);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 7c6241c..e96e31c 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -13674,6 +13674,15 @@ c_build_qualified_type (tree type, int type_quals, tree orig_qual_type,
   return var_type;
 }
 
+/* Callback function for c_build_va_arg to compare
+   va_list types.  */
+
+static bool
+va_list_comptypes (tree type1, tree type2)
+{
+  return comptypes (type1, type2) == 1;
+}
+
 /* Build a VA_ARG_EXPR for the C parser.  */
 
 tree
@@ -13698,7 +13707,7 @@ c_build_va_arg (location_t loc1, tree expr, location_t loc2, tree type)
   else if (warn_cxx_compat && TREE_CODE (type) == ENUMERAL_TYPE)
     warning_at (loc2, OPT_Wc___compat,
 		"C++ requires promoted type, not enum type, in %<va_arg%>");
-  return build_va_arg (loc2, expr, type);
+  return build_va_arg (loc2, expr, type, va_list_comptypes);
 }
 
 /* Return truthvalue of whether T1 is the same tree structure as T2.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index b739fa0..5e38326 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6956,11 +6956,13 @@ build_x_va_arg (source_location loc, tree expr, tree type)
 		 "through %<...%> is conditionally-supported", type);
 
       tree ref = cp_build_reference_type (type, false);
-      expr = build_va_arg (loc, expr, ref);
+      expr = build_va_arg (loc, expr, ref,
+			   same_type_ignoring_top_level_qualifiers_p);
       return convert_from_reference (expr);
     }
 
-  return build_va_arg (loc, expr, type);
+  return build_va_arg (loc, expr, type,
+		       same_type_ignoring_top_level_qualifiers_p);
 }
 
 /* TYPE has been given to va_arg.  Apply the default conversions which
diff --git a/gcc/testsuite/c-c++-common/va-arg-va-list-type.c b/gcc/testsuite/c-c++-common/va-arg-va-list-type.c
new file mode 100644
index 0000000..cdd97cf
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/va-arg-va-list-type.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+
+__builtin_va_list *pap;
+
+void
+fn1 (void)
+{
+  __builtin_va_arg (pap, double); /* { dg-error "first argument to 'va_arg' not of type 'va_list'" } */
+}