Skip to end of metadata
Go to start of metadata

Self-copy assignment can occur in situations of varying complexity, but essentially, all self-copy assignments entail some variation of the following.

#include <utility>
 
struct S { /* ... */ }
 
void f() {
  S s;
  s = s; // Self-copy assignment
}

User-provided copy operators must properly handle self-copy assignment.

The postconditions required for copy assignment are specified by the C++ Standard, [utility.arg.requirements], Table 23 [ISO/IEC 14882-2014], which states that for x = y, the value of y is unchanged. When &x == &y, this postcondition translates into the values of both x and y remaining unchanged. A naive implementation of copy assignment could destroy object-local resources in the process of copying resources from the given parameter. If the given parameter is the same object as the local object, the act of destroying object-local resources will invalidate them. The subsequent copy of those resources will be left in an indeterminate state, which violates the postcondition.

A user-provided copy assignment operator must prevent self-copy assignment from leaving the object in an indeterminate state. This can be accomplished by self-assignment tests, copy-and-swap, or other idiomatic design patterns.

The C++ Standard, [copyassignable], specifies that types must ensure that self-copy assignment leave the object in a consistent state when passed to Standard Template Library (STL) functions. Since objects of STL types are used in contexts where CopyAssignable is required, STL types are required to gracefully handle self-copy assignment.

Noncompliant Code Example

In this noncompliant code example, the copy assignment operator does not protect against self-copy assignment. If self-copy assignment occurs, this->s1 is deleted, which results in rhs.s1 also being deleted. The invalidated memory for rhs.s1 is then passed into the copy constructor for S, which can result in dereferencing an invalid pointer.

#include <new>
 
struct S { /* ... */ }; // Has nonthrowing copy constructor
 
class T {
  int n;
  S *s1;
 
public:
  T(const T &rhs) : n(rhs.n), s1(rhs.s1 ? new S(*rhs.s1) : nullptr) {}
  ~T() { delete s1; }
 
  // ...
 
  T& operator=(const T &rhs) {
    n = rhs.n;
    delete s1;
    s1 = new S(*rhs.s1);
    return *this;
  }
};

Compliant Solution (Self-Test)

This compliant solution guards against self-copy assignment by testing whether the given parameter is the same as this. If self-copy assignment occurs, then operator= does nothing; otherwise, the copy proceeds as in the original example.

#include <new>
 
struct S { /* ... */ }; // Has nonthrowing copy constructor
 
class T {
  int n;
  S *s1;
 
public:
  T(const T &rhs) : n(rhs.n), s1(rhs.s1 ? new S(*rhs.s1) : nullptr) {}
  ~T() { delete s1; }

  // ...
 
  T& operator=(const T &rhs) {
    if (this != &rhs) {
      n = rhs.n;
      delete s1;
      try {
        s1 = new S(*rhs.s1);
      } catch (std::bad_alloc &) {
        s1 = nullptr; // For basic exception guarantees
        throw;
      }
    }
    return *this;
  }
};

This solution does not provide a strong exception guarantee for the copy assignment. Specifically, if an exception is called when evaluating the new expression, this has already been modified. However, this solution does provide a basic exception guarantee because no resources are leaked and all data members contain valid values. Consequently, this code complies with ERR56-CPP. Guarantee exception safety.

Compliant Solution (Copy and Swap)

This compliant solution avoids self-copy assignment by constructing a temporary object from rhs that is then swapped with *this. This compliant solution provides a strong exception guarantee because swap() will never be called if resource allocation results in an exception being thrown while creating the temporary object.

#include <new>
#include <utility>
 
struct S { /* ... */ }; // Has nonthrowing copy constructor
 
class T {
  int n;
  S *s1;
 
public:
  T(const T &rhs) : n(rhs.n), s1(rhs.s1 ? new S(*rhs.s1) : nullptr) {}
  ~T() { delete s1; }

  // ...
 
  void swap(T &rhs) noexcept {
    using std::swap;
    swap(n, rhs.n);
    swap(s1, rhs.s1);
  }
 
  T& operator=(T rhs) noexcept {
    rhs.swap(*this);
    return *this;
  }
};

Compliant Solution (Move and Swap)

This compliant solution uses the same classes S and T from the previous compliant solution, but adds the following public constructor and methods:

  T(T &&rhs) { *this = std::move(rhs); }

  // ... everything except operator= ..

  T& operator=(T &&rhs) noexcept {
    using std::swap;
    swap(n, rhs.n);
    swap(s1, rhs.s1);
    return *this;
  }

The copy assignment operator uses std::move() rather than swap() to achieve safe self-assignment and a strong exception guarantee. The move assignment operator uses a move (via the method parameter) and swap.

The move constructor is not strictly necessary, but defining a move constructor along with a move assignment operator is conventional for classes that support move operations.

Note that unlike copy assignment operators, the signature of a move assignment operator accepts a non-const reference to its object with the expectation that the moved-from object will be left in an unspecified, but valid state. Move constructors have the same difference from copy constructors.

Risk Assessment

Allowing a copy assignment operator to corrupt an object could lead to undefined behavior.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

OOP54-CPP

Low

Probable

High

P2

L3

Automated Detection

Tool

Version

Checker

Description

Clang
9.0 (r361550)
cert-oop54-cppChecked by clang-tidy.
Klocwork
2018
CL.SELF-ASSIGN
Parasoft C/C++test
10.4.2

CERT_CPP-OOP54-a

Check for assignment to self in operator=
Polyspace Bug Finder

R2019b

CERT C++: OOP54-CPPChecks for copy assignment operators where self-assignment is not tested (rule partially covered)
PRQA QA-C++
4.3
4072, 4073, 4075, 4076

Related Vulnerabilities

Search for other vulnerabilities resulting from the violation of this rule on the CERT website.

Related Guidelines

This rule is a partial subset of OOP58-CPP. Copy operations must not mutate the source object when copy operations do not gracefully handle self-copy assignment, because the copy operation may mutate both the source and destination objects (due to them being the same object).

Bibliography

[Henricson 1997]Rule 5.12, Copy assignment operators should be protected from doing destructive actions if an object is assigned to itself
[ISO/IEC 14882-2014]Subclause 17.6.3.1, "Template Argument Requirements"
Subclause 17.6.4.9, "Function Arguments"
[Meyers 2005]Item 11, "Handle Assignment to Self in operator="
[Meyers 2014]



20 Comments

  1. What about this solution:

    T& T::operator=(T const& rhs) const
        S * tmp = new S(*rhs.s1);
        // commit the operation
        delete s1; // noexcept
        s1 = tmp;  // noexcept
        n = rhs.n; // noexcept
        return *this;
    }

    ?

    It offers a Strong Exception Guarantee, it supports self-assignment, it's likely more efficient than the one that tests addresses in the probable case. However, in the improbable case, it'll be slower, and it'll mutate the input. I haven't found whether the postcondition "v is unchanged" in utility.arg.requirements/CopyAssignable means the copy-and-swap idiom is not that perfect. Does it concerns only the observable value (as we are in a value semantics, the observable state won't be modified), or does it also concern the internals?

    I guess I'll have to ask on SO or on clc++(.m).

    1. It doesn't handle the case where rhs.s1 is nullptr, but that's easy enough to handle by doing S *tmp = rhs.s1 ? new S(*rhs.s1) : nullptr;, but it does comply with this rule.

      > Does it concerns only the observable value (as we are in a value semantics, the observable state won't be modified), or does it also concern the internals?

      Your example poses an interesting question, so I asked someone from the Library Working Group. Their response is that your code example does satisfy the CopyAssignable concept. Basically, the concept does not care about internal implementation details that are not part of the identity of the object. e.g., this is fine, but if T had a function S& get_s() { return *s1; }, then it would not satisfy the concept because an outstanding reference to s1 could be observably invalidated by the assignment operator on self-copy assignment.

  2. Thank you for this information. This is really interesting to know.

  3. While working on a clang-tidy check for this rule, there was a bit of confusion over whether move-and-swap is a compliant solution here or not. I would contend that it is and that it would be worth adding a new CS that demonstrates it. WDYT?

    1. Studying this question, I discovered that there is a problem in the "Copy and Swap" CS. The assignment operator takes a reference to rhs. Calling swap() on rhs() will therefore modify rhs.  This could be fixed by making rhs a value, rather than a reference.

      That said, a move-and-swap CS would be a good addition.

      Reference: https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom


      1. The assignment operator takes rhs as a const reference and then passes rhs to the copy constructor for T (which also takes a const reference). I don't see any way for the current CS to mutate rhs itself (only the copy of rhs), so I don't think there's a bug there.

        1. Ah, you're right. Our CS constructs a temporary variable from rhs and swaps that. The StackOverflow link has a very similar solution that uses a value (not reference) for rhs and swaps that. Both have the same semantics, just different syntax.


    2. How would it be implemented?


      1. OK. I've seen it.

        The way `operator(T&&)` is implemented, this is exactly `swap()`. As such, IMO it deserves to exist as `swap()`. I was wondering whether it could be implemented with `std::exchange` instead of `swap()`, but this won't permit to release previously acquired resources.
        IMO, this solution is convoluted.

        Also, the copy-assignment operator cannot be `noexcept`.

        1. The way `operator(T&&)` is implemented, this is exactly `swap()`. As such, IMO it deserves to exist as `swap()`. 

          I sort of agree, especially since the copy version has an explicit swap method. A reason to leave it out is that it somewhat distracts from the more-complicated example because it adds one more layer of indirection. You could correct this by turning the move assignment operator body into: this->swap(rhs); return *this; and rely on the fact that the swap() function is present in the preceding CS. However, that may make the example even harder to understand. I don't have strong opinions either way though. 

          IMO, this solution is convoluted.

          It's an idiomatic solution that solves the issue (and works nicely for types that have both explicit copy and move operations).

          Also, the copy-assignment operator cannot be `noexcept`.

          That's true for both the copy and swap as well as move and swap CSes – good catch! There are two issues there: 1) std::swap() is conditionally noexcept, and 2) the constructor for T can throw a std::bad_alloc if the allocation fails. I'd probably drop the noexcept specifiers from the copy assignment operators but leave the one in swap (because we control the data members in the example).


          1. I took 'noexcept' off of both the copy and move assignment operators.

            I do wonder about that, though. The copy assignment operator (in both CS's) can throw because it has to construct a temp object to swap with, and that constructor can throw. But if we make it take a value as argument (rather than a reference), then could it be noexcept?  Does noexcept address if the implicit copy operation of a value parameter can throw?



            1. I took 'noexcept' off of both the copy and move assignment operators.

              The move assignment operator is noexcept safe if we're going to concede the swap function in the copy and swap CS is noexcept safe. I'd either add back the noexcept to the move assignment operator or remove the noexcept from the swap function (slight preference for leaving the noexcept move assignment).

              I do wonder about that, though. The copy assignment operator (in both CS's) can throw because it has to construct a temp object to swap with, and that constructor can throw. But if we make it take a value as argument (rather than a reference), then could it be noexcept?  Does noexcept address if the implicit copy operation of a value parameter can throw?

              Yes, it could be noexcept in that case because the copy for the argument to the assignment operator is created on the caller's side of the fence, so the exception will be thrown before entering the assignment operator function call.

              1. Sigh. I've added back the noexcepts to the assignment operatiors. If this is still controversial, I'm taking them back out for good, because noexcept is complicated and getting it right (especially on swap()) is out of scope of this rule.

                1. The `noexcept` annotation is fine only with the following functions:

                  First possibility (sorry I cannot make the code editor work in my browser)

                  T& operator=(T) noexcept; // the copy that may throw is done on client side

                  Or (it's exclusive: we cannot have passing by value and passing by any kind of reference together)

                  T& operator=(T&&) noexcept; // no copy here, we steal and move around stuff (1)
                  T& operator=(T const&) /* NO noexcept */; // a copy that may throw is done inside


                  (1) actually we can use noexcept in that case if and only if moving around internal data is guaranteed noexcept. This is usually what we aim for.

                  1. I changed the final copy-assignment operator to accept a pass-by-value argument, rather than a const reference. That puts the copy operation outside the operator, allowing it to be noexcept.


                    1. If we have

                      T& operator=(T&&) noexcept /* or not*/;
                      T& operator=(T) noexcept /* or not*/;

                      The compiler will refuse to compile as it cannot choose which overload select in case of a rvalue.

                      1. Agreed. Fixed by removing the final assignment operator.

          2. I think I've pinpointed what's bothering me: the "other" approach, IMO, is not exactly "move and swap", but "move a copy into self". The move is implemented here with a swap, but this is a detail. I'd have called this other way of doing things "copy and move" instead of "move and swap".

            The move assignment operator could have been implemented without relying on swap().

            As, in your example, it relies on swapping, I'd have factored out the `swap()` function and have the move-assignment operator explicitely call `swap()` – for DRY reasons. The advantage of using swap() internally is that it avoid objects in a state that permits destruction and assignment but with invariants weakened (see https://akrzemi1.wordpress.com/2018/05/16/rvalues-redefined/ and https://akrzemi1.wordpress.com/2016/04/07/sessions-and-object-lifetimes/ on this topic).

  4. Btw, as of r361550, clang-tidy supports a check for this rule under the cert-oop54-cpp alias.

    1. Thanks, Aaron. I've added an entry for clang under the Static Analysis section.