* [C++ PATCH] Fix ubsan downcast -fsanitize=vptr sanitization (PR c++/68508)
@ 2015-11-24 21:09 Jakub Jelinek
2015-11-25 14:46 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2015-11-24 21:09 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4281 bytes --]
Hi!
Since the introduction of -fsanitize=vptr we ICE on the attached testcase,
because for -std=c++14 the FE has strict assumptions on what the trees from
force_paren_expr should look like, but with -fsanitize=vptr there is extra
sanitization (COMPOUND_EXPR with UBSAN_VPTR call and the ADDR_EXPR it is
looking for), thus I wrote the second (for now untested) patch.
But then got surprised that we actually sanitize the A * to A & static
cast at all, I believe we ought to only sanitize downcasts, but:
1) in the second build_static_cast_1 caller of
cp_ubsan_maybe_instrument_downcast we call build_base_path which apparently
already converts the intype to type, so cp_ubsan_maybe_instrument_downcast
then doesn't know if it is a downcast or not
2) the downcast check is just DERIVED_FROM_P check, but my undestanding
of that is that DERIVED_FROM_P (x, x) is true too
So, this first included (non-attached) patch is my attempt to only sanitize
real downcasts. This patch has been bootstrapped/regtested on x86_64-linux
and i686-linux on the trunk, ok for trunk?
If yes, is the second patch better for the branch?
2015-11-24 Jakub Jelinek <jakub@redhat.com>
PR c++/68508
* cp-tree.h (cp_ubsan_maybe_instrument_downcast): Add INTYPE argument.
* cp-ubsan.c (cp_ubsan_maybe_instrument_downcast): Likewise. Use
it instead of or in addition to TREE_TYPE (op). Return NULL_TREE if
TREE_TYPE (intype) and TREE_TYPE (type) are the same type.
* typeck.c (build_static_cast_1): Adjust callers.
* g++.dg/ubsan/pr68508.C: New test.
--- gcc/cp/cp-tree.h.jj 2015-11-20 09:52:25.000000000 +0100
+++ gcc/cp/cp-tree.h 2015-11-24 13:42:30.096194615 +0100
@@ -6854,7 +6854,7 @@ extern bool cilk_valid_spawn
/* In cp-ubsan.c */
extern void cp_ubsan_maybe_instrument_member_call (tree);
extern void cp_ubsan_instrument_member_accesses (tree *);
-extern tree cp_ubsan_maybe_instrument_downcast (location_t, tree, tree);
+extern tree cp_ubsan_maybe_instrument_downcast (location_t, tree, tree, tree);
extern tree cp_ubsan_maybe_instrument_cast_to_vbase (location_t, tree, tree);
/* -- end of C++ */
--- gcc/cp/cp-ubsan.c.jj 2015-11-14 19:35:53.000000000 +0100
+++ gcc/cp/cp-ubsan.c 2015-11-24 13:45:12.837927413 +0100
@@ -245,13 +245,18 @@ cp_ubsan_instrument_member_accesses (tre
/* Instrument downcast. */
tree
-cp_ubsan_maybe_instrument_downcast (location_t loc, tree type, tree op)
+cp_ubsan_maybe_instrument_downcast (location_t loc, tree type,
+ tree intype, tree op)
{
if (!POINTER_TYPE_P (type)
+ || !POINTER_TYPE_P (intype)
|| !POINTER_TYPE_P (TREE_TYPE (op))
|| !CLASS_TYPE_P (TREE_TYPE (type))
+ || !CLASS_TYPE_P (TREE_TYPE (intype))
|| !CLASS_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
- || !DERIVED_FROM_P (TREE_TYPE (TREE_TYPE (op)), TREE_TYPE (type)))
+ || !DERIVED_FROM_P (TREE_TYPE (intype), TREE_TYPE (type))
+ || same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (intype)),
+ TYPE_MAIN_VARIANT (TREE_TYPE (type))))
return NULL_TREE;
return cp_ubsan_maybe_instrument_vptr (loc, op, TREE_TYPE (type), true,
--- gcc/cp/typeck.c.jj 2015-11-20 08:17:50.000000000 +0100
+++ gcc/cp/typeck.c 2015-11-24 13:46:37.618746306 +0100
@@ -6590,7 +6590,8 @@ build_static_cast_1 (tree type, tree exp
if (flag_sanitize & SANITIZE_VPTR)
{
tree ubsan_check
- = cp_ubsan_maybe_instrument_downcast (input_location, type, expr);
+ = cp_ubsan_maybe_instrument_downcast (input_location, type,
+ intype, expr);
if (ubsan_check)
expr = ubsan_check;
}
@@ -6737,7 +6738,8 @@ build_static_cast_1 (tree type, tree exp
if (flag_sanitize & SANITIZE_VPTR)
{
tree ubsan_check
- = cp_ubsan_maybe_instrument_downcast (input_location, type, expr);
+ = cp_ubsan_maybe_instrument_downcast (input_location, type,
+ intype, expr);
if (ubsan_check)
expr = ubsan_check;
}
--- gcc/testsuite/g++.dg/ubsan/pr68508.C.jj 2015-11-24 13:58:27.506683395 +0100
+++ gcc/testsuite/g++.dg/ubsan/pr68508.C 2015-11-24 13:53:41.000000000 +0100
@@ -0,0 +1,15 @@
+// PR c++/68508
+// { dg-do compile }
+// { dg-options "-std=c++14 -fsanitize=vptr" }
+
+struct A
+{
+ virtual int foo () { return 0; }
+};
+
+const A &
+bar ()
+{
+ static A x = A ();
+ return (x);
+}
Jakub
[-- Attachment #2: V614b --]
[-- Type: text/plain, Size: 1649 bytes --]
2015-11-24 Jakub Jelinek <jakub@redhat.com>
PR c++/68508
* typeck.c (check_return_expr): Strip away also
cp_ubsan_maybe_instrument_downcast added instrumentation.
* g++.dg/ubsan/pr68508.C: New test.
--- gcc/cp/typeck.c.jj 2015-11-20 08:17:50.000000000 +0100
+++ gcc/cp/typeck.c 2015-11-24 13:46:37.618746306 +0100
@@ -8802,9 +8804,23 @@ check_return_expr (tree retval, bool *no
&& REF_PARENTHESIZED_P (retval))
{
retval = TREE_OPERAND (retval, 0);
- while (TREE_CODE (retval) == NON_LVALUE_EXPR
- || TREE_CODE (retval) == NOP_EXPR)
- retval = TREE_OPERAND (retval, 0);
+ do
+ {
+ if (TREE_CODE (retval) == NON_LVALUE_EXPR
+ || TREE_CODE (retval) == NOP_EXPR)
+ retval = TREE_OPERAND (retval, 0);
+ /* Also look through cp_ubsan_maybe_instrument_downcast
+ instrumentation. */
+ else if (TREE_CODE (retval) == COMPOUND_EXPR
+ && TREE_CODE (TREE_OPERAND (retval, 0)) == CALL_EXPR
+ && CALL_EXPR_FN (TREE_OPERAND (retval, 0)) == NULL_TREE
+ && (CALL_EXPR_IFN (TREE_OPERAND (retval, 0))
+ == IFN_UBSAN_VPTR))
+ retval = TREE_OPERAND (retval, 1);
+ else
+ break;
+ }
+ while (1);
gcc_assert (TREE_CODE (retval) == ADDR_EXPR);
retval = TREE_OPERAND (retval, 0);
}
--- gcc/testsuite/g++.dg/ubsan/pr68508.C.jj 2015-11-24 13:58:27.506683395 +0100
+++ gcc/testsuite/g++.dg/ubsan/pr68508.C 2015-11-24 13:53:41.000000000 +0100
@@ -0,0 +1,15 @@
+// PR c++/68508
+// { dg-do compile }
+// { dg-options "-std=c++14 -fsanitize=vptr" }
+
+struct A
+{
+ virtual int foo () { return 0; }
+};
+
+const A &
+bar ()
+{
+ static A x = A ();
+ return (x);
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [C++ PATCH] Fix ubsan downcast -fsanitize=vptr sanitization (PR c++/68508)
2015-11-24 21:09 [C++ PATCH] Fix ubsan downcast -fsanitize=vptr sanitization (PR c++/68508) Jakub Jelinek
@ 2015-11-25 14:46 ` Jason Merrill
2015-11-25 20:32 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Jason Merrill @ 2015-11-25 14:46 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 11/24/2015 04:01 PM, Jakub Jelinek wrote:
> 2) the downcast check is just DERIVED_FROM_P check, but my undestanding
> of that is that DERIVED_FROM_P (x, x) is true too
Yes, but you can use is_properly_derived_from instead.
With that change the first cast is OK for trunk and branch.
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [C++ PATCH] Fix ubsan downcast -fsanitize=vptr sanitization (PR c++/68508)
2015-11-25 14:46 ` Jason Merrill
@ 2015-11-25 20:32 ` Jakub Jelinek
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2015-11-25 20:32 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Wed, Nov 25, 2015 at 09:45:46AM -0500, Jason Merrill wrote:
> On 11/24/2015 04:01 PM, Jakub Jelinek wrote:
> >2) the downcast check is just DERIVED_FROM_P check, but my undestanding
> >of that is that DERIVED_FROM_P (x, x) is true too
>
> Yes, but you can use is_properly_derived_from instead.
>
> With that change the first cast is OK for trunk and branch.
Thanks, this passed bootstrap/regtest on x86_64-linux and i686-linux
too, will apply tomorrow.
2015-11-25 Jakub Jelinek <jakub@redhat.com>
PR c++/68508
* cp-tree.h (cp_ubsan_maybe_instrument_downcast): Add INTYPE argument.
* cp-ubsan.c (cp_ubsan_maybe_instrument_downcast): Likewise. Use
it instead of or in addition to TREE_TYPE (op). Use
is_properly_derived_from, return NULL_TREE if TREE_TYPE (intype) and
TREE_TYPE (type) are the same type minus qualifiers.
* typeck.c (build_static_cast_1): Adjust callers.
* g++.dg/ubsan/pr68508.C: New test.
--- gcc/cp/cp-ubsan.c.jj 2015-11-25 09:49:50.850231457 +0100
+++ gcc/cp/cp-ubsan.c 2015-11-25 18:07:11.083778395 +0100
@@ -245,13 +245,14 @@ cp_ubsan_instrument_member_accesses (tre
/* Instrument downcast. */
tree
-cp_ubsan_maybe_instrument_downcast (location_t loc, tree type, tree op)
+cp_ubsan_maybe_instrument_downcast (location_t loc, tree type,
+ tree intype, tree op)
{
if (!POINTER_TYPE_P (type)
+ || !POINTER_TYPE_P (intype)
|| !POINTER_TYPE_P (TREE_TYPE (op))
- || !CLASS_TYPE_P (TREE_TYPE (type))
|| !CLASS_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
- || !DERIVED_FROM_P (TREE_TYPE (TREE_TYPE (op)), TREE_TYPE (type)))
+ || !is_properly_derived_from (TREE_TYPE (type), TREE_TYPE (intype)))
return NULL_TREE;
return cp_ubsan_maybe_instrument_vptr (loc, op, TREE_TYPE (type), true,
--- gcc/cp/cp-tree.h.jj 2015-11-25 09:49:50.816231943 +0100
+++ gcc/cp/cp-tree.h 2015-11-25 18:04:12.867310549 +0100
@@ -6854,7 +6854,7 @@ extern bool cilk_valid_spawn
/* In cp-ubsan.c */
extern void cp_ubsan_maybe_instrument_member_call (tree);
extern void cp_ubsan_instrument_member_accesses (tree *);
-extern tree cp_ubsan_maybe_instrument_downcast (location_t, tree, tree);
+extern tree cp_ubsan_maybe_instrument_downcast (location_t, tree, tree, tree);
extern tree cp_ubsan_maybe_instrument_cast_to_vbase (location_t, tree, tree);
/* -- end of C++ */
--- gcc/cp/typeck.c.jj 2015-11-25 09:49:50.879231042 +0100
+++ gcc/cp/typeck.c 2015-11-25 18:04:12.871310493 +0100
@@ -6590,7 +6590,8 @@ build_static_cast_1 (tree type, tree exp
if (flag_sanitize & SANITIZE_VPTR)
{
tree ubsan_check
- = cp_ubsan_maybe_instrument_downcast (input_location, type, expr);
+ = cp_ubsan_maybe_instrument_downcast (input_location, type,
+ intype, expr);
if (ubsan_check)
expr = ubsan_check;
}
@@ -6737,7 +6738,8 @@ build_static_cast_1 (tree type, tree exp
if (flag_sanitize & SANITIZE_VPTR)
{
tree ubsan_check
- = cp_ubsan_maybe_instrument_downcast (input_location, type, expr);
+ = cp_ubsan_maybe_instrument_downcast (input_location, type,
+ intype, expr);
if (ubsan_check)
expr = ubsan_check;
}
--- gcc/testsuite/g++.dg/ubsan/pr68508.C.jj 2015-11-25 18:04:12.871310493 +0100
+++ gcc/testsuite/g++.dg/ubsan/pr68508.C 2015-11-25 18:04:12.871310493 +0100
@@ -0,0 +1,15 @@
+// PR c++/68508
+// { dg-do compile }
+// { dg-options "-std=c++14 -fsanitize=vptr" }
+
+struct A
+{
+ virtual int foo () { return 0; }
+};
+
+const A &
+bar ()
+{
+ static A x = A ();
+ return (x);
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-11-25 20:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-24 21:09 [C++ PATCH] Fix ubsan downcast -fsanitize=vptr sanitization (PR c++/68508) Jakub Jelinek
2015-11-25 14:46 ` Jason Merrill
2015-11-25 20:32 ` Jakub Jelinek
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).