On Wed, Jan 03, 2018 at 02:02:16PM +0100, Jakub Jelinek wrote:
> Though, the above snippet reminds me I should probably also replace:
> + expr = build_base_path (MINUS_EXPR, expr, base,
> + /*nonnull=*/!sanitize_null_p, complain);
> with:
> + expr = build_base_path (MINUS_EXPR, expr, base,
> + /*nonnull=*/(!sanitize_null_p
> + && flag_delete_null_pointer_checks),
> + complain);
>
> Instead of checking for generated asm without sanitization, I think it will
> be easier/more portable to verify no conditionals in *.optimized dump.
> Let me repost the updated patch.
Here is an updated patch (which just uses flag_delete_null_pointer_checks,
because -fsanitize=null disables that option) and with a testcase that makes
sure there is no conditional testing b != NULL.
Queued for bootstrap/regtest.
2018-01-03 Jakub Jelinek <ja...@redhat.com>
PR c++/83555
* typeck.c (build_static_cast_1): For static casts to reference types,
call build_base_path with flag_delete_null_pointer_checks as nonnull
instead of always false. When -fsanitize=null, call
ubsan_maybe_instrument_reference on the NULL reference INTEGER_CST.
* cp-gimplify.c (cp_genericize_r): Don't walk subtrees of UBSAN_NULL
call if the first argument is INTEGER_CST with REFERENCE_TYPE.
* g++.dg/opt/pr83555.C: New test.
* g++.dg/ubsan/pr83555.C: New test.
--- gcc/cp/typeck.c.jj 2018-01-03 10:20:17.457537524 +0100
+++ gcc/cp/typeck.c 2018-01-03 14:28:46.189830697 +0100
@@ -6943,8 +6943,11 @@ build_static_cast_1 (tree type, tree exp
}
/* Convert from "B*" to "D*". This function will check that "B"
- is not a virtual base of "D". */
- expr = build_base_path (MINUS_EXPR, expr, base, /*nonnull=*/false,
+ is not a virtual base of "D". Even if we don't have a guarantee
+ that expr is NULL, if the static_cast is to a reference type,
+ it is UB if it would be NULL, so omit the non-NULL check. */
+ expr = build_base_path (MINUS_EXPR, expr, base,
+ /*nonnull=*/flag_delete_null_pointer_checks,
complain);
/* Convert the pointer to a reference -- but then remember that
@@ -6955,7 +6958,18 @@ build_static_cast_1 (tree type, tree exp
is a variable with the same type, the conversion would get folded
away, leaving just the variable and causing lvalue_kind to give
the wrong answer. */
- return convert_from_reference (rvalue (cp_fold_convert (type, expr)));
+ expr = cp_fold_convert (type, expr);
+
+ /* When -fsanitize=null, make sure to diagnose reference binding to
+ NULL even when the reference is converted to pointer later on. */
+ if (sanitize_flags_p (SANITIZE_NULL)
+ && TREE_CODE (expr) == COND_EXPR
+ && TREE_OPERAND (expr, 2)
+ && TREE_CODE (TREE_OPERAND (expr, 2)) == INTEGER_CST
+ && TREE_TYPE (TREE_OPERAND (expr, 2)) == type)
+ ubsan_maybe_instrument_reference (&TREE_OPERAND (expr, 2));
+
+ return convert_from_reference (rvalue (expr));
}
/* "A glvalue of type cv1 T1 can be cast to type rvalue reference to
--- gcc/cp/cp-gimplify.c.jj 2018-01-03 13:15:42.799817380 +0100
+++ gcc/cp/cp-gimplify.c 2018-01-03 14:27:39.383797996 +0100
@@ -1506,6 +1506,12 @@ cp_genericize_r (tree *stmt_p, int *walk
if (sanitize_flags_p (SANITIZE_VPTR) && !is_ctor)
cp_ubsan_maybe_instrument_member_call (stmt);
}
+ else if (fn == NULL_TREE
+ && CALL_EXPR_IFN (stmt) == IFN_UBSAN_NULL
+ && TREE_CODE (CALL_EXPR_ARG (stmt, 0)) == INTEGER_CST
+ && (TREE_CODE (TREE_TYPE (CALL_EXPR_ARG (stmt, 0)))
+ == REFERENCE_TYPE))
+ *walk_subtrees = 0;
}
break;
--- gcc/testsuite/g++.dg/opt/pr83555.C.jj 2018-01-03 15:17:56.294130008
+0100
+++ gcc/testsuite/g++.dg/opt/pr83555.C 2018-01-03 15:19:06.764162730 +0100
@@ -0,0 +1,15 @@
+// PR c++/83555
+// { dg-do compile }
+// { dg-options "-O2 -fdump-tree-optimized -fdelete-null-pointer-checks" }
+
+struct A { int a; };
+struct B { int b; };
+struct C : A, B { int c; };
+
+C *
+foo (B *b)
+{
+ return &static_cast<C &>(*b);
+}
+
+// { dg-final { scan-tree-dump-not "if \\(b_\[0-9]*\\(D\\) .= 0" "optimized" }
}
--- gcc/testsuite/g++.dg/ubsan/pr83555.C.jj 2018-01-03 14:27:39.383797996
+0100
+++ gcc/testsuite/g++.dg/ubsan/pr83555.C 2018-01-03 14:27:39.383797996
+0100
@@ -0,0 +1,40 @@
+// PR c++/83555
+// { dg-do run }
+// { dg-options "-fsanitize=null" }
+// { dg-output ":25:\[^\n\r]*reference binding to null pointer of type 'struct
C'" }
+
+struct A { int a; };
+struct B { int b; };
+struct C : A, B { int c; };
+
+__attribute__((noipa)) C *
+foo (B *b)
+{
+ return static_cast<C *>(b);
+}
+
+__attribute__((noipa)) C *
+bar (B *b)
+{
+ return &static_cast<C &>(*b);
+}
+
+__attribute__((noipa)) C *
+baz (B *b)
+{
+ return &static_cast<C &>(*b);
+}
+
+int
+main ()
+{
+ C c;
+ if (foo (static_cast<B *> (&c)) != &c)
+ __builtin_abort ();
+ if (foo (0))
+ __builtin_abort ();
+ if (bar (static_cast<B *> (&c)) != &c)
+ __builtin_abort ();
+ baz (0);
+ return 0;
+}
Jakub
