[RFA][PATCH][tree-optimization/78496] 02/03 Do not exploit __builtin_unreachable if -fsanitize=unreachable

[RFA][PATCH][tree-optimization/78496] 02/03 Do not exploit __builtin_unreachable if -fsanitize=unreachable

[Jeff Law]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]


As I brought my patches for pr78496 up to the tip of the trunk I noticed
a couple testsuite regressions with -fsanitize=unreachable tests.

The problem is VRP and EVRP analysis/optimization could exploit
__builtin_unreachable to narrow the range of an object, then use that
narrowed range to eliminate the __builtin_unreachable.  That seems
fundamentally wrong if we're compiling with -fsanitize=unreachable.

So this patch changes both to not exploit __builtin_unreachable when
-fsanitize=unreachable.

Bootstrapped and regression tested with all three patches in this kit.

OK for the trunk?

Jeff

[-- Attachment #2: P2 --]
[-- Type: text/plain, Size: 1338 bytes --]

	* gimple-ssa-evrp-analyze.c
	(evrp_range_analyzer::record_ranges_from_incoming_edge): Do not
	exploit __builtin_unreachable for -fsanitize=unreachable.
	* tree-vrp.c (remove_range_assertions): Similarly.


diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
index fb3d329..3cdf271 100644
--- a/gcc/gimple-ssa-evrp-analyze.c
+++ b/gcc/gimple-ssa-evrp-analyze.c
@@ -193,7 +193,8 @@ evrp_range_analyzer::record_ranges_from_incoming_edge (basic_block bb)
 
 	  /* If pred_e is really a fallthru we can record value ranges
 	     in SSA names as well.  */
-	  bool is_fallthru = assert_unreachable_fallthru_edge_p (pred_e);
+	  bool is_fallthru = (assert_unreachable_fallthru_edge_p (pred_e)
+			      && (flag_sanitize & SANITIZE_UNREACHABLE) == 0);
 
 	  /* Push updated ranges only after finding all of them to avoid
 	     ordering issues that can lead to worse ranges.  */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index a86b382..d0435a0 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -5181,7 +5181,8 @@ remove_range_assertions (void)
 		    is_unreachable = 0;
 		    if (single_pred_p (bb)
 			&& assert_unreachable_fallthru_edge_p
-						    (single_pred_edge (bb)))
+						    (single_pred_edge (bb))
+			&& (flag_sanitize & SANITIZE_UNREACHABLE) == 0)
 		      is_unreachable = 1;
 		  }
 		/* Handle

Re: [RFA][PATCH][tree-optimization/78496] 02/03 Do not exploit __builtin_unreachable if -fsanitize=unreachable

[Richard Biener]

On Mon, Dec 4, 2017 at 6:55 AM, Jeff Law <law@redhat.com> wrote:
>
> As I brought my patches for pr78496 up to the tip of the trunk I noticed
> a couple testsuite regressions with -fsanitize=unreachable tests.
>
> The problem is VRP and EVRP analysis/optimization could exploit
> __builtin_unreachable to narrow the range of an object, then use that
> narrowed range to eliminate the __builtin_unreachable.  That seems
> fundamentally wrong if we're compiling with -fsanitize=unreachable.
>
> So this patch changes both to not exploit __builtin_unreachable when
> -fsanitize=unreachable.
>
> Bootstrapped and regression tested with all three patches in this kit.
>
> OK for the trunk?

Jakub already fixed this in sligthly different ways.

Richard.

> Jeff
>
>         * gimple-ssa-evrp-analyze.c
>         (evrp_range_analyzer::record_ranges_from_incoming_edge): Do not
>         exploit __builtin_unreachable for -fsanitize=unreachable.
>         * tree-vrp.c (remove_range_assertions): Similarly.
>
>
> diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
> index fb3d329..3cdf271 100644
> --- a/gcc/gimple-ssa-evrp-analyze.c
> +++ b/gcc/gimple-ssa-evrp-analyze.c
> @@ -193,7 +193,8 @@ evrp_range_analyzer::record_ranges_from_incoming_edge (basic_block bb)
>
>           /* If pred_e is really a fallthru we can record value ranges
>              in SSA names as well.  */
> -         bool is_fallthru = assert_unreachable_fallthru_edge_p (pred_e);
> +         bool is_fallthru = (assert_unreachable_fallthru_edge_p (pred_e)
> +                             && (flag_sanitize & SANITIZE_UNREACHABLE) == 0);
>
>           /* Push updated ranges only after finding all of them to avoid
>              ordering issues that can lead to worse ranges.  */
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index a86b382..d0435a0 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -5181,7 +5181,8 @@ remove_range_assertions (void)
>                     is_unreachable = 0;
>                     if (single_pred_p (bb)
>                         && assert_unreachable_fallthru_edge_p
> -                                                   (single_pred_edge (bb)))
> +                                                   (single_pred_edge (bb))
> +                       && (flag_sanitize & SANITIZE_UNREACHABLE) == 0)
>                       is_unreachable = 1;
>                   }
>                 /* Handle
>

Re: [RFA][PATCH][tree-optimization/78496] 02/03 Do not exploit __builtin_unreachable if -fsanitize=unreachable

[Jeff Law]

On 12/04/2017 03:40 AM, Richard Biener wrote:
> On Mon, Dec 4, 2017 at 6:55 AM, Jeff Law <law@redhat.com> wrote:
>>
>> As I brought my patches for pr78496 up to the tip of the trunk I noticed
>> a couple testsuite regressions with -fsanitize=unreachable tests.
>>
>> The problem is VRP and EVRP analysis/optimization could exploit
>> __builtin_unreachable to narrow the range of an object, then use that
>> narrowed range to eliminate the __builtin_unreachable.  That seems
>> fundamentally wrong if we're compiling with -fsanitize=unreachable.
>>
>> So this patch changes both to not exploit __builtin_unreachable when
>> -fsanitize=unreachable.
>>
>> Bootstrapped and regression tested with all three patches in this kit.
>>
>> OK for the trunk?
> 
> Jakub already fixed this in sligthly different ways.
ACK.  Just looked at his fix.  I'd pondered doing it in one of those
support routines as well.  I'll verify his fix is sufficient with my
work.  Consider this patch dropped as I expect Jakub's patch to be
sufficient.

jeff