From patchwork Tue Mar 20 02:06:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 887970 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-474980-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="JzGs7GsZ"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 404xB313Wrz9sVQ for ; Tue, 20 Mar 2018 13:06:17 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=DcL32CsUMsjDWVLApQhm1MpOgDrLzMQLpYyc/ayilL/eISxlJj akrt6zBVwXemxjoMC9SPqGQK4Y5iz6WE2wNa/V4iq9Bnu1CvtrH3Jfr2G7netf+C BzoHKiZnx32E8xKBSfBIqtyPEHaJnlnBcTADFUGeFHEun0h4PIz5K30Zs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=8mb+ljFWB1OiSoRthCDQheqV1u8=; b=JzGs7GsZSZbZVn0Gcbns rlgQHzQ2rDm1/gIQnO0qkGoVNMHuHqNn9esKutNaUAzI+J/pYoEvv7e6yX27tFFu JGoqUZOiFswvltKzXi7mRTYJcGbM5mH+N5NjSQw3KwrEPgW9SxW29jYvASBdOMpM ob1n8w9HpB9vM8gEIpxr6CU= Received: (qmail 44784 invoked by alias); 20 Mar 2018 02:06:09 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 43508 invoked by uid 89); 20 Mar 2018 02:06:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=H*Ad:U*msebor, relaxes, H*r:sk:40.2018 X-HELO: mail-ot0-f180.google.com Received: from mail-ot0-f180.google.com (HELO mail-ot0-f180.google.com) (74.125.82.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Mar 2018 02:06:06 +0000 Received: by mail-ot0-f180.google.com with SMTP id y11-v6so92390otg.0 for ; Mon, 19 Mar 2018 19:06:05 -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; bh=n/BXX4NP6c/dOwr+OeT1O4LNcCJVG3eKaEIgVrPG1JQ=; b=IHWNLsTAlwd97gjWDHHerqj9NWkmzL24JnEN1Gb03uqq44nKW5DZoyzct7Iy9XsvAN 6eF3kU7RdTSq+4mcENWiFxu3rSEWIifTFagQAsuJDrnHeE4ayMVcS9YbO37kqT4D4xRD Pv/BNC33NlJaIjTMTNj8tdn1GvU/djID9rHYKmUDdsiSKY4979xkfDLUwvP6NgImvGzE J/r/WDw2kpmJnlIp5gMgwoPgGXfrHWEqfHwPsKE0gC+hfWRo5L6mhBHrFkWCBIE+ZRTg uxHf2FiE7bu/aqu+Z114jYvDGhHkTXgG6cJlTo9jPidFuywfbc97THCRvfHqBubDsJO6 xZFA== X-Gm-Message-State: AElRT7FsjD4BovF9/L8gYWjHqhvAllTacZv6kYT75gDZW9RgA/iLq203 8MQWugP8nqQ4+nSO0eULq8s= X-Google-Smtp-Source: AG47ELu/nZg19lXqTcUQkh+pcneENMoE9gxBQmtXqQ68ZHcx4n5eAu+TkD/zWcGa3J9QaMMgU7dl4Q== X-Received: by 2002:a9d:1465:: with SMTP id h92-v6mr9654665oth.301.1521511564160; Mon, 19 Mar 2018 19:06:04 -0700 (PDT) Received: from localhost.localdomain (97-118-127-195.hlrn.qwest.net. [97.118.127.195]) by smtp.gmail.com with ESMTPSA id s143sm424494ois.40.2018.03.19.19.06.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 19 Mar 2018 19:06:02 -0700 (PDT) To: Gcc Patch List , Jason Merrill From: Martin Sebor Subject: [PATCH] relax -Wclass-memaccess for class members (PR 84850) Message-ID: <896452a4-b727-5e50-1a26-2647b5713aea@gmail.com> Date: Mon, 19 Mar 2018 20:06:00 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 X-IsSubscribed: yes The -Wclass-memaccess warning makes an exception for ctors and dtors of non-trivial classes with no bases to avoid triggering for uses of raw memory functions with this as the destination. All other members as well as non-members trigger the warning. In bug 84850 the reporter shows that some code (squid in this case) calls memset(this, 0. ...) in both the copy assignment operator and the copy ctor as a short-hand to clear all trivial members without having to explicitly enumerate them. A similar idiom has been seen in Firefox (some of the bugs linked from https://bugzilla.mozilla.org/show_bug.cgi?id=1411029). Since calling memset(this, 0, sizeof *this) from any non-static member of a non-trivial class containing no non-trivial members is safe, the attached patch relaxes the warning to allow this idiom. I also noticed that the exemption for ctors and dtors is overly permissive and causes the warning not to trigger for classes with no bases or virtual functions but containing non-trivial members. This is bug 84851. Jakub suggested to fix just 84850 for GCC 8 and defer 84851 to GCC 9, so the patch follows that suggestion. Fixing the latter is a matter of removing the block in warn_for_restrict() with the FIXME comment above it. Martin PR c++/84850 - -Wclass-memaccess on a memcpy in a copy assignment operator with no nontrivial bases or members gcc/cp/ChangeLog: PR c++/84850 * call.c (first_non_public_field): New template and function. (first_non_trivial_field): New function. (maybe_warn_class_memaccess): Call them. gcc/testsuite/ChangeLog: PR c++/84850 * g++.dg/Wclass-memaccess-3.C: New test. * g++.dg/Wclass-memaccess-4.C: New test. Index: gcc/cp/call.c =================================================================== --- gcc/cp/call.c (revision 258646) +++ gcc/cp/call.c (working copy) @@ -8261,13 +8261,17 @@ build_over_call (struct z_candidate *cand, int fla return call; } -/* Return the DECL of the first non-public data member of class TYPE - or null if none can be found. */ +namespace +{ -static tree -first_non_public_field (tree type) +/* Return the DECL of the first non-static subobject of class TYPE + that satisfies the predicate PRED or null if none can be found. */ + +template +tree +first_non_static_field (tree type, Predicate pred) { - if (!CLASS_TYPE_P (type)) + if (!type || !CLASS_TYPE_P (type)) return NULL_TREE; for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) @@ -8276,7 +8280,7 @@ build_over_call (struct z_candidate *cand, int fla continue; if (TREE_STATIC (field)) continue; - if (TREE_PRIVATE (field) || TREE_PROTECTED (field)) + if (pred (field)) return field; } @@ -8286,8 +8290,9 @@ build_over_call (struct z_candidate *cand, int fla BINFO_BASE_ITERATE (binfo, i, base_binfo); i++) { tree base = TREE_TYPE (base_binfo); - - if (tree field = first_non_public_field (base)) + if (pred (base)) + return base; + if (tree field = first_non_static_field (base, pred)) return field; } @@ -8294,6 +8299,42 @@ build_over_call (struct z_candidate *cand, int fla return NULL_TREE; } +struct NonPublicField +{ + bool operator() (const_tree t) + { + return DECL_P (t) && (TREE_PRIVATE (t) || TREE_PROTECTED (t)); + } +}; + +/* Return the DECL of the first non-public subobject of class TYPE + or null if none can be found. */ + +static inline tree +first_non_public_field (tree type) +{ + return first_non_static_field (type, NonPublicField ()); +} + +struct NonTrivialField +{ + bool operator() (const_tree t) + { + return !trivial_type_p (DECL_P (t) ? TREE_TYPE (t) : t); + } +}; + +/* Return the DECL of the first non-trivial subobject of class TYPE + or null if none can be found. */ + +static inline tree +first_non_trivial_field (tree type) +{ + return first_non_static_field (type, NonTrivialField ()); +} + +} /* unnamed namespace */ + /* Return true if all copy and move assignment operator overloads for class TYPE are trivial and at least one of them is not deleted and, when ACCESS is set, accessible. Return false otherwise. Set @@ -8419,23 +8460,31 @@ maybe_warn_class_memaccess (location_t loc, tree f if (!desttype || !COMPLETE_TYPE_P (desttype) || !CLASS_TYPE_P (desttype)) return; - /* Check to see if the raw memory call is made by a ctor or dtor - with this as the destination argument for the destination type. - If so, be more permissive. */ + /* Check to see if the raw memory call is made by a non-static member + function with THIS as the destination argument for the destination + type. If so, and if the class has no non-trivial bases or members, + be more permissive. */ if (current_function_decl - && (DECL_CONSTRUCTOR_P (current_function_decl) - || DECL_DESTRUCTOR_P (current_function_decl)) + && DECL_NONSTATIC_MEMBER_FUNCTION_P (current_function_decl) && is_this_parameter (tree_strip_nop_conversions (dest))) { tree ctx = DECL_CONTEXT (current_function_decl); bool special = same_type_ignoring_top_level_qualifiers_p (ctx, desttype); - tree binfo = TYPE_BINFO (ctx); - /* A ctor and dtor for a class with no bases and no virtual functions - can do whatever they want. Bail early with no further checking. */ - if (special && !BINFO_VTABLE (binfo) && !BINFO_N_BASE_BINFOS (binfo)) + /* FIXME: The following if statement is overly permissive (see + bug 84851). Remove it in GCC 9. */ + if (special + && !BINFO_VTABLE (binfo) + && !BINFO_N_BASE_BINFOS (binfo) + && (DECL_CONSTRUCTOR_P (current_function_decl) + || DECL_DESTRUCTOR_P (current_function_decl))) return; + + if (special + && !BINFO_VTABLE (binfo) + && !first_non_trivial_field (desttype)) + return; } /* True if the class is trivial. */ Index: gcc/testsuite/g++.dg/Wclass-memaccess-3.C =================================================================== --- gcc/testsuite/g++.dg/Wclass-memaccess-3.C (nonexistent) +++ gcc/testsuite/g++.dg/Wclass-memaccess-3.C (working copy) @@ -0,0 +1,287 @@ +/* PR c++/84850 - -Wclass-memaccess on a memcpy in a copy assignment + operator with no nontrivial bases or members + { dg-do compile } + { dg-options "-Wclass-memaccess -ftrack-macro-expansion=0" } */ + +typedef __SIZE_TYPE__ size_t; + +extern "C" void* memcpy (void*, const void*, size_t); +extern "C" void* memset (void*, int, size_t); + +template +struct EmptyClass { }; + +template +struct TrivialClass +{ + bool a; + int b; + void *c; + double d[2]; + void (*e)(); +}; + +template +struct HasDefault +{ + HasDefault (); +}; + +/* Verify that raw memory accesses from non-static members of a class with + an empty base is not diagnosed. */ + +struct EmptyWithBase: EmptyClass<0>, EmptyClass<1>, EmptyClass<2> +{ + EmptyWithBase () + { + memset (this, 0, sizeof *this); + } + + EmptyWithBase (const EmptyWithBase &x) + { + memcpy (this, &x, sizeof *this); + } + + ~EmptyWithBase () + { + memset (this, 0, sizeof *this); + } + + void operator= (const EmptyWithBase &x) + { + memcpy (this, &x, sizeof *this); + } + + void clear () + { + memset (this, 0, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + void copy (const void *p) + { + memcpy (this, p, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + static void bad_clear (EmptyWithBase &x) + { + memset (&x, 0, sizeof x); // { dg-warning "\\\[-Wclass-memaccess" } + } + + static void bad_copy (EmptyWithBase &x, const void *p) + { + memcpy (&x, p, sizeof x); // { dg-warning "\\\[-Wclass-memaccess" } + } +}; + +/* Verify that raw memory accesses from non-static members of a class with + all trivial members is not diagnosed. */ + +struct HasTrivialMembers +{ + bool a; + int b; + void *c; + double d[2]; + void (*e)(); + + TrivialClass<1> trivial; + + HasTrivialMembers () + { + memset (this, 0, sizeof *this); + } + + HasTrivialMembers (const HasTrivialMembers &x) + { + memcpy (this, &x, sizeof *this); + } + + ~HasTrivialMembers () + { + memset (this, 0, sizeof *this); + } + + void operator= (const HasTrivialMembers &x) + { + memcpy (this, &x, sizeof *this); + } + + void clear () + { + memset (this, 0, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + void copy (const void *p) + { + memcpy (this, p, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + static void bad_clear (HasTrivialMembers &x) + { + memset (&x, 0, sizeof x); // { dg-warning "\\\[-Wclass-memaccess" } + } + + static void bad_copy (HasTrivialMembers &x, const void *p) + { + memcpy (&x, p, sizeof x); // { dg-warning "\\\[-Wclass-memaccess" } + } +}; + +/* Verify that raw memory accesses from non-static members of a class with + a trivial base class and no non-trivial members is not diagnosed. */ + +struct HasTrivialBase: TrivialClass<1> +{ + TrivialClass<2> a[2]; + + HasTrivialBase () + { + memset (this, 0, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + HasTrivialBase (const HasTrivialBase &x) + { + memcpy (this, &x, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + ~HasTrivialBase () + { + memset (this, 0, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + void operator= (const HasTrivialBase &x) + { + memcpy (this, &x, sizeof *this); + } + + void clear () + { + memset (this, 0, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + void copy (void *p) + { + memcpy (this, p, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } +}; + + +struct HasTrivialBases: TrivialClass<1>, TrivialClass<2> +{ + TrivialClass<3> a[2]; + + HasTrivialBases () + { + memset (this, 0, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + HasTrivialBases (const HasTrivialBases &x) + { + memcpy (this, &x, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + ~HasTrivialBases () + { + memset (this, 0, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + void operator= (const HasTrivialBases &x) + { + memcpy (this, &x, sizeof *this); + } + + void clear () + { + memset (this, 0, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } + + void copy (void *p) + { + memcpy (this, p, sizeof *this); // { dg-bogus "\\\[-Wclass-memaccess" } + } +}; + +struct DerivesFromNontrivialClass: HasDefault<1> { }; + +/* Verify that raw memory accesses from members of a class with a non-trivial + base class is diagnosed. */ + +struct HasNonTrivialBase: TrivialClass<1>, TrivialClass<2>, + DerivesFromNontrivialClass, + TrivialClass<3>, TrivialClass<4> +{ + HasNonTrivialBase () + { + memset (this, 0, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } + + HasNonTrivialBase (const HasNonTrivialBase &x) + { + memcpy (this, &x, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } + + ~HasNonTrivialBase () + { + memset (this, 0, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } + + HasNonTrivialBase& operator= (const HasNonTrivialBase &x) + { + memcpy (this, &x, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + return *this; + } + + void clear () + { + memset (this, 0, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } + + void copy (void *p) + { + memcpy (this, p, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } +}; + +struct DerivesIndidirectlyFromNontrivialClass: + TrivialClass<1>, TrivialClass<2>, + DerivesFromNontrivialClass, + TrivialClass<3>, TrivialClass<4> { }; + +/* Verify that raw memory accesses from members of a class with a non-trivial + indirect base class is diagnosed. */ + +struct HasIndirectNonTrivialBase: TrivialClass<5>, TrivialClass<6>, + TrivialClass<7>, TrivialClass<8>, + DerivesIndidirectlyFromNontrivialClass +{ + HasIndirectNonTrivialBase () + { + memset (this, 0, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } + + HasIndirectNonTrivialBase (const HasIndirectNonTrivialBase &x) + { + memcpy (this, &x, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } + + ~HasIndirectNonTrivialBase () + { + memset (this, 0, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } + + HasIndirectNonTrivialBase& operator= (const HasIndirectNonTrivialBase &x) + { + memcpy (this, &x, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + return *this; + } + + void clear () + { + memset (this, 0, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } + + void copy (void *p) + { + memcpy (this, p, sizeof *this); // { dg-warning "\\\[-Wclass-memaccess" } + } +}; Index: gcc/testsuite/g++.dg/Wclass-memaccess-4.C =================================================================== --- gcc/testsuite/g++.dg/Wclass-memaccess-4.C (nonexistent) +++ gcc/testsuite/g++.dg/Wclass-memaccess-4.C (working copy) @@ -0,0 +1,39 @@ +/* PR c++/84850 - missing -Wclass-memaccess for a memcpy in a copy ctor + with a non-trivial member + { dg-do compile } + { dg-options "-Wclass-memaccess -ftrack-macro-expansion=0" } */ + +typedef __SIZE_TYPE__ size_t; + +extern "C" void* memcpy (void*, const void*, size_t); + +struct A +{ + const int &r; + + A (); + + A (const A&); + + virtual ~A (); +}; + +struct C +{ + A a; + + C (const C&); + + C& operator= (const C&); +}; + +C::C (const C &c) +{ + memcpy (this, &c, sizeof c); // { dg-warning "\\\[-Wclass-memaccess]" "pr84851" { xfail *-*-*} } +} + +C& C::operator= (const C &c) +{ + memcpy (this, &c, sizeof c); // { dg-warning "\\\[-Wclass-memaccess]" } + return *this; +}