Calling memset, memcpy, or similar to write to an object of
a non-trivial type (such as one that defines a ctor or dtor,
or has such a member) can break the invariants otherwise
maintained by the class and cause undefined behavior.
The motivating example that prompted this work was a review of
a change that added to a plain old struct a new member with a ctor
and dtor (in this instance the member was of type std::vector).
To help catch problems of this sort some projects (such as GDB)
have apparently even devised their own clever solutions to detect
them: https://sourceware.org/ml/gdb-patches/2017-04/msg00378.html.
The attached patch adds a new warning, -Wnon-trivial-memaccess,
that has GCC detect these mistakes. The patch also fixes up
a handful of instances of the problem in GCC. These instances
correspond to the two patterns below:
struct A
{
void *p;
void foo (int n) { p = malloc (n); }
~A () { free (p); }
};
void init (A *a)
{
memset (a, 0, sizeof *a);
}
and
struct B
{
int i;
~A ();
};
void copy (B *p, const B *q)
{
memcpy (p, q, sizeof *p);
...
}
These aren't undefined and the patch could be tweaked to allow
them. I decided not to invest effort into it because, although
not strictly erroneous, I think they represent poor practice.
The code would be more clearly (and more in the spirit of "good"
C++) written in terms of the default constructor and assignment
operator. The first one like so:
*a = A ();
and the second one like so:
*b = *q;
Martin
PR c++/80560 - warn on undefined memory operations involving non-trivial types
gcc/c-family/ChangeLog:
PR c++/80560
* c.opt (-Wnon-trivial-memaccess): New option.
gcc/cp/ChangeLog:
PR c++/80560
* call.c (maybe_warn_nontrivial_memaccess): New function.
(build_cxx_call): Call it.
gcc/ChangeLog:
PR c++/80560
* doc/invoke.texi (Wnon-trivial-memaccess): Document new option.
libitm/ChangeLog:
PR c++/80560
* beginend.cc (GTM::gtm_thread::rollback): Avoid calling memset
on an object of a non-trivial type.
libcpp/ChangeLog:
PR c++/80560
* line-map.c (line_maps::~line_maps): Avoid calling htab_delete
with a null pointer.
(linemap_init): Avoid calling memset on an object of a non-trivial
type.
gcc/testsuite/ChangeLog:
PR c++/80560
* g++.dg/Wnon-trivial-memaccess.C: New test.
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9ad2f6e..dae5e80 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -792,6 +792,10 @@ Wnon-template-friend
C++ ObjC++ Var(warn_nontemplate_friend) Init(1) Warning
Warn when non-templatized friend functions are declared within a template.
+Wnon-trivial-memaccess
+C++ ObjC++ Var(warn_nontrival_memaccess) Warning LangEnabledBy(C++ ObjC++, Wall)
+Warn for raw memory writes to objects of non-trivial types.
+
Wnon-virtual-dtor
C++ ObjC++ Var(warn_nonvdtor) Warning LangEnabledBy(C++ ObjC++,Weffc++)
Warn about non-virtual destructors.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index c15b8e4..8655b53 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8152,6 +8152,43 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
return call;
}
+/* Issue a warning on a call to the built-in function FNDECL if it is
+ a memory write whose destination is an object of a non-trivial type. */
+
+static void
+maybe_warn_nontrivial_memaccess (location_t loc, tree fndecl, tree dest)
+{
+ /* Warn about raw memory operations whose destination is an object
+ of a non-trivial type because they are undefined. */
+ bool memfunc = false;
+ switch (DECL_FUNCTION_CODE (fndecl))
+ {
+ case BUILT_IN_BZERO:
+ case BUILT_IN_MEMCPY:
+ case BUILT_IN_MEMMOVE:
+ case BUILT_IN_MEMPCPY:
+ case BUILT_IN_MEMSET:
+ memfunc = true;
+ break;
+
+ default:
+ break;
+ }
+
+ if (memfunc)
+ {
+ if (TREE_CODE (dest) == NOP_EXPR)
+ dest = TREE_OPERAND (dest, 0);
+
+ tree desttype = TREE_TYPE (TREE_TYPE (dest));
+
+ if (COMPLETE_TYPE_P (desttype) && !trivial_type_p (desttype))
+ warning_at (loc, OPT_Wnon_trivial_memaccess,
+ "calling %qD with a pointer to a non-trivial type %#qT",
+ fndecl, desttype);
+ }
+}
+
/* Build and return a call to FN, using NARGS arguments in ARGARRAY.
This function performs no overload resolution, conversion, or other
high-level operations. */
@@ -8184,6 +8221,9 @@ build_cxx_call (tree fn, int nargs, tree *argarray,
if (!check_builtin_function_arguments (EXPR_LOCATION (fn), vNULL, fndecl,
nargs, argarray))
return error_mark_node;
+
+ /* Warn if the built-in writes to an object of a non-trivial type. */
+ maybe_warn_nontrivial_memaccess (loc, fndecl, argarray[0]);
}
/* If it is a built-in array notation function, then the return type of
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 0eeea7b..e1d01a9 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -215,7 +215,8 @@ in the following sections.
-Wabi=@var{n} -Wabi-tag -Wconversion-null -Wctor-dtor-privacy @gol
-Wdelete-non-virtual-dtor -Wliteral-suffix -Wmultiple-inheritance @gol
-Wnamespaces -Wnarrowing @gol
--Wnoexcept -Wnoexcept-type -Wnon-virtual-dtor -Wreorder -Wregister @gol
+-Wnoexcept -Wnoexcept-type -Wnon-trivial-memaccess @gol
+-Wnon-virtual-dtor -Wreorder -Wregister @gol
-Weffc++ -Wstrict-null-sentinel -Wtemplates @gol
-Wno-non-template-friend -Wold-style-cast @gol
-Woverloaded-virtual -Wno-pmf-conversions @gol
@@ -2911,6 +2912,21 @@ void g() noexcept;
void h() @{ f(g); @} // in C++14 calls f<void(*)()>, in C++1z calls f<void(*)()noexcept>
@end smallexample
+@item -Wnon-trivial-memaccess @r{(C++ and Objective-C++ only)}
+@opindex Wnon-trivial-memaccess
+Warn when the destination of a call to a raw memory function such as
+@code{memset} or @code{memcpy} is an object of a non-trivial class type.
+Modifying the representation of such an object may violate invariants
+maintained by member functions of the class.
+For example, the call to @code{memset} below is undefined becase it
+modifies a non-trivial class object and is, therefore, diagnosed.
+The safe way to either initialize or "reset" objects of non-trivial
+types is by using the appropriate constructor.
+@smallexample
+std::string str = "abc";
+memset (&str, 0, 3);
+@end smallexample
+The @option{-Wnon-trivial-memaccess} option is enabled by @option{-Wall}.
@item -Wnon-virtual-dtor @r{(C++ and Objective-C++ only)}
@opindex Wnon-virtual-dtor
diff --git a/gcc/testsuite/g++.dg/Wnon-trivial-memaccess.C b/gcc/testsuite/g++.dg/Wnon-trivial-memaccess.C
new file mode 100644
index 0000000..812a9eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wnon-trivial-memaccess.C
@@ -0,0 +1,122 @@
+/* PR c++/80560 - warn on undefined memory operations involving non-trivial
+ types
+ { dg-do compile }
+ { dg-options "-Wnon-trivial-memaccess" } */
+
+struct Trivial { int i; char *s; char a[4]; };
+struct HasDefaultCtor { HasDefaultCtor (); };
+struct HasCopyCtor { HasCopyCtor (); };
+struct HasDtor { HasDtor (); };
+struct HasAssign { void operator= (HasAssign&); };
+
+typedef __SIZE_TYPE__ size_t;
+
+extern "C" {
+ void bzero (void*, size_t);
+ void* memcpy (void*, const void*, size_t);
+ void* memmove (void*, const void*, size_t);
+ void* mempcpy (void*, const void*, size_t);
+ void* memset (void*, int, size_t);
+}
+
+void sink (void*);
+
+#define T(fn, arglist) (fn arglist, sink (p))
+
+void test (Trivial *p, void *q)
+{
+ T (bzero, (p, sizeof *p));
+ T (bzero, (q, sizeof *p));
+
+ T (memcpy, (p, q, sizeof *p));
+ T (memcpy, (q, p, sizeof *p));
+
+ T (memset, (p, 0, sizeof *p));
+ T (memset, (q, 0, sizeof *p));
+
+ T (memmove, (p, q, sizeof *p));
+ T (memmove, (q, p, sizeof *p));
+}
+
+void test (HasDefaultCtor *p, const void *q)
+{
+ T (bzero, (p, sizeof *p)); // { dg-warning "calling .void bzero(\[^\n\r\]*). with a pointer to a non-trivial type 'struct HasDefaultCtor." }
+
+ T (memcpy, (p, q, sizeof *p)); // { dg-warning "calling .void\\* memcpy" }
+
+ T (memset, (p, 0, sizeof *p)); // { dg-warning "calling .void\\* memset" }
+
+ T (memmove, (p, q, sizeof *p)); // { dg-warning "calling .void\\* memmove" }
+
+ T (mempcpy, (p, q, sizeof *p)); // { dg-warning "calling .void\\* mempcpy" }
+}
+
+void test (HasCopyCtor *p, const void *q)
+{
+ T (bzero, (p, sizeof *p)); // { dg-warning "calling .void bzero" }
+
+ T (memcpy, (p, q, sizeof *p)); // { dg-warning "calling .void\\* memcpy" }
+
+ T (memset, (p, 0, sizeof *p)); // { dg-warning "calling .void\\* memset" }
+
+ T (memmove, (p, q, sizeof *p)); // { dg-warning "calling .void\\* memmove" }
+
+ T (mempcpy, (p, q, sizeof *p)); // { dg-warning "calling .void\\* mempcpy" }
+}
+
+void test (HasDtor *p, const void *q)
+{
+ T (bzero, (p, sizeof *p)); // { dg-warning "calling .void bzero" }
+
+ T (memcpy, (p, q, sizeof *p)); // { dg-warning "calling .void\\* memcpy" }
+
+ T (memset, (p, 0, sizeof *p)); // { dg-warning "calling .void\\* memset" }
+
+ T (memmove, (p, q, sizeof *p)); // { dg-warning "calling .void\\* memmove" }
+
+ T (mempcpy, (p, q, sizeof *p)); // { dg-warning "calling .void\\* mempcpy" }
+}
+
+void test (HasAssign *p, const void *q)
+{
+ T (bzero, (p, sizeof *p)); // { dg-warning "calling .void bzero" }
+
+ T (memcpy, (p, q, sizeof *p)); // { dg-warning "calling .void\\* memcpy" }
+
+ T (memset, (p, 0, sizeof *p)); // { dg-warning "calling .void\\* memset" }
+
+ T (memmove, (p, q, sizeof *p)); // { dg-warning "calling .void\\* memmove" }
+
+ T (mempcpy, (p, q, sizeof *p)); // { dg-warning "calling .void\\* mempcpy" }
+}
+
+
+void test_expr (int i)
+{
+ struct TestClass: HasDefaultCtor { };
+ TestClass a, b;
+
+ static void *p;
+
+ T (bzero, (i < 0 ? &a : &b, 1)); // { dg-warning "calling .void bzero" }
+}
+
+void test_this ()
+{
+#undef T
+#define T(fn, arglist) (fn arglist, sink (this))
+
+ static const void *p;
+
+ struct TestDefaultCtor: HasDefaultCtor
+ {
+ TestDefaultCtor ()
+ {
+ T (bzero, (this, sizeof *this)); // { dg-warning "calling .void bzero" }
+ T (memset, (this, 0, sizeof *this)); // { dg-warning "calling .void\\* memset" }
+ T (memcpy, (this, p, sizeof *this)); // { dg-warning "calling .void\\* memcpy" }
+ T (memmove, (this, p, sizeof *this)); // { dg-warning "calling .void\\* memmove" }
+ T (mempcpy, (this, p, sizeof *this)); // { dg-warning "calling .void\\* mempcpy" }
+ }
+ };
+}
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 949489e..4e36e38 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -62,7 +62,8 @@ extern unsigned num_macro_tokens_counter;
line_maps::~line_maps ()
{
- htab_delete (location_adhoc_data_map.htab);
+ if (location_adhoc_data_map.htab)
+ htab_delete (location_adhoc_data_map.htab);
}
/* Hash function for location_adhoc_data hashtable. */
@@ -347,7 +348,7 @@ void
linemap_init (struct line_maps *set,
source_location builtin_location)
{
- memset (set, 0, sizeof (struct line_maps));
+ *set = line_maps ();
set->highest_location = RESERVED_LOCATION_COUNT - 1;
set->highest_line = RESERVED_LOCATION_COUNT - 1;
set->location_adhoc_data_map.htab =
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index d04f3e9..c6550a3 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -431,7 +431,7 @@ GTM::gtm_transaction_cp::save(gtm_thread* tx)
// Save everything that we might have to restore on restarts or aborts.
jb = tx->jb;
undolog_size = tx->undolog.size();
- memcpy(&alloc_actions, &tx->alloc_actions, sizeof(alloc_actions));
+ alloc_actions = tx->alloc_actions;
user_actions_size = tx->user_actions.size();
id = tx->id;
prop = tx->prop;
@@ -449,7 +449,7 @@ GTM::gtm_transaction_cp::commit(gtm_thread* tx)
// commits of nested transactions. Allocation actions must be committed
// before committing the snapshot.
tx->jb = jb;
- memcpy(&tx->alloc_actions, &alloc_actions, sizeof(alloc_actions));
+ tx->alloc_actions = alloc_actions;
tx->id = id;
tx->prop = prop;
}
@@ -485,7 +485,7 @@ GTM::gtm_thread::rollback (gtm_transaction_cp *cp, bool aborting)
prop = cp->prop;
if (cp->disp != abi_disp())
set_abi_disp(cp->disp);
- memcpy(&alloc_actions, &cp->alloc_actions, sizeof(alloc_actions));
+ alloc_actions = cp->alloc_actions;
nesting = cp->nesting;
}
else
