This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/5] Poison non-POD memset & non-trivially-copyable memcpy/memmove
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<foo> 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 <palves@redhat.com>
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; <template-parameter-1-2> = 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; <template-parameter-1-2> = 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; <template-parameter-1-2> = void; size_t = long unsigned int]’
...
gdb/ChangeLog:
2017-04-25 Pedro Alves <palves@redhat.com>
* 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 <type_traits>.
(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 <palves@redhat.com>
+ * 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 <type_traits>.
+ (Not, Or, Requires): New, moved from common/function-view.h.
+
+2017-04-25 Pedro Alves <palves@redhat.com>
+
* 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<typename Predicate>
- struct Not : public std::integral_constant<bool, !Predicate::value>
- {};
-
- template<typename...>
- struct Or;
-
- template<>
- struct Or<> : public std::false_type
- {};
-
- template<typename B1>
- struct Or<B1> : public B1
- {};
-
- template<typename B1, typename B2>
- struct Or<B1, B2>
- : public std::conditional<B1::value, B1, B2>::type
- {};
-
- template<typename B1,typename B2,typename B3, typename... Bn>
- struct Or<B1, B2, B3, Bn...>
- : public std::conditional<B1::value, B1, Or<B2, B3, Bn...>>::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<Res (Args...)>
{
template<typename From, typename To>
using CompatibleReturnType
- = traits::Or<std::is_void<To>,
- std::is_same<From, To>,
- std::is_convertible<From, To>>;
+ = Or<std::is_void<To>,
+ std::is_same<From, To>,
+ std::is_convertible<From, To>>;
/* 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<Res (Args...)>
: std::is_same<function_view, typename std::decay<Callable>::type>
{};
- /* Helper to make SFINAE logic easier to read. */
- template<typename Condition>
- using Requires = typename std::enable_if<Condition::value, void>::type;
-
public:
/* NULL by default. */
@@ -248,7 +216,7 @@ class function_view<Res (Args...)>
compatible. */
template
<typename Callable,
- typename = Requires<traits::Not<IsFunctionView<Callable>>>,
+ typename = Requires<Not<IsFunctionView<Callable>>>,
typename = Requires<IsCompatibleCallable<Callable>>>
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 <http://www.gnu.org/licenses/>. */
+
+#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<foo> 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<typename T>
+struct IsMemsettable
+ : gdb::Or<std::is_void<T>,
+ std::is_pod<T>>
+{};
+
+template <typename T,
+ typename = gdb::Requires<gdb::Not<IsMemsettable<T>>>>
+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<typename T>
+struct IsRelocatable
+ : gdb::Or<std::is_void<T>,
+ std::is_trivially_copyable<T>>
+{};
+
+/* True if both source and destination are relocatable. */
+
+template <typename D, typename S>
+using BothAreRelocatable
+ = gdb::And<IsRelocatable<D>, IsRelocatable<S>>;
+
+template <typename D, typename S,
+ typename = gdb::Requires<gdb::Not<BothAreRelocatable<D, S>>>>
+void *memcpy (D *dest, const S *src, size_t n) = delete;
+
+template <typename D, typename S,
+ typename = gdb::Requires<gdb::Not<BothAreRelocatable<D, S>>>>
+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<typename... Ts>
using void_t = typename make_void<Ts...>::type;
/* A few trait helpers, mainly stolen from libstdc++. Uppercase
- because "and" is a keyword. */
+ because "and/or", etc. are reserved keywords. */
+
+template<typename Predicate>
+struct Not : public std::integral_constant<bool, !Predicate::value>
+{};
+
+template<typename...>
+struct Or;
+
+template<>
+struct Or<> : public std::false_type
+{};
+
+template<typename B1>
+struct Or<B1> : public B1
+{};
+
+template<typename B1, typename B2>
+struct Or<B1, B2>
+ : public std::conditional<B1::value, B1, B2>::type
+{};
+
+template<typename B1,typename B2,typename B3, typename... Bn>
+struct Or<B1, B2, B3, Bn...>
+ : public std::conditional<B1::value, B1, Or<B2, B3, Bn...>>::type
+{};
template<typename...>
struct And;
@@ -55,6 +80,9 @@ struct And<B1, B2, B3, Bn...>
: public std::conditional<B1::value, And<B2, B3, Bn...>, B1>::type
{};
+/* Concepts-light-like helper to make SFINAE logic easier to read. */
+template<typename Condition>
+using Requires = typename std::enable_if<Condition::value, void>::type;
}
#endif /* COMMON_TRAITS_H */
--
2.5.5