diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -68,6 +68,7 @@ bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion, const MemRegion *OtherSmartPtrRegion) const; void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const; + bool handleComparisionOp(const CallEvent &Call, CheckerContext &C) const; using SmartPtrMethodHandlerFn = void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const; @@ -264,6 +265,11 @@ if (handleAssignOp(Call, C)) return true; + // FIXME: This won't work + // Check for one arg or the other being a smart-ptr. + if (handleComparisionOp(Call, C)) + return true; + const SmartPtrMethodHandlerFn *Handler = SmartPtrMethodHandlers.lookup(Call); if (!Handler) return false; @@ -272,6 +278,177 @@ return C.isDifferent(); } +bool SmartPtrModeling::handleComparisionOp(const CallEvent &Call, + CheckerContext &C) const { + const auto *MC = llvm::dyn_cast(&Call); + if (!MC) + return false; + const OverloadedOperatorKind OOK = MC->getOperator(); + if (!(OOK == OO_Equal || OOK == OO_ExclaimEqual || OOK == OO_Less || + OOK == OO_LessEqual || OOK == OO_Greater || OOK == OO_GreaterEqual || + OOK == OO_Spaceship)) + return false; + + SVal First = Call.getArgSVal(0); + SVal Second = Call.getArgSVal(1); + + // There are some special cases about which we can infer about + // the resulting answer. + // For reference, there is a discussion at https://reviews.llvm.org/D104616. + // Also, the cppreference page is good to look at + // https://en.cppreference.com/w/cpp/memory/unique_ptr/operator_cmp. + + bool addTransition = false; + ProgramStateRef State = C.getState(); + SVal RetVal = C.getSValBuilder().conjureSymbolVal( + Call.getOriginExpr(), C.getLocationContext(), Call.getResultType(), + C.blockCount()); + + if (!First.isZeroConstant() && !Second.isZeroConstant()) { + // Neither are nullptr, so they are both std::unique_ptr. (whether the smart + // pointers are null or not is an entire different question.) + const MemRegion *FirstReg = First.getAsRegion(); + const MemRegion *SecondReg = Second.getAsRegion(); + if (!FirstReg || !SecondReg) + return false; + + // First and Second may refer to the same object + if (FirstReg == SecondReg) { + switch (OOK) { + case OO_Equal: + case OO_GreaterEqual: + case OO_LessEqual: + State = + State->assume(llvm::dyn_cast(RetVal), true); + addTransition = true; + break; + case OO_Greater: + case OO_Less: + State = + State->assume(llvm::dyn_cast(RetVal), false); + addTransition = true; + break; + case OO_Spaceship: + // TODO: What would be a good thing to do here? + default: + assert(false && "shouldn't reach here"); + } + } else { + const auto *FirstPtr = State->get(FirstReg); + const auto *SecondPtr = State->get(SecondReg); + + if (!FirstPtr && !SecondPtr) + return false; + + // Now, we know the inner pointer of at least one + + if (FirstPtr && !SecondPtr && + State->assume(llvm::dyn_cast(*FirstPtr), + false)) { + // FirstPtr is null, SecondPtr is unknown + if (OOK == OO_LessEqual) { + State = + State->assume(llvm::dyn_cast(RetVal), true); + addTransition = true; + } + } + if (SecondPtr && !FirstPtr && + State->assume(llvm::dyn_cast(*SecondPtr), + false)) { + // SecondPtr is null, FirstPtr is unknown + if (OOK == OO_GreaterEqual) { + State = + State->assume(llvm::dyn_cast(RetVal), true); + addTransition = true; + } + } + + if (FirstPtr && SecondPtr) { + bool FirstNull{!State->assume( + llvm::dyn_cast(*FirstPtr), true)}; + bool SecondNull{!State->assume( + llvm::dyn_cast(*SecondPtr), true)}; + + if (FirstNull && SecondNull) { + switch (OOK) { + case OO_Equal: + case OO_GreaterEqual: + case OO_LessEqual: + State = State->assume(llvm::dyn_cast(RetVal), + true); + addTransition = true; + break; + case OO_Greater: + case OO_Less: + State = State->assume(llvm::dyn_cast(RetVal), + false); + addTransition = true; + break; + case OO_Spaceship: + // TODO: What would be a good thing to do here? + default: + assert(false && "shouldn't reach here"); + } + } + + if (FirstNull && !SecondNull) { + switch (OOK) { + case OO_Less: + case OO_LessEqual: + State = State->assume(llvm::dyn_cast(RetVal), + true); + addTransition = true; + break; + case OO_Equal: + case OO_GreaterEqual: + case OO_Greater: + State = State->assume(llvm::dyn_cast(RetVal), + false); + addTransition = true; + break; + case OO_Spaceship: + // TODO: What would be a good thing to do here? + default: + assert(false && "shouldn't reach here"); + } + } + + if (!FirstNull && SecondNull) { + switch (OOK) { + case OO_GreaterEqual: + case OO_Greater: + State = State->assume(llvm::dyn_cast(RetVal), + true); + addTransition = true; + break; + case OO_Less: + case OO_LessEqual: + case OO_Equal: + State = State->assume(llvm::dyn_cast(RetVal), + false); + addTransition = true; + break; + case OO_Spaceship: + // TODO: What would be a good thing to do here? + default: + assert(false && "shouldn't reach here"); + } + } + } + } + } + + // TODO: Now handle all the cases with one arg as nullptr + + if (addTransition) { + State = + State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), RetVal); + C.addTransition(State); + return true; + } + return false; +} + void SmartPtrModeling::checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const { ProgramStateRef State = C.getState();