[PATCH] fold-const: Fix up -fsanitize=null in C++ [PR105729]

[PATCH] fold-const: Fix up -fsanitize=null in C++ [PR105729]

[Jakub Jelinek]

Hi!

The following testcase triggers a false positive UBSan binding a reference
to null diagnostics.
In the FE we instrument conversions from pointer to reference type
to diagnose at runtime if the operand of such a conversion is 0.
The problem is that a GENERIC folding folds
((const struct Bar *) ((const struct Foo *) this)->data) + (sizetype) range_check (x)
conversion to const struct Bar & by converting to that the first
operand of the POINTER_PLUS_EXPR.  But that changes when the -fsanitize=null
binding to reference runtime check occurs.  Without the optimization,
it is invoked on the result of the POINTER_PLUS_EXPR, and as range_check
call throws, that means it never triggers in the testcase.
With the optimization, it checks whether this->data is NULL and it is.

The following patch avoids that optimization during GENERIC folding when
-fsanitize=null is enabled and it is a cast from non-REFERENCE_TYPE to
REFERENCE_TYPE.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2022-05-27  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/105729
	* fold-const.cc (fold_unary_loc): Don't optimize (X &) ((Y *) z + w)
	to (X &) z + w if -fsanitize=null during GENERIC folding.

	* g++.dg/ubsan/pr105729.C: New test.

--- gcc/fold-const.cc.jj	2022-05-13 09:34:23.967147706 +0200
+++ gcc/fold-const.cc	2022-05-26 17:12:50.145526094 +0200
@@ -9516,6 +9516,16 @@ fold_unary_loc (location_t loc, enum tre
 		  > min_align_of_type (TREE_TYPE (TREE_TYPE (arg00)))))
 	    return NULL_TREE;
 
+	  /* Similarly, avoid this optimization in GENERIC for -fsanitize=null
+	     when type is a reference type and arg00's type is not,
+	     because arg00 could be validly nullptr and if arg01 doesn't return,
+	     we don't want false positive binding of reference to nullptr.  */
+	  if (TREE_CODE (type) == REFERENCE_TYPE
+	      && !in_gimple_form
+	      && sanitize_flags_p (SANITIZE_NULL)
+	      && TREE_CODE (TREE_TYPE (arg00)) != REFERENCE_TYPE)
+	    return NULL_TREE;
+
 	  arg00 = fold_convert_loc (loc, type, arg00);
 	  return fold_build_pointer_plus_loc (loc, arg00, arg01);
 	}
--- gcc/testsuite/g++.dg/ubsan/pr105729.C.jj	2022-05-26 19:54:58.725009300 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr105729.C	2022-05-26 19:56:50.785848337 +0200
@@ -0,0 +1,29 @@
+// PR sanitizer/105729
+// { dg-do run }
+// { dg-options "-fsanitize=null -fno-sanitize-recover=null" }
+
+int
+foo (int x)
+{
+  throw 0;
+}
+
+struct S {};
+struct T {
+  S *data;
+  T () : data (0) {}
+  const S &bar (int x) const { return data[foo (x)]; }
+};
+
+int
+main ()
+{
+  T t;
+  try
+    {
+      t.bar (-1);
+    }
+  catch (...)
+    {
+    }
+}

	Jakub

Re: [PATCH] fold-const: Fix up -fsanitize=null in C++ [PR105729]

[Richard Biener]

On Fri, 27 May 2022, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase triggers a false positive UBSan binding a reference
> to null diagnostics.
> In the FE we instrument conversions from pointer to reference type
> to diagnose at runtime if the operand of such a conversion is 0.
> The problem is that a GENERIC folding folds
> ((const struct Bar *) ((const struct Foo *) this)->data) + (sizetype) range_check (x)
> conversion to const struct Bar & by converting to that the first
> operand of the POINTER_PLUS_EXPR.  But that changes when the -fsanitize=null
> binding to reference runtime check occurs.  Without the optimization,
> it is invoked on the result of the POINTER_PLUS_EXPR, and as range_check
> call throws, that means it never triggers in the testcase.
> With the optimization, it checks whether this->data is NULL and it is.
> 
> The following patch avoids that optimization during GENERIC folding when
> -fsanitize=null is enabled and it is a cast from non-REFERENCE_TYPE to
> REFERENCE_TYPE.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2022-05-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/105729
> 	* fold-const.cc (fold_unary_loc): Don't optimize (X &) ((Y *) z + w)
> 	to (X &) z + w if -fsanitize=null during GENERIC folding.
> 
> 	* g++.dg/ubsan/pr105729.C: New test.
> 
> --- gcc/fold-const.cc.jj	2022-05-13 09:34:23.967147706 +0200
> +++ gcc/fold-const.cc	2022-05-26 17:12:50.145526094 +0200
> @@ -9516,6 +9516,16 @@ fold_unary_loc (location_t loc, enum tre
>  		  > min_align_of_type (TREE_TYPE (TREE_TYPE (arg00)))))
>  	    return NULL_TREE;
>  
> +	  /* Similarly, avoid this optimization in GENERIC for -fsanitize=null
> +	     when type is a reference type and arg00's type is not,
> +	     because arg00 could be validly nullptr and if arg01 doesn't return,
> +	     we don't want false positive binding of reference to nullptr.  */
> +	  if (TREE_CODE (type) == REFERENCE_TYPE
> +	      && !in_gimple_form
> +	      && sanitize_flags_p (SANITIZE_NULL)
> +	      && TREE_CODE (TREE_TYPE (arg00)) != REFERENCE_TYPE)
> +	    return NULL_TREE;
> +
>  	  arg00 = fold_convert_loc (loc, type, arg00);
>  	  return fold_build_pointer_plus_loc (loc, arg00, arg01);
>  	}
> --- gcc/testsuite/g++.dg/ubsan/pr105729.C.jj	2022-05-26 19:54:58.725009300 +0200
> +++ gcc/testsuite/g++.dg/ubsan/pr105729.C	2022-05-26 19:56:50.785848337 +0200
> @@ -0,0 +1,29 @@
> +// PR sanitizer/105729
> +// { dg-do run }
> +// { dg-options "-fsanitize=null -fno-sanitize-recover=null" }
> +
> +int
> +foo (int x)
> +{
> +  throw 0;
> +}
> +
> +struct S {};
> +struct T {
> +  S *data;
> +  T () : data (0) {}
> +  const S &bar (int x) const { return data[foo (x)]; }
> +};
> +
> +int
> +main ()
> +{
> +  T t;
> +  try
> +    {
> +      t.bar (-1);
> +    }
> +  catch (...)
> +    {
> +    }
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)