From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x82a.google.com (mail-qt1-x82a.google.com [IPv6:2607:f8b0:4864:20::82a]) by sourceware.org (Postfix) with ESMTPS id 1F517385503C for ; Thu, 5 Aug 2021 22:02:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1F517385503C Received: by mail-qt1-x82a.google.com with SMTP id z24so5029952qtn.8 for ; Thu, 05 Aug 2021 15:02:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version:content-language; bh=ZW3aM2j4G8LtpmA1bB76c9EeB0fM9/9kLODN5yVfJPA=; b=Lkg0xTIBxCxAgKKe5qjdUXuSXJx5+Eiq6Ik5ACBw1RwDjcdfGrm7Ksc2CJUfWwrGpR L5FA4Px1cGE3C1DF9EMV7cdG0NbWj97sLvD2w4fV5ggNr6RW+X8s8ZYC+8CBaIB2f0G8 ZPITnFS9bWXefRgHV3NHbc9IKals8stbLZa9j16JxL5YqO+Hk9iPZDT3gCoAkWebGLez wDGKkL/RwG2eo7OAurRb43l1IH0+YWzJYPCtq2dB54se2EDuiBmLtRb3IJTZK6jSTYdr h2XJe22MZ4HgOhcdijAVPo/pD7q0wtc7PnatIrWAzEQ0EEcRFs2s0IuAgA1ygWl+m/T3 ekLw== X-Gm-Message-State: AOAM533QW06+dQ+YZ1MQmgJi2m4FNKWIAoNq7ULoJCmB/pFZ9CqUiWLN /lUN/qPHPStCt39yNf7xQwH9Sr7p6sc= X-Google-Smtp-Source: ABdhPJxyBx0xYJdXl4tk+uz5gASx1GKwsAKIQfZdxjKXign8O4Ii97rqxjLstr8uumMk3OPBG4rqiw== X-Received: by 2002:ac8:4d8e:: with SMTP id a14mr6466675qtw.157.1628200940574; Thu, 05 Aug 2021 15:02:20 -0700 (PDT) Received: from [192.168.0.41] (75-166-102-22.hlrn.qwest.net. [75.166.102.22]) by smtp.gmail.com with ESMTPSA id q11sm3531563qkm.56.2021.08.05.15.02.19 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 05 Aug 2021 15:02:20 -0700 (PDT) To: gcc-patches From: Martin Sebor Subject: [PATCH] diagnose more new/delete mismatches (PR 101791) Message-ID: <908b84b7-e229-7b47-7a29-32f16d60f190@gmail.com> Date: Thu, 5 Aug 2021 16:02:19 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------C48C3A20E3347A73B997727C" Content-Language: en-US X-Spam-Status: No, score=-10.1 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, 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-List-Received-Date: Thu, 05 Aug 2021 22:02:23 -0000 This is a multi-part message in MIME format. --------------C48C3A20E3347A73B997727C Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit -Wmismatched-new-delete partly relies on valid_new_delete_pair_p() to detect matching calls to operator new and delete. The function returns a conservative result which, when indicating a mismatch, the warning then works to make more accurate before it triggers. As it turns out, the logic is inadvertently overly permissive in the most common case of a mismatch: between the array and scalar forms of the global operators new and delete such as in: delete new int[2]; // should be delete[] The attached patch solves the problem by adding a new argument to valid_new_delete_pair_p() for the function to set to indicate when its negative result is definitive rather than a conservative guess. Tested on x86_64-linux. Martin --------------C48C3A20E3347A73B997727C Content-Type: text/x-patch; charset=UTF-8; name="gcc-101791.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-101791.diff" Diagnose mismatches between array and scalar new and delete [PR101791]. Resolves: PR middle-end/101791 - missing warning on a mismatch between scalar and array forms of new and delete gcc/ChangeLog: PR middle-end/101791 * gimple-ssa-warn-access.cc (new_delete_mismatch_p): Use new argument to valid_new_delete_pair_p. * tree.c (valid_new_delete_pair_p): Add argument. * tree.h (valid_new_delete_pair_p): Same. gcc/testsuite/ChangeLog: PR middle-end/101791 * g++.dg/warn/Wmismatched-new-delete-6.C: New test. * g++.dg/warn/Wmismatched-new-delete-7.C: New test. diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index e4d98b2ec28..c1ced98780f 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -1233,9 +1782,14 @@ new_delete_mismatch_p (tree new_decl, tree delete_decl) /* valid_new_delete_pair_p() returns a conservative result (currently it only handles global operators). A true result is reliable but - a false result doesn't necessarily mean the operators don't match. */ - if (valid_new_delete_pair_p (new_name, delete_name)) + a false result doesn't necessarily mean the operators don't match + unless CERTAIN is set. */ + bool certain; + if (valid_new_delete_pair_p (new_name, delete_name, &certain)) return false; + /* CERTAIN is set when the negative result is certain. */ + if (certain) + return true; /* For anything not handled by valid_new_delete_pair_p() such as member operators compare the individual demangled components of the mangled diff --git a/gcc/tree.c b/gcc/tree.c index e923e67b694..5c747321002 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -14311,17 +14311,28 @@ verify_type_context (location_t loc, type_context_kind context, || targetm.verify_type_context (loc, context, type, silent_p)); } -/* Return that NEW_ASM and DELETE_ASM name a valid pair of new and - delete operators. */ +/* Return true if NEW_ASM and DELETE_ASM name a valid pair of new and + delete operators. Return false if they may or may not name such + a pair and, when nonnull, set *PCERTAIN to true if they certainly + do not. */ bool -valid_new_delete_pair_p (tree new_asm, tree delete_asm) +valid_new_delete_pair_p (tree new_asm, tree delete_asm, + bool *pcertain /* = NULL */) { + bool certain; + if (!pcertain) + pcertain = &certain; + const char *new_name = IDENTIFIER_POINTER (new_asm); const char *delete_name = IDENTIFIER_POINTER (delete_asm); unsigned int new_len = IDENTIFIER_LENGTH (new_asm); unsigned int delete_len = IDENTIFIER_LENGTH (delete_asm); + /* The following failures are due to invalid names so they're not + considered certain mismatches. */ + *pcertain = false; + if (new_len < 5 || delete_len < 6) return false; if (new_name[0] == '_') @@ -14334,11 +14345,19 @@ valid_new_delete_pair_p (tree new_asm, tree delete_asm) ++delete_name, --delete_len; if (new_len < 4 || delete_len < 5) return false; + + /* The following failures are due to names of user-defined operators + so they're also not considered certain mismatches. */ + /* *_len is now just the length after initial underscores. */ if (new_name[0] != 'Z' || new_name[1] != 'n') return false; if (delete_name[0] != 'Z' || delete_name[1] != 'd') return false; + + /* The following failures are certain mismatches. */ + *pcertain = true; + /* _Znw must match _Zdl, _Zna must match _Zda. */ if ((new_name[2] != 'w' || delete_name[2] != 'l') && (new_name[2] != 'a' || delete_name[2] != 'a')) @@ -14377,6 +14396,9 @@ valid_new_delete_pair_p (tree new_asm, tree delete_asm) && !memcmp (delete_name + 5, "St11align_val_tRKSt9nothrow_t", 29)) return true; } + + /* The negative result is conservative. */ + *pcertain = false; return false; } diff --git a/gcc/tree.h b/gcc/tree.h index 972ceb370f8..69d12193686 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -5396,7 +5396,9 @@ extern bool gimple_canonical_types_compatible_p (const_tree, const_tree, extern bool type_with_interoperable_signedness (const_tree); extern bitmap get_nonnull_args (const_tree); extern int get_range_pos_neg (tree); -extern bool valid_new_delete_pair_p (tree, tree); + +/* Return true for a valid pair of new and delete operators. */ +extern bool valid_new_delete_pair_p (tree, tree, bool * = NULL); /* Return simplified tree code of type that is used for canonical type merging. */ diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-6.C b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-6.C new file mode 100644 index 00000000000..e19d00058b3 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-6.C @@ -0,0 +1,158 @@ +/* PR middle-end/101791 - missing warning on a mismatch between scalar + and array forms of new and delete + { dg-do compile } + { dg-options "-Wall" } */ + +typedef __SIZE_TYPE__ size_t; + +namespace std +{ +#if __cplusplus >= 201703L +enum class align_val_t: size_t { }; +#else +enum align_val_t { }; +#endif + +struct nothrow_t { }; +const nothrow_t nothrow = { }; + +} + +void* operator new (size_t); +void* operator new (size_t, std::align_val_t); +void* operator new (size_t, std::nothrow_t) throw (); +void* operator new (size_t, std::align_val_t, std::nothrow_t) throw (); + +void* operator new[] (size_t); +void* operator new[] (size_t, std::align_val_t); +void* operator new[] (size_t, std::nothrow_t) throw (); +void* operator new[] (size_t, std::align_val_t, std::nothrow_t) throw (); + +void operator delete (void*); +void operator delete (void*, size_t); +void operator delete (void*, std::align_val_t); +void operator delete (void*, size_t, std::align_val_t); +void operator delete (void*, std::nothrow_t) throw (); +void operator delete (void*, std::align_val_t, std::nothrow_t) throw (); + +void operator delete[] (void*); +void operator delete[] (void*, size_t); +void operator delete[] (void*, std::align_val_t); +void operator delete[] (void*, size_t, std::align_val_t); +void operator delete[] (void*, std::nothrow_t) throw (); +void operator delete[] (void*, std::align_val_t, std::nothrow_t) throw (); + + +void sink (void*, ...); + + +void nowarn_scalar_scalar () +{ + { + int *p = new int; + sink (p); + delete p; + } + + { + int *p = new (std::align_val_t (8)) int; + sink (p); + delete p; + } + + { + int *p = new (std::nothrow) int; + sink (p); + delete p; + } + + { + int *p = new (std::align_val_t (8), std::nothrow) int; + sink (p); + delete p; + } +} + +void nowarn_array_array () +{ + { + int *p = new int[__LINE__]; + sink (p); + delete[] p; + } + + { + int *p = new (std::align_val_t (8)) int[__LINE__]; + sink (p); + delete[] p; + } + + { + int *p = new (std::nothrow) int[__LINE__]; + sink (p); + delete[] p; + } + + { + int *p = new (std::align_val_t (8), std::nothrow) int[__LINE__]; + sink (p); + delete[] p; + } +} + + + +void nowarn_scalar_array () +{ + { + int *p = new int; // { dg-message "returned from" } + sink (p); + delete[] p; // { dg-warning "\\\[-Wmismatched-new-delete" } + } + + { + int *p = new (std::align_val_t (8)) int; + sink (p); + delete[] p; // { dg-warning "\\\[-Wmismatched-new-delete" } + } + + { + int *p = new (std::nothrow) int; + sink (p); + delete[] p; // { dg-warning "\\\[-Wmismatched-new-delete" } + } + + { + int *p = new (std::align_val_t (8), std::nothrow) int; + sink (p); + delete[] p; // { dg-warning "\\\[-Wmismatched-new-delete" } + } +} + + +void nowarn_array_scalar () +{ + { + int *p = new int[__LINE__]; + sink (p); + delete p; // { dg-warning "\\\[-Wmismatched-new-delete" } + } + + { + int *p = new (std::align_val_t (8)) int[__LINE__]; + sink (p); + delete p; // { dg-warning "\\\[-Wmismatched-new-delete" } + } + + { + int *p = new (std::nothrow) int[__LINE__]; + sink (p); + delete p; // { dg-warning "\\\[-Wmismatched-new-delete" } + } + + { + int *p = new (std::align_val_t (8), std::nothrow) int[__LINE__]; + sink (p); + delete p; // { dg-warning "\\\[-Wmismatched-new-delete" } + } +} diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-7.C b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-7.C new file mode 100644 index 00000000000..67a5354b91d --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-7.C @@ -0,0 +1,91 @@ +/* PR middle-end/101791 - missing warning on a mismatch between scalar + and array forms of new and delete + Verify that likely safe calls to technically mismatched member operator + new and delete are not diagnosed. This test might need to be adjusted + if it turns out the assumptions GCC makes are overly conservative. + { dg-do compile } + { dg-options "-Wall" } */ + +typedef __SIZE_TYPE__ size_t; + +namespace std +{ +#if __cplusplus >= 201703L +enum class align_val_t: size_t { }; +#else +enum align_val_t { }; +#endif + +struct nothrow_t { }; +const nothrow_t nothrow = { }; + +} + +void sink (void*, ...); + +struct X { } x; +struct Y { } y; + +struct A +{ + void* operator new (size_t); + void* operator new (size_t, std::align_val_t); + + void* operator new (size_t, X); + void* operator new (size_t, Y); + + void* operator new (size_t, std::align_val_t, X); + void* operator new (size_t, std::nothrow_t, Y); + + /* A single operator delete callable on the result of calls to any + of the operator new overloads above (this may be too optimistic). */ + void operator delete (void*); +}; + +A* nowarn_align () +{ + /* The following are likely okay given A's definition above but would + not be if A also defined an align_val_t overload of operator delete. */ + A *p = new (std::align_val_t (8)) A; + delete p; + + return new (std::align_val_t (16)) A; +} + +A* nowarn_X () +{ + /* The following are also likely okay given A's definition above but + also would not be if A also defined an overload of operator delete + for X. */ + A *p = new (x) A; + delete p; + return new (x) A; +} + +A* nowarn_Y () +{ + // Same as above. + A *p = new (y) A; + delete p; + return new (y) A; +} + + +A* nowarn_align_X () +{ + // Same as above. + A *p = new (std::align_val_t (32), x) A; + delete p; + + return new (std::align_val_t (64), x) A; +} + + +A* nowarn_nothrow_Y () +{ + // Same as above. + A *p = new (std::nothrow, y) A; + delete p; + return new (std::nothrow, y) A; +} + --------------C48C3A20E3347A73B997727C--