From: Tom de Vries <Tom_deVries@mentor.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH, PR71602] Give error for invalid va_list argument to va_arg
Date: Thu, 23 Jun 2016 10:27:00 -0000 [thread overview]
Message-ID: <576BB984.1040401@mentor.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 1755 bytes --]
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
[-- Attachment #2: 0001-Give-error-for-invalid-va_list-argument-to-va_arg.patch --]
[-- Type: text/x-patch, Size: 5804 bytes --]
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'" } */
+}
next reply other threads:[~2016-06-23 10:27 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-23 10:27 Tom de Vries [this message]
2016-06-23 21:22 ` Jason Merrill
2016-08-24 6:22 ` [PATCH, PR71602, 4/4] Make canonical_va_list_type more strict Tom de Vries
2016-08-29 15:51 ` Joseph Myers
2016-08-29 16:12 ` Tom de Vries
2016-09-09 22:12 ` Jeff Law
2016-09-11 9:39 ` Christophe Lyon
2016-09-19 11:20 ` Szabolcs Nagy
2016-09-19 11:25 ` Tom de Vries
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=576BB984.1040401@mentor.com \
--to=tom_devries@mentor.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).