From patchwork Thu Jan 27 18:57:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 50495 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 59607385E02C for ; Thu, 27 Jan 2022 18:57:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 59607385E02C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1643309878; bh=13LT7tK9CyuvJ465bcl9G1c1pzg7mVHgv45pdHk5VW4=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=IW1ws4LjZ/fYxdLaVdZ9vB0scoIC7UJQlz2h60qfMYf3FhunYbMS9FkLCeSBPqtUs iYrz5j0VmaiRqPvcjTs0Uxcj6gmaBoMuxBNX0RfuDq5l+LWCKMkcnv9fvV5lSO2H4x KslZQFbioVx92NFSDqRBYucgYnnaJQ+rV+MORreo= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-il1-x12a.google.com (mail-il1-x12a.google.com [IPv6:2607:f8b0:4864:20::12a]) by sourceware.org (Postfix) with ESMTPS id C626D385DC31 for ; Thu, 27 Jan 2022 18:57:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C626D385DC31 Received: by mail-il1-x12a.google.com with SMTP id e8so3292431ilm.13 for ; Thu, 27 Jan 2022 10:57:10 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:from:subject; bh=13LT7tK9CyuvJ465bcl9G1c1pzg7mVHgv45pdHk5VW4=; b=mEKY0C5WC7A9Fw93CJdEaPB2RvdeycX1Yw8t+0jgZEmbftsPvzxOoFZ0tvxSVkPgef +Bqw8ptzEgN5im/Q83nvKw51RddmmSaDgEsYT1Q3v07Yr4KEALMr5ShtYp4AoT13WQRI J7jHtPLcEwDM4tECIK3G2mXPliwCsf+nc1o6xbA/ir3uWG44pfAb7pDoFUMi+TEMmj4Q Q9/5XhehVC7lr/sFJDudA0Z0pWvLDflDs3w/gTlnqtq3dJCZQq04rSgSonMqGAeun6s8 ffy9sjdoEuvA2T3h9yBd1pQwCeHQ0jBxJxsdBzw4w58Znwwu1SL7ip9rZqBMA3uQBny/ ObMQ== X-Gm-Message-State: AOAM5313xJbBokjveduFKS3jTIBg77dCQjmj1iE2Ns62LTcV/4LCcTHA 1WASEatfRtiicP491cayVQNTu82k26w= X-Google-Smtp-Source: ABdhPJwu9XkEmGJpzDobqLBi0vZ/ge05Y1ARxvt2g/tS03Sq7qzJ+bWk6dUShQ6+CQM7to3l/hP30w== X-Received: by 2002:a05:6e02:1c4b:: with SMTP id d11mr3466820ilg.273.1643309830069; Thu, 27 Jan 2022 10:57:10 -0800 (PST) Received: from [192.168.0.41] (97-118-100-142.hlrn.qwest.net. [97.118.100.142]) by smtp.gmail.com with ESMTPSA id g21sm3591209iow.4.2022.01.27.10.57.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 27 Jan 2022 10:57:09 -0800 (PST) Message-ID: <91beddcc-1d69-48f4-24ee-4dfed823f59a@gmail.com> Date: Thu, 27 Jan 2022 11:57:08 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 Content-Language: en-US To: gcc-patches Subject: [PATCH] constrain PHI handling in -Wuse-after-free (PR104232) X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SCC_5_SHORT_WORD_LINES, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Martin Sebor via Gcc-patches From: Martin Sebor Reply-To: Martin Sebor Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" The indiscriminate PHI handling by -Wuse-after-free has caused the false positive reported in PR 104232. The attached patch refines the handling to only consider PHIs all of whose operands refer to the same object and disregard the rest. Tested on x86_64-linux and by compiling a few toolchain projects, including Glibc and Binutils/GDB, to verify the absence of false positives. Martin 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 --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; +}