diff options
Diffstat (limited to 'clang/lib/Analysis')
20 files changed, 644 insertions, 113 deletions
diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp index 5a52056..f188fc6 100644 --- a/clang/lib/Analysis/AnalysisDeclContext.cpp +++ b/clang/lib/Analysis/AnalysisDeclContext.cpp @@ -117,6 +117,11 @@ Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const { return BD->getBody(); else if (const auto *FunTmpl = dyn_cast_or_null<FunctionTemplateDecl>(D)) return FunTmpl->getTemplatedDecl()->getBody(); + else if (const auto *VD = dyn_cast_or_null<VarDecl>(D)) { + if (VD->isFileVarDecl()) { + return const_cast<Stmt *>(dyn_cast_or_null<Stmt>(VD->getInit())); + } + } llvm_unreachable("unknown code decl"); } diff --git a/clang/lib/Analysis/BodyFarm.cpp b/clang/lib/Analysis/BodyFarm.cpp index c5f35b3..94ab69a 100644 --- a/clang/lib/Analysis/BodyFarm.cpp +++ b/clang/lib/Analysis/BodyFarm.cpp @@ -293,15 +293,14 @@ static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M, FunctionDecl *callOperatorDecl = CallbackDecl->getLambdaCallOperator(); assert(callOperatorDecl != nullptr); - DeclRefExpr *callOperatorDeclRef = - DeclRefExpr::Create(/* Ctx =*/ C, - /* QualifierLoc =*/ NestedNameSpecifierLoc(), - /* TemplateKWLoc =*/ SourceLocation(), - const_cast<FunctionDecl *>(callOperatorDecl), - /* RefersToEnclosingVariableOrCapture=*/ false, - /* NameLoc =*/ SourceLocation(), - /* T =*/ callOperatorDecl->getType(), - /* VK =*/ VK_LValue); + DeclRefExpr *callOperatorDeclRef = DeclRefExpr::Create( + /* Ctx =*/C, + /* QualifierLoc =*/NestedNameSpecifierLoc(), + /* TemplateKWLoc =*/SourceLocation(), callOperatorDecl, + /* RefersToEnclosingVariableOrCapture=*/false, + /* NameLoc =*/SourceLocation(), + /* T =*/callOperatorDecl->getType(), + /* VK =*/VK_LValue); return CXXOperatorCallExpr::Create( /*AstContext=*/C, OO_Call, callOperatorDeclRef, diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index cdde849..f8407ad 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -1666,6 +1666,12 @@ std::unique_ptr<CFG> CFGBuilder::buildCFG(const Decl *D, Stmt *Statement) { assert(Succ == &cfg->getExit()); Block = nullptr; // the EXIT block is empty. Create all other blocks lazily. + // Add parameters to the initial scope to handle their dtos and lifetime ends. + LocalScope *paramScope = nullptr; + if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D)) + for (ParmVarDecl *PD : FD->parameters()) + paramScope = addLocalScopeForVarDecl(PD, paramScope); + if (BuildOpts.AddImplicitDtors) if (const CXXDestructorDecl *DD = dyn_cast_or_null<CXXDestructorDecl>(D)) addImplicitDtorsForDestructor(DD); @@ -2246,6 +2252,11 @@ LocalScope* CFGBuilder::addLocalScopeForVarDecl(VarDecl *VD, if (!VD->hasLocalStorage()) return Scope; + // Reference parameters are aliases to objects that live elsewhere, so they + // don't require automatic destruction or lifetime tracking. + if (isa<ParmVarDecl>(VD) && VD->getType()->isReferenceType()) + return Scope; + if (!BuildOpts.AddLifetime && !BuildOpts.AddScopes && !needsAutomaticDestruction(VD)) { assert(BuildOpts.AddImplicitDtors); @@ -5616,8 +5627,15 @@ public: bool handleDecl(const Decl *D, raw_ostream &OS) { DeclMapTy::iterator I = DeclMap.find(D); - if (I == DeclMap.end()) + if (I == DeclMap.end()) { + // ParmVarDecls are not declared in the CFG itself, so they do not appear + // in DeclMap. + if (auto *PVD = dyn_cast_or_null<ParmVarDecl>(D)) { + OS << "[Parm: " << PVD->getNameAsString() << "]"; + return true; + } return false; + } if (currentBlock >= 0 && I->second.first == (unsigned) currentBlock && I->second.second == currStmt) { diff --git a/clang/lib/Analysis/Consumed.cpp b/clang/lib/Analysis/Consumed.cpp index f2c714a..efc7098 100644 --- a/clang/lib/Analysis/Consumed.cpp +++ b/clang/lib/Analysis/Consumed.cpp @@ -1354,12 +1354,13 @@ void ConsumedAnalyzer::run(AnalysisDeclContext &AC) { case CFGElement::AutomaticObjectDtor: { const CFGAutomaticObjDtor &DTor = B.castAs<CFGAutomaticObjDtor>(); + const auto *DD = DTor.getDestructorDecl(AC.getASTContext()); + if (!DD) + break; + SourceLocation Loc = DTor.getTriggerStmt()->getEndLoc(); const VarDecl *Var = DTor.getVarDecl(); - - Visitor.checkCallability(PropagationInfo(Var), - DTor.getDestructorDecl(AC.getASTContext()), - Loc); + Visitor.checkCallability(PropagationInfo(Var), DD, Loc); break; } diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp index 2f40c7e..86d7dca 100644 --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -135,6 +135,11 @@ class ExprPointeeResolve { if (const auto *PE = dyn_cast<ParenExpr>(E)) return resolveExpr(PE->getSubExpr()); + if (const auto *UO = dyn_cast<UnaryOperator>(E)) { + if (UO->getOpcode() == UO_AddrOf) + return resolveExpr(UO->getSubExpr()); + } + if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E)) { // only implicit cast needs to be treated as resolvable. // explicit cast will be checked in `findPointeeToNonConst` diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp index 431b1f2..e8113fc 100644 --- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp +++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp @@ -22,8 +22,8 @@ #include "clang/AST/Type.h" #include "clang/Analysis/FlowSensitive/StorageLocation.h" #include "clang/Basic/LLVM.h" -#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SetVector.h" #include <cassert> #include <iterator> #include <vector> @@ -81,7 +81,7 @@ bool containsSameFields(const FieldSet &Fields, const RecordStorageLocation::FieldToLoc &FieldLocs) { if (Fields.size() != FieldLocs.size()) return false; - for ([[maybe_unused]] auto [Field, Loc] : FieldLocs) + for (const auto &Field : FieldLocs.keys()) if (!Fields.contains(cast_or_null<FieldDecl>(Field))) return false; return true; @@ -164,21 +164,21 @@ RecordInitListHelper::RecordInitListHelper( } static void insertIfGlobal(const Decl &D, - llvm::DenseSet<const VarDecl *> &Globals) { + llvm::SetVector<const VarDecl *> &Globals) { if (auto *V = dyn_cast<VarDecl>(&D)) if (V->hasGlobalStorage()) Globals.insert(V); } static void insertIfLocal(const Decl &D, - llvm::DenseSet<const VarDecl *> &Locals) { + llvm::SetVector<const VarDecl *> &Locals) { if (auto *V = dyn_cast<VarDecl>(&D)) if (V->hasLocalStorage() && !isa<ParmVarDecl>(V)) Locals.insert(V); } static void insertIfFunction(const Decl &D, - llvm::DenseSet<const FunctionDecl *> &Funcs) { + llvm::SetVector<const FunctionDecl *> &Funcs) { if (auto *FD = dyn_cast<FunctionDecl>(&D)) Funcs.insert(FD); } diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 0fa333e..d90f5d4 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -1153,26 +1153,34 @@ auto buildDiagnoseMatchSwitch( // FIXME: Evaluate the efficiency of matchers. If using matchers results in a // lot of duplicated work (e.g. string comparisons), consider providing APIs // that avoid it through memoization. - auto IgnorableOptional = ignorableOptional(Options); - return CFGMatchSwitchBuilder< - const Environment, - llvm::SmallVector<UncheckedOptionalAccessDiagnostic>>() - // optional::value - .CaseOfCFGStmt<CXXMemberCallExpr>( - valueCall(IgnorableOptional), - [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, - const Environment &Env) { - return diagnoseUnwrapCall(E->getImplicitObjectArgument(), Env); - }) - - // optional::operator*, optional::operator-> - .CaseOfCFGStmt<CallExpr>(valueOperatorCall(IgnorableOptional), - [](const CallExpr *E, + const auto IgnorableOptional = ignorableOptional(Options); + + auto DiagBuilder = + CFGMatchSwitchBuilder< + const Environment, + llvm::SmallVector<UncheckedOptionalAccessDiagnostic>>() + // optional::operator*, optional::operator-> + .CaseOfCFGStmt<CallExpr>( + valueOperatorCall(IgnorableOptional), + [](const CallExpr *E, const MatchFinder::MatchResult &, + const Environment &Env) { + return diagnoseUnwrapCall(E->getArg(0), Env); + }); + + auto Builder = Options.IgnoreValueCalls + ? std::move(DiagBuilder) + : std::move(DiagBuilder) + // optional::value + .CaseOfCFGStmt<CXXMemberCallExpr>( + valueCall(IgnorableOptional), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, const Environment &Env) { - return diagnoseUnwrapCall(E->getArg(0), Env); - }) - .Build(); + return diagnoseUnwrapCall( + E->getImplicitObjectArgument(), Env); + }); + + return std::move(Builder).Build(); } } // namespace diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index b42bfa3..0949432 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -25,6 +25,7 @@ #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/MatchSwitch.h" #include "clang/Analysis/FlowSensitive/RecordOps.h" +#include "clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h" #include "clang/Analysis/FlowSensitive/StorageLocation.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Basic/LLVM.h" @@ -211,6 +212,74 @@ static auto isStatusConstructor() { using namespace ::clang::ast_matchers; // NOLINT: Too many names return cxxConstructExpr(hasType(statusType())); } +static auto isLoggingGetReferenceableValueCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr(callee( + functionDecl(hasName("::absl::log_internal::GetReferenceableValue")))); +} + +static auto isLoggingCheckEqImpl() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr( + callee(functionDecl(hasName("::absl::log_internal::Check_EQImpl")))); +} + +static auto isAsStatusCallWithStatus() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr( + callee(functionDecl(hasName("::absl::log_internal::AsStatus"))), + hasArgument(0, hasType(statusClass()))); +} + +static auto isAsStatusCallWithStatusOr() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr( + callee(functionDecl(hasName("::absl::log_internal::AsStatus"))), + hasArgument(0, hasType(statusOrType()))); +} + +static auto possiblyReferencedStatusOrType() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return anyOf(statusOrType(), referenceType(pointee(statusOrType()))); +} + +static auto isConstStatusOrAccessorMemberCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxMemberCallExpr(callee( + cxxMethodDecl(parameterCountIs(0), isConst(), + returns(qualType(possiblyReferencedStatusOrType()))))); +} + +static auto isConstStatusOrAccessorMemberOperatorCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr( + callee(cxxMethodDecl(parameterCountIs(0), isConst(), + returns(possiblyReferencedStatusOrType())))); +} + +static auto isConstStatusOrPointerAccessorMemberCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxMemberCallExpr(callee(cxxMethodDecl( + parameterCountIs(0), isConst(), + returns(pointerType(pointee(possiblyReferencedStatusOrType())))))); +} + +static auto isConstStatusOrPointerAccessorMemberOperatorCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr(callee(cxxMethodDecl( + parameterCountIs(0), isConst(), + returns(pointerType(pointee(possiblyReferencedStatusOrType())))))); +} + +static auto isNonConstMemberCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxMemberCallExpr(callee(cxxMethodDecl(unless(isConst())))); +} + +static auto isNonConstMemberOperatorCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr(callee(cxxMethodDecl(unless(isConst())))); +} static auto buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) { @@ -602,6 +671,194 @@ static void transferStatusConstructor(const CXXConstructExpr *Expr, if (State.Env.getValue(locForOk(StatusLoc)) == nullptr) initializeStatus(StatusLoc, State.Env); } +static void +transferLoggingGetReferenceableValueCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() == 1); + if (Expr->getArg(0)->isPRValue()) + return; + auto *ArgLoc = State.Env.getStorageLocation(*Expr->getArg(0)); + if (ArgLoc == nullptr) + return; + + State.Env.setStorageLocation(*Expr, *ArgLoc); +} + +static void transferLoggingCheckEqImpl(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() > 2); + + auto *EqVal = evaluateEquality(Expr->getArg(0), Expr->getArg(1), State.Env); + if (EqVal == nullptr) + return; + + // Consider modelling this more accurately instead of assigning BoolValue + // as the value of an expression of pointer type. + // For now, this is being handled in transferPointerToBoolean. + State.Env.setValue(*Expr, State.Env.makeNot(*EqVal)); +} + +static void transferAsStatusCallWithStatus(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() == 1); + + auto *ArgLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0)); + if (ArgLoc == nullptr) + return; + + if (State.Env.getValue(locForOk(*ArgLoc)) == nullptr) + initializeStatus(*ArgLoc, State.Env); + + auto &ExprVal = State.Env.create<PointerValue>(*ArgLoc); + State.Env.setValue(*Expr, ExprVal); +} + +static void transferAsStatusCallWithStatusOr(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() == 1); + + auto *ArgLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0)); + if (ArgLoc == nullptr) + return; + + RecordStorageLocation &StatusLoc = locForStatus(*ArgLoc); + + if (State.Env.getValue(locForOk(StatusLoc)) == nullptr) + initializeStatusOr(*ArgLoc, State.Env); + + auto &ExprVal = State.Env.create<PointerValue>(StatusLoc); + State.Env.setValue(*Expr, ExprVal); +} + +static void transferPointerToBoolean(const ImplicitCastExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + if (auto *SubExprVal = + dyn_cast_or_null<BoolValue>(State.Env.getValue(*Expr->getSubExpr()))) + State.Env.setValue(*Expr, *SubExprVal); +} + +static void transferStatusOrReturningCall(const CallExpr *Expr, + LatticeTransferState &State) { + RecordStorageLocation *StatusOrLoc = + Expr->isPRValue() ? &State.Env.getResultObjectLocation(*Expr) + : State.Env.get<RecordStorageLocation>(*Expr); + if (StatusOrLoc != nullptr && + State.Env.getValue(locForOk(locForStatus(*StatusOrLoc))) == nullptr) + initializeStatusOr(*StatusOrLoc, State.Env); +} + +static bool doHandleConstStatusOrAccessorMemberCall( + const CallExpr *Expr, RecordStorageLocation *RecordLoc, + const MatchFinder::MatchResult &Result, LatticeTransferState &State) { + assert(isStatusOrType(Expr->getType())); + if (RecordLoc == nullptr) + return false; + const FunctionDecl *DirectCallee = Expr->getDirectCallee(); + if (DirectCallee == nullptr) + return false; + StorageLocation &Loc = + State.Lattice.getOrCreateConstMethodReturnStorageLocation( + *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) { + initializeStatusOr(cast<RecordStorageLocation>(Loc), State.Env); + }); + if (Expr->isPRValue()) { + auto &ResultLoc = State.Env.getResultObjectLocation(*Expr); + copyRecord(cast<RecordStorageLocation>(Loc), ResultLoc, State.Env); + } else { + State.Env.setStorageLocation(*Expr, Loc); + } + return true; +} + +static void handleConstStatusOrAccessorMemberCall( + const CallExpr *Expr, RecordStorageLocation *RecordLoc, + const MatchFinder::MatchResult &Result, LatticeTransferState &State) { + if (!doHandleConstStatusOrAccessorMemberCall(Expr, RecordLoc, Result, State)) + transferStatusOrReturningCall(Expr, State); +} +static void handleConstStatusOrPointerAccessorMemberCall( + const CallExpr *Expr, RecordStorageLocation *RecordLoc, + const MatchFinder::MatchResult &Result, LatticeTransferState &State) { + if (RecordLoc == nullptr) + return; + auto *Val = State.Lattice.getOrCreateConstMethodReturnValue(*RecordLoc, Expr, + State.Env); + State.Env.setValue(*Expr, *Val); +} + +static void +transferConstStatusOrAccessorMemberCall(const CXXMemberCallExpr *Expr, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + handleConstStatusOrAccessorMemberCall( + Expr, getImplicitObjectLocation(*Expr, State.Env), Result, State); +} + +static void transferConstStatusOrAccessorMemberOperatorCall( + const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + auto *RecordLoc = cast_or_null<RecordStorageLocation>( + State.Env.getStorageLocation(*Expr->getArg(0))); + handleConstStatusOrAccessorMemberCall(Expr, RecordLoc, Result, State); +} + +static void transferConstStatusOrPointerAccessorMemberCall( + const CXXMemberCallExpr *Expr, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + handleConstStatusOrPointerAccessorMemberCall( + Expr, getImplicitObjectLocation(*Expr, State.Env), Result, State); +} + +static void transferConstStatusOrPointerAccessorMemberOperatorCall( + const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + auto *RecordLoc = cast_or_null<RecordStorageLocation>( + State.Env.getStorageLocation(*Expr->getArg(0))); + handleConstStatusOrPointerAccessorMemberCall(Expr, RecordLoc, Result, State); +} + +static void handleNonConstMemberCall(const CallExpr *Expr, + RecordStorageLocation *RecordLoc, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + if (RecordLoc) { + State.Lattice.clearConstMethodReturnValues(*RecordLoc); + State.Lattice.clearConstMethodReturnStorageLocations(*RecordLoc); + } + if (isStatusOrType(Expr->getType())) + transferStatusOrReturningCall(Expr, State); +} + +static void transferNonConstMemberCall(const CXXMemberCallExpr *Expr, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + handleNonConstMemberCall(Expr, getImplicitObjectLocation(*Expr, State.Env), + Result, State); +} + +static void +transferNonConstMemberOperatorCall(const CXXOperatorCallExpr *Expr, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + auto *RecordLoc = cast_or_null<RecordStorageLocation>( + State.Env.getStorageLocation(*Expr->getArg(0))); + handleNonConstMemberCall(Expr, RecordLoc, Result, State); +} + +static RecordStorageLocation * +getSmartPtrLikeStorageLocation(const Expr &E, const Environment &Env) { + if (!E.isPRValue()) + return dyn_cast_or_null<RecordStorageLocation>(Env.getStorageLocation(E)); + if (auto *PointerVal = dyn_cast_or_null<PointerValue>(Env.getValue(E))) + return dyn_cast_or_null<RecordStorageLocation>( + &PointerVal->getPointeeLoc()); + return nullptr; +} CFGMatchSwitch<LatticeTransferState> buildTransferMatchSwitch(ASTContext &Ctx, @@ -652,6 +909,68 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferValueAssignmentCall) .CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(), transferValueConstructor) + .CaseOfCFGStmt<CallExpr>(isAsStatusCallWithStatus(), + transferAsStatusCallWithStatus) + .CaseOfCFGStmt<CallExpr>(isAsStatusCallWithStatusOr(), + transferAsStatusCallWithStatusOr) + .CaseOfCFGStmt<CallExpr>(isLoggingGetReferenceableValueCall(), + transferLoggingGetReferenceableValueCall) + .CaseOfCFGStmt<CallExpr>(isLoggingCheckEqImpl(), + transferLoggingCheckEqImpl) + // This needs to go before the const accessor call matcher, because these + // look like them, but we model `operator`* and `get` to return the same + // object. Also, we model them for non-const cases. + .CaseOfCFGStmt<CXXOperatorCallExpr>( + isPointerLikeOperatorStar(), + [](const CXXOperatorCallExpr *E, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedDeref( + E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env), + State, [](StorageLocation &Loc) {}); + }) + .CaseOfCFGStmt<CXXOperatorCallExpr>( + isPointerLikeOperatorArrow(), + [](const CXXOperatorCallExpr *E, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedGet( + E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env), + State, [](StorageLocation &Loc) {}); + }) + .CaseOfCFGStmt<CXXMemberCallExpr>( + isSmartPointerLikeValueMethodCall(), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedDeref( + E, getImplicitObjectLocation(*E, State.Env), State, + [](StorageLocation &Loc) {}); + }) + .CaseOfCFGStmt<CXXMemberCallExpr>( + isSmartPointerLikeGetMethodCall(), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedGet( + E, getImplicitObjectLocation(*E, State.Env), State, + [](StorageLocation &Loc) {}); + }) + // const accessor calls + .CaseOfCFGStmt<CXXMemberCallExpr>(isConstStatusOrAccessorMemberCall(), + transferConstStatusOrAccessorMemberCall) + .CaseOfCFGStmt<CXXOperatorCallExpr>( + isConstStatusOrAccessorMemberOperatorCall(), + transferConstStatusOrAccessorMemberOperatorCall) + .CaseOfCFGStmt<CXXMemberCallExpr>( + isConstStatusOrPointerAccessorMemberCall(), + transferConstStatusOrPointerAccessorMemberCall) + .CaseOfCFGStmt<CXXOperatorCallExpr>( + isConstStatusOrPointerAccessorMemberOperatorCall(), + transferConstStatusOrPointerAccessorMemberOperatorCall) + // non-const member calls that may modify the state of an object. + .CaseOfCFGStmt<CXXMemberCallExpr>(isNonConstMemberCall(), + transferNonConstMemberCall) + .CaseOfCFGStmt<CXXOperatorCallExpr>(isNonConstMemberOperatorCall(), + transferNonConstMemberOperatorCall) // N.B. These need to come after all other CXXConstructExpr. // These are there to make sure that every Status and StatusOr object // have their ok boolean initialized when constructed. If we were to @@ -662,6 +981,9 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferStatusOrConstructor) .CaseOfCFGStmt<CXXConstructExpr>(isStatusConstructor(), transferStatusConstructor) + .CaseOfCFGStmt<ImplicitCastExpr>( + implicitCastExpr(hasCastKind(CK_PointerToBoolean)), + transferPointerToBoolean) .Build(); } diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 06f1278..0574835 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -769,8 +769,29 @@ public: StorageLocation *TrueLoc = TrueEnv->getStorageLocation(*S->getTrueExpr()); StorageLocation *FalseLoc = FalseEnv->getStorageLocation(*S->getFalseExpr()); - if (TrueLoc == FalseLoc && TrueLoc != nullptr) + if (TrueLoc == FalseLoc && TrueLoc != nullptr) { Env.setStorageLocation(*S, *TrueLoc); + } else if (!S->getType()->isRecordType()) { + // Ideally, we would have something like an "alias set" to say that the + // result StorageLocation can be either of the locations from the + // TrueEnv or FalseEnv. Then, when this ConditionalOperator is + // (a) used in an LValueToRValue cast, the value is the join of all of + // the values in the alias set. + // (b) or, used in an assignment to the resulting LValue, the assignment + // *may* update all of the locations in the alias set. + // For now, we do the simpler thing of creating a new StorageLocation + // and joining the values right away, handling only case (a). + // Otherwise, the dataflow framework needs to be updated be able to + // represent alias sets and weak updates (for the "may"). + if (Value *Val = Environment::joinValues( + S->getType(), TrueEnv->getValue(*S->getTrueExpr()), *TrueEnv, + FalseEnv->getValue(*S->getFalseExpr()), *FalseEnv, Env, + Model)) { + StorageLocation &Loc = Env.createStorageLocation(*S); + Env.setStorageLocation(*S, Loc); + Env.setValue(Loc, *Val); + } + } } else if (!S->getType()->isRecordType()) { // The conditional operator can evaluate to either of the values of the // two branches. To model this, join these two values together to yield diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index c443c3a..7479276 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -43,13 +43,14 @@ namespace { /// Struct to store the complete context for a potential lifetime violation. struct PendingWarning { SourceLocation ExpiryLoc; // Where the loan expired. - const Expr *UseExpr; // Where the origin holding this loan was used. + llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact; Confidence ConfidenceLevel; }; class LifetimeChecker { private: llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap; + llvm::DenseMap<const ParmVarDecl *, const Expr *> AnnotationWarningsMap; const LoanPropagationAnalysis &LoanPropagation; const LiveOriginsAnalysis &LiveOrigins; const FactManager &FactMgr; @@ -65,10 +66,29 @@ public: for (const Fact *F : FactMgr.getFacts(B)) if (const auto *EF = F->getAs<ExpireFact>()) checkExpiry(EF); + else if (const auto *OEF = F->getAs<OriginEscapesFact>()) + checkAnnotations(OEF); issuePendingWarnings(); + suggestAnnotations(); } - /// Checks for use-after-free errors when a loan expires. + /// Checks if an escaping origin holds a placeholder loan, indicating a + /// missing [[clang::lifetimebound]] annotation. + void checkAnnotations(const OriginEscapesFact *OEF) { + OriginID EscapedOID = OEF->getEscapedOriginID(); + LoanSet EscapedLoans = LoanPropagation.getLoans(EscapedOID, OEF); + for (LoanID LID : EscapedLoans) { + const Loan *L = FactMgr.getLoanMgr().getLoan(LID); + if (const auto *PL = dyn_cast<PlaceholderLoan>(L)) { + const ParmVarDecl *PVD = PL->getParmVarDecl(); + if (PVD->hasAttr<LifetimeBoundAttr>()) + continue; + AnnotationWarningsMap.try_emplace(PVD, OEF->getEscapeExpr()); + } + } + } + + /// Checks for use-after-free & use-after-return errors when a loan expires. /// /// This method examines all live origins at the expiry point and determines /// if any of them hold the expiring loan. If so, it creates a pending @@ -83,7 +103,11 @@ public: LoanID ExpiredLoan = EF->getLoanID(); LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); Confidence CurConfidence = Confidence::None; - const UseFact *BadUse = nullptr; + // The UseFact or OriginEscapesFact most indicative of a lifetime error, + // prioritized by earlier source location. + llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> + BestCausingFact = nullptr; + for (auto &[OID, LiveInfo] : Origins) { LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF); if (!HeldLoans.contains(ExpiredLoan)) @@ -92,17 +116,17 @@ public: Confidence NewConfidence = livenessKindToConfidence(LiveInfo.Kind); if (CurConfidence < NewConfidence) { CurConfidence = NewConfidence; - BadUse = LiveInfo.CausingUseFact; + BestCausingFact = LiveInfo.CausingFact; } } - if (!BadUse) + if (!BestCausingFact) return; // We have a use-after-free. Confidence LastConf = FinalWarningsMap.lookup(ExpiredLoan).ConfidenceLevel; if (LastConf >= CurConfidence) return; FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(), - /*UseExpr=*/BadUse->getUseExpr(), + /*BestCausingFact=*/BestCausingFact, /*ConfidenceLevel=*/CurConfidence}; } @@ -110,12 +134,32 @@ public: if (!Reporter) return; for (const auto &[LID, Warning] : FinalWarningsMap) { - const Loan &L = FactMgr.getLoanMgr().getLoan(LID); - const Expr *IssueExpr = L.IssueExpr; - Reporter->reportUseAfterFree(IssueExpr, Warning.UseExpr, - Warning.ExpiryLoc, Warning.ConfidenceLevel); + const Loan *L = FactMgr.getLoanMgr().getLoan(LID); + const auto *BL = cast<PathLoan>(L); + const Expr *IssueExpr = BL->getIssueExpr(); + llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> + CausingFact = Warning.CausingFact; + Confidence Confidence = Warning.ConfidenceLevel; + SourceLocation ExpiryLoc = Warning.ExpiryLoc; + + if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) + Reporter->reportUseAfterFree(IssueExpr, UF->getUseExpr(), ExpiryLoc, + Confidence); + else if (const auto *OEF = + CausingFact.dyn_cast<const OriginEscapesFact *>()) + Reporter->reportUseAfterReturn(IssueExpr, OEF->getEscapeExpr(), + ExpiryLoc, Confidence); + else + llvm_unreachable("Unhandled CausingFact type"); } } + + void suggestAnnotations() { + if (!Reporter) + return; + for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) + Reporter->suggestAnnotation(PVD, EscapeExpr); + } }; } // namespace diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index de821bb..05c20d6 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -170,8 +170,8 @@ private: return D->transfer(In, *F->getAs<ExpireFact>()); case Fact::Kind::OriginFlow: return D->transfer(In, *F->getAs<OriginFlowFact>()); - case Fact::Kind::ReturnOfOrigin: - return D->transfer(In, *F->getAs<ReturnOfOriginFact>()); + case Fact::Kind::OriginEscapes: + return D->transfer(In, *F->getAs<OriginEscapesFact>()); case Fact::Kind::Use: return D->transfer(In, *F->getAs<UseFact>()); case Fact::Kind::TestPoint: @@ -184,7 +184,7 @@ public: Lattice transfer(Lattice In, const IssueFact &) { return In; } Lattice transfer(Lattice In, const ExpireFact &) { return In; } Lattice transfer(Lattice In, const OriginFlowFact &) { return In; } - Lattice transfer(Lattice In, const ReturnOfOriginFact &) { return In; } + Lattice transfer(Lattice In, const OriginEscapesFact &) { return In; } Lattice transfer(Lattice In, const UseFact &) { return In; } Lattice transfer(Lattice In, const TestPointFact &) { return In; } }; diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 190c038..6831731 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -20,7 +20,7 @@ void Fact::dump(llvm::raw_ostream &OS, const LoanManager &, void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &OM) const { OS << "Issue ("; - LM.getLoan(getLoanID()).dump(OS); + LM.getLoan(getLoanID())->dump(OS); OS << ", ToOrigin: "; OM.dump(getOriginID(), OS); OS << ")\n"; @@ -29,7 +29,7 @@ void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, void ExpireFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &) const { OS << "Expire ("; - LM.getLoan(getLoanID()).dump(OS); + LM.getLoan(getLoanID())->dump(OS); OS << ")\n"; } @@ -43,10 +43,10 @@ void OriginFlowFact::dump(llvm::raw_ostream &OS, const LoanManager &, OS << ")\n"; } -void ReturnOfOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { - OS << "ReturnOfOrigin ("; - OM.dump(getReturnedOriginID(), OS); +void OriginEscapesFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "OriginEscapes ("; + OM.dump(getEscapedOriginID(), OS); OS << ")\n"; } @@ -95,4 +95,14 @@ void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC) const { } } +llvm::ArrayRef<const Fact *> +FactManager::getBlockContaining(ProgramPoint P) const { + for (const auto &BlockToFactsVec : BlockToFacts) { + for (const Fact *F : BlockToFactsVec) + if (F == P) + return BlockToFactsVec; + } + return {}; +} + } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index bec8e1d..2f270b0 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -15,18 +15,6 @@ namespace clang::lifetimes::internal { using llvm::isa_and_present; -static bool isGslPointerType(QualType QT) { - if (const auto *RD = QT->getAsCXXRecordDecl()) { - // We need to check the template definition for specializations. - if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) - return CTSD->getSpecializedTemplate() - ->getTemplatedDecl() - ->hasAttr<PointerAttr>(); - return RD->hasAttr<PointerAttr>(); - } - return false; -} - static bool isPointerType(QualType QT) { return QT->isPointerOrReferenceType() || isGslPointerType(QT); } @@ -43,29 +31,38 @@ static bool hasOrigin(const VarDecl *VD) { /// This function should be called whenever a DeclRefExpr represents a borrow. /// \param DRE The declaration reference expression that initiates the borrow. /// \return The new Loan on success, nullptr otherwise. -static const Loan *createLoan(FactManager &FactMgr, const DeclRefExpr *DRE) { +static const PathLoan *createLoan(FactManager &FactMgr, + const DeclRefExpr *DRE) { if (const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl())) { AccessPath Path(VD); // The loan is created at the location of the DeclRefExpr. - return &FactMgr.getLoanMgr().addLoan(Path, DRE); + return FactMgr.getLoanMgr().createLoan<PathLoan>(Path, DRE); } return nullptr; } void FactsGenerator::run() { llvm::TimeTraceScope TimeProfile("FactGenerator"); + const CFG &Cfg = *AC.getCFG(); + llvm::SmallVector<Fact *> PlaceholderLoanFacts = issuePlaceholderLoans(); // Iterate through the CFG blocks in reverse post-order to ensure that // initializations and destructions are processed in the correct sequence. for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) { CurrentBlockFacts.clear(); + EscapesInCurrentBlock.clear(); + if (Block == &Cfg.getEntry()) + CurrentBlockFacts.append(PlaceholderLoanFacts.begin(), + PlaceholderLoanFacts.end()); for (unsigned I = 0; I < Block->size(); ++I) { const CFGElement &Element = Block->Elements[I]; if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>()) Visit(CS->getStmt()); - else if (std::optional<CFGAutomaticObjDtor> DtorOpt = - Element.getAs<CFGAutomaticObjDtor>()) - handleDestructor(*DtorOpt); + else if (std::optional<CFGLifetimeEnds> LifetimeEnds = + Element.getAs<CFGLifetimeEnds>()) + handleLifetimeEnds(*LifetimeEnds); } + CurrentBlockFacts.append(EscapesInCurrentBlock.begin(), + EscapesInCurrentBlock.end()); FactMgr.addBlockFacts(Block, CurrentBlockFacts); } } @@ -94,7 +91,7 @@ void FactsGenerator::VisitDeclRefExpr(const DeclRefExpr *DRE) { if (const Loan *L = createLoan(FactMgr, DRE)) { OriginID ExprOID = FactMgr.getOriginMgr().getOrCreate(*DRE); CurrentBlockFacts.push_back( - FactMgr.createFact<IssueFact>(L->ID, ExprOID)); + FactMgr.createFact<IssueFact>(L->getID(), ExprOID)); } } } @@ -166,7 +163,8 @@ void FactsGenerator::VisitReturnStmt(const ReturnStmt *RS) { if (const Expr *RetExpr = RS->getRetValue()) { if (hasOrigin(RetExpr)) { OriginID OID = FactMgr.getOriginMgr().getOrCreate(*RetExpr); - CurrentBlockFacts.push_back(FactMgr.createFact<ReturnOfOriginFact>(OID)); + EscapesInCurrentBlock.push_back( + FactMgr.createFact<OriginEscapesFact>(OID, RetExpr)); } } } @@ -176,6 +174,15 @@ void FactsGenerator::VisitBinaryOperator(const BinaryOperator *BO) { handleAssignment(BO->getLHS(), BO->getRHS()); } +void FactsGenerator::VisitConditionalOperator(const ConditionalOperator *CO) { + if (hasOrigin(CO)) { + // Merge origins from both branches of the conditional operator. + // We kill to clear the initial state and merge both origins into it. + killAndFlowOrigin(*CO, *CO->getTrueExpr()); + flowOrigin(*CO, *CO->getFalseExpr()); + } +} + void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) { // Assignment operators have special "kill-then-propagate" semantics // and are handled separately. @@ -216,25 +223,20 @@ void FactsGenerator::VisitMaterializeTemporaryExpr( killAndFlowOrigin(*MTE, *MTE->getSubExpr()); } -void FactsGenerator::handleDestructor(const CFGAutomaticObjDtor &DtorOpt) { - /// TODO: Also handle trivial destructors (e.g., for `int` - /// variables) which will never have a CFGAutomaticObjDtor node. +void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { /// TODO: Handle loans to temporaries. - /// TODO: Consider using clang::CFG::BuildOptions::AddLifetime to reuse the - /// lifetime ends. - const VarDecl *DestructedVD = DtorOpt.getVarDecl(); - if (!DestructedVD) + const VarDecl *LifetimeEndsVD = LifetimeEnds.getVarDecl(); + if (!LifetimeEndsVD) return; // Iterate through all loans to see if any expire. - /// TODO(opt): Do better than a linear search to find loans associated with - /// 'DestructedVD'. - for (const Loan &L : FactMgr.getLoanMgr().getLoans()) { - const AccessPath &LoanPath = L.Path; - // Check if the loan is for a stack variable and if that variable - // is the one being destructed. - if (LoanPath.D == DestructedVD) - CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( - L.ID, DtorOpt.getTriggerStmt()->getEndLoc())); + for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { + if (const auto *BL = dyn_cast<PathLoan>(Loan)) { + // Check if the loan is for a stack variable and if that variable + // is the one being destructed. + if (BL->getAccessPath().D == LifetimeEndsVD) + CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( + BL->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc())); + } } } @@ -347,4 +349,24 @@ void FactsGenerator::markUseAsWrite(const DeclRefExpr *DRE) { UseFacts[DRE]->markAsWritten(); } +// Creates an IssueFact for a new placeholder loan for each pointer or reference +// parameter at the function's entry. +llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { + const auto *FD = dyn_cast<FunctionDecl>(AC.getDecl()); + if (!FD) + return {}; + + llvm::SmallVector<Fact *> PlaceholderLoanFacts; + for (const ParmVarDecl *PVD : FD->parameters()) { + if (hasOrigin(PVD)) { + const PlaceholderLoan *L = + FactMgr.getLoanMgr().createLoan<PlaceholderLoan>(PVD); + OriginID OID = FactMgr.getOriginMgr().getOrCreate(*PVD); + PlaceholderLoanFacts.push_back( + FactMgr.createFact<IssueFact>(L->getID(), OID)); + } + } + return PlaceholderLoanFacts; +} + } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index ad61a42..54e343f 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -10,6 +10,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" @@ -70,4 +71,34 @@ bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { return isNormalAssignmentOperator(FD); } +template <typename T> static bool isRecordWithAttr(QualType Type) { + auto *RD = Type->getAsCXXRecordDecl(); + if (!RD) + return false; + // Generally, if a primary template class declaration is annotated with an + // attribute, all its specializations generated from template instantiations + // should inherit the attribute. + // + // However, since lifetime analysis occurs during parsing, we may encounter + // cases where a full definition of the specialization is not required. In + // such cases, the specialization declaration remains incomplete and lacks the + // attribute. Therefore, we fall back to checking the primary template class. + // + // Note: it is possible for a specialization declaration to have an attribute + // even if the primary template does not. + // + // FIXME: What if the primary template and explicit specialization + // declarations have conflicting attributes? We should consider diagnosing + // this scenario. + bool Result = RD->hasAttr<T>(); + + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) + Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>(); + + return Result; +} + +bool isGslPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); } +bool isGslOwnerType(QualType QT) { return isRecordWithAttr<OwnerAttr>(QT); } + } // namespace clang::lifetimes diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index 59f594e..5733812 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -53,6 +53,14 @@ struct Lattice { } }; +static SourceLocation GetFactLoc(CausingFactType F) { + if (const auto *UF = F.dyn_cast<const UseFact *>()) + return UF->getUseExpr()->getExprLoc(); + if (const auto *OEF = F.dyn_cast<const OriginEscapesFact *>()) + return OEF->getEscapeExpr()->getExprLoc(); + llvm_unreachable("unhandled causing fact in PointerUnion"); +} + /// The analysis that tracks which origins are live, with granular information /// about the causing use fact and confidence level. This is a backward /// analysis. @@ -74,11 +82,14 @@ public: /// one. Lattice join(Lattice L1, Lattice L2) const { LivenessMap Merged = L1.LiveOrigins; - // Take the earliest UseFact to make the join hermetic and commutative. - auto CombineUseFact = [](const UseFact &A, - const UseFact &B) -> const UseFact * { - return A.getUseExpr()->getExprLoc() < B.getUseExpr()->getExprLoc() ? &A - : &B; + // Take the earliest Fact to make the join hermetic and commutative. + auto CombineCausingFact = [](CausingFactType A, + CausingFactType B) -> CausingFactType { + if (!A) + return B; + if (!B) + return A; + return GetFactLoc(A) < GetFactLoc(B) ? A : B; }; auto CombineLivenessKind = [](LivenessKind K1, LivenessKind K2) -> LivenessKind { @@ -93,12 +104,11 @@ public: const LivenessInfo *L2) -> LivenessInfo { assert((L1 || L2) && "unexpectedly merging 2 empty sets"); if (!L1) - return LivenessInfo(L2->CausingUseFact, LivenessKind::Maybe); + return LivenessInfo(L2->CausingFact, LivenessKind::Maybe); if (!L2) - return LivenessInfo(L1->CausingUseFact, LivenessKind::Maybe); - return LivenessInfo( - CombineUseFact(*L1->CausingUseFact, *L2->CausingUseFact), - CombineLivenessKind(L1->Kind, L2->Kind)); + return LivenessInfo(L1->CausingFact, LivenessKind::Maybe); + return LivenessInfo(CombineCausingFact(L1->CausingFact, L2->CausingFact), + CombineLivenessKind(L1->Kind, L2->Kind)); }; return Lattice(utils::join( L1.LiveOrigins, L2.LiveOrigins, Factory, CombineLivenessInfo, @@ -120,6 +130,14 @@ public: LivenessInfo(&UF, LivenessKind::Must))); } + /// An escaping origin (e.g., via return) makes the origin live with definite + /// confidence, as it dominates this program point. + Lattice transfer(Lattice In, const OriginEscapesFact &OEF) { + OriginID OID = OEF.getEscapedOriginID(); + return Lattice(Factory.add(In.LiveOrigins, OID, + LivenessInfo(&OEF, LivenessKind::Must))); + } + /// Issuing a new loan to an origin kills its liveness. Lattice transfer(Lattice In, const IssueFact &IF) { return Lattice(Factory.remove(In.LiveOrigins, IF.getOriginID())); diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 0e6c194..23ce1b7 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -58,12 +58,10 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, CheckOrigin(OF->getSrcOriginID()); break; } - case Fact::Kind::ReturnOfOrigin: - CheckOrigin(F->getAs<ReturnOfOriginFact>()->getReturnedOriginID()); - break; case Fact::Kind::Use: CheckOrigin(F->getAs<UseFact>()->getUsedOrigin()); break; + case Fact::Kind::OriginEscapes: case Fact::Kind::Expire: case Fact::Kind::TestPoint: break; diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index 2c85a3c..fdfdbb4 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -10,9 +10,13 @@ namespace clang::lifetimes::internal { -void Loan::dump(llvm::raw_ostream &OS) const { - OS << ID << " (Path: "; +void PathLoan::dump(llvm::raw_ostream &OS) const { + OS << getID() << " (Path: "; OS << Path.D->getNameAsString() << ")"; } +void PlaceholderLoan::dump(llvm::raw_ostream &OS) const { + OS << getID() << " (Placeholder loan)"; +} + } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index ea51a75..0f2eaa9 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -34,6 +34,8 @@ Origin &OriginManager::addOrigin(OriginID ID, const clang::Expr &E) { // TODO: Mark this method as const once we remove the call to getOrCreate. OriginID OriginManager::get(const Expr &E) { + if (auto *ParenIgnored = E.IgnoreParens(); ParenIgnored != &E) + return get(*ParenIgnored); auto It = ExprToOriginID.find(&E); if (It != ExprToOriginID.end()) return It->second; diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index 77750cf..c8d6153 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -43,6 +43,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Allocator.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TrailingObjects.h" #include "llvm/Support/raw_ostream.h" @@ -2820,7 +2821,13 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { case CFGElement::AutomaticObjectDtor: { CFGAutomaticObjDtor AD = BI.castAs<CFGAutomaticObjDtor>(); const auto *DD = AD.getDestructorDecl(AC.getASTContext()); - if (!DD->hasAttrs()) + // Function parameters as they are constructed in caller's context and + // the CFG does not contain the ctors. Ignore them as their + // capabilities cannot be analysed because of this missing + // information. + if (isa_and_nonnull<ParmVarDecl>(AD.getVarDecl())) + break; + if (!DD || !DD->hasAttrs()) break; LocksetBuilder.handleCall( diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index f5a3686..da155d3 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -781,9 +781,25 @@ struct LibcFunNamePrefixSuffixParser { } }; +// Constant fold a conditional expression 'cond ? A : B' to +// - 'A', if 'cond' has constant true value; +// - 'B', if 'cond' has constant false value. +static const Expr *tryConstantFoldConditionalExpr(const Expr *E, + const ASTContext &Ctx) { + // FIXME: more places can use this function + if (const auto *CE = dyn_cast<ConditionalOperator>(E)) { + bool CondEval; + + if (CE->getCond()->EvaluateAsBooleanCondition(CondEval, Ctx)) + return CondEval ? CE->getLHS() : CE->getRHS(); + } + return E; +} + // A pointer type expression is known to be null-terminated, if it has the // form: E.c_str(), for any expression E of `std::string` type. -static bool isNullTermPointer(const Expr *Ptr) { +static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) { + Ptr = tryConstantFoldConditionalExpr(Ptr, Ctx); if (isa<clang::StringLiteral>(Ptr->IgnoreParenImpCasts())) return true; if (isa<PredefinedExpr>(Ptr->IgnoreParenImpCasts())) @@ -874,7 +890,7 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg, const Expr *Arg = Call->getArg(ArgIdx); - if (isNullTermPointer(Arg)) + if (isNullTermPointer(Arg, Ctx)) // If Arg is a null-terminated pointer, it is safe anyway. return true; // continue parsing @@ -922,8 +938,8 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg, // (including the format argument) is unsafe pointer. return llvm::any_of( llvm::make_range(Call->arg_begin() + FmtArgIdx, Call->arg_end()), - [&UnsafeArg](const Expr *Arg) -> bool { - if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg)) { + [&UnsafeArg, &Ctx](const Expr *Arg) -> bool { + if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg, Ctx)) { UnsafeArg = Arg; return true; } @@ -1175,7 +1191,7 @@ static bool hasUnsafePrintfStringArg(const CallExpr &Node, ASTContext &Ctx, // We don't really recognize this "normal" printf, the only thing we // can do is to require all pointers to be null-terminated: for (const auto *Arg : Node.arguments()) - if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg)) { + if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg, Ctx)) { Result.addNode(Tag, DynTypedNode::create(*Arg)); return true; } |