diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -238,6 +238,14 @@ const LocationContext *LCtx, unsigned Count); + /// Conjure a symbol representing heap allocated memory region. + /// + /// Note, now, the expression *doesn't* need to represent a location. + /// But the type need to! + DefinedOrUnknownSVal getConjuredHeapSymbolVal(const Expr *E, + const LocationContext *LCtx, + QualType type, unsigned Count); + DefinedOrUnknownSVal getDerivedRegionValueSymbolVal( SymbolRef parentSymbol, const TypedValueRegion *region); 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 @@ -38,6 +38,7 @@ using namespace ento; namespace { + class SmartPtrModeling : public Checker { @@ -88,6 +89,9 @@ {{"swap", 1}, &SmartPtrModeling::handleSwapMethod}, {{"get"}, &SmartPtrModeling::handleGet}}; const CallDescription StdSwapCall{{"std", "swap"}, 2}; + const CallDescription StdMakeUniqueCall{{"std", "make_unique"}}; + const CallDescription StdMakeUniqueForOverwriteCall{ + {"std", "make_unique_for_overwrite"}}; }; } // end of anonymous namespace @@ -187,12 +191,27 @@ return {}; auto TemplateArgs = TSD->getTemplateArgs().asArray(); - if (TemplateArgs.size() == 0) + if (TemplateArgs.empty()) return {}; auto InnerValueType = TemplateArgs[0].getAsType(); return C.getASTContext().getPointerType(InnerValueType.getCanonicalType()); } +// This is for use with standalone-functions like std::make_unique, +// std::make_unique_for_overwrite, etc. It reads the template parameter and +// returns the pointer type corresponding to it, +static QualType getPointerTypeFromTemplateArg(const CallEvent &Call, + CheckerContext &C) { + const auto *FD = dyn_cast_or_null(Call.getDecl()); + if (!FD || !FD->isFunctionTemplateSpecialization()) + return {}; + const auto &TemplateArgs = FD->getTemplateSpecializationArgs()->asArray(); + if (TemplateArgs.size() == 0) + return {}; + auto ValueType = TemplateArgs[0].getAsType(); + return C.getASTContext().getPointerType(ValueType.getCanonicalType()); +} + // Helper method to get the inner pointer type of specialized smart pointer // Returns empty type if not found valid inner pointer type. static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) { @@ -248,6 +267,7 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call, CheckerContext &C) const { + ProgramStateRef State = C.getState(); // If any one of the arg is a unique_ptr, then @@ -271,6 +291,50 @@ return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C); } + if (Call.isCalled(StdMakeUniqueCall) || + Call.isCalled(StdMakeUniqueForOverwriteCall)) { + if (!ModelSmartPtrDereference) + return false; + + const Optional ThisRegionOpt = Call.getReturnValueUnderConstruction(); + if (!ThisRegionOpt) + return false; + + const auto PtrVal = C.getSValBuilder().getConjuredHeapSymbolVal( + Call.getOriginExpr(), C.getLocationContext(), + getPointerTypeFromTemplateArg(Call, C), C.blockCount()); + + const MemRegion *ThisRegion = ThisRegionOpt->getAsRegion(); + State = State->set(ThisRegion, PtrVal); + State = State->assume(PtrVal, true); + + // TODO: ExprEngine should do this for us. + // For a bit more context: + // 1) Why do we need this? Since we are modelling a "function" + // that returns a constructed object we need to store this information in + // the program state. + // + // 2) Why does this work? + // `updateObjectsUnderConstruction` does exactly as it sounds. + // + // 3) How should it look like when moved to the Engine? + // It would be nice if we can just + // pretend we don't need to know about this - ie, completely automatic work. + // However, realistically speaking, I think we would need to "signal" the + // ExprEngine evalCall handler that we are constructing an object with this + // function call (constructors obviously construct, hence can be + // automatically deduced). + auto &Engine = State->getStateManager().getOwningEngine(); + State = Engine.updateObjectsUnderConstruction( + *ThisRegionOpt, nullptr, State, C.getLocationContext(), + Call.getConstructionContext(), {}); + + // We don't leave a note here since it is guaranteed the + // unique_ptr from this call is non-null (hence is safe to de-reference). + C.addTransition(State); + return true; + } + if (!smartptr::isStdSmartPtrCall(Call)) return false; diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp --- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -192,12 +192,19 @@ const LocationContext *LCtx, unsigned VisitCount) { QualType T = E->getType(); - assert(Loc::isLocType(T)); - assert(SymbolManager::canSymbolicate(T)); - if (T->isNullPtrType()) - return makeZeroVal(T); + return getConjuredHeapSymbolVal(E, LCtx, T, VisitCount); +} + +DefinedOrUnknownSVal +SValBuilder::getConjuredHeapSymbolVal(const Expr *E, + const LocationContext *LCtx, + QualType type, unsigned VisitCount) { + assert(Loc::isLocType(type)); + assert(SymbolManager::canSymbolicate(type)); + if (type->isNullPtrType()) + return makeZeroVal(type); - SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, T, VisitCount); + SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, type, VisitCount); return loc::MemRegionVal(MemMgr.getSymbolicHeapRegion(sym)); } diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -1033,6 +1033,16 @@ template bool operator<=(nullptr_t x, const unique_ptr &y); +template +unique_ptr make_unique(Args &&...args); + +#if __cplusplus >= 202002L + +template +unique_ptr make_unique_for_overwrite(); + +#endif + } // namespace std #endif diff --git a/clang/test/Analysis/smart-ptr-text-output.cpp b/clang/test/Analysis/smart-ptr-text-output.cpp --- a/clang/test/Analysis/smart-ptr-text-output.cpp +++ b/clang/test/Analysis/smart-ptr-text-output.cpp @@ -1,10 +1,17 @@ // RUN: %clang_analyze_cc1\ -// RUN: -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\ +// RUN: -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\ +// RUN: -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\ +// RUN: -analyzer-output=text -std=c++20 %s -verify=expected + +// RUN: %clang_analyze_cc1\ +// RUN: -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\ // RUN: -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\ // RUN: -analyzer-output=text -std=c++11 %s -verify=expected #include "Inputs/system-header-simulator-cxx.h" +void clang_analyzer_eval(bool); + class A { public: A(){}; @@ -310,3 +317,55 @@ // expected-note@-1{{Dereference of null smart pointer 'P'}} } } + +void makeUniqueReturnsNonNullUniquePtr() { + auto P = std::make_unique(); + if (!P) { // expected-note {{Taking false branch}} + P->foo(); // should have no warning here, path is impossible + } + P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}} + // Now P is null + if (!P) { + // expected-note@-1 {{Taking true branch}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1{{Dereference of null smart pointer 'P'}} + } +} + +#if __cplusplus >= 202002L + +void makeUniqueForOverwriteReturnsNullUniquePtr() { + auto P = std::make_unique_for_overwrite(); + if (!P) { // expected-note {{Taking false branch}} + P->foo(); // should have no warning here, path is impossible + } + P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}} + // Now P is null + if (!P) { + // expected-note@-1 {{Taking true branch}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1{{Dereference of null smart pointer 'P'}} + } +} + +#endif + +struct G { + int *p; + G(int *p): p(p) {} + ~G() { *p = 0; } +}; + +void foo() { + int x = 1; + { + auto P = std::make_unique(&x); + clang_analyzer_eval(*P->p == 1); // expected-warning {{TRUE}} + // expected-note@-1 {{Assuming the condition is false}} + // expected-note@-2 {{FALSE}} + // expected-note@-3 {{Assuming the condition is true}} + // expected-note@-4 {{TRUE}} + } + clang_analyzer_eval(x == 0); // expected-warning {{FALSE}} + // expected-note@-1 {{FALSE}} +}