From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1930) id F2AD93858C1F; Mon, 31 Jan 2022 19:06:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F2AD93858C1F MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Martin Sebor To: gcc-cvs@gcc.gnu.org Subject: [gcc r12-6947] Constrain PHI handling in -Wuse-after-free [PR104232]. X-Act-Checkin: gcc X-Git-Author: Martin Sebor X-Git-Refname: refs/heads/master X-Git-Oldrev: 31ab99f7c854d654bf05abd50e3300714df381f3 X-Git-Newrev: 48d3191e7bd6245bd2df625731f1ad9207a26655 Message-Id: <20220131190640.F2AD93858C1F@sourceware.org> Date: Mon, 31 Jan 2022 19:06:40 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Jan 2022 19:06:41 -0000 https://gcc.gnu.org/g:48d3191e7bd6245bd2df625731f1ad9207a26655 commit r12-6947-g48d3191e7bd6245bd2df625731f1ad9207a26655 Author: Martin Sebor Date: Mon Jan 31 12:04:55 2022 -0700 Constrain PHI handling in -Wuse-after-free [PR104232]. 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. Diff: --- gcc/gimple-ssa-warn-access.cc | 50 +++++++++++- gcc/testsuite/g++.dg/warn/Wuse-after-free4.C | 27 +++++++ gcc/testsuite/gcc.dg/Wuse-after-free-2.c | 115 +++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/Wuse-after-free-3.c | 22 +++++ 4 files changed, 212 insertions(+), 2 deletions(-) diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index 3dcaf4230b8..ad5e2f4458e 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -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 index 00000000000..599d9bfe3c7 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuse-after-free4.C @@ -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 index 00000000000..9f7ed4529f0 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wuse-after-free-2.c @@ -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 index 00000000000..d1bcfcb3dda --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wuse-after-free-3.c @@ -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; +}