[gcc r14-9707] profile-count: Avoid overflows into uninitialized [PR112303]

Jakub Jelinek jakub@gcc.gnu.org
Thu Mar 28 14:05:51 GMT 2024


https://gcc.gnu.org/g:d5a3b4afcdf4d517334a2717dbb65ae0d2c26507

commit r14-9707-gd5a3b4afcdf4d517334a2717dbb65ae0d2c26507
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 28 15:00:44 2024 +0100

    profile-count: Avoid overflows into uninitialized [PR112303]
    
    The testcase in the patch ICEs with
    --- gcc/tree-scalar-evolution.cc
    +++ gcc/tree-scalar-evolution.cc
    @@ -3881,7 +3881,7 @@ final_value_replacement_loop (class loop *loop)
    
           /* Propagate constants immediately, but leave an unused initialization
             around to avoid invalidating the SCEV cache.  */
    -      if (CONSTANT_CLASS_P (def) && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rslt))
    +      if (0 && CONSTANT_CLASS_P (def) && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rslt))
            replace_uses_by (rslt, def);
    
           /* Create the replacement statements.  */
    (the addition of the above made the ICE latent), because profile_count
    addition doesn't check for overflows and if unlucky, we can even overflow
    into the uninitialized value.
    Getting really huge profile counts is very easy even when not using
    recursive inlining in loops, e.g.
    __attribute__((noipa)) void
    bar (void)
    {
      __builtin_exit (0);
    }
    
    __attribute__((noipa)) void
    foo (void)
    {
      for (int i = 0; i < 1000; ++i)
      for (int j = 0; j < 1000; ++j)
      for (int k = 0; k < 1000; ++k)
      for (int l = 0; l < 1000; ++l)
      for (int m = 0; m < 1000; ++m)
      for (int n = 0; n < 1000; ++n)
      for (int o = 0; o < 1000; ++o)
      for (int p = 0; p < 1000; ++p)
      for (int q = 0; q < 1000; ++q)
      for (int r = 0; r < 1000; ++r)
      for (int s = 0; s < 1000; ++s)
      for (int t = 0; t < 1000; ++t)
      for (int u = 0; u < 1000; ++u)
      for (int v = 0; v < 1000; ++v)
      for (int w = 0; w < 1000; ++w)
      for (int x = 0; x < 1000; ++x)
      for (int y = 0; y < 1000; ++y)
      for (int z = 0; z < 1000; ++z)
      for (int a = 0; a < 1000; ++a)
      for (int b = 0; b < 1000; ++b)
        bar ();
    }
    
    int
    main ()
    {
      foo ();
    }
    reaches the maximum count already on the 11th loop.
    
    Some other methods of profile_count like apply_scale already
    do use MIN (val, max_count) before assignment to m_val, this patch
    just extends that to operator{+,+=} methods.
    Furthermore, one overload of apply_probability wasn't using
    safe_scale_64bit and so could very easily overflow as well
    - prob is required to be [0, 10000] and if m_val is near the max_count,
    it can overflow even with multiplications by 8.
    
    2024-03-28  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/112303
            * profile-count.h (profile_count::operator+): Perform
            addition in uint64_t variable and set m_val to MIN of that
            val and max_count.
            (profile_count::operator+=): Likewise.
            (profile_count::operator-=): Formatting fix.
            (profile_count::apply_probability): Use safe_scale_64bit
            even in the int overload.
    
            * gcc.c-torture/compile/pr112303.c: New test.

Diff:
---
 gcc/profile-count.h                            | 12 ++++++++----
 gcc/testsuite/gcc.c-torture/compile/pr112303.c | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/gcc/profile-count.h b/gcc/profile-count.h
index b3d776475e2..6bbd476cb7f 100644
--- a/gcc/profile-count.h
+++ b/gcc/profile-count.h
@@ -910,7 +910,8 @@ public:
 
       profile_count ret;
       gcc_checking_assert (compatible_p (other));
-      ret.m_val = m_val + other.m_val;
+      uint64_t ret_val = m_val + other.m_val;
+      ret.m_val = MIN (ret_val, max_count);
       ret.m_quality = MIN (m_quality, other.m_quality);
       return ret;
     }
@@ -929,7 +930,8 @@ public:
       else
 	{
           gcc_checking_assert (compatible_p (other));
-	  m_val += other.m_val;
+	  uint64_t ret_val = m_val + other.m_val;
+	  m_val = MIN (ret_val, max_count);
 	  m_quality = MIN (m_quality, other.m_quality);
 	}
       return *this;
@@ -957,7 +959,7 @@ public:
       else
 	{
           gcc_checking_assert (compatible_p (other));
-	  m_val = m_val >= other.m_val ? m_val - other.m_val: 0;
+	  m_val = m_val >= other.m_val ? m_val - other.m_val : 0;
 	  m_quality = MIN (m_quality, other.m_quality);
 	}
       return *this;
@@ -1127,7 +1129,9 @@ public:
       if (!initialized_p ())
 	return uninitialized ();
       profile_count ret;
-      ret.m_val = RDIV (m_val * prob, REG_BR_PROB_BASE);
+      uint64_t tmp;
+      safe_scale_64bit (m_val, prob, REG_BR_PROB_BASE, &tmp);
+      ret.m_val = tmp;
       ret.m_quality = MIN (m_quality, ADJUSTED);
       return ret;
     }
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr112303.c b/gcc/testsuite/gcc.c-torture/compile/pr112303.c
new file mode 100644
index 00000000000..01937da53cf
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr112303.c
@@ -0,0 +1,25 @@
+/* PR tree-optimization/112303 */
+
+int a, b, d, e, f, **g, h;
+char c;
+
+int *
+foo (void)
+{
+  for (int i = 0; i < 3; i++)
+    {
+      for (h = 0; h < 2; h++)
+	;
+      if (!b)
+	break;
+    }
+  while (f)
+    while (e)
+      {
+	c = 0;
+	while (d)
+	  while (a)
+	    *g = foo ();
+      }
+  return 0;
+}


More information about the Gcc-cvs mailing list