std::copy() calls memmove on nontrivially-copyable type

[AaronBallman]

Bugzilla Link	28527
Version	3.8
OS	Windows NT
CC	@efriedma-quic,@mclow,@zygoloid

Extended Description

Consider the following snippet:

#include <algorithm>

int main() {
  volatile int *v1[100];
  volatile int *v2[100];

  std::copy(v1, v1 + 100, v2);
}

This results in a call to memmove() rather than copying individual volatile elements of the array (https://godbolt.org/g/b0fv3R). This should be handled a bit more kindly, since volatile-qualified types are not trivially-copyable types ([basic.types]p9 states in part: "Cv-unqualified scalar types, trivially copyable class types (Clause 9), arrays of such types, and nonvolatile const-qualified versions of these types (3.9.3) are collectively called trivially copyable types.").

[AaronBallman]

assigned to @mclow

[mclow]

The problem is that this is true:
static_assert(std::is_trivially_copy_assignable::value, "" );

[efriedma-quic]

"volatile int*" is a pointer to a volatile type, but it isn't itself a volatile type. I think you meant the following?

#include <algorithm>

int main() {
  volatile int v1[100];
  volatile int v2[100];

  std::copy(v1, v1 + 100, v2);
}

Gives:

error: cannot initialize a parameter of type 'void *' with an lvalue of type 'volatile int *'

[AaronBallman]

"volatile int*" is a pointer to a volatile type, but it isn't itself a
volatile type. I think you meant the following?

#include

int main() {
volatile int v1[100];
volatile int v2[100];

std::copy(v1, v1 + 100, v2);
}

Gives:

error: cannot initialize a parameter of type 'void *' with an lvalue of type
'volatile int *'

No, I meant 'volatile int *' -- a pointer to volatile ints. Those cannot be copied via memcpy() or memmove() under as-if because volatile int is not a trivially copyable type.

[efriedma-quic]

No, I meant 'volatile int *' -- a pointer to volatile ints. Those cannot be
copied via memcpy() or memmove() under as-if because volatile int is not a
trivially copyable type.

Your original testcase passes a volatile int** to std::copy... which can be copied with memcpy (there aren't any volatile objects involved).

[AaronBallman]

No, I meant 'volatile int *' -- a pointer to volatile ints. Those cannot be
copied via memcpy() or memmove() under as-if because volatile int is not a
trivially copyable type.

Your original testcase passes a volatile int** to std::copy... which can be
copied with memcpy (there aren't any volatile objects involved).

Ugh, my brain some days. :-( You're absolutely right, this does pass a volatile int **.

However, when I correct my test case to not be wrong, I still get misbehavior.

https://godbolt.org/g/tC0y7b

In file included from /tmp/gcc-explorer-compiler116613-82-1qhm7jl/example.cpp:1:
/opt/clang+llvm-3.8.0-x86_64-linux-gnu-ubuntu-14.04/bin/../include/c++/v1/algorithm:1748:24: error: cannot initialize a parameter of type 'void *' with an lvalue of type 'volatile int *'
_VSTD::memmove(__result, __first, __n * sizeof(_Up));
^~~~~~~~
/opt/clang+llvm-3.8.0-x86_64-linux-gnu-ubuntu-14.04/bin/../include/c++/v1/algorithm:1757:19: note: in instantiation of function template specialization 'std::__1::__copy<volatile int, volatile int>' requested here
return _VSTD::__copy(__unwrap_iter(__first), __unwrap_iter(__last), __unwrap_iter(__result));
^
7 : note: in instantiation of function template specialization 'std::__1::copy<volatile int *, volatile int *>' requested here
std::copy(v1, v1 + 100, v2);
^
/usr/include/string.h:50:29: note: passing argument to parameter '__dest' here
extern void *memmove (void *__dest, const void *__src, size_t __n)
^
1 error generated.
Compilation failed

[mclow]

I get the same behavior (compiler error) on gcc, but not on MSVC.

[zygoloid]

(Responding to Marshall's question from IRC)

Per http://wg21.link/cwg2094, clang is correct to consider volatile types and types with volatile subobjects as being trivially copyable. Perhaps we need a separate trait to determine if copying an object is really equivalent to memcpy (trivially copyable and has no volatile subobjects). =(

[mclow]

Perhaps we need a separate trait to determine if copying an object is really equivalent to memcpy.

If we do that, then we can get rid of "is_trivially_copyable/is_trivially_copy_constructible/etc", because that's pretty much all it they're used for.

[mclow]

*** Bug llvm/llvm-bugzilla-archive#45103 has been marked as a duplicate of this bug. ***

[mclow]

mentioned in issue llvm/llvm-bugzilla-archive#45103

[ldionne]

This should be solved by https://reviews.llvm.org/D139235. Per http://eel.is/c++draft/basic.types.general#3, we are allowed to memcpy trivially copyable types. Per http://wg21.link/cwg2094, volatile objects and objects with volatile subobjects may be considered trivially copyable as well.

I think any change in behavior here would require the core wording to change. FWIW if it's unsafe to perform this optimization for some types, I think we'll need a way for the compiler to tell us that, because we can't start pessimizing every legitimate trivially copyable std::copy for the sake of volatile subobjects.

Please let me know if you think I'm wrong or if you think I'm making a bad judgement call here.

[AaronBallman]

Per http://wg21.link/cwg2094, volatile objects and objects with volatile subobjects may be considered trivially copyable as well.

I agree with this interpretation but find it unfortunate (assuming I understand it properly, of course).

As I read it, volatile semantics seem to now be broken in C++ because you can no longer trust that the volatile objects will actually be accessed properly if you use std::copy, but it works fine in a for loop, unless the optimizer decides to "as-if" your for loop into a memcpy.

The scenario is: you have a volatile int array that represents a register bank, you go to copy all of the data out of the register bank (perhaps you want to restore the register state later), and oops, it was memcpy/memmove so you got the wrong values because the volatile access didn't happen.

It's not the most common use case, to be sure, but it'd be nice if what we provided was reliably useful. But I agree with you, it'd require a change to standards wording to get that.

[var-const]

@AaronBallman Great, sounds like we're on the same page here.

Just curious -- for the memmove optimization to apply, the struct with the volatile int array would have to be trivially copyable. It seems like a user could "opt out" by writing a user-provided copy constructor (although that might end up doing more harm than good and in either case shouldn't be required). Would the case you have in mind normally be trivially copyable (under current language rules)?

One more question -- if I'm reading the conversation right, it seems the title of the issue should be updated to something like std::copy() issues a compiler error when trying to copy between volatile pointers. Does that look reasonable to you? (if it does, I can update the title)

[philnik777]

Closing this, since the original issue is fixed. Fixing volatile semantics seems out of scope.