Hi!
The following testcase is miscompiled, because when cp_build_modify_expr
processes assignment into lvalue COND_EXPR, the rhs doesn't have
side-effects and so stabilize_expr doesn't do anything to it, then we
use that rhs in both COND_EXPR branches (but shared) and finally during
genericization we ubsan instrument that rhs, adding some SAVE_EXPRs in there
because we want to evaluate parts of it multiple times. Unfortunately,
the SAVE_EXPRs will be initialized only in one of the two COND_EXPR branches
and will just use uninitialized temporary in the other one.
Fixed by unsharing the rhs if we do this (but only for ubsan, so that
we don't waste compile time/memory on that when we don't need that).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-03-29 Jakub Jelinek <ja...@redhat.com>
PR sanitizer/89869
* typeck.c: Include gimplify.h.
(cp_build_modify_expr) <case COND_EXPR>: Unshare rhs before using it
for second time. Formatting fixes.
* g++.dg/ubsan/vptr-14.C: New test.
--- gcc/cp/typeck.c.jj 2019-03-22 15:42:02.456993944 +0100
+++ gcc/cp/typeck.c 2019-03-29 11:23:36.823067316 +0100
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.
#include "stringpool.h"
#include "attribs.h"
#include "asan.h"
+#include "gimplify.h"
static tree cp_build_addr_expr_strict (tree, tsubst_flags_t);
static tree cp_build_function_call (tree, tree, tsubst_flags_t);
@@ -8129,8 +8130,6 @@ cp_build_modify_expr (location_t loc, tr
/* Produce (a ? (b = rhs) : (c = rhs))
except that the RHS goes through a save-expr
so the code to compute it is only emitted once. */
- tree cond;
-
if (VOID_TYPE_P (TREE_TYPE (rhs)))
{
if (complain & tf_error)
@@ -8145,13 +8144,21 @@ cp_build_modify_expr (location_t loc, tr
if (!lvalue_or_else (lhs, lv_assign, complain))
return error_mark_node;
- cond = build_conditional_expr
- (input_location, TREE_OPERAND (lhs, 0),
- cp_build_modify_expr (loc, TREE_OPERAND (lhs, 1),
- modifycode, rhs, complain),
- cp_build_modify_expr (loc, TREE_OPERAND (lhs, 2),
- modifycode, rhs, complain),
- complain);
+ tree op1 = cp_build_modify_expr (loc, TREE_OPERAND (lhs, 1),
+ modifycode, rhs, complain);
+ /* When sanitizing undefined behavior, even when rhs doesn't need
+ stabilization at this point, the sanitization might add extra
+ SAVE_EXPRs in there and so make sure there is no tree sharing
+ in the rhs, otherwise those SAVE_EXPRs will have initialization
+ only in one of the two branches. */
+ if (sanitize_flags_p (SANITIZE_UNDEFINED
+ | SANITIZE_UNDEFINED_NONDEFAULT))
+ rhs = unshare_expr (rhs);
+ tree op2 = cp_build_modify_expr (loc, TREE_OPERAND (lhs, 2),
+ modifycode, rhs, complain);
+ tree cond = build_conditional_expr (input_location,
+ TREE_OPERAND (lhs, 0), op1, op2,
+ complain);
if (cond == error_mark_node)
return cond;
--- gcc/testsuite/g++.dg/ubsan/vptr-14.C.jj 2019-03-29 11:25:19.634389671
+0100
+++ gcc/testsuite/g++.dg/ubsan/vptr-14.C 2019-03-29 11:24:37.564076159
+0100
@@ -0,0 +1,18 @@
+// PR sanitizer/89869
+// { dg-do run }
+// { dg-options "-fsanitize=vptr -fno-sanitize-recover=vptr" }
+
+struct S { S *s = 0; virtual ~S () {} };
+
+void
+foo (S *x, S *y)
+{
+ (x->s ? y : x) = x->s;
+}
+
+int
+main ()
+{
+ S a;
+ foo (&a, 0);
+}
Jakub
