From 830421fcd37719426fa8eb7f0d545c744db497b4 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Wed, 3 Jan 2018 21:37:41 +0100 Subject: [PATCH] re PR c++/83555 (Unnecessary null check when static_cast is used with references.) 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. From-SVN: r256186 --- gcc/cp/ChangeLog | 10 +++++++ gcc/cp/cp-gimplify.c | 6 +++++ gcc/cp/typeck.c | 20 +++++++++++--- gcc/testsuite/ChangeLog | 6 +++++ gcc/testsuite/g++.dg/opt/pr83555.C | 15 +++++++++++ gcc/testsuite/g++.dg/ubsan/pr83555.C | 40 ++++++++++++++++++++++++++++ 6 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/opt/pr83555.C create mode 100644 gcc/testsuite/g++.dg/ubsan/pr83555.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 4f8f8394273..959f412ebf3 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,13 @@ +2018-01-03 Jakub Jelinek + + 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. + 2018-01-03 Nathan Sidwell PR c++/83667 diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index c090ee7fcbd..eda493a7bec 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -1506,6 +1506,12 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data) 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; diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index ccbef1db35c..76fd9302edf 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -6943,8 +6943,11 @@ build_static_cast_1 (tree type, tree expr, bool c_cast_p, } /* 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 expr, bool c_cast_p, 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 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 503a8397ab6..7c4e6e9a349 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2018-01-03 Jakub Jelinek + + PR c++/83555 + * g++.dg/opt/pr83555.C: New test. + * g++.dg/ubsan/pr83555.C: New test. + 2018-01-03 David Malcolm PR c/82050 diff --git a/gcc/testsuite/g++.dg/opt/pr83555.C b/gcc/testsuite/g++.dg/opt/pr83555.C new file mode 100644 index 00000000000..c6f810d1f00 --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/pr83555.C @@ -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(*b); +} + +// { dg-final { scan-tree-dump-not "if \\(b_\[0-9]*\\(D\\) .= 0" "optimized" } } diff --git a/gcc/testsuite/g++.dg/ubsan/pr83555.C b/gcc/testsuite/g++.dg/ubsan/pr83555.C new file mode 100644 index 00000000000..6574923740c --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/pr83555.C @@ -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(b); +} + +__attribute__((noipa)) C * +bar (B *b) +{ + return &static_cast(*b); +} + +__attribute__((noipa)) C * +baz (B *b) +{ + return &static_cast(*b); +} + +int +main () +{ + C c; + if (foo (static_cast (&c)) != &c) + __builtin_abort (); + if (foo (0)) + __builtin_abort (); + if (bar (static_cast (&c)) != &c) + __builtin_abort (); + baz (0); + return 0; +} -- 2.30.2