Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)

[Pedro Alves <palves at redhat dot com>]

On 05/16/2017 08:41 PM, Jason Merrill wrote:

> I agree that it makes sense to
> check for a trivial assignment operator specifically.  I guess we want
> a slightly stronger "trivially copyable" that also requires a
> non-deleted assignment operator.
> 
> It seems to me that the relevant tests are:
> 
> bcopy/memcpy/memmove want trivally copyable + non-deleted assignment.
> bzero/memset want trivial + non-deleted assignment.
> 
> I'm still not convinced we need to consider standard-layout at all.

What do you think of warning for memset of types with references?  Since NULL
references are undefined behavior (N4659, [dcl.ref]/5), IMHO such code is quite
smelly and most likely a sign of incomplete refactoring, not design.

(My original intention for poisoning memset of standard-layout type in gdb was
mainly as proxy/approximation for "type with references".  Other properties
that make a type non-standard-layout are not as interesting to me.)

While at it, maybe the same reasoning would justify warn of memcpy/memset
of types with const data members?  memcpy of such types kind of sounds like a
recipe for subtle breakage that could only be salvaged with std::launder.
And if you know about that, you'll probably be copying objects of type T
to/from raw byte/char storage, not to/from another T.

Actually, now that I look at Martin's new patch, I see he's already warning
about const data members and references, both memcpy and memset.  I'm not
super convinced on warning about memcpy of references (unlike memset), but
I don't feel strongly against it either.  I'd be fine (FWIW) with giving it a
try and see what breaks out there.

> +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.

May I suggest renaming the warning (and the description above) from
-Wnon-trivial-memaccess to something else that avoids "trivial" in
its name?  Because it's no longer strictly about "trivial" in the
standard C++ sense.  The documentation talks about "The safe way",
and "does not warn on safe calls", so maybe call it -Wunsafe-memaccess?

(I spotted a couple typos in the new patch: "otherwse", "becase", btw.)

Thanks,
Pedro Alves