From patchwork Wed Jun 2 21:40:49 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1486887 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=oFbnl5WB; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FwMsm4f0Lz9s1l for ; Thu, 3 Jun 2021 07:41:16 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8317A39730F7 for ; Wed, 2 Jun 2021 21:41:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8317A39730F7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1622670073; bh=bYZM/sw2ceE+vojrYq5AsA7PK1XPt/JgLIGmSMZfFz4=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=oFbnl5WB+ec5LSRErS0NFwJLK36K9LwSPRKDsXYXCUMNxikdFYb7E0RTuCPN/Xdvz XXIJ3WX038IG/CRhte+vjrrPSDusY5j/Qj7v9g4jW/T4qXAbPOfeI/Al/a9dbn1Y5E Nw9lFqCiIu/KSCvKiIP38mBdCyqBV706Ql56t/GE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-oi1-x22e.google.com (mail-oi1-x22e.google.com [IPv6:2607:f8b0:4864:20::22e]) by sourceware.org (Postfix) with ESMTPS id EC4DC3844072 for ; Wed, 2 Jun 2021 21:40:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EC4DC3844072 Received: by mail-oi1-x22e.google.com with SMTP id m137so263497oig.6 for ; Wed, 02 Jun 2021 14:40:51 -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=bYZM/sw2ceE+vojrYq5AsA7PK1XPt/JgLIGmSMZfFz4=; b=Fgrjp069sZ+El2z+jKqE1FXukEx1k40bxF9hH43UTqcKM0MPpvirH8kCHLJejOOvA2 QWfc+tIX4RwtIYngVkZpNWdEJClBEjtaJTOZ1zY9Jo7ug2cNfRbUPn63IKIrRuBl10Q9 AKq4NL+yVKN6aPwYJ4lkP1VtuanDN3tAotba5Dgz1KXwRFn4bisFuy67YCDMUjXO4b1u +CEaImdDo77yI9RjbUchdise9tzWS89kCDZaqf4f59pnm3S+2Pu02MXg3DfXqaq6IAcw GVNPhJwmHYTi5m/WwXZuJVHDDE9PD3qKxA2xH4pIDD8TfQQT+CE6wBJ/UNwRh8JFwsYQ Qw2A== X-Gm-Message-State: AOAM5315EPIO79AUEfYcZchX5p7j1tV9/MalctZ5dpJKB5G1G9/n/5LM 73ErKM6gJeToopuYrD3JOhdA4HVCGAA= X-Google-Smtp-Source: ABdhPJyBuat/FJVKw9lnqmo3Hzv40eps95eJ8mFcbcFzCgCYCshBBJoKQmAhXZZ6cqK/Rds3EsVIHQ== X-Received: by 2002:aca:1a05:: with SMTP id a5mr5127847oia.26.1622670051075; Wed, 02 Jun 2021 14:40:51 -0700 (PDT) Received: from [192.168.0.41] (184-96-226-253.hlrn.qwest.net. [184.96.226.253]) by smtp.gmail.com with ESMTPSA id i4sm274668oih.13.2021.06.02.14.40.50 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 02 Jun 2021 14:40:50 -0700 (PDT) To: gcc-patches Subject: [PATCH] teach compute_objsize about placement new (PR 100876) Message-ID: <6ccb6e6b-61b4-822b-d291-d2712192758e@gmail.com> Date: Wed, 2 Jun 2021 15:40:49 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 Content-Language: en-US X-Spam-Status: No, score=-10.0 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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" The two forms of placement operator new defined in return their pointer argument and may not be displaced by user-defined functions. But because they are ordinary (not built-in) functions this property isn't reflected in their declarations alone, and there's no user- level attribute to annotate them with. When they are inlined the property is transparent in the IL but when they are not (without inlining such as -O0), calls to the operators appear in the IL and cause -Wmismatched-new-delete to try to match them with the functions called to deallocate memory. When the pointer to the memory was obtained from a function that matches the deallocator but not the placement new, the warning falsely triggers. The attached patch solves this by detecting calls to placement new and treating them the same as those to other pass-through calls (such as memset). In addition, it also teaches -Wfree-nonheap-object about placement delete, for a similar reason as above. Finally, it also adds a test for attribute fn spec indicating a function returns its argument. It's not necessary for the fix (I had initially though placement new might have the attribute) but it seems appropriate to check. Tested on x86_64-linux. Martin PR c++/100876 - -Wmismatched-new-delete should understand placement new when it's not inlined gcc/ChangeLog: PR c++/100876 * builtins.c (gimple_call_return_array): Check for attribute fn spec. Handle calls to placement new. (ndecl_dealloc_argno): Avoid placement delete. gcc/testsuite/ChangeLog: PR c++/100876 * g++.dg/warn/Wmismatched-new-delete-4.C: New test. * g++.dg/warn/Wmismatched-new-delete-5.C: New test. * g++.dg/warn/Wstringop-overflow-7.C: New test. * g++.dg/warn/Wfree-nonheap-object-6.C: New test. * g++.dg/analyzer/placement-new.C: Prune out expected warning. diff --git a/gcc/builtins.c b/gcc/builtins.c index af1fe49bb48..fb0717a0248 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -5159,11 +5159,43 @@ static tree gimple_call_return_array (gimple *stmt, offset_int offrng[2], range_query *rvals) { - if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL) - || gimple_call_num_args (stmt) < 1) + { + /* Check for attribute fn spec to see if the function returns one + of its arguments. */ + attr_fnspec fnspec = gimple_call_fnspec (as_a (stmt)); + unsigned int argno; + if (fnspec.returns_arg (&argno)) + { + offrng[0] = offrng[1] = 0; + return gimple_call_arg (stmt, argno); + } + } + + if (gimple_call_num_args (stmt) < 1) return NULL_TREE; tree fn = gimple_call_fndecl (stmt); + if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) + { + /* See if this is a call to placement new. */ + if (!fn + || !DECL_IS_OPERATOR_NEW_P (fn) + || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fn)) + return NULL_TREE; + + tree fname = DECL_ASSEMBLER_NAME (fn); + const char *name = IDENTIFIER_POINTER (fname); + if (strcmp (name, "_ZnwmPv") // ordinary form + && strcmp (name, "_ZnamPv")) // array form + return NULL_TREE; + + if (gimple_call_num_args (stmt) != 2) + return NULL_TREE; + + offrng[0] = offrng[1] = 0; + return gimple_call_arg (stmt, 1); + } + switch (DECL_FUNCTION_CODE (fn)) { case BUILT_IN_MEMCPY: @@ -13285,7 +13317,18 @@ fndecl_dealloc_argno (tree fndecl) { /* A call to operator delete isn't recognized as one to a built-in. */ if (DECL_IS_OPERATOR_DELETE_P (fndecl)) - return 0; + { + if (DECL_IS_REPLACEABLE_OPERATOR (fndecl)) + return 0; + + /* Avoid placement delete that's not been inlined. */ + tree fname = DECL_ASSEMBLER_NAME (fndecl); + const char *name = IDENTIFIER_POINTER (fname); + if (strcmp (name, "_ZdlPvS_") == 0 // ordinary form + || strcmp (name, "_ZdaPvS_") == 0) // array form + return UINT_MAX; + return 0; + } /* TODO: Handle user-defined functions with attribute malloc? Handle known non-built-ins like fopen? */ diff --git a/gcc/testsuite/g++.dg/analyzer/placement-new.C b/gcc/testsuite/g++.dg/analyzer/placement-new.C index 8250f45b9d9..b648a428247 100644 --- a/gcc/testsuite/g++.dg/analyzer/placement-new.C +++ b/gcc/testsuite/g++.dg/analyzer/placement-new.C @@ -24,3 +24,5 @@ void test_3 (void) int *p = new(buf) int (42); delete p; // { dg-warning "memory not on the heap" } } + +// { dg-prune-output "-Wfree-nonheap-object" } diff --git a/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object-6.C b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object-6.C new file mode 100644 index 00000000000..83b6ff9157c --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object-6.C @@ -0,0 +1,45 @@ +/* { dg-do compile } + { dg-options "-O0 -Wall" } */ + +#if __cplusplus < 201103L +# define noexcept throw () +#endif + +void* operator new (__SIZE_TYPE__, void* __p) noexcept; +void operator delete (void*, void*); + +void* operator new[] (__SIZE_TYPE__, void* __p) noexcept; +void operator delete[] (void*, void*) noexcept; + +struct A { A (); ~A (); int i; }; + +extern void *p; + +void nowarn_placement_new () +{ + char a[sizeof (A)]; + p = new (a) A (); // { dg-bogus "-Wfree-nonheap-object" } +} + + +void warn_placement_new () +{ + char a[sizeof (A)]; + p = new (a + 1) A (); // { dg-warning "\\\[-Wplacement-new" } + // { dg-bogus "-Wfree-nonheap-object" "bogus" { target *-*-* } .-1 } +} + + +void nowarn_placement_new_array () +{ + char a[sizeof (A)]; + p = new (a) A[1]; // { dg-bogus "-Wfree-nonheap-object" } +} + + +void warn_placement_new_array () +{ + char a[sizeof (A)]; + p = new (a + 1) A[1]; // { dg-warning "\\\[-Wplacement-new" } + // { dg-bogus "-Wfree-nonheap-object" "bogus" { target *-*-* } .-1 } +} diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-4.C b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-4.C new file mode 100644 index 00000000000..4320181e4d7 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-4.C @@ -0,0 +1,37 @@ +/* PR c++/100876 - -Wmismatched-new-delete should either look through + or ignore placement new + { dg-do compile } + { dg-options "-O0 -Wall" } */ + +extern "C" { + void* malloc (__SIZE_TYPE__); + void free (void*); +} + +void* operator new (__SIZE_TYPE__, void*); +void* operator new[] (__SIZE_TYPE__, void*); + +void nowarn_placement_new () +{ + free (new (malloc (sizeof (int))) int ()); // { dg-bogus "-Wmismatched-new-delete" } +} + +void nowarn_placement_array_new () +{ + free (new (malloc (sizeof (int) * 2)) int[2]); // { dg-bogus "-Wmismatched-new-delete" } +} + + +void warn_placement_new () +{ + void *p = malloc (sizeof (int)); + int *q = new (p) int (); + delete q; // { dg-warning "-Wmismatched-new-delete" } +} + +void warn_placement_array_new () +{ + void *p = malloc (sizeof (int)); + int *q = new (p) int[2]; + delete q; // { dg-warning "-Wmismatched-new-delete" } +} diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-5.C b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-5.C new file mode 100644 index 00000000000..92c75df40d9 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-5.C @@ -0,0 +1,37 @@ +/* PR c++/100876 - -Wmismatched-new-delete should either look through + or ignore placement new + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +extern "C" { + void* malloc (__SIZE_TYPE__); + void free (void*); +} + +inline void* operator new (__SIZE_TYPE__, void *p) { return p; } +inline void* operator new[] (__SIZE_TYPE__, void *p) { return p; } + +void nowarn_placement_new () +{ + free (new (malloc (sizeof (int))) int ()); // { dg-bogus "-Wmismatched-new-delete" } +} + +void nowarn_placement_array_new () +{ + free (new (malloc (sizeof (int) * 2)) int[2]); // { dg-bogus "-Wmismatched-new-delete" } +} + + +void warn_placement_new () +{ + void *p = malloc (sizeof (int)); + int *q = new (p) int (); + delete q; // { dg-warning "-Wmismatched-new-delete" } +} + +void warn_placement_array_new () +{ + void *p = malloc (sizeof (int)); + int *q = new (p) int[2]; + delete q; // { dg-warning "-Wmismatched-new-delete" } +} diff --git a/gcc/testsuite/g++.dg/warn/Wstringop-overflow-7.C b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-7.C new file mode 100644 index 00000000000..d3d28f43d0a --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-7.C @@ -0,0 +1,42 @@ +/* PR c++/100876 - -Wmismatched-new-delete should either look through + or ignore placement new + { dg-do compile } + { dg-options "-O0 -Wall -Wno-array-bounds" } */ + +inline void* operator new (__SIZE_TYPE__, void *p) { return p; } +inline void* operator new[] (__SIZE_TYPE__, void *p) { return p; } + +void* nowarn_placement_new_memset () +{ + struct S { int i; }; + void *p = __builtin_malloc (sizeof (S)); + S *q = new (p) S; + __builtin_memset (q, 0, sizeof (S)); + return q; +} + +void* warn_placement_new_memset () +{ + struct S { int i; }; + void *p = __builtin_malloc (sizeof (S)); + S *q = new (p) S; + __builtin_memset (q, 0, sizeof (S) + 1); // { dg-warning "\\\[-Wstringop-overflow" } + return q; +} + +void* nowarn_placement_new_array_strncpy (const char *s) +{ + void *p = __builtin_malloc (5); + char *q = new (p) char[5]; + __builtin_strncpy (q, s, 5); + return q; + +} + +void* warn_placement_new_array_strncpy (const char *s) +{ + void *p = __builtin_malloc (4); + char *q = new (p) char[5]; + __builtin_strncpy (q, s, 5); // { dg-warning "\\\[-Wstringop-overflow" } + return q; +}