Re: [C++ PATCH] Improve code generation for static_cast of pointers to reference types (PR c++/83555)

[Jakub Jelinek <jakub at redhat dot com>]

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  <jakub@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