[C++ PATCH] -Wclass-memaccess fixes and improvements (PR c++/81327)
Jakub Jelinek
jakub@redhat.com
Fri Jan 5 22:02:00 GMT 2018
Hi!
Apparently LLVM allows similar warning to -Wclass-memaccess (is it just
similar or same?; if the latter, perhaps we should use the same option for
that) to be disabled not just by casting the destination pointer to
e.g. (char *) or some other pointer to non-class, but also to (void *),
which is something that Qt uses or wants to use.
The problem is that the warning is diagnosed too late and whether we had
explicit or implicit conversion to void * is lost at that point.
This patch moves the warning tiny bit earlier (from build_cxx_call to the
caller) where we still have information about the original parsed arguments
before conversions to the builtin argument types.
In addition it fixes various small issues I run into when reading the code,
e.g. STRIP_NOPS was changing the argument in the argument array passed to
it, or e.g. the if (!warned && !warnfmt) return; was redundant with the
following code doing just
if (warnfmt)
something;
if (warned)
somethingelse;
return;
or not verifying the INTEGER_CSTs fit into uhwi before calling tree_to_uhwi
etc.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2018-01-05 Jakub Jelinek <jakub@redhat.com>
PR c++/81327
* call.c (maybe_warn_class_memaccess): Add forward declaration.
Change last argument from tree * to const vec<tree, va_gc> *, adjust
args uses and check number of operands too. Don't strip away any
nops. Use maybe_constant_value when looking for INTEGER_CST args.
Deal with src argument not having pointer type. Check
tree_fits_uhwi_p before calling tree_to_uhwi. Remove useless
test.
(build_over_call): Call maybe_warn_class_memaccess here on the
original arguments.
(build_cxx_call): Rather than here on converted arguments.
* g++.dg/Wclass-memaccess-2.C: Don't expect a warning when explicitly
cast to void *.
--- gcc/cp/call.c.jj 2018-01-04 00:43:16.109702768 +0100
+++ gcc/cp/call.c 2018-01-05 15:03:40.255312975 +0100
@@ -147,6 +147,8 @@ static int equal_functions (tree, tree);
static int joust (struct z_candidate *, struct z_candidate *, bool,
tsubst_flags_t);
static int compare_ics (conversion *, conversion *);
+static void maybe_warn_class_memaccess (location_t, tree,
+ const vec<tree, va_gc> *);
static tree build_over_call (struct z_candidate *, int, tsubst_flags_t);
#define convert_like(CONV, EXPR, COMPLAIN) \
convert_like_real ((CONV), (EXPR), NULL_TREE, 0, \
@@ -8180,6 +8182,12 @@ build_over_call (struct z_candidate *can
&& !mark_used (fn, complain))
return error_mark_node;
+ /* Warn if the built-in writes to an object of a non-trivial type. */
+ if (warn_class_memaccess
+ && vec_safe_length (args) >= 2
+ && DECL_BUILT_IN_CLASS (fn) == BUILT_IN_NORMAL)
+ maybe_warn_class_memaccess (input_location, fn, args);
+
if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0
/* Don't mess with virtual lookup in instantiate_non_dependent_expr;
virtual functions can't be constexpr. */
@@ -8360,7 +8368,8 @@ has_trivial_copy_p (tree type, bool acce
assignments. */
static void
-maybe_warn_class_memaccess (location_t loc, tree fndecl, tree *args)
+maybe_warn_class_memaccess (location_t loc, tree fndecl,
+ const vec<tree, va_gc> *args)
{
/* Except for bcopy where it's second, the destination pointer is
the first argument for all functions handled here. Compute
@@ -8368,15 +8377,10 @@ maybe_warn_class_memaccess (location_t l
unsigned dstidx = DECL_FUNCTION_CODE (fndecl) == BUILT_IN_BCOPY;
unsigned srcidx = !dstidx;
- tree dest = args[dstidx];
- if (!dest || !TREE_TYPE (dest) || !POINTER_TYPE_P (TREE_TYPE (dest)))
+ tree dest = (*args)[dstidx];
+ if (!TREE_TYPE (dest) || !POINTER_TYPE_P (TREE_TYPE (dest)))
return;
- /* Remove the outermost (usually implicit) conversion to the void*
- argument type. */
- if (TREE_CODE (dest) == NOP_EXPR)
- dest = TREE_OPERAND (dest, 0);
-
tree srctype = NULL_TREE;
/* Determine the type of the pointed-to object and whether it's
@@ -8436,7 +8440,7 @@ maybe_warn_class_memaccess (location_t l
switch (DECL_FUNCTION_CODE (fndecl))
{
case BUILT_IN_MEMSET:
- if (!integer_zerop (args[1]))
+ if (!integer_zerop (maybe_constant_value ((*args)[1])))
{
/* Diagnose setting non-copy-assignable or non-trivial types,
or types with a private member, to (potentially) non-zero
@@ -8497,8 +8501,11 @@ maybe_warn_class_memaccess (location_t l
case BUILT_IN_MEMMOVE:
case BUILT_IN_MEMPCPY:
/* Determine the type of the source object. */
- srctype = STRIP_NOPS (args[srcidx]);
- srctype = TREE_TYPE (TREE_TYPE (srctype));
+ srctype = TREE_TYPE ((*args)[srcidx]);
+ if (!srctype || !POINTER_TYPE_P (srctype))
+ srctype = void_type_node;
+ else
+ srctype = TREE_TYPE (srctype);
/* Since it's impossible to determine wheter the byte copy is
being used in place of assignment to an existing object or
@@ -8547,13 +8554,16 @@ maybe_warn_class_memaccess (location_t l
"assignment or copy-initialization instead",
fndecl, desttype, access, fld, srctype);
}
- else if (!trivial && TREE_CODE (args[2]) == INTEGER_CST)
+ else if (!trivial && vec_safe_length (args) > 2)
{
+ tree sz = maybe_constant_value ((*args)[2]);
+ if (!tree_fits_uhwi_p (sz))
+ break;
+
/* Finally, warn on partial copies. */
unsigned HOST_WIDE_INT typesize
= tree_to_uhwi (TYPE_SIZE_UNIT (desttype));
- if (unsigned HOST_WIDE_INT partial
- = tree_to_uhwi (args[2]) % typesize)
+ if (unsigned HOST_WIDE_INT partial = tree_to_uhwi (sz) % typesize)
warned = warning_at (loc, OPT_Wclass_memaccess,
(typesize - partial > 1
? G_("%qD writing to an object of "
@@ -8577,16 +8587,17 @@ maybe_warn_class_memaccess (location_t l
else if (!get_dtor (desttype, tf_none))
warnfmt = G_("%qD moving an object of type %#qT with deleted "
"destructor");
- else if (!trivial
- && TREE_CODE (args[1]) == INTEGER_CST
- && tree_int_cst_lt (args[1], TYPE_SIZE_UNIT (desttype)))
+ else if (!trivial)
{
- /* Finally, warn on reallocation into insufficient space. */
- warned = warning_at (loc, OPT_Wclass_memaccess,
- "%qD moving an object of non-trivial type "
- "%#qT and size %E into a region of size %E",
- fndecl, desttype, TYPE_SIZE_UNIT (desttype),
- args[1]);
+ tree sz = maybe_constant_value ((*args)[1]);
+ if (TREE_CODE (sz) == INTEGER_CST
+ && tree_int_cst_lt (sz, TYPE_SIZE_UNIT (desttype)))
+ /* Finally, warn on reallocation into insufficient space. */
+ warned = warning_at (loc, OPT_Wclass_memaccess,
+ "%qD moving an object of non-trivial type "
+ "%#qT and size %E into a region of size %E",
+ fndecl, desttype, TYPE_SIZE_UNIT (desttype),
+ sz);
}
break;
@@ -8594,9 +8605,6 @@ maybe_warn_class_memaccess (location_t l
return;
}
- if (!warned && !warnfmt)
- return;
-
if (warnfmt)
{
if (suggest)
@@ -8643,10 +8651,6 @@ build_cxx_call (tree fn, int nargs, tree
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. */
- if (nargs)
- maybe_warn_class_memaccess (loc, fndecl, argarray);
}
if (VOID_TYPE_P (TREE_TYPE (fn)))
--- gcc/testsuite/g++.dg/Wclass-memaccess-2.C.jj 2017-06-27 09:16:08.184683033 +0200
+++ gcc/testsuite/g++.dg/Wclass-memaccess-2.C 2018-01-05 14:52:01.277207043 +0100
@@ -53,9 +53,7 @@ void c_cast_uchar (NonTrivial *p)
__builtin_memset ((unsigned char*)p, 0, sizeof *p);
}
-// A cast to void* does not suppress the warning. That is (or can be)
-// considered a feature.
void c_cast_void (NonTrivial *p)
{
- __builtin_memset ((void*)p, 0, sizeof *p); // { dg-warning "\\\[-Wclass-memaccess]" }
+ __builtin_memset ((void*)p, 0, sizeof *p); // { dg-bogus "\\\[-Wclass-memaccess]" }
}
Jakub
More information about the Gcc-patches
mailing list