From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11489 invoked by alias); 23 Nov 2017 17:31:58 -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 11480 invoked by uid 89); 23 Nov 2017 17:31:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Nov 2017 17:31:56 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 55BB93D97F; Thu, 23 Nov 2017 17:31:55 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7A35018233; Thu, 23 Nov 2017 17:31:54 +0000 (UTC) Subject: Re: [PATCH 4/4] Poison XNEW and friends for types that should use new/delete To: Simon Marchi References: <1511368867-19365-1-git-send-email-simon.marchi@ericsson.com> <1511368867-19365-5-git-send-email-simon.marchi@ericsson.com> Cc: Simon Marchi , gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Thu, 23 Nov 2017 17:31:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-11/txt/msg00567.txt.bz2 On 11/23/2017 05:27 PM, Simon Marchi wrote: > On 2017-11-23 10:02, Pedro Alves wrote: >> On 11/22/2017 04:41 PM, Simon Marchi wrote: >>> From: Simon Marchi >> The criterion I >>> initially used to allow a type to use XNEW (which calls malloc in the >>> end) was std::is_trivially_constructible, but then realized that gcc 4.8 >>> did not have it. Instead, I went with: >>> >> >> Yeah, in the memset poisoning I just disabled the poisoning on older >> compilers. This is the "#if HAVE_IS_TRIVIALLY_COPYABLE" check in >> poison.h. > > I'm not sure if you are suggesting adding > HAVE_IS_TRIVIALLY_CONSTRUCTIBLE, but since we can use std::is_pod > easily, I think it's simpler just to use that (even if it's a bit more > strict than needed). Nope, I was really just stating a fact. >>> which is just a bit more strict, which doesn't hurt. A similar thing is >>> done for macros that free instead of allocated, the criterion is: >>> >>> using IsFreeable = gdb::Or, >>> std::is_void>; >>> >>> For simplicity, we could also do for std::is_pod for IsFreeable as well, >>> if you prefer. >>> >>> I chose to put static_assert in the functions, instead of using >>> gdb::Requires in the template as SFINAE, because it allows to put a >>> message, which I think makes the compiler error more understandable. >>> With gdb::Requires, the error is: >> >> Right, I think static_assert is what I had suggest initially too. >> SFINAE alone wouldn't sound right to me here. >> >> The right alternative to static_assert would be SFINAE+"=delete;", >> which is what we do for the memcpy/memcpy poisoning, and what >> you're doing to "free". >> >> =delete keeps the function/overload in the overload set, serving >> as "honeypot", so you get an error about trying to call a >> deleted function. >> >> You could do that to xfree instead >> of the static_assert, but I guess inlining xfree ends up being >> a good thing. > > Ah yeah, it would probably make the message a bit clearer. Instead of > saying "I can't find a function that matches", it would say "I found a > function that matches, but you can't use it". Exactly. > I'll just keep > static_assert for xfree as well, since it ends up clearer anyway. *nod* >> Other than the formatting, this LGTM. Thanks a lot for doing this. > > Thanks, I'm glad that I won't have to carry that branch on the side > anymore :) > :-) Thanks, Pedro Alves