diff mbox series

Selectively trap if ranger and vr-values disagree on range builtins.

Message ID 20201027152911.65293-1-aldyh@redhat.com
State New
Headers show
Series Selectively trap if ranger and vr-values disagree on range builtins. | expand

Commit Message

Aldy Hernandez Oct. 27, 2020, 3:29 p.m. UTC
The UBSAN builtins degrade into PLUS/MINUS/MULT and call
extract_range_from_binary_expr, which as the PR shows, can special
case some symbolics which the ranger doesn't currently handle.

Looking at vr_values::extract_range_builtin(), I see that every single
place where we ask for a range, we bail on non-integers (symbolics,
etc).  That is, with the exception of the UBSAN builtins.

Since this seems to be particular to UBSAN, we could still go with the
original plan of removing the duplicity in ranger vs vr-values, but
leave in the UBSAN builtin handling.  This isn't ideal, as we'd like
to remove all the common code, but I'd be willing to put up with UBSAN
duplication for the time being.

This patch disables the assert on the UBSAN builtins, while still
trapping if any other differences are found between the vr_values and
the ranger versions of builtin range handling.

As a follow-up, once Fedora can test this approach, I'll remove all
the builtin code from extract_range_builtin, with the exception of the
UBSAN stuff (renaming it to extract_range_ubsan_builtin).

Since the builtin code has proven fickle across architectures, I've
tested this with {-m32,-m64,-fsanitize=signed-integer-overflow} on
x86, ppc64le, and aarch64.  I think this should be enough.  If it
isn't, we can revert the patch, and leave the duplicate code until
the next release cycle when hopefully vr_values, evrp, and friends
will all be overhauled.

Andrew, do you have any thoughts on this?

Aldy

gcc/ChangeLog:

	PR tree-optimization/97505
	* vr-values.c (vr_values::extract_range_basic): Enable
	trap again for everything except UBSAN builtins.
---
 gcc/vr-values.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Aldy Hernandez Oct. 27, 2020, 3:58 p.m. UTC | #1
For the record, this is what I envision the follow-up patch to be (untested).

Aldy

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 9f5943a1ab6..3db72a360a6 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -1159,188 +1159,16 @@ check_for_binary_op_overflow (range_query *query,
    successful.  */

 bool
-vr_values::extract_range_builtin (value_range_equiv *vr, gimple *stmt)
+vr_values::extract_range_from_ubsan_builtin (value_range_equiv *vr,
gimple *stmt)
 {
   gcc_assert (is_gimple_call (stmt));
   tree type = gimple_expr_type (stmt);
-  tree arg;
-  int mini, maxi, zerov = 0, prec;
   enum tree_code subcode = ERROR_MARK;
   combined_fn cfn = gimple_call_combined_fn (stmt);
   scalar_int_mode mode;

   switch (cfn)
     {
-    case CFN_BUILT_IN_CONSTANT_P:
-      /* Resolve calls to __builtin_constant_p after inlining.  */
-      if (cfun->after_inlining)
-    {
-      vr->set_zero (type);
-      vr->equiv_clear ();
-      return true;
-    }
-      break;
-      /* Both __builtin_ffs* and __builtin_popcount return
-     [0, prec].  */
-    CASE_CFN_FFS:
-    CASE_CFN_POPCOUNT:
-      arg = gimple_call_arg (stmt, 0);
-      prec = TYPE_PRECISION (TREE_TYPE (arg));
-      mini = 0;
-      maxi = prec;
-      if (TREE_CODE (arg) == SSA_NAME)
-    {
-      const value_range_equiv *vr0 = get_value_range (arg);
-      /* If arg is non-zero, then ffs or popcount are non-zero.  */
-      if (range_includes_zero_p (vr0) == 0)
-        mini = 1;
-      /* If some high bits are known to be zero,
-         we can decrease the maximum.  */
-      if (vr0->kind () == VR_RANGE
-          && TREE_CODE (vr0->max ()) == INTEGER_CST
-          && !operand_less_p (vr0->min (),
-                  build_zero_cst (TREE_TYPE (vr0->min ()))))
-        maxi = tree_floor_log2 (vr0->max ()) + 1;
-    }
-      goto bitop_builtin;
-      /* __builtin_parity* returns [0, 1].  */
-    CASE_CFN_PARITY:
-      mini = 0;
-      maxi = 1;
-      goto bitop_builtin;
-      /* __builtin_clz* return [0, prec-1], except for
-     when the argument is 0, but that is undefined behavior.
-     Always handle __builtin_clz* which can be only written
-     by user as UB on 0 and so [0, prec-1] range, and the internal-fn
-     calls depending on how CLZ_DEFINED_VALUE_AT_ZERO is defined.  */
-    CASE_CFN_CLZ:
-      arg = gimple_call_arg (stmt, 0);
-      prec = TYPE_PRECISION (TREE_TYPE (arg));
-      mini = 0;
-      maxi = prec - 1;
-      mode = SCALAR_INT_TYPE_MODE (TREE_TYPE (arg));
-      if (gimple_call_internal_p (stmt))
-    {
-      if (optab_handler (clz_optab, mode) != CODE_FOR_nothing
-          && CLZ_DEFINED_VALUE_AT_ZERO (mode, zerov) == 2)
-        {
-          /* Handle only the single common value.  */
-          if (zerov == prec)
-        maxi = prec;
-          /* Magic value to give up, unless vr0 proves
-         arg is non-zero.  */
-          else
-        mini = -2;
-        }
-    }
-      if (TREE_CODE (arg) == SSA_NAME)
-    {
-      const value_range_equiv *vr0 = get_value_range (arg);
-      /* From clz of VR_RANGE minimum we can compute
-         result maximum.  */
-      if (vr0->kind () == VR_RANGE
-          && TREE_CODE (vr0->min ()) == INTEGER_CST
-          && integer_nonzerop (vr0->min ()))
-        {
-          maxi = prec - 1 - tree_floor_log2 (vr0->min ());
-          if (mini == -2)
-        mini = 0;
-        }
-      else if (vr0->kind () == VR_ANTI_RANGE
-           && integer_zerop (vr0->min ()))
-        {
-          maxi = prec - 1;
-          mini = 0;
-        }
-      if (mini == -2)
-        break;
-      /* From clz of VR_RANGE maximum we can compute
-         result minimum.  */
-      if (vr0->kind () == VR_RANGE
-          && TREE_CODE (vr0->max ()) == INTEGER_CST)
-        {
-          int newmini = prec - 1 - tree_floor_log2 (vr0->max ());
-          if (newmini == prec)
-        {
-          if (maxi == prec)
-            mini = prec;
-        }
-          else
-        mini = newmini;
-        }
-    }
-      if (mini == -2)
-    break;
-      goto bitop_builtin;
-      /* __builtin_ctz* return [0, prec-1], except for
-     when the argument is 0, but that is undefined behavior.
-     Always handle __builtin_ctz* which can be only written
-     by user as UB on 0 and so [0, prec-1] range, and the internal-fn
-     calls depending on how CTZ_DEFINED_VALUE_AT_ZERO is defined.  */
-    CASE_CFN_CTZ:
-      arg = gimple_call_arg (stmt, 0);
-      prec = TYPE_PRECISION (TREE_TYPE (arg));
-      mini = 0;
-      maxi = prec - 1;
-      mode = SCALAR_INT_TYPE_MODE (TREE_TYPE (arg));
-      if (gimple_call_internal_p (stmt))
-    {
-      if (optab_handler (ctz_optab, mode) != CODE_FOR_nothing
-          && CTZ_DEFINED_VALUE_AT_ZERO (mode, zerov) == 2)
-        {
-          /* Handle only the two common values.  */
-          if (zerov == -1)
-        mini = -1;
-          else if (zerov == prec)
-        maxi = prec;
-          else
-        /* Magic value to give up, unless vr0 proves
-           arg is non-zero.  */
-        mini = -2;
-        }
-    }
-      if (TREE_CODE (arg) == SSA_NAME)
-    {
-      const value_range_equiv *vr0 = get_value_range (arg);
-      /* If arg is non-zero, then use [0, prec - 1].  */
-      if ((vr0->kind () == VR_RANGE
-           && integer_nonzerop (vr0->min ()))
-          || (vr0->kind () == VR_ANTI_RANGE
-          && integer_zerop (vr0->min ())))
-        {
-          mini = 0;
-          maxi = prec - 1;
-        }
-      /* If some high bits are known to be zero,
-         we can decrease the result maximum.  */
-      if (vr0->kind () == VR_RANGE
-          && TREE_CODE (vr0->max ()) == INTEGER_CST)
-        {
-          int newmaxi = tree_floor_log2 (vr0->max ());
-          if (newmaxi == -1)
-        {
-          if (mini == -1)
-            maxi = -1;
-          else if (maxi == prec)
-            mini = prec;
-        }
-          else if (maxi != prec)
-        maxi = newmaxi;
-        }
-    }
-      if (mini == -2)
-    break;
-      goto bitop_builtin;
-      /* __builtin_clrsb* returns [0, prec-1].  */
-    CASE_CFN_CLRSB:
-      arg = gimple_call_arg (stmt, 0);
-      prec = TYPE_PRECISION (TREE_TYPE (arg));
-      mini = 0;
-      maxi = prec - 1;
-      goto bitop_builtin;
-    bitop_builtin:
-      vr->set (build_int_cst (type, mini), build_int_cst (type, maxi));
-      return true;
     case CFN_UBSAN_CHECK_ADD:
       subcode = PLUS_EXPR;
       break;
@@ -1350,47 +1178,6 @@ vr_values::extract_range_builtin
(value_range_equiv *vr, gimple *stmt)
     case CFN_UBSAN_CHECK_MUL:
       subcode = MULT_EXPR;
       break;
-    case CFN_GOACC_DIM_SIZE:
-    case CFN_GOACC_DIM_POS:
-      /* Optimizing these two internal functions helps the loop
-     optimizer eliminate outer comparisons.  Size is [1,N]
-     and pos is [0,N-1].  */
-      {
-    bool is_pos = cfn == CFN_GOACC_DIM_POS;
-    int axis = oacc_get_ifn_dim_arg (stmt);
-    int size = oacc_get_fn_dim_size (current_function_decl, axis);
-
-    if (!size)
-      /* If it's dynamic, the backend might know a hardware
-         limitation.  */
-      size = targetm.goacc.dim_limit (axis);
-
-    tree type = TREE_TYPE (gimple_call_lhs (stmt));
-    vr->set(build_int_cst (type, is_pos ? 0 : 1),
-        size
-        ? build_int_cst (type, size - is_pos) : vrp_val_max (type));
-      }
-      return true;
-    case CFN_BUILT_IN_STRLEN:
-      if (tree lhs = gimple_call_lhs (stmt))
-    if (ptrdiff_type_node
-        && (TYPE_PRECISION (ptrdiff_type_node)
-        == TYPE_PRECISION (TREE_TYPE (lhs))))
-      {
-        tree type = TREE_TYPE (lhs);
-        tree max = vrp_val_max (ptrdiff_type_node);
-        wide_int wmax = wi::to_wide (max, TYPE_PRECISION (TREE_TYPE (max)));
-        tree range_min = build_zero_cst (type);
-        /* To account for the terminating NUL, the maximum length
-           is one less than the maximum array size, which in turn
-           is one  less than PTRDIFF_MAX (or SIZE_MAX where it's
-           smaller than the former type).
-           FIXME: Use max_object_size() - 1 here.  */
-        tree range_max = wide_int_to_tree (type, wmax - 2);
-        vr->set (range_min, range_max);
-        return true;
-      }
-      break;
     default:
       break;
     }
@@ -1430,20 +1217,27 @@ vr_values::extract_range_basic
(value_range_equiv *vr, gimple *stmt)
   bool sop;
   tree type = gimple_expr_type (stmt);

-  if (is_gimple_call (stmt) && extract_range_builtin (vr, stmt))
+  if (is_gimple_call (stmt))
     {
       combined_fn cfn = gimple_call_combined_fn (stmt);
-      if (cfn == CFN_UBSAN_CHECK_ADD
-      || cfn == CFN_UBSAN_CHECK_SUB
-      || cfn == CFN_UBSAN_CHECK_MUL)
-    return;
-
-      value_range_equiv tmp;
-      /* Assert that any ranges vr_values::extract_range_builtin gets
-     are also handled by the ranger counterpart.  */
-      gcc_assert (range_of_builtin_call (*this, tmp, as_a<gcall *> (stmt)));
-      gcc_assert (tmp.equal_p (*vr, /*ignore_equivs=*/false));
-      return;
+      switch (cfn)
+    {
+    case CFN_UBSAN_CHECK_ADD:
+    case CFN_UBSAN_CHECK_SUB:
+    case CFN_UBSAN_CHECK_MUL:
+      if (extract_range_from_ubsan_builtin (vr, stmt))
+        return;
+      break;
+    default:
+      if (range_of_builtin_call (*this, *vr, as_a<gcall *> (stmt)))
+        {
+          /* The original code nuked equivalences every time a
+         range was found, so do the same here.  */
+          vr->equiv_clear ();
+          return;
+        }
+      break;
+    }
     }
   /* Handle extraction of the two results (result of arithmetics and
      a flag whether arithmetics overflowed) from {ADD,SUB,MUL}_OVERFLOW
diff --git a/gcc/vr-values.h b/gcc/vr-values.h
index 59fac0c4b1e..712d029909f 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -148,7 +148,7 @@ class vr_values : public range_query
   void extract_range_from_comparison (value_range_equiv *, gimple *);
   void vrp_visit_assignment_or_call (gimple*, tree *, value_range_equiv *);
   void vrp_visit_switch_stmt (gswitch *, edge *);
-  bool extract_range_builtin (value_range_equiv *, gimple *);
+  bool extract_range_from_ubsan_builtin (value_range_equiv *, gimple *);

   /* This probably belongs in the lattice rather than in here.  */
   bool values_propagated;

On Tue, Oct 27, 2020 at 4:29 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> The UBSAN builtins degrade into PLUS/MINUS/MULT and call
> extract_range_from_binary_expr, which as the PR shows, can special
> case some symbolics which the ranger doesn't currently handle.
>
> Looking at vr_values::extract_range_builtin(), I see that every single
> place where we ask for a range, we bail on non-integers (symbolics,
> etc).  That is, with the exception of the UBSAN builtins.
>
> Since this seems to be particular to UBSAN, we could still go with the
> original plan of removing the duplicity in ranger vs vr-values, but
> leave in the UBSAN builtin handling.  This isn't ideal, as we'd like
> to remove all the common code, but I'd be willing to put up with UBSAN
> duplication for the time being.
>
> This patch disables the assert on the UBSAN builtins, while still
> trapping if any other differences are found between the vr_values and
> the ranger versions of builtin range handling.
>
> As a follow-up, once Fedora can test this approach, I'll remove all
> the builtin code from extract_range_builtin, with the exception of the
> UBSAN stuff (renaming it to extract_range_ubsan_builtin).
>
> Since the builtin code has proven fickle across architectures, I've
> tested this with {-m32,-m64,-fsanitize=signed-integer-overflow} on
> x86, ppc64le, and aarch64.  I think this should be enough.  If it
> isn't, we can revert the patch, and leave the duplicate code until
> the next release cycle when hopefully vr_values, evrp, and friends
> will all be overhauled.
>
> Andrew, do you have any thoughts on this?
>
> Aldy
>
> gcc/ChangeLog:
>
>         PR tree-optimization/97505
>         * vr-values.c (vr_values::extract_range_basic): Enable
>         trap again for everything except UBSAN builtins.
> ---
>  gcc/vr-values.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 7a0e70eab64..9f5943a1ab6 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -1432,14 +1432,17 @@ vr_values::extract_range_basic (value_range_equiv *vr, gimple *stmt)
>
>    if (is_gimple_call (stmt) && extract_range_builtin (vr, stmt))
>      {
> +      combined_fn cfn = gimple_call_combined_fn (stmt);
> +      if (cfn == CFN_UBSAN_CHECK_ADD
> +         || cfn == CFN_UBSAN_CHECK_SUB
> +         || cfn == CFN_UBSAN_CHECK_MUL)
> +       return;
> +
>        value_range_equiv tmp;
>        /* Assert that any ranges vr_values::extract_range_builtin gets
>          are also handled by the ranger counterpart.  */
>        gcc_assert (range_of_builtin_call (*this, tmp, as_a<gcall *> (stmt)));
> -#if 0
> -      /* Disable this while PR97505 is resolved.  */
>        gcc_assert (tmp.equal_p (*vr, /*ignore_equivs=*/false));
> -#endif
>        return;
>      }
>    /* Handle extraction of the two results (result of arithmetics and
> --
> 2.26.2
>
Andrew MacLeod Oct. 29, 2020, 1:53 p.m. UTC | #2
On 10/27/20 11:29 AM, Aldy Hernandez wrote:
> The UBSAN builtins degrade into PLUS/MINUS/MULT and call
> extract_range_from_binary_expr, which as the PR shows, can special
> case some symbolics which the ranger doesn't currently handle.
>
> Looking at vr_values::extract_range_builtin(), I see that every single
> place where we ask for a range, we bail on non-integers (symbolics,
> etc).  That is, with the exception of the UBSAN builtins.
>
> Since this seems to be particular to UBSAN, we could still go with the
> original plan of removing the duplicity in ranger vs vr-values, but
> leave in the UBSAN builtin handling.  This isn't ideal, as we'd like
> to remove all the common code, but I'd be willing to put up with UBSAN
> duplication for the time being.
>
> This patch disables the assert on the UBSAN builtins, while still
> trapping if any other differences are found between the vr_values and
> the ranger versions of builtin range handling.
>
> As a follow-up, once Fedora can test this approach, I'll remove all
> the builtin code from extract_range_builtin, with the exception of the
> UBSAN stuff (renaming it to extract_range_ubsan_builtin).
>
> Since the builtin code has proven fickle across architectures, I've
> tested this with {-m32,-m64,-fsanitize=signed-integer-overflow} on
> x86, ppc64le, and aarch64.  I think this should be enough.  If it
> isn't, we can revert the patch, and leave the duplicate code until
> the next release cycle when hopefully vr_values, evrp, and friends
> will all be overhauled.
>
> Andrew, do you have any thoughts on this?
>
>
OK.

I think we want to remove as much duplication as possible, which will 
then give us confidence that that ranger versions are correct.
THe UBSAN versions will have to get tighter ranger integration with 
relationals when they are available in order to handle things like this.

I dont suppose you can create a testcase for this?   Otherwise we'll 
have to tag it somehow so we dont forget to come back to it when we 
start handling these ubsan builtins differently.

Andrew

PS. and might as well create and test the follow up patch so its ready 
to go.  I'd leave this as-is with the asserts for a week or so.
Aldy Hernandez Oct. 29, 2020, 2:39 p.m. UTC | #3
On 10/29/20 2:53 PM, Andrew MacLeod wrote:
> On 10/27/20 11:29 AM, Aldy Hernandez wrote:
>> The UBSAN builtins degrade into PLUS/MINUS/MULT and call
>> extract_range_from_binary_expr, which as the PR shows, can special
>> case some symbolics which the ranger doesn't currently handle.
>>
>> Looking at vr_values::extract_range_builtin(), I see that every single
>> place where we ask for a range, we bail on non-integers (symbolics,
>> etc).  That is, with the exception of the UBSAN builtins.
>>
>> Since this seems to be particular to UBSAN, we could still go with the
>> original plan of removing the duplicity in ranger vs vr-values, but
>> leave in the UBSAN builtin handling.  This isn't ideal, as we'd like
>> to remove all the common code, but I'd be willing to put up with UBSAN
>> duplication for the time being.
>>
>> This patch disables the assert on the UBSAN builtins, while still
>> trapping if any other differences are found between the vr_values and
>> the ranger versions of builtin range handling.
>>
>> As a follow-up, once Fedora can test this approach, I'll remove all
>> the builtin code from extract_range_builtin, with the exception of the
>> UBSAN stuff (renaming it to extract_range_ubsan_builtin).
>>
>> Since the builtin code has proven fickle across architectures, I've
>> tested this with {-m32,-m64,-fsanitize=signed-integer-overflow} on
>> x86, ppc64le, and aarch64.  I think this should be enough.  If it
>> isn't, we can revert the patch, and leave the duplicate code until
>> the next release cycle when hopefully vr_values, evrp, and friends
>> will all be overhauled.
>>
>> Andrew, do you have any thoughts on this?
>>
>>
> OK.
> 
> I think we want to remove as much duplication as possible, which will 
> then give us confidence that that ranger versions are correct.
> THe UBSAN versions will have to get tighter ranger integration with 
> relationals when they are available in order to handle things like this.
> 
> I dont suppose you can create a testcase for this?   Otherwise we'll 
> have to tag it somehow so we dont forget to come back to it when we 
> start handling these ubsan builtins differently.

Ughh...not easily.  It's deep in a Fortran testcase, and AFAICT the 
range that is determined (~[0,0]) does not affect the code generated.

I'll post as is, while I ponder this.  Perhaps I can hand craft a gimple 
FE test that will trigger different code out of evrp that we can somehow 
test.  If/when I do, I'll push the test.

Aldy

> Andrew
> 
> PS. and might as well create and test the follow up patch so its ready 
> to go.  I'd leave this as-is with the asserts for a week or so.
> 
> 
> 
>
Aldy Hernandez Nov. 2, 2020, 10:44 a.m. UTC | #4
On 10/29/20 3:39 PM, Aldy Hernandez wrote:
> 
> 
> On 10/29/20 2:53 PM, Andrew MacLeod wrote:
>> On 10/27/20 11:29 AM, Aldy Hernandez wrote:
>>> The UBSAN builtins degrade into PLUS/MINUS/MULT and call
>>> extract_range_from_binary_expr, which as the PR shows, can special
>>> case some symbolics which the ranger doesn't currently handle.
>>>
>>> Looking at vr_values::extract_range_builtin(), I see that every single
>>> place where we ask for a range, we bail on non-integers (symbolics,
>>> etc).  That is, with the exception of the UBSAN builtins.
>>>
>>> Since this seems to be particular to UBSAN, we could still go with the
>>> original plan of removing the duplicity in ranger vs vr-values, but
>>> leave in the UBSAN builtin handling.  This isn't ideal, as we'd like
>>> to remove all the common code, but I'd be willing to put up with UBSAN
>>> duplication for the time being.
>>>
>>> This patch disables the assert on the UBSAN builtins, while still
>>> trapping if any other differences are found between the vr_values and
>>> the ranger versions of builtin range handling.
>>>
>>> As a follow-up, once Fedora can test this approach, I'll remove all
>>> the builtin code from extract_range_builtin, with the exception of the
>>> UBSAN stuff (renaming it to extract_range_ubsan_builtin).
>>>
>>> Since the builtin code has proven fickle across architectures, I've
>>> tested this with {-m32,-m64,-fsanitize=signed-integer-overflow} on
>>> x86, ppc64le, and aarch64.  I think this should be enough.  If it
>>> isn't, we can revert the patch, and leave the duplicate code until
>>> the next release cycle when hopefully vr_values, evrp, and friends
>>> will all be overhauled.
>>>
>>> Andrew, do you have any thoughts on this?
>>>
>>>
>> OK.
>>
>> I think we want to remove as much duplication as possible, which will 
>> then give us confidence that that ranger versions are correct.
>> THe UBSAN versions will have to get tighter ranger integration with 
>> relationals when they are available in order to handle things like this.
>>
>> I dont suppose you can create a testcase for this?   Otherwise we'll 
>> have to tag it somehow so we dont forget to come back to it when we 
>> start handling these ubsan builtins differently.
> 
> Ughh...not easily.  It's deep in a Fortran testcase, and AFAICT the 
> range that is determined (~[0,0]) does not affect the code generated.
> 
> I'll post as is, while I ponder this.  Perhaps I can hand craft a gimple 
> FE test that will trigger different code out of evrp that we can somehow 
> test.  If/when I do, I'll push the test.

This took some work, but I was finally able to distill a C testcase 
where the behavior is visible in the generated code.  It's a small 
testcase that will serve as a simple test for relationals in the world 
of built-ins.

Basically the Y - X is turned into a .UBSAN_CHECK_SUB call that must 
play nice with the (x < y) relationship.

Pushed.

gcc/testsuite/ChangeLog:

	PR tree-optimization/97505
	* gcc.dg/pr97505.c: New test.
---
  gcc/testsuite/gcc.dg/pr97505.c | 23 +++++++++++++++++++++++
  1 file changed, 23 insertions(+)
  create mode 100644 gcc/testsuite/gcc.dg/pr97505.c

diff --git a/gcc/testsuite/gcc.dg/pr97505.c b/gcc/testsuite/gcc.dg/pr97505.c
new file mode 100644
index 00000000000..f01d912067a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr97505.c
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-options "-Os -fsanitize=signed-integer-overflow -fdump-tree-evrp" }
+
+// Test that .UBSAN_CHECK_SUB(y, x) is treated as y-x for range
+// purposes, where X and Y are related to each other.
+//
+// This effectively checks that range relationals work with builtins.
+
+void unreachable();
+
+int foobar(int x, int y)
+{
+  if (x < y)
+    {
+      int z = y - x;
+      if (z == 0)
+        unreachable();
+      return z;
+    }
+  return 5;
+}
+
+// { dg-final { scan-tree-dump-not "unreachable" "evrp" } }
diff mbox series

Patch

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 7a0e70eab64..9f5943a1ab6 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -1432,14 +1432,17 @@  vr_values::extract_range_basic (value_range_equiv *vr, gimple *stmt)
 
   if (is_gimple_call (stmt) && extract_range_builtin (vr, stmt))
     {
+      combined_fn cfn = gimple_call_combined_fn (stmt);
+      if (cfn == CFN_UBSAN_CHECK_ADD
+	  || cfn == CFN_UBSAN_CHECK_SUB
+	  || cfn == CFN_UBSAN_CHECK_MUL)
+	return;
+
       value_range_equiv tmp;
       /* Assert that any ranges vr_values::extract_range_builtin gets
 	 are also handled by the ranger counterpart.  */
       gcc_assert (range_of_builtin_call (*this, tmp, as_a<gcall *> (stmt)));
-#if 0
-      /* Disable this while PR97505 is resolved.  */
       gcc_assert (tmp.equal_p (*vr, /*ignore_equivs=*/false));
-#endif
       return;
     }
   /* Handle extraction of the two results (result of arithmetics and