From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38171 invoked by alias); 25 Apr 2017 01:14:40 -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 38132 invoked by uid 89); 25 Apr 2017 01:14:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=Ie, I.e, UD:I.e, stolen X-HELO: mail-wm0-f52.google.com Received: from mail-wm0-f52.google.com (HELO mail-wm0-f52.google.com) (74.125.82.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 25 Apr 2017 01:14:33 +0000 Received: by mail-wm0-f52.google.com with SMTP id w64so10348103wma.0 for ; Mon, 24 Apr 2017 18:14:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=yYljMwlNeM4hCe/TzohmhHTmtAQL1yvBHYZ7b09itZU=; b=mMcvJpdjMwE+WJ64tdJhNPsQtj6KPyfKEGARJmTxdtse2/fpL6FqfN8R8V5YdwXJFN ngWjQ3jOM9fhh1s33kc02x67CgQS+f2tI/YzBi1pD5RK42/XkfFea/zz39zKrMfMZUoR rIX5OGWN7ztQTAbrcN9MGtiamP72SUtY3VnX86rEQskT8YUYrj+JlsiMF/qWGG+IANWw QJR+s8xxockUhkGyupNT8UaZ9YOFdthfIKWLFaA58HzJoRUJRw8L+R52I31YXHGAn0rp gvGSyKSNgDigWHcXZhlAkRuPcjbsPSMfNFyNqwVqYG+MtJwzqeveaztUOVzv+2B79QdY TlmA== X-Gm-Message-State: AN3rC/5NiRy3d62tK55B4W2uZ0c9V2UCZE8QWyg0fmQyO6aj9NB7HtZN AazfBRaCdqPV6xheAxWk3A== X-Received: by 10.28.26.149 with SMTP id a143mr10882498wma.18.1493082872792; Mon, 24 Apr 2017 18:14:32 -0700 (PDT) Received: from [192.168.0.102] ([37.189.166.198]) by smtp.gmail.com with ESMTPSA id p197sm1704903wmb.34.2017.04.24.18.14.31 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Apr 2017 18:14:31 -0700 (PDT) Subject: Re: [PATCH 1/5] Poison non-POD memset & non-trivially-copyable memcpy/memmove To: Simon Marchi References: <1492050475-9238-1-git-send-email-palves@redhat.com> <1492050475-9238-2-git-send-email-palves@redhat.com> <44b612a2b2dadc142c054a1967dc2600@polymtl.ca> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Tue, 25 Apr 2017 01:14: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: <44b612a2b2dadc142c054a1967dc2600@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2017-04/txt/msg00656.txt.bz2 On 04/20/2017 04:27 AM, Simon Marchi wrote: >> I'll move this to the end of the series before pushing (if agreed). (I've moved it to the end of the series before pushing, and removed that comment from the commit log.) > That's really nice. I'm actually surprised we didn't get random crashes > because of that yet! Yeah. >> + struct S { >> + S() { m_data.reserve (10); } >> + std::vector m_data; >> + }; > > Here it says struct S ... > >> + >> +and old code was initializing B objects like this: >> + >> + struct B b; >> + memset (&b, 0, sizeof (B)); // whoops, now wipes vector. > > ... and here struct B? Bah. These should of course be S, not B. Thanks for spotting this! Below's what I pushed. >From b0b92aeb3828219075fce23543fb39fee8608e99 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 25 Apr 2017 01:27:41 +0100 Subject: [PATCH] Poison non-POD memset & non-trivially-copyable memcpy/memmove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch catches invalid initialization of non-POD types with memset, at compile time. This is what I used to catch the problems fixed by the previous patches in the series: $ make -k 2>&1 | grep "deleted function" src/gdb/breakpoint.c:951:53: error: use of deleted function ‘void* memset(T*, int, size_t) [with T = bp_location; = void; size_t = long unsigned int]’ src/gdb/breakpoint.c:7325:32: error: use of deleted function ‘void* memset(T*, int, size_t) [with T = bp_location; = void; size_t = long unsigned int]’ src/gdb/btrace.c:1153:42: error: use of deleted function ‘void* memset(T*, int, size_t) [with T = btrace_insn; = void; size_t = long unsigned int]’ ... gdb/ChangeLog: 2017-04-25 Pedro Alves * common/common-defs.h: Include "common/poison.h". * common/function-view.h: (Not, Or, Requires): Move to traits.h and adjust. * common/poison.h: New file. * common/traits.h: Include . (Not, Or, Requires): New, moved from common/function-view.h. --- gdb/ChangeLog | 9 +++++ gdb/common/common-defs.h | 1 + gdb/common/function-view.h | 40 +++------------------- gdb/common/poison.h | 83 ++++++++++++++++++++++++++++++++++++++++++++++ gdb/common/traits.h | 30 ++++++++++++++++- 5 files changed, 126 insertions(+), 37 deletions(-) create mode 100644 gdb/common/poison.h diff --git a/gdb/ChangeLog b/gdb/ChangeLog index dc321a2..26e6370 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,14 @@ 2017-04-25 Pedro Alves + * common/common-defs.h: Include "common/poison.h". + * common/function-view.h: (Not, Or, Requires): Move to traits.h + and adjust. + * common/poison.h: New file. + * common/traits.h: Include . + (Not, Or, Requires): New, moved from common/function-view.h. + +2017-04-25 Pedro Alves + * breakpoint.h (struct breakpoint): In-class initialize all fields. Make boolean fields "bool". * breakpoint.c (init_raw_breakpoint_without_location): Remove diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h index af37111..439bce6 100644 --- a/gdb/common/common-defs.h +++ b/gdb/common/common-defs.h @@ -82,6 +82,7 @@ #include "common-debug.h" #include "cleanups.h" #include "common-exceptions.h" +#include "common/poison.h" #define EXTERN_C extern "C" #define EXTERN_C_PUSH extern "C" { diff --git a/gdb/common/function-view.h b/gdb/common/function-view.h index 66a691b..de9a634 100644 --- a/gdb/common/function-view.h +++ b/gdb/common/function-view.h @@ -153,34 +153,6 @@ namespace gdb { -namespace traits { - /* A few trait helpers. */ - template - struct Not : public std::integral_constant - {}; - - template - struct Or; - - template<> - struct Or<> : public std::false_type - {}; - - template - struct Or : public B1 - {}; - - template - struct Or - : public std::conditional::type - {}; - - template - struct Or - : public std::conditional>::type - {}; -} /* namespace traits */ - namespace fv_detail { /* Bits shared by all function_view instantiations that do not depend on the template parameters. */ @@ -209,9 +181,9 @@ class function_view { template using CompatibleReturnType - = traits::Or, - std::is_same, - std::is_convertible>; + = Or, + std::is_same, + std::is_convertible>; /* True if Func can be called with Args, and either the result is Res, convertible to Res or Res is void. */ @@ -227,10 +199,6 @@ class function_view : std::is_same::type> {}; - /* Helper to make SFINAE logic easier to read. */ - template - using Requires = typename std::enable_if::type; - public: /* NULL by default. */ @@ -248,7 +216,7 @@ class function_view compatible. */ template >>, + typename = Requires>>, typename = Requires>> function_view (Callable &&callable) noexcept { diff --git a/gdb/common/poison.h b/gdb/common/poison.h new file mode 100644 index 0000000..a875568 --- /dev/null +++ b/gdb/common/poison.h @@ -0,0 +1,83 @@ +/* Poison symbols at compile time. + + Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef COMMON_POISON_H +#define COMMON_POISON_H + +#include "traits.h" + +/* Poison memset of non-POD types. The idea is catching invalid + initialization of non-POD structs that is easy to be introduced as + side effect of refactoring. For example, say this: + + struct S { VEC(foo_s) *m_data; }; + +is converted to this at some point: + + struct S { + S() { m_data.reserve (10); } + std::vector m_data; + }; + +and old code was initializing S objects like this: + + struct S s; + memset (&s, 0, sizeof (S)); // whoops, now wipes vector. + +Declaring memset as deleted for non-POD types makes the memset above +be a compile-time error. */ + +/* Helper for SFINAE. True if "T *" is memsettable. I.e., if T is + either void, or POD. */ +template +struct IsMemsettable + : gdb::Or, + std::is_pod> +{}; + +template >>> +void *memset (T *s, int c, size_t n) = delete; + +/* Similarly, poison memcpy and memmove of non trivially-copyable + types, which is undefined. */ + +/* True if "T *" is relocatable. I.e., copyable with memcpy/memmove. + I.e., T is either trivially copyable, or void. */ +template +struct IsRelocatable + : gdb::Or, + std::is_trivially_copyable> +{}; + +/* True if both source and destination are relocatable. */ + +template +using BothAreRelocatable + = gdb::And, IsRelocatable>; + +template >>> +void *memcpy (D *dest, const S *src, size_t n) = delete; + +template >>> +void *memmove (D *dest, const S *src, size_t n) = delete; + +#endif /* COMMON_POISON_H */ diff --git a/gdb/common/traits.h b/gdb/common/traits.h index c8f29cc..4f7161b 100644 --- a/gdb/common/traits.h +++ b/gdb/common/traits.h @@ -32,7 +32,32 @@ template using void_t = typename make_void::type; /* A few trait helpers, mainly stolen from libstdc++. Uppercase - because "and" is a keyword. */ + because "and/or", etc. are reserved keywords. */ + +template +struct Not : public std::integral_constant +{}; + +template +struct Or; + +template<> +struct Or<> : public std::false_type +{}; + +template +struct Or : public B1 +{}; + +template +struct Or + : public std::conditional::type +{}; + +template +struct Or + : public std::conditional>::type +{}; template struct And; @@ -55,6 +80,9 @@ struct And : public std::conditional, B1>::type {}; +/* Concepts-light-like helper to make SFINAE logic easier to read. */ +template +using Requires = typename std::enable_if::type; } #endif /* COMMON_TRAITS_H */ -- 2.5.5