What can go wrong implementing swap copying the bits at low level?

Question

Consider this code:

#include <cstring>
#include <memory>

namespace mstd {
template <typename T>
void swap(T& lhs, T& rhs) {
    char tmp[sizeof(T)];

    std::memcpy(tmp, std::addressof(lhs), sizeof(T));
    std::memcpy(std::addressof(lhs), std::addressof(rhs), sizeof(T));
    std::memcpy(std::addressof(rhs), tmp, sizeof(T));
}
}

Using mstd::swap is in general not safe; it is only if std::is_trivially_copyable<T>::value is true.

However I cannot see how it can go wrong. Do anyone know a real example where using this swap will bring a behavior that is not a correct swap, and why?

Do I understand you correctly that you are asking for problems in the general case, i.e. non-trivially copyable types? Then the anser is it will go wrong if the (copy) ctor has side effects, like registering an object in a central registry or such. — Peter - Reinstate Monica, Apr 28 '15 at 07:04
Or if he binary-copies some pointers to objects now being pointed to by multiple pointers and subsequently destroyed multiple times. Scratch that - its a swap :) — BitTickler, Apr 28 '15 at 07:05
@PeterSchneider has a point; for example if the central registry keep the address of the objects. The address now points to the swapped objects and it not updated as it could it be via copy ctor. — Paolo.Bolzoni, Apr 28 '15 at 07:07
@user2225104, not really. The duplicate pointer is copied in a array that makes nothing in destruction. The two Ts will be destroyed normally. — Paolo.Bolzoni, Apr 28 '15 at 07:08
Objects that contain pointers into themselves, for instance. We had a recent discussion on this subject (`memcpy` on non-trivially-copyable types): http://stackoverflow.com/questions/29777492/why-would-the-behavior-of-stdmemcpy-be-undefined-for-objects-that-are-not-triv — T.C., Apr 28 '15 at 07:11
@T.C., nice one. It needs a fairly insane programmer to not use ```this```, but it is definitely a case. Edit: Oh, right. It is a duplicate. Deleting. Edit(2): I cannot delete questions with answers :( — Paolo.Bolzoni, Apr 28 '15 at 07:13
@Paolo.Bolzoni It comes up more often than you might think. `std::fstream` and `std::stringstream`'s `rdbuf()` pointer usually points back at a streambuf embedded in the stream object itself. — T.C., Apr 28 '15 at 07:29

Mike Seymour · Answer 1 · 2015-04-28T07:31:11.590

3

Suppose a class contains a pointer, which must point to one of its own members. It will need copy/move semantics which preserve that invariant. A bytewise copy will ignore those semantics, leaving it incorrectly pointing a member of a different object.

For a specific example, consider a string with "small string optimisation". It contains an embedded buffer to use instead of dynamic memory, if the string is small enough; and a pointer pointing to either that buffer, or dynamic memory. It needs non-trivial copy (and move) operations to make sure the pointer isn't copied, but rather points to the target object's buffer:

string(string const & other) :
    size(other.size),
    data(other.data == other.buffer ? buffer : new char[size])
{
    copy(other.data, other.data+size, data);
}

A bytewise swap (of two small strings) will make each string point to the other's buffer. You'll end up with a dangling pointer if one is destroyed before each other; worse, a destructor implemented as

~string() {if (data != buffer) delete [] buffer;}

will delete something it shouldn't, giving undefined behaviour.

edited Apr 28 '15 at 07:31

answered Apr 28 '15 at 07:13

Mike Seymour

249,747
28
448
644

I think you are mistaken. Swap is usually implemented using move assignment in which case there is no need to allocate new memory. – MikeMB Apr 28 '15 at 07:15
1

@MikeMB: Moving will also have to preserve the invariant that the pointer points to its own object's member, not some other object's, in order for `std::swap` to work correctly. A bytewise swap will break this invariant. – Mike Seymour Apr 28 '15 at 07:18
Moving as well as the OP's swap function will copy the pointers around. So in the end, rhs will end up pointing to the child of the former lhs and vice versa, what is exactly what swap is supposed to do. For swap and move, you only need shallow copies as nothing new is created or destroyed (otherwise move would not be faster than copy). – MikeMB Apr 28 '15 at 07:25
@MikeMB: The point is that the class has an invariant: the pointer **must** point to a member of its own object, not that of another object. So it needs non-default move and copy semantics to preserve that invariant, as my "string" example shows. `std::swap` will correctly use those semantics; a byte-wise swap doesn't obey those semantics, and breaks the invariant, giving problems like the ones I described. – Mike Seymour Apr 28 '15 at 07:28
Sorry, I misread your answer. You are completely right (when I read "to its own member" I was thinking of a pointer to some object on the heap like in vector) – MikeMB Apr 28 '15 at 07:35
If thats ok, I've one more question about small string optimization. I was under the impression, that at least some implementations don't point their pointers to a local buffer, but they have a union of either two (or three) pointers or an array of chars - reusing the space for the pointers as part of the local buffer. How that union is used it is then decided based on a boolean flag. In that case, the OPs swap would work as expected, even though they are not trivially copyable right? – MikeMB Apr 28 '15 at 08:03
@MikeMB: I've no idea, I haven't studied string optimisations in that much detail. I'm just giving an example of a class which will break when incorrectly swapped. – Mike Seymour Apr 28 '15 at 08:07

score 2 · Answer 2 · answered Apr 28 '15 at 07:13

I think I understood you correctly, so I'll make this an answer.

If the ctor and consequently the copy ctor of a class has certain side effects, those will not be respected by a bitwise copy. As an example: if an object registers itself upon creation in a central registry by address, e.g. because it is a callback functor. Then the address will point to the wrong object after the swap. In the example, the wrong callback will be executed by the callback holder.

score 0 · Answer 3 · answered Apr 28 '15 at 07:28

One example from the top of my mind is a class, where a child has a pointer to its own parent:

#include <memory>

struct Parent;  
struct Child{
    Parent* parent;
};
struct Parent {
    std::unique_ptr<Child> child;
};
void swap(Parent& lhs, Parent& rhs) {
    std::unique_ptr<Child> tmp(std::move(lhs.child));
    lhs.child=std::move(rhs.child);
    rhs.child=std::move(tmp);

    //there is no equivalence for those two lines in your example
    lhs.child->parent=&lhs;
    rhs.child->parent=&rhs;
}

You encounter this e.g. quite often in Qt.

What can go wrong implementing swap copying the bits at low level?

3 Answers3