From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57147 invoked by alias); 30 Apr 2017 01:51:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 56854 invoked by uid 89); 30 Apr 2017 01:51:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=smell, Mix, bulletproof, sneak X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 30 Apr 2017 01:51:43 +0000 Received: by simark.ca (Postfix, from userid 33) id 81F0C1E4C1; Sat, 29 Apr 2017 21:51:44 -0400 (EDT) To: Pedro Alves Subject: Re: [PATCH 1/5] Poison non-POD memset & non-trivially-copyable memcpy/memmove X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 30 Apr 2017 01:51:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: References: <1492050475-9238-1-git-send-email-palves@redhat.com> <1492050475-9238-2-git-send-email-palves@redhat.com> <9ee5551a7999a72a0040f15e6e5410a1@polymtl.ca> Message-ID: <9c15c7f2d50cdb53f39719dba3eb589e@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00822.txt.bz2 On 2017-04-27 09:58, Pedro Alves wrote: > On 04/24/2017 02:53 AM, Simon Marchi wrote: >> Actually, it should probably use std::is_trivially_constructible. >> And I >> suppose we could do the same with xfree, delete it when >> !std::is_trivially_destructible. > > > I think you wanted std::is_trivially_default_constructible > for XNEW. From what I understand, using is_trivially_default_constructible is the same as is_trivially_constructible. We can of course use is_trivially_default_constructible if it's clearer. > I think that we want _both_ conditions (*constructible > and *destructible) on both XNEW and xfree. For example, it'll be > good to catch the mismatching new/delete that could sneak in otherwise: That seems reasonnable. We want to "upgrade" to new and delete in a lock step anyway. > // type with trivial constructor > struct A > { > // A() = default; > ~A() { /* do something with side effects */ } // not trivial > }; > > // type with trivial destructor > struct B > { > B() { /* do something with side effects */ } // not trivial > //~B() = default; > }; > > void foo () > { > A *a = XNEW (struct A); > delete a; > B *b = new B; > xfree (b); > } > > Calling delete on a pointer not allocated with new is undefined > behavior. > These mismatches are also flagged by -fsanitize=address, but > making them compile-time errors would be even better. > > This wouldn't catch allocating types that are both trivially > default constructible and trivially destructible, and which _also_ > have non-default ctors, like this, for example: > > struct C > { > C() = default; > explicit C(int) { /* some side effects */ } > }; > > static_assert (std::is_trivially_default_constructible::value, ""); > static_assert (std::is_trivially_destructible::value, ""); > > C *b = new C(1); > xfree (b); // whoops, technically undefined. -fsanitify=address > likely complains. > > but std::is_pod wouldn't either. > > If we make a type non-standard-layout, then it no longer is POD: > > struct D > { > // Mix of public/private fields => not POD > public: > int a; > private: > int b; > }; > > This (D) case is likely to not really be problematic in practice WRT > to allocation/deallocation with malloc/free, but it still feels > like a code smell to me. I'd be willing to try forcing use > of new/delete for these types too. This would suggest using the > bigger std::is_pod hammer in XNEW/xfree instead of just > std::is_trivially_*ctible. But I'd understand if others disagree. I think it would be a good guideline to use new/delete for types that have some C++-related stuff in them, even if it's not technically necessary. Note that this won't be bulletproof also because at many places xfree is used on a void pointer, so we don't know what we're really free'ing. In some other cases, objects are freed using a pointer to their "C base class". Simon