Clang-tidy’s use-after-move checker and path sensitive analyzer (clang-analyzer-cplusplus.Move) can find bugs where a moved from object is used (before it is potentially reassigned). For the cases where the static checkers are not able to find use after move bugs, an instrumented executable could find such bugs. A hypothetical instrumented + runtime supported build could track objects that are passed to a move constructor or assignment operator, and raise a failure if that object is “used” before it is reassigned or destructed. Could this type of check fall under one of the existing sanitizers (and/or, is there any active work to support one)?
As someone who worked on the static analyzer use-after-move check, the most annoying part is that use after move isn’t exactly illegal. For most standard classes it simply leaves the object in a “valid but unspecified state” which leaves a lot of room for creative ways to reuse the object later. For non-standard classes it boils down to “whatever the move-constructor says” which most of the time means that you can’t diagnose anything.
An interesting hack I did in the static analyzer was to diagnose even the non-standard classes when the moved variable is local, which indicates that there’s relatively few good reasons to worry about reusing memory. But in this case the problem is very local, so static analysis actually catches most of these bugs.
I think this could work as a “library sanitizer” thing, due to standard library being the best place to classify standard operations into legal and illegal after move (@ldionne WDYT?). But outside of that, I think it’ll probably require programmers to annotate their code before using such tool. (And I’ll happily consume such annotations in static analyzer as well.)
1 Like
It is true that what I’m interested in is an opinionated tool which assumes any use, except for reassigning the moved from object to another value, is potentially indicative of a bug. Reusing a unique_ptr without first reassigning it would be something I’d like to know my code is doing, for instance (even if it’s specified that the moved from pointer is null). But I definitely see that many programs may write code that skips any explicit reassigning to null in this example. I wouldn’t be surprised if libc++ or others relied on stuff like this internally. As I understand it, the sanitizers are concerned with objectively buggy programs (a memory error is always a memory error). It wouldn’t make sense to abort the program if it detected such a use as ASAN would. I’ll do some more thinking on this - thanks for the feedback.
You can do a lot more you can do than just reassignment. For example, in case of unique_ptr it makes sense to use operator bool():
unique_ptr<int> P = ...;
if (cond1) {
P = nullptr;
}
if (cond2) {
foo(std::move(P));
}
if (P) { // Is the value still there for *any* reason?
...
}
Sometimes calling operator bool() like this may indicate a bug (especially when the prior move was unconditional) but generally it isn’t.
In case of collections, it may be more idiomatic to call .clear() than to assign a {} (just like .reset() may be a valid way to reset a unique_ptr). In a custom collection that reliably clears itself after move, an .empty() check may make sense for the same reason that operator bool() makes sense on unique_ptr. So there’s no easy way around this, you have to somehow teach the tool about everything that may potentially make sense.
Also classes don’t need to be perfectly incapsulated. One way to reset the object is to assign values to the fields directly. So your tool might need to instrument raw assignment operations as well.