Copy operations (copy constructors and copy assignment operators) are expected to copy the salient properties of a source object into the destination object, with the resulting object being a "copy" of the original. What is considered to be a salient property of the type is type-dependent, but for types that expose comparison or equality operators, includes any properties used for those comparison operations. This expectation leads to assumptions in code that a copy operation results in a destination object with a value representation that is equivalent to the source object value representation. Violation of this basic assumption can lead to unexpected behavior.
Ideally, the copy operator should have an idiomatic signature. For copy constructors, that is T(const T&);
and for copy assignment operators, that is T& operator=(const T&);
. Copy constructors and copy assignment operators that do not use an idiomatic signature do not meet the requirements of the CopyConstructible
or CopyAssignable
concept, respectively. This precludes the type from being used with common standard library functionality [ISO/IEC 14882-2014].
When implementing a copy operator, do not mutate any externally observable members of the source object operand or globally accessible information. Externally observable members include, but are not limited to, members that participate in comparison or equality operations, members whose values are exposed via public APIs, and global variables.
Before C++11, a copy operation that mutated the source operand was the only way to provide move-like semantics. However, the language did not provide a way to enforce that this operation only occurred when the source operand was at the end of its lifetime, which led to fragile APIs like std::auto_ptr
. In C++11 and later, such a situation is a good candidate for a move operation instead of a copy operation.
auto_ptr
For example, in C++03, std::auto_ptr
had the following copy operation signatures [ISO/IEC 14882-2003]:
Copy constructor | auto_ptr(auto_ptr &A); |
Copy assignment | auto_ptr& operator=(auto_ptr &A); |
Both copy construction and copy assignment would mutate the source argument, A
, by effectively calling this->reset(A.release())
. However, this invalidated assumptions made by standard library algorithms such as std::sort()
, which may need to make a copy of an object for later comparisons [Hinnant 05]. Consider the following implementation of std::sort()
that implements the quick sort algorithm.
// ... value_type pivot_element = *mid_point; // ...
At this point, the sorting algorithm assumes that pivot_element
and *mid_point
have equivalent value representations and will compare equal. However, for std::auto_ptr
, this is not the case because *mid_point
has been mutated and results in unexpected behavior.
In C++11, the std::unique_ptr
smart pointer class was introduced as a replacement for std::auto_ptr
to better specify the ownership semantics of pointer objects. Rather than mutate the source argument in a copy operation, std::unique_ptr
explicitly deletes the copy constructor and copy assignment operator, and instead uses a move constructor and move assignment operator. Subsequently, std::auto_ptr
was deprecated in C++11.
Noncompliant Code Example
In this noncompliant code example, the copy operations for A
mutate the source operand by resetting its member variable m
to 0
. When std::fill()
is called, the first element copied will have the original value of obj.m
, 12
, at which point obj.m
is set to 0
. The subsequent nine copies will all retain the value 0
.
#include <algorithm> #include <vector> class A { mutable int m; public: A() : m(0) {} explicit A(int m) : m(m) {} A(const A &other) : m(other.m) { other.m = 0; } A& operator=(const A &other) { if (&other != this) { m = other.m; other.m = 0; } return *this; } int get_m() const { return m; } }; void f() { std::vector<A> v{10}; A obj(12); std::fill(v.begin(), v.end(), obj); }
Compliant Solution
In this compliant solution, the copy operations for A
no longer mutate the source operand, ensuring that the vector contains equivalent copies of obj
. Instead, A
has been given move operations that perform the mutation when it is safe to do so.
#include <algorithm> #include <vector> class A { int m; public: A() : m(0) {} explicit A(int m) : m(m) {} A(const A &other) : m(other.m) {} A(A &&other) : m(other.m) { other.m = 0; } A& operator=(const A &other) { if (&other != this) { m = other.m; } return *this; } A& operator=(A &&other) { m = other.m; other.m = 0; return *this; } int get_m() const { return m; } }; void f() { std::vector<A> v{10}; A obj(12); std::fill(v.begin(), v.end(), obj); }
Exceptions
OOP58-CPP-EX0: Reference counting, and implementations such as std::shared_ptr<>
constitute an exception to this rule. Any copy or assignment operation of a reference-counted object requires the reference count to be incremented. The semantics of reference counting are well-understood, and it can be argued that the reference count is not a salient part of the shared_pointer
object.
Risk Assessment
Copy operations that mutate the source operand or global state can lead to unexpected program behavior. Using such a type in a Standard Template Library container or algorithm can also lead to undefined behavior.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
OOP58-CPP | Low | Likely | Low | P9 | L2 |
Automated Detection
Tool | Version | Checker | Description |
---|---|---|---|
CodeSonar | 8.1p0 | LANG.FUNCS.COPINC | Copy Operation Parameter Is Not const |
Helix QAC | 2024.3 | C++4075 | |
Klocwork | 2024.3 | CERT.OOP.COPY_MUTATES | |
Parasoft C/C++test | 2023.1 | CERT_CPP-OOP58-a | Copy operations must not mutate the source object |
Polyspace Bug Finder | R2024a | CERT C++: OOP58-CPP | Checks for copy operation modifying source operand (rule partially covered) |
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
Related Guidelines
Bibliography
[ISO/IEC 14882-2014] | Subclause 12.8, "Copying and Moving Class Objects" Table 21, "CopyConstructible Requirements" Table 23, "CopyAssignable Requirements" |
[ISO/IEC 14882-2003] | |
[Hinnant 2005] | "Rvalue Reference Recommendations for Chapter 20" |
11 Comments
Peter Brockamp
The code snippet for the noncompliant sample is wrong, as it declares the arguments for the copy constructor and assignment operators as being const. Thus they will fail to compile when trying to mutate the argument. This should be corrected into versions taking non-const arguments.
Aaron Ballman
The code examples will compile with a conforming compiler implementation – the member variable
M
is declared asmutable
, so it can be modified within aconst
context.Peter Brockamp
Yep, you are right, I overlooked this, sorry.
However if a member variable is declared mutable I must expect this variable being changed even by a const method. And this need not necessarily be wrong, it drills down to the old discussion about internal and external constness of a class, more concrete the latter being the observable constness. Thus a copy constructor or assignment operator mutating a mutable member of the source object is not broken in general, it just depends. So for me this insight broadens in another (not yet present) rule "Declare only those members as mutable that don't break the observable constness".
I often see code fragments where developers neglected const correctness or missed it. With "missed it" I mean they failed to declare a method const, albeit this would've been reasonable and possible without much effort. Thus they write sloppy copy constructors and assignment operator with non-const calling parameters - "But they do work, so what's the point?":
Equally, on a regular basis I would expect any getter method to be a const method. The name itself implies constness. Thus either try to make it const if possible or else rename it to not look like a getter. E.g. "getSize()" will become "calculateSize()".So there would be a rule "Make getters const methods or rename them appropriately to not appear like genuine getters" and a recommendation "Try to make method parameters const if feasible".
And as everything has its counterexample: Imagine a class that is persistent by offering a save() method that writes its state to a file. It naturally implies that save shall be const, as it doesn't (or at least shouldn't) change the object. Plus (if not throwing an error in case of failed I/O) you'll probably like to have a getter to query the state of the last I/O operation getLastError() (which is const as well). The obvious (and IMHO correct) solution is to introduce a mutable member that stores the I/O state. save(), albeit being const, will write to that member and getLastError() will return it. Thus this solution will violate above observable-constness-rule, nevertheless it "feels right". And I would regularly tend to implement things so that they "feel right", i. e. they get a "natural" touch, which makes it more likely developers will intuitively use them in the intended, correct way and less likely they will misuse or misunderstand them. But here I fail to formulate this as a rule or recommendation...
Just my 2 cents...
Aaron Ballman
I agree with your observation regarding mutable members, but that's why the normative wording specifies what it means for a member to be "externally observable."
In the example, because
M
is exposed viaGet_M()
, it is externally observable. However, the wording is not intended to prevent things like locking a mutablestd::mutex
object that is an implementation detail of the class and not otherwise observable.As for your example of neglected const correctness, it does actually matter in practice! Those are non-idiomatic copy constructor and copy assignment operators. They will cause
std::is_copy_constructible<T>::value
andstd::is_copy_assignable<T>::value
to evaluate tofalse
. They also do not meet the standard's definition of CopyConstructible or CopyAssignable (as a library term, see [copyconstructible] and [copyassignable]), and so the type will not behave as likely anticipated by the library. See http://coliru.stacked-crooked.com/a/ee2a78afe47506d9 as an example. This rule actually started out by forbidding such non-idiomatic constructs, but unfortunately that doesn't rise to the metric of a rule because it's not a security concern, just a best practices concern. It doesn't seem right to fail a security audit simply because the user messed up the const-correctness of a parameter. However, it is a security concern when the non-idiomatic signature abuses the intent of what a copy constructor (or copy assignment operator) should be used for by mutating the source object.Btw, we do have a recommendation for const-qualification: VOID DCL00-CPP. Const-qualify immutable objects, though it likely needs to be updated.
David Svoboda
Don't copy operations of std::shared_ptr<> mutate the source pointer (by incrementing the reference count)? And doesn't this apply to all reference-counted objects?
Aaron Ballman
Yes and no.
Yes – when you copy a std::shared_ptr, a reference count is updated in the control block. This is a modification to (an implementation detail of) the object.
No – the control block that is modified is not a salient part of the shared_ptr object's identity. For instance, this does not impact operator==() behavior on the shared_ptr object.
You could argue that this is still mutating an externally observable member of the shared_ptr because of shared_ptr::use_count(), but that would be a bit of a pathological definition of "identity" for reference counted objects. However, it might make sense to spell this out more explicitly with somewhat better words. This is a hard property to capture in prose.
David Svoboda
These rules exist precisely to capture such details in prose :) However you wish to argue what "mutate" and "salient properties" means, I think the rule deserves an explicit exception for reference-counted objects, including std::shared_ptr<>.
Personally I would argue that std::shared_ptr<> currently violates this rule as it is currently worded. I consider it permissible because the semantics of reference counting are well understood and do not violate assumptions of generic algorithms like std::fill(). (The std::auto_ptr<> was deprecated because it failed these conditions.)
Aaron Ballman
I think that's reasonable.
I wouldn't make that argument. I think that the signature of the std::shared_ptr copy constructor is a pretty clear signal that the mutation is not impacting the salient state of the object.
David Svoboda
It occurs to me that reference-counting works only because no external code reads or writes the reference counts (eg what you meant by "salient part"). For example, if I had a vector of shared pointers, and I tried to sort it by reference counts, I'm guessing the behavior would be undefined.
Aaron Ballman
That's why I mentioned std::shared_ptr::use_count() – it does allow you to externally read the reference counts (but not write to it). So it's a bit squishy as to whether the reference count is salient or not. The information is exposed, but it doesn't contribute to the identity of the object, so it's grey area.
David Svoboda
Exception added.