public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/107361] New: Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types?
@ 2022-10-23  7:22 carlosgalvezp at gmail dot com
  2022-10-23 23:52 ` [Bug c++/107361] " pinskia at gcc dot gnu.org
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: carlosgalvezp at gmail dot com @ 2022-10-23  7:22 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107361

            Bug ID: 107361
           Summary: Why does -Wclass-memaccess require trivial types,
                    instead of trivially-copyable types?
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: carlosgalvezp at gmail dot com
  Target Milestone: ---

I find -Wclass-memaccess warns on this kind of code:

#include <cstring>

struct Foo
{
    int x{};
    char y{};
    int z{};
};

int main()
{
    Foo a;
    std::memset(&a, 0, sizeof(a));
}

All because Foo has default member initializers, making it a non-trivial type.
It's still trivially-copyable, which is the requirement for std::memset.
Therefore I ask: why is the warning stricter than it needs to be?

Making sure structs are initialized is defensive programming, and consistent
with coding guidelines that require classes to initialize their data members. 

Also, there are cases where we cannot use the C++ initialization (Foo a = {};).
For example, this class has implicit padding, and the C++ initialization will
not initialize that. This is a problem for serializer/deserializer type of code
(copying/reading uninitialized padding bytes, valgrind will complain).

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug c++/107361] Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types?
  2022-10-23  7:22 [Bug c++/107361] New: Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types? carlosgalvezp at gmail dot com
@ 2022-10-23 23:52 ` pinskia at gcc dot gnu.org
  2022-10-24  6:19 ` carlosgalvezp at gmail dot com
  2024-05-28 15:33 ` pkasting at google dot com
  2 siblings, 0 replies; 4+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-10-23 23:52 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107361

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
https://gcc.gnu.org/legacy-ml/gcc-patches/2017-04/msg01571.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug c++/107361] Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types?
  2022-10-23  7:22 [Bug c++/107361] New: Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types? carlosgalvezp at gmail dot com
  2022-10-23 23:52 ` [Bug c++/107361] " pinskia at gcc dot gnu.org
@ 2022-10-24  6:19 ` carlosgalvezp at gmail dot com
  2024-05-28 15:33 ` pkasting at google dot com
  2 siblings, 0 replies; 4+ messages in thread
From: carlosgalvezp at gmail dot com @ 2022-10-24  6:19 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107361

--- Comment #2 from Carlos Galvez <carlosgalvezp at gmail dot com> ---
I understand the motivation and I think the warning makes a lot of sense!
However I don't see how it could possibly be dangerous in the provided example.

Would it suffice to trigger the warning only when a class has
ctors/dtors/copy/move, which seems to be the intended use case?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug c++/107361] Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types?
  2022-10-23  7:22 [Bug c++/107361] New: Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types? carlosgalvezp at gmail dot com
  2022-10-23 23:52 ` [Bug c++/107361] " pinskia at gcc dot gnu.org
  2022-10-24  6:19 ` carlosgalvezp at gmail dot com
@ 2024-05-28 15:33 ` pkasting at google dot com
  2 siblings, 0 replies; 4+ messages in thread
From: pkasting at google dot com @ 2024-05-28 15:33 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107361

Peter Kasting <pkasting at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pkasting at google dot com

--- Comment #3 from Peter Kasting <pkasting at google dot com> ---
This warning fired in Chromium on safe code that clang doesn't warn about, for
similar reasons as the reporter here complains about.

I think the linked patch was too aggressive. At the very least, `memcpy()` from
some instance `t1` of a `T` into another instance `t2` of a `T` should only
require that `T` is trivially-copyable. Since we can assume that `t1`'s
invariants held before the statement, after it `t2`'s invariants must hold (or
the coder should not have allowed trivial copyability, which is a distinct
bug).

I do agree with the original patch author that `memset()` probably requires the
type to be trivial rather than just trivially-copyable.

We can work around with cast-to-`void*` prior to `memcpy()`, which feels
unfortunate given that the intent of that is less obvious to a reader, and it's
probably easier to introduce dangerous behavior later.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-05-28 15:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-23  7:22 [Bug c++/107361] New: Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types? carlosgalvezp at gmail dot com
2022-10-23 23:52 ` [Bug c++/107361] " pinskia at gcc dot gnu.org
2022-10-24  6:19 ` carlosgalvezp at gmail dot com
2024-05-28 15:33 ` pkasting at google dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).