From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23687 invoked by alias); 11 May 2017 16:23:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 23672 invoked by uid 89); 11 May 2017 16:23:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=mistakes, safer, challenge, guilty X-HELO: mail-io0-f173.google.com Received: from mail-io0-f173.google.com (HELO mail-io0-f173.google.com) (209.85.223.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 11 May 2017 16:23:48 +0000 Received: by mail-io0-f173.google.com with SMTP id k91so24757673ioi.1 for ; Thu, 11 May 2017 09:23:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:subject:to:references:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=KYoEEkRJ1Yy1JsImhD40lu72F9XOZZ9WArvhjc3bZKU=; b=HB3geK6q2iLUdlVpZlot1bk+jVIi6IBTr9QiIiFtWYG5KNoZzfxyWnm3gr+KM7Plxl Z23vig0+rx8xjJoxWqJXoDlzM1ojbUYnaTjdUhEEMNLjhoSoJG6Il7aClVi/2OQ6G2/g 8YPJliJ7ULIk9+aRPd7FHaLQNMIQl9dwmHAmaf+LiBIrslw0nGWeEWNj7DrY2Ng/v30i OVmAbaBI7c5gIDRjsvp9f7vJLduNrbQS5FbUCZBtkmS3UsQc8p6DPM+l34kR2OlOnmnI 3/8amBIwa/IAQxrOzl601QunwkaDAxP1gQxn6vuObEgiZVSrBuwSBoOnokBELkUhPInb kysg== X-Gm-Message-State: AODbwcBxzjyMtrFo9MyHzvuhtuLG/c41U14p5FWsdUTz3Hv1jgC8Vwrb XKa/FEyxmAxMAA== X-Received: by 10.107.18.85 with SMTP id a82mr1304472ioj.7.1494519829526; Thu, 11 May 2017 09:23:49 -0700 (PDT) Received: from localhost.localdomain (75-166-101-229.hlrn.qwest.net. [75.166.101.229]) by smtp.gmail.com with ESMTPSA id y125sm450497itb.4.2017.05.11.09.23.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 May 2017 09:23:49 -0700 (PDT) From: Martin Sebor Subject: Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560) To: Pedro Alves , Gcc Patch List , Jason Merrill References: <656ca1db-1082-b1ed-a911-ba7bf48f09c0@redhat.com> Message-ID: Date: Thu, 11 May 2017 16:34:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <656ca1db-1082-b1ed-a911-ba7bf48f09c0@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00926.txt.bz2 On 04/30/2017 02:02 PM, Pedro Alves wrote: > Hi Martin, > > Thanks much for doing this. A few comments below, in light of my > experience doing the equivalent checks in the gdb patch linked below, > using standard C++11. Thanks for the feedback! It gave me quite a bit to think about. The challenge with using memcpy or memset with class types is figuring out if it's being called to copy-construct a new object or assign a new value to an existing one. In general it's not possible to tell so the conservative assumption is that it's the latter. Because of that, relying on the trivially copyable property to determine whether it's safe to assign a new value to an object is not sufficient (or even necessary). The class must be at a minimum trivially assignable. But it turns out that even trivial assignability as defined by C++ isn't enough. A class with a const data member or a member of a reference type is considered "trivially assignable" but its copy assignment is deleted, and so it can't be assigned to. Calling memset or memcpy to write over an object of such a class can violate the const invariant or corrupt the reference, respectively. On the other hand, relying on the standard layout property to decide whether or not calling memset on an object is safe, while on the surface reasonable, is at the same time too strict and overly permissive. It's too strict because it warns for calls where the destination is an object of a trivial derived class that declares data members in one of its bases as well as in the derived class (GCC has code like that). It's not strict enough because it doesn't catch cases where the class contains only private or only protected data members (GCC is guilty of abusing memset to violate encapsulation like that as well). That said, I'm testing a solution that overcomes these problems. I adjusted it so it doesn't warn on the GDB code in your example (or any GDB code on trunk), even though in my opinion "tricks" like that would best be avoided in favor of safer alternatives. Unlike in C, the preferred way to initialize objects in C++ is to use some form of initialization (as opposed to memset). The preferred way to copy objects is using the copy ctor or assignment operator (as opposed to memcpy). Using the special member functions is clearer, less prone to mistakes and so safer, and in some cases can also be more efficient. Memcpy and memset should be reserved for manipulating raw storage, not typed objects. Martin