]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
Constrain PHI handling in -Wuse-after-free [PR104232].
authorMartin Sebor <msebor@redhat.com>
Mon, 31 Jan 2022 19:04:55 +0000 (12:04 -0700)
committerMartin Sebor <msebor@redhat.com>
Mon, 31 Jan 2022 19:04:55 +0000 (12:04 -0700)
Resolves:
PR middle-end/104232 - spurious -Wuse-after-free after conditional free

gcc/ChangeLog:

PR middle-end/104232
* gimple-ssa-warn-access.cc (pointers_related_p): Add argument.
Handle PHIs.  Add a synonymous overload.
(pass_waccess::check_pointer_uses): Call pointers_related_p.

gcc/testsuite/ChangeLog:

PR middle-end/104232
* g++.dg/warn/Wuse-after-free4.C: New test.
* gcc.dg/Wuse-after-free-2.c: New test.
* gcc.dg/Wuse-after-free-3.c: New test.

gcc/gimple-ssa-warn-access.cc
gcc/testsuite/g++.dg/warn/Wuse-after-free4.C [new file with mode: 0644]
gcc/testsuite/gcc.dg/Wuse-after-free-2.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/Wuse-after-free-3.c [new file with mode: 0644]

index 3dcaf4230b85bfeee304cfbc734c09696eda1d7a..ad5e2f4458eaa1a07dc5eae9b3569e20355170f2 100644 (file)
@@ -4080,7 +4080,8 @@ maybe_warn_mismatched_realloc (tree ptr, gimple *realloc_stmt, gimple *stmt)
    either don't or their relationship cannot be determined.  */
 
 static bool
-pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
+pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry,
+                   auto_bitmap &visited)
 {
   if (!ptr_derefs_may_alias_p (p, q))
     return false;
@@ -4093,7 +4094,47 @@ pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
        it involves a self-referential PHI.  Return a conservative result.  */
     return false;
 
-  return pref.ref == qref.ref;
+  if (pref.ref == qref.ref)
+    return true;
+
+  /* If either pointer is a PHI, iterate over all its operands and
+     return true if they're all related to the other pointer.  */
+  tree ptr = q;
+  unsigned version;
+  gphi *phi = pref.phi ();
+  if (phi)
+    version = SSA_NAME_VERSION (pref.ref);
+  else
+    {
+      phi = qref.phi ();
+      if (!phi)
+       return false;
+
+      ptr = p;
+      version = SSA_NAME_VERSION (qref.ref);
+    }
+
+  if (!bitmap_set_bit (visited, version))
+    return true;
+
+  unsigned nargs = gimple_phi_num_args (phi);
+  for (unsigned i = 0; i != nargs; ++i)
+    {
+      tree arg = gimple_phi_arg_def (phi, i);
+      if (!pointers_related_p (stmt, arg, ptr, qry, visited))
+       return false;
+    }
+
+  return true;
+}
+
+/* Convenience wrapper for the above.  */
+
+static bool
+pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
+{
+  auto_bitmap visited;
+  return pointers_related_p (stmt, p, q, qry, visited);
 }
 
 /* For a STMT either a call to a deallocation function or a clobber, warn
@@ -4192,7 +4233,12 @@ pass_waccess::check_pointer_uses (gimple *stmt, tree ptr,
            {
              if (gimple_code (use_stmt) == GIMPLE_PHI)
                {
+                 /* Only add a PHI result to POINTERS if all its
+                    operands are related to PTR, otherwise continue.  */
                  tree lhs = gimple_phi_result (use_stmt);
+                 if (!pointers_related_p (stmt, lhs, ptr, m_ptr_qry))
+                   continue;
+
                  if (TREE_CODE (lhs) == SSA_NAME)
                    {
                      pointers.safe_push (lhs);
diff --git a/gcc/testsuite/g++.dg/warn/Wuse-after-free4.C b/gcc/testsuite/g++.dg/warn/Wuse-after-free4.C
new file mode 100644 (file)
index 0000000..599d9bf
--- /dev/null
@@ -0,0 +1,27 @@
+/* PR middle-end/104232 - spurious -Wuse-after-free after conditional free
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+char* f (void);
+
+struct A
+{
+  char *p;
+  A (): p () { }
+  ~A ()
+  {
+    __builtin_free (p);           // { dg-bogus "-Wuse-after-free" }
+  }
+};
+
+int test_no_warn (void)
+{
+  A px, qx;
+
+  qx.p = f ();
+  if (!qx.p)
+    return 0;
+
+  px.p = f ();
+  return 1;
+}
diff --git a/gcc/testsuite/gcc.dg/Wuse-after-free-2.c b/gcc/testsuite/gcc.dg/Wuse-after-free-2.c
new file mode 100644 (file)
index 0000000..9f7ed45
--- /dev/null
@@ -0,0 +1,115 @@
+/* PR middle-end/104232 - spurious -Wuse-after-free after conditional free
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+void free (void*);
+
+void sink (void*);
+
+void nowarn_cond_2 (char *p0, char *q0, int i)
+{
+  char *r = i ? p0 : q0;
+
+  free (p0);
+
+  /* The use of a PHI operand could be diagnosed using the "maybe" form
+     of the warning at level 2 but it's not done.  If it ever changes
+     this test and those below will need to be updated.  */
+  sink (r);
+}
+
+void nowarn_cond_2_null (char *p0, int i)
+{
+  char *r = i ? p0 : 0;
+
+  free (p0);
+  sink (r);
+}
+
+void nowarn_cond_3 (char *p0, char *q0, int i)
+{
+  char *r = i < 0 ? p0 - 1 : 0 < i ? p0 + 1 : q0;
+
+  free (p0);
+  sink (r);
+}
+
+void nowarn_cond_3_null (char *p0, int i)
+{
+  char *r = i < 0 ? p0 - 1 : 0 < i ? p0 + 1 : 0;
+
+  free (p0);
+  sink (r);
+}
+
+void nowarn_cond_4 (char *p0, char *q0, int i)
+{
+  char *r = i < -1 ? p0 - 2 : i < 0 ? p0 - 1 : 1 < i ? p0 + 1 : q0;
+
+  free (p0);
+  sink (r);
+}
+
+int nowarn_cond_loop (char *p)
+{
+  char *q = p;
+  while (*q)
+    {
+      if (*q == 'x')
+        {
+          q = "";
+          break;
+        }
+      ++q;
+    }
+
+  free (p);
+  return *q;
+}
+
+
+void warn_cond_2_cst (char *p, int i)
+{
+  /* Same as nowarn_cond_2() above but with R being derived only from
+     P, which means that any R's use after P has been freed should be
+     diagnosed.  */
+  char *r = i ? p + 1 : p + 2;
+
+  free (p);         // { dg-message "call to 'free'" }
+  sink (r);         // { dg-warning "pointer used after 'free'" }
+}
+
+void warn_cond_2_var (char *p, int i, int j)
+{
+  char *r = i ? p + i : p + j;
+
+  free (p);         // { dg-message "call to 'free'" }
+  sink (r);         // { dg-warning "pointer used after 'free'" }
+}
+
+void warn_cond_3_var (char *p0, int i, int j)
+{
+  char *r = i < 0 ? p0 - i : 0 < i ? p0 + j : p0 + i + j;
+
+  free (p0);        // { dg-message "call to 'free'" }
+  sink (r + 1);     // { dg-warning "pointer used after 'free'" }
+}
+
+int warn_cond_4 (char *p0, char *q0, int i)
+{
+  char *r = i < -1 ? p0 - 2 : i < 0 ? p0 - 1 : 1 < i ? p0 + 2 : p0 + 1;
+
+  free (p0);        // { dg-message "call to 'free'" }
+  return *r;        // { dg-warning "pointer used after 'free'" }
+}
+
+int warn_cond_loop (char *p)
+{
+  char *q = p;
+
+  while (*q)
+    ++q;
+
+  free (p);         // { dg-message "call to 'free'" }
+  return *q;        // { dg-warning "pointer 'q' used after 'free'" }
+}
diff --git a/gcc/testsuite/gcc.dg/Wuse-after-free-3.c b/gcc/testsuite/gcc.dg/Wuse-after-free-3.c
new file mode 100644 (file)
index 0000000..d1bcfcb
--- /dev/null
@@ -0,0 +1,22 @@
+/* PR middle-end/104232 - spurious -Wuse-after-free after conditional free
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+char* f (void);
+
+static inline void freep (void *p)
+{
+  __builtin_free (*(void**)p);    // { dg-bogus "-Wuse-after-free" }
+}
+
+int test_no_warn (void)
+{
+  __attribute__ ((__cleanup__ (freep))) char *s = 0, *t = 0;
+
+  t = f ();
+  if (!t)
+    return 0;
+
+  s = f ();
+  return 1;
+}