diff --git a/clang/include/clang/Sema/BoundsAnalysis.h b/clang/include/clang/Sema/BoundsAnalysis.h index f25e98cceb5f..98b5a524f2b8 100644 --- a/clang/include/clang/Sema/BoundsAnalysis.h +++ b/clang/include/clang/Sema/BoundsAnalysis.h @@ -116,9 +116,6 @@ namespace clang { EdgeBoundsTy Gen, Out; // The Kill set for the block. StmtDeclSetTy Kill; - // The set of all variables used in bounds expr for each ntptr in the - // block. - BoundsVarTy BoundsVars; // To compute In[B] we compute the intersection of Out[B*->B], where B* // are all preds of B. When there is a back edge from block B' to B (for @@ -156,9 +153,9 @@ namespace clang { // to lookup ElevatedCFGBlock from CFGBlock. BlockMapTy BlockMap; - // A set of all ntptrs in scope. Currently, we simply collect all ntptrs - // defined in the function. - DeclSetTy NtPtrsInScope; + // The mapping of all ntptrs in the function and all variables occurring in + // the bounds expr for each ntptr. + BoundsVarTy NtPtrsInScope; // To compute In[B] we compute the intersection of Out[B*->B], where B* are // all preds of B. When there is a back edge from block B' to B (for @@ -246,14 +243,13 @@ namespace clang { // @param[in] Dest block for the edge for which the Gen set is updated. void FillGenSet(Expr *E, ElevatedCFGBlock *EB, ElevatedCFGBlock *SuccEB); - // Uniformize the expr, fill Gen set and get variables used in bounds expr - // for the ntptr. + // Uniformize the expr, fill Gen set for the edge EB->SuccEB. // @param[in] E is an ntptr dereference or array subscript expr. // @param[in] Source block for the edge for which the Gen set is updated. // @param[in] Dest block for the edge for which the Gen set is updated. - void FillGenSetAndGetBoundsVars(const Expr *E, - ElevatedCFGBlock *EB, - ElevatedCFGBlock *SuccEB); + void FillGenSetForEdge(const Expr *E, + ElevatedCFGBlock *EB, + ElevatedCFGBlock *SuccEB); // Collect all variables used in bounds expr E. // @param[in] E represents the bounds expr for an ntptr. @@ -297,17 +293,9 @@ namespace clang { // Get the DeclRefExpr from an expression E. // @param[in] An expression E which is known to be either an LValueToRValue // cast or an ArrayToPointerDecay cast. - // @return The DeclRefExpr from the expression E. + // @return The DeclRefExpr from the expression E or nullptr. DeclRefExpr *GetDeclOperand(const Expr *E); - // A DeclRefExpr can be a reference either to an array subscript (in which - // case it is wrapped around a ArrayToPointerDecay cast) or to a pointer - // dereference (in which case it is wrapped around an LValueToRValue cast). - // @param[in] An expression E. - // @return Whether E is an expression containing a reference to an array - // subscript or a pointer dereference. - bool IsDeclOperand(const Expr *E); - // Make an expression uniform by moving all DeclRefExpr to the LHS and all // IntegerLiterals to the RHS. // @param[in] E is the expression which should be made uniform. @@ -343,10 +331,10 @@ namespace clang { // @return The intersection of sets A and B. template T Intersect(T &A, T &B) const; - // Compute the union of sets A and B. + // Compute the union of sets A and B and widen the bounds where applicable. // @param[in] A is a set. // @param[in] B is a set. - // @return The union of sets A and B. + // @return The union of sets A and B containing the widened bounds. template T Union(T &A, T &B) const; // Compute the set difference of sets A and B. diff --git a/clang/lib/Sema/BoundsAnalysis.cpp b/clang/lib/Sema/BoundsAnalysis.cpp index 1084420ec1f1..0d2eeea5698f 100644 --- a/clang/lib/Sema/BoundsAnalysis.cpp +++ b/clang/lib/Sema/BoundsAnalysis.cpp @@ -217,44 +217,40 @@ void BoundsAnalysis::CollectBoundsVars(const Expr *E, DeclSetTy &BoundsVars) { CollectBoundsVars(BO->getRHS(), BoundsVars); } - if (IsDeclOperand(E)) { - const DeclRefExpr *D = GetDeclOperand(E); + if (DeclRefExpr *D = GetDeclOperand(E)) if (const auto *V = dyn_cast(D->getDecl())) BoundsVars.insert(V); - } } DeclRefExpr *BoundsAnalysis::GetDeclOperand(const Expr *E) { - if (!E || !isa(E)) - return nullptr; - auto *CE = dyn_cast(E); - assert(CE->getSubExpr() && "invalid CastExpr expression"); - - return dyn_cast(IgnoreCasts(CE->getSubExpr())); -} - -bool BoundsAnalysis::IsDeclOperand(const Expr *E) { - if (auto *CE = dyn_cast(E)) { - assert(CE->getSubExpr() && "invalid CastExpr expression"); + if (auto *CE = dyn_cast_or_null(E)) { + const Expr *SubE = CE->getSubExpr(); + assert(SubE && "Invalid CastExpr expression"); if (CE->getCastKind() == CastKind::CK_LValueToRValue || - CE->getCastKind() == CastKind::CK_ArrayToPointerDecay) - return isa(IgnoreCasts(CE->getSubExpr())); + CE->getCastKind() == CastKind::CK_ArrayToPointerDecay) { + E = Lex.IgnoreValuePreservingOperations(Ctx, const_cast(SubE)); + return dyn_cast_or_null(const_cast(E)); + } } - return false; + return nullptr; } void BoundsAnalysis::CollectNtPtrsInScope(FunctionDecl *FD) { - // TODO: Currently, we simply collect all ntptrs defined in the current - // function. Ultimately, we need to do a liveness analysis of what ntptrs are - // in scope for a block. + // TODO: Currently, we simply collect all ntptrs and variables used in their + // declared bounds for the entire function. Ultimately, we need to do a + // liveness analysis of what ntptrs are in scope for a block. assert(FD && "invalid function"); // Collect ntptrs passed as parameters to the current function. - for (const ParmVarDecl *PD : FD->parameters()) + // Note: NtPtrsInScope is a mapping from ntptr to variables used in the + // declared bounds of the ntptr. In this function we store an empty set for + // the bounds variables. This will later be filled in FillGenSetForEdge. + for (const ParmVarDecl *PD : FD->parameters()) { if (IsNtArrayType(PD)) - NtPtrsInScope.insert(PD); + NtPtrsInScope[PD] = DeclSetTy(); + } // Collect all ntptrs defined in the current function. BlockMap contains all // blocks of the current function. We iterate through all blocks in BlockMap @@ -273,15 +269,15 @@ void BoundsAnalysis::CollectNtPtrsInScope(FunctionDecl *FD) { for (const Decl *D : DS->decls()) if (const auto *V = dyn_cast(D)) if (IsNtArrayType(V)) - NtPtrsInScope.insert(V); + NtPtrsInScope[V] = DeclSetTy(); } } } } -void BoundsAnalysis::FillGenSetAndGetBoundsVars(const Expr *E, - ElevatedCFGBlock *EB, - ElevatedCFGBlock *SuccEB) { +void BoundsAnalysis::FillGenSetForEdge(const Expr *E, + ElevatedCFGBlock *EB, + ElevatedCFGBlock *SuccEB) { // TODO: Handle accesses of the form: // "if (*(p + i) && *(p + j) && *(p + k))" @@ -327,29 +323,23 @@ void BoundsAnalysis::FillGenSetAndGetBoundsVars(const Expr *E, // if (*(p + 1)) // no widening // if (*(p + i)) // widen p and q by 1 - // TODO: Currently, we iterate and re-compute info for all ntptrs in scope - // for each ntptr dereference. We can optimize this at the cost of space by - // storing the VarDecls, variables used in bounds exprs and base/offset for - // the declared upper bounds expr for the VarDecl. Then we simply have to - // look this up instead of re-computing. + for (auto item : NtPtrsInScope) { + const VarDecl *V = item.first; - for (const VarDecl *V : NtPtrsInScope) { - // In case the bounds expr for V is not a RangeBoundsExpr, invoke - // ExpandBoundsToRange to expand it to RangeBoundsExpr. - const BoundsExpr *BE = S.ExpandBoundsToRange(V, V->getBoundsExpr()); - const auto *RBE = dyn_cast(BE); + BoundsExpr *NormalizedBounds = S.NormalizeBounds(V); + if (!NormalizedBounds) + continue; + const auto *RBE = dyn_cast(NormalizedBounds); if (!RBE) continue; // Collect all variables involved in the upper and lower bounds exprs for // the ntptr. An assignment to any such variable would kill the widenend // bounds for the ntptr. - if (!SuccEB->BoundsVars.count(V)) { - DeclSetTy BoundsVars; - CollectBoundsVars(RBE, BoundsVars); - SuccEB->BoundsVars[V] = BoundsVars; - } + DeclSetTy BoundsVars; + CollectBoundsVars(NormalizedBounds, BoundsVars); + NtPtrsInScope[V] = BoundsVars; // Update the bounds of p on the edge EB->SuccEB only if we haven't already // updated them. @@ -427,8 +417,8 @@ ExprIntPairTy BoundsAnalysis::SplitIntoBaseOffset(const Expr *E) { E = E->IgnoreParens(); - if (IsDeclOperand(E)) - return std::make_pair(GetDeclOperand(E), Zero); + if (DeclRefExpr *D = GetDeclOperand(E)) + return std::make_pair(D, Zero); if (!isa(E)) return std::make_pair(nullptr, Zero); @@ -450,20 +440,22 @@ ExprIntPairTy BoundsAnalysis::SplitIntoBaseOffset(const Expr *E) { // p + i ==> return (p, i) llvm::APSInt IntVal; - if (IsDeclOperand(LHS) && RHS->isIntegerConstantExpr(IntVal, Ctx)) - return std::make_pair(GetDeclOperand(LHS), IntVal); + if (DeclRefExpr *D = GetDeclOperand(LHS)) + if (RHS->isIntegerConstantExpr(IntVal, Ctx)) + return std::make_pair(D, IntVal); // Case 2: LHS is IntegerLiteral and RHS is DeclRefExpr. We simply need to // swap LHS and RHS to make expr uniform. // i + p ==> return (p, i) - if (LHS->isIntegerConstantExpr(IntVal, Ctx) && IsDeclOperand(RHS)) - return std::make_pair(GetDeclOperand(RHS), IntVal); + if (DeclRefExpr *D = GetDeclOperand(RHS)) + if (LHS->isIntegerConstantExpr(IntVal, Ctx)) + return std::make_pair(D, IntVal); // Case 3: LHS and RHS are both DeclRefExprs. This means there is no // IntegerLiteral in the expr. In this case, we return the incoming // BinaryOperator expr with a nullptr for the RHS. // p + q ==> return (p + q, nullptr) - if (IsDeclOperand(LHS) && IsDeclOperand(RHS)) + if (GetDeclOperand(LHS) && GetDeclOperand(RHS)) return std::make_pair(BO, Zero); // To make parsing simpler, we always try to keep BinaryOperator on the LHS. @@ -580,22 +572,23 @@ void BoundsAnalysis::FillGenSet(Expr *E, AE->getValueKind(), AE->getObjectKind(), AE->getExprLoc(), FPOptions()); - FillGenSetAndGetBoundsVars(BO, EB, SuccEB); + FillGenSetForEdge(BO, EB, SuccEB); } else if (auto *UO = dyn_cast(E)) { if (UO->getOpcode() == UO_Deref) { assert(UO->getSubExpr() && "invalid UnaryOperator expression"); const Expr *UE = IgnoreCasts(UO->getSubExpr()); - FillGenSetAndGetBoundsVars(UE, EB, SuccEB); + FillGenSetForEdge(UE, EB, SuccEB); } } } void BoundsAnalysis::ComputeKillSets(StmtSet NestedStmts) { - // For a block B, a variable v is added to Kill[B][S] if v is assigned to in + // For a block B, a variable V is added to Kill[B][S] if V is assigned to in // B by Stmt S or some child S1 of S. + // Compute vars killed in the current block. for (const auto item : BlockMap) { ElevatedCFGBlock *EB = item.second; @@ -604,10 +597,12 @@ void BoundsAnalysis::ComputeKillSets(StmtSet NestedStmts) { const Stmt *S = Elem.castAs().getStmt(); if (!S) continue; + // Skip top-level statements that are nested in another // top-level statement. if (NestedStmts.find(S) != NestedStmts.end()) continue; + FillKillSet(EB, S, S); } } @@ -641,23 +636,23 @@ void BoundsAnalysis::FillKillSet(ElevatedCFGBlock *EB, EB->Kill[TopLevelStmt].insert(V); else { - // Else look for the variable in BoundsVars. + // Else look for the variable in NtPtrsInScope. - // BoundsVars is a mapping from an ntptr to all the variables used in - // its upper and lower bounds exprs. For example: + // NtPtrsInScope is a mapping from an ntptr to all the variables used + // in its upper and lower bounds exprs. For example: // _Nt_array_ptr p : bounds(p + i, i + p + j + 10); // _Nt_array_ptr q : bounds(i + q, i + p + q + m); - // EB->BoundsVars: {p: {p, i, j}, q: {i, q, p, m}} + // NtPtrsInScope: {p: {p, i, j}, q: {i, q, p, m}} - for (auto item : EB->BoundsVars) { + for (auto item : NtPtrsInScope) { const VarDecl *NtPtr = item.first; - DeclSetTy Vars = item.second; + DeclSetTy BoundsVars = item.second; // If the variable exists in the bounds declaration for the ntptr, // then add the Stmt:ntptr pair to the Kill set for the block. - if (Vars.count(V)) + if (BoundsVars.count(V)) EB->Kill[TopLevelStmt].insert(NtPtr); } } @@ -695,15 +690,15 @@ void BoundsAnalysis::ComputeOutSets(ElevatedCFGBlock *EB, WorkListTy &WorkList) { // Out[B1->B2] = (In[B1] - Kill[B1]) u Gen[B1->B2]. - // EB->Kill is a mapping from Stmt to ntptrs. We extract just the ntptrs for - // the block and then use that to compute (In - Kill). + // EB->Kill is a mapping from Stmt to ntptrs. We extract just the ntptrs + // killed for the block and use that to compute (In - Kill). DeclSetTy KilledVars; for (auto item : EB->Kill) { const DeclSetTy Vars = item.second; KilledVars.insert(Vars.begin(), Vars.end()); } - BoundsMapTy Diff = Difference(EB->In, KilledVars); + BoundsMapTy InMinusKill = Difference(EB->In, KilledVars); for (const CFGBlock *succ : EB->Block->succs()) { if (SkipBlock(succ)) @@ -728,7 +723,7 @@ void BoundsAnalysis::ComputeOutSets(ElevatedCFGBlock *EB, // B2: if (*(p + 1)) { // In[B2] = {p:1}, Gen[B1->B2] = {p:1} ==> bounds(p) = 2. // B3: if (*(p + 2)) { // In[B2] = {p:2}, Gen[B1->B2] = {p:2} ==> bounds(p) = 3. - EB->Out[succ] = Union(Diff, EB->Gen[succ]); + EB->Out[succ] = Union(InMinusKill, EB->Gen[succ]); // The Out set on an edge is marked "empty" if the In set is marked "empty" // and the Gen set on that edge is empty. @@ -776,8 +771,8 @@ Expr *BoundsAnalysis::IgnoreCasts(const Expr *E) { } bool BoundsAnalysis::IsNtArrayType(const VarDecl *V) const { - return V->getType()->isCheckedPointerNtArrayType() || - V->getType()->isNtCheckedArrayType(); + return V && (V->getType()->isCheckedPointerNtArrayType() || + V->getType()->isNtCheckedArrayType()); } bool BoundsAnalysis::SkipBlock(const CFGBlock *B) const { diff --git a/clang/test/CheckedC/inferred-bounds/widened-bounds.c b/clang/test/CheckedC/inferred-bounds/widened-bounds.c index 58e5678b5f1a..cad60aa51323 100644 --- a/clang/test/CheckedC/inferred-bounds/widened-bounds.c +++ b/clang/test/CheckedC/inferred-bounds/widened-bounds.c @@ -233,7 +233,6 @@ void f11(int i, int j) { // CHECK: [B3] // CHECK: 1: *(p + j) -// CHECK: T: if [B3.1] // CHECK: [B2] // CHECK: 1: j = 0 // CHECK: 2: *(p + j + 1) @@ -1242,3 +1241,28 @@ void f33() { // CHECK: [B1] // CHECK: upper_bound(p) = 1 } + +void f34(_Nt_array_ptr p : count(i), int i, int flag) { + if (*(p + i)) { + flag ? i++ : i; // expected-error {{inferred bounds for 'p' are unknown after statement}} + + if (*(p + i + 1)) + {} + } + +// CHECK: In function: f34 +// CHECK: [B6] +// CHECK: 1: *(p + i) +// CHECK: [B5] +// CHECK: 1: flag +// CHECK: upper_bound(p) = 1 +// CHECK: [B4] +// CHECK: 1: i +// CHECK: upper_bound(p) = 1 +// CHECK: [B3] +// CHECK: 1: i++ +// CHECK: upper_bound(p) = 1 +// CHECK: [B2] +// CHECK: 2: *(p + i + 1) +// CHECK: upper_bound(p) = 1 +}