This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Add modeling for unique_ptr move constructor
ClosedPublic

Authored by vrnithinkumar on Aug 21 2020, 2:37 PM.

Details

Summary
  • Modeling unique_ptr move constructor unique_ptr( unique_ptr&& u ) noexcept

Diff Detail

Event Timeline

vrnithinkumar created this revision.Aug 21 2020, 2:37 PM
vrnithinkumar requested review of this revision.Aug 21 2020, 2:37 PM
vrnithinkumar edited the summary of this revision. (Show Details)Aug 21 2020, 2:39 PM
vrnithinkumar added inline comments.Aug 21 2020, 2:42 PM
clang/test/Analysis/smart-ptr.cpp
295

I was trying to test the below code.

void foo_() {
  std::unique_ptr<A> PToMove;
  std::unique_ptr<A>&& AfterRValeRefCast = std::move(functionReturnsRValueRef());
  std::unique_ptr<A> P(AfterRValeRefCast);
  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}

But passing the local RValue reference variable to move constructor is always invoking the deleted copy constructor.

NoQ added a comment.Aug 21 2020, 5:44 PM

This is easier than D86293 because there's no old value in the freshly constructed smart pointer, right? I suspect that you can still re-use a lot of the implementation with D86293, at least partially.

clang/test/Analysis/smart-ptr.cpp
295

Yeah, you still need an explicit move over it. Implicit moves don't happen because it's an rvalue reference, it has more to do with the anonymity of the object; the compiler only auto-moves when there's no possibility for accidental implicit use-after-move, and in presence of a named reference to the value, accidental implicit use-after-move is still very much possible, so an explicit move is required. That's not the exact rules but that's, as far as i understand, the logic behind the exact rules.

  • Refactoring to reuse common duplicated code
vrnithinkumar added inline comments.Aug 26 2020, 1:31 PM
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
352–394

Same note tag is used for move assignment and move constructor here.

clang/test/Analysis/smart-ptr.cpp
20

Now both "use after move" and "null pointer dereference" warnings are coming. I hope it will be only "null pointer dereference" after smart pointer modeling is complete

vrnithinkumar marked an inline comment as done.Aug 26 2020, 1:32 PM
vrnithinkumar added inline comments.
clang/test/Analysis/smart-ptr.cpp
295

Okay
Thanks for the clarification.

vrnithinkumar marked an inline comment as done.Aug 26 2020, 1:33 PM
In D86373#2231605, @NoQ wrote:

This is easier than D86293 because there's no old value in the freshly constructed smart pointer, right? I suspect that you can still re-use a lot of the implementation with D86293, at least partially.

Refactored to reuse the implementation.

NoQ accepted this revision.Aug 27 2020, 4:02 PM

Yup, that's more like it!~

This revision is now accepted and ready to land.Aug 27 2020, 4:02 PM