From 603792cb70825ac22de26911781068444b3e54b9 Mon Sep 17 00:00:00 2001 From: Mandeep Singh Grang Date: Mon, 9 Aug 2021 15:23:12 -0700 Subject: [PATCH 1/8] [BoundsWidening] Handle complex conditionals in bounds widening Support bounds widening in presence of complex conditionals like: "if (*p != 0)", "if ((c = *p) == 'a')", etc. --- .../clang/Sema/BoundsWideningAnalysis.h | 65 +- clang/lib/Sema/BoundsWideningAnalysis.cpp | 281 ++++++-- .../widened-bounds-complex-conditionals.c | 603 ++++++++++++++++++ 3 files changed, 894 insertions(+), 55 deletions(-) create mode 100644 clang/test/CheckedC/inferred-bounds/widened-bounds-complex-conditionals.c diff --git a/clang/include/clang/Sema/BoundsWideningAnalysis.h b/clang/include/clang/Sema/BoundsWideningAnalysis.h index 8be477f3f729..6d868360920c 100644 --- a/clang/include/clang/Sema/BoundsWideningAnalysis.h +++ b/clang/include/clang/Sema/BoundsWideningAnalysis.h @@ -63,6 +63,39 @@ namespace clang { using InvertibleStmtMapTy = llvm::DenseMap; + // A struct representing various information about the terminating condition + // of a block. + struct TermCondInfoTy { + // The expression that dereferences a pointer or subscripts an array. For + // example: + // if (*(p + i) == 0) ==> DerefExpr = p + i + Expr *DerefExpr; + + // Whether the value that an expression is compared against is null. For + // example: + // if (*p == 0) ==> IsComparedValNull = True + // if (*p == 'a') ==> IsComparedValNull = False + bool IsComparedValNull; + + // Whether the operator of the conditional is positive. For example: + // if (*p == 0) ==> IsCheckPositive = True + // if (*p != 0) ==> IsCheckPositive = False + bool IsCheckPositive; + + // Whether the terminating condition is an expression involving a binary + // operator. For example: + // if (*p == 0) ==> HasBinaryOp = True + // if (!*p) ==> HasBinaryOp = False + bool HasBinaryOp; + + // Whether the terminating condition is an expression involving a unary not + // operator. For example: + // if (!*p) ==> HasUnaryNot = True + // if (!(*p == 0)) ==> HasUnaryNot = True + // if (*p == 0) ==> HasUnaryNot = False + bool HasUnaryNot; + }; + } // end namespace clang namespace clang { @@ -159,11 +192,19 @@ namespace clang { // @return Returns the expression E + Offset. Expr *AddOffsetToExpr(Expr *E, unsigned Offset) const; - // From the given expression get the dereference expression. A dereference - // expression can be of the form "*(p + 1)" or "p[1]". - // @param[in] E is the given expression. - // @return Returns the dereference expression, if it exists. - Expr *GetDerefExpr(const Expr *E) const; + // Get various information about the terminating condition of a block. + // @param[in] TermCond is the terminating condition of a block. + // @return A struct containing various information about the terminating + // condition. + TermCondInfoTy GetTermCondInfo(const Expr *TermCond) const; + + // Fill the TermCondInfo parameter with information about the terminating + // condition TermCond. + // @param[in] TermCond is the terminating condition of a block. + // @param[out] TermCondInfo is the struct that is filled with various + // information about the terminating condition. + void FillTermCondInfo(const Expr *TermCond, + TermCondInfoTy &TermCondInfo) const; // Get the variable in an expression that is a pointer to a null-terminated // array. @@ -178,6 +219,11 @@ namespace clang { // @return E with casts stripped off. Expr *IgnoreCasts(const Expr *E) const; + // Strip off more casts than IgnoreCasts. + // @param[in] E is the expression whose casts must be stripped. + // @return E with casts stripped off. + Expr *StripCasts(const Expr *E) const; + // We do not want to run dataflow analysis on null blocks or the exit // block. So we skip them. // @param[in] B is the block which may need to be skipped from dataflow @@ -289,7 +335,7 @@ namespace clang { // The terminating condition that dereferences a pointer. This is nullptr // if the terminating condition does not dereference a pointer. - Expr *TermCondDerefExpr = nullptr; + TermCondInfoTy TermCondInfo; // The In set of the last statment of each block. BoundsMapTy InOfLastStmt; @@ -523,6 +569,13 @@ namespace clang { const Stmt *CurrStmt, VarSetTy PtrsWithAffectedBounds) const; + // Check whether the terminating condition of a block tests for a + // null-terminator. + // @param[in] EB is the current ElevatedCFGBlock. + // @return Returns true if the terminating condition of block EB tests for + // a null-terminator, false otherwise. + bool DoesTermCondCheckNull(ElevatedCFGBlock *EB) const; + // Update the bounds in StmtOut with the adjusted bounds for the current // statement, if they exist. // @param[in] EB is the current ElevatedCFGBlock. diff --git a/clang/lib/Sema/BoundsWideningAnalysis.cpp b/clang/lib/Sema/BoundsWideningAnalysis.cpp index f6d8639f2092..a817baf5a837 100644 --- a/clang/lib/Sema/BoundsWideningAnalysis.cpp +++ b/clang/lib/Sema/BoundsWideningAnalysis.cpp @@ -361,6 +361,10 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( // Check if the edge from pred to the current block is a true edge. bool IsEdgeTrue = BWUtil.IsTrueEdge(PredBlock, CurrBlock); + // Check if the terminating condition of the pred block tests for a null + // terminator. + bool TermCondChecksNull = DoesTermCondCheckNull(PredEB); + // Get the In of the last statement in the pred block. If the pred // block does not have any statements then InOfLastStmtOfPred is set to // the In set of the pred block. @@ -433,9 +437,28 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( // Note: Switch cases are handled separately later in this function. - if (!IsSwitchCase && !IsEdgeTrue) { - PrunedOutSet[V] = BoundsOfVInStmtIn; - continue; + if (!IsSwitchCase) { + // if (*p != 0) { + // IsEdgeTrue = True, TermCondChecksNull = False + // ==> widen bounds of p + // + // } else { + // IsEdgeTrue = False, TermCondChecksNull = False + // ==> do not widen bounds of p + // } + + // if (*p == 0) { + // IsEdgeTrue = True, TermCondChecksNull = True + // ==> do not widen bounds of p + // + // } else { + // IsEdgeTrue = False, TermCondChecksNull = True + // ==> widen bounds of p + // } + if (IsEdgeTrue == TermCondChecksNull) { + PrunedOutSet[V] = BoundsOfVInStmtIn; + continue; + } } // If the terminating condition of the pred block does not @@ -457,8 +480,8 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( // upon entry to blocks 2 and 3. bool IsDerefAtUpperBound = - PredEB->TermCondDerefExpr && BoundsOfVInStmtIn != Top && - Lex.CompareExprSemantically(PredEB->TermCondDerefExpr, + PredEB->TermCondInfo.DerefExpr && BoundsOfVInStmtIn != Top && + Lex.CompareExprSemantically(PredEB->TermCondInfo.DerefExpr, BoundsOfVInStmtIn->getUpperExpr()); if (!IsDerefAtUpperBound) { @@ -729,24 +752,24 @@ void BoundsWideningAnalysis::GetVarsAndBoundsInPtrDeref( if (!TermCond) return; - // If the terminating condition is a dereference expression get that - // expression. For example, from a terminating condition like "if (*(p + - // 1))", extract the expression "p + 1". - Expr *DerefExpr = BWUtil.GetDerefExpr(TermCond); - if (!DerefExpr) - return; + // Get information about the terminating condition such as the dereference + // expression, whether the condition check is positive or not (== vs !=), + // whether the compared value is null or not (== '0' vs == 'a'). + EB->TermCondInfo = BWUtil.GetTermCondInfo(TermCond); - // Store the dereference condition for the block. We will use it later in the - // In set calculation. - EB->TermCondDerefExpr = DerefExpr; + // If the terminating condition does not contain a dereference (or an array + // subscript) we cannot possibly widen a pointer. + if (!EB->TermCondInfo.DerefExpr) + return; - // Get the variable in the expression that is a pointers to a null-terminated + // Get the variable in the expression that is a pointer to a null-terminated // array. For example: On a dereference expression like "*(p + i + j + 1)" // GetNullTermPtrInExpr() will return p if p is a pointer to a null-terminated // array. // Note: We assume that a dereference expression can only contain at most one // pointer to a null-terminated array. - const VarDecl *NullTermPtrInExpr = BWUtil.GetNullTermPtrInExpr(DerefExpr); + const VarDecl *NullTermPtrInExpr = + BWUtil.GetNullTermPtrInExpr(EB->TermCondInfo.DerefExpr); if (!NullTermPtrInExpr) return; @@ -785,8 +808,9 @@ void BoundsWideningAnalysis::GetVarsAndBoundsInPtrDeref( RangeBoundsExpr *WidenedBounds = new (Ctx) RangeBoundsExpr(R->getLowerExpr(), - BWUtil.AddOffsetToExpr(DerefExpr, 1), - SourceLocation(), SourceLocation()); + BWUtil.AddOffsetToExpr(EB->TermCondInfo.DerefExpr, 1), + SourceLocation(), SourceLocation()); + VarsAndBounds[V] = WidenedBounds; } } @@ -907,6 +931,25 @@ void BoundsWideningAnalysis::CheckStmtInvertibility(ElevatedCFGBlock *EB, } } +bool BoundsWideningAnalysis::DoesTermCondCheckNull( + ElevatedCFGBlock *EB) const { + + // *p == 0 + // ==> IsComparedValNull = True, IsCheckPositive = True, return True + + // *p != 0 + // ==> IsComparedValNull = True, IsCheckPositive = False, return False + + // *p == 'a' + // ==> IsComparedValNull = False, IsCheckPositive = True, return False + + // *p != 'a' + // ==> IsComparedValNull = False, IsCheckPositive = False, return True + + return EB->TermCondInfo.IsComparedValNull == + EB->TermCondInfo.IsCheckPositive; +} + void BoundsWideningAnalysis::UpdateAdjustedBounds( ElevatedCFGBlock *EB, const Stmt *CurrStmt, BoundsMapTy &StmtOut) const { @@ -1064,7 +1107,7 @@ void BoundsWideningAnalysis::DumpWidenedBounds(FunctionDecl *FD, } else if (PrintOption == 1) { OS << "\n Stmt: "; } - + // Print the current statement. PrintStmt(CurrStmt); @@ -1353,45 +1396,166 @@ Expr *BoundsWideningUtil::AddOffsetToExpr(Expr *E, unsigned Offset) const { BinaryOperatorKind::BO_Add); } -Expr *BoundsWideningUtil::GetDerefExpr(const Expr *TermCond) const { - if (!TermCond) - return nullptr; +TermCondInfoTy BoundsWideningUtil::GetTermCondInfo( + const Expr *TermCond) const { - Expr *E = const_cast(TermCond); + TermCondInfoTy TermCondInfo; + // Initialize fields of TermCondInfo. + TermCondInfo.DerefExpr = nullptr; + TermCondInfo.IsComparedValNull = false; + TermCondInfo.IsCheckPositive = false; + TermCondInfo.HasBinaryOp = false; + TermCondInfo.HasUnaryNot = false; - // According to C11 standard section 6.5.13, the logical AND Operator shall - // yield 1 if both of its operands compare unequal to 0; otherwise, it yields - // 0. The result has type int. An IntegralCast is generated for "if (e1 && - // e2)" Here we strip off the IntegralCast. - if (auto *CE = dyn_cast(E)) { - if (CE->getCastKind() == CastKind::CK_IntegralCast) - E = CE->getSubExpr(); - } + FillTermCondInfo(TermCond, TermCondInfo); + return TermCondInfo; +} - if (auto *CE = dyn_cast(E)) - if (CE->getCastKind() == CastKind::CK_LValueToRValue) - E = CE->getSubExpr(); +void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, + TermCondInfoTy &TermCondInfo) const { + Expr *E = StripCasts(TermCond); - E = IgnoreCasts(E); + if (auto *UO = dyn_cast(E)) { + UnaryOperatorKind UnaryOp = UO->getOpcode(); + + if (UnaryOp == UO_Deref) { + Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); + + // *p ==> DerefExpr = p + // *(p + i) ==> DerefExpr = p + i + TermCondInfo.DerefExpr = SubExpr; + + if (TermCondInfo.HasBinaryOp) { + // *p == 0 ==> HasUnaryNot = False, IsCheckPositive = True + // *p != 0 ==> HasUnaryNot = True, IsCheckPositive = False + // !(*p == 0) ==> HasUnaryNot = True, IsCheckPositive = False + // !(*p != 0) ==> HasUnaryNot = False, IsCheckPositive = True + TermCondInfo.IsCheckPositive = + TermCondInfo.IsCheckPositive != TermCondInfo.HasUnaryNot; + + } else { + // *p ==> *p != 0 ==> HasUnaryNot = False + // IsCheckPositive = False, + // IsComparedValNull = True + + // !*p ==> *p == 0 ==> HasUnaryNot = True + // IsCheckPositive = True, + // IsComparedValNull = True + TermCondInfo.IsCheckPositive = TermCondInfo.HasUnaryNot; + TermCondInfo.IsComparedValNull = true; + } - // A dereference expression can contain an array subscript or a pointer - // dereference. + } else if (UnaryOp == UO_LNot) { + Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); - // If a dereference expression is of the form "*(p + i)". - if (auto *UO = dyn_cast(E)) { - if (UO->getOpcode() == UO_Deref) - return IgnoreCasts(UO->getSubExpr()); + // *p ==> HasUnaryNot = False + // !*p ==> HasUnaryNot = True + // !!*p ==> HasUnaryNot = False + // !!!*p ==> HasUnaryNot = True + TermCondInfo.HasUnaryNot = !TermCondInfo.HasUnaryNot; + + // !*p ==> FillTermCondInfo(*p); + // !!*p ==> FillTermCondInfo(!*p); + // !!!*p ==> FillTermCondInfo(!!*p); + // !(*p == 0) ==> FillTermCondInfo(*p == 0); + FillTermCondInfo(SubExpr, TermCondInfo); + } - // Else if a dereference expression is an array access. An array access can - // be written A[i] or i[A] (both are equivalent). getBase() and getIdx() - // always present the normalized view: A[i]. In this case getBase() returns - // "A" and getIdx() returns "i". + // If dereference expression contains an array access. An array access can + // be written A[i] or i[A] (both are equivalent). getBase() and getIdx() + // always present the normalized view: A[i]. In this case getBase() returns + // "A" and getIdx() returns "i". } else if (auto *AE = dyn_cast(E)) { - return ExprCreatorUtil::CreateBinaryOperator(SemaRef, AE->getBase(), - AE->getIdx(), - BinaryOperatorKind::BO_Add); + // p[i] ==> DerefExpr = p + i + TermCondInfo.DerefExpr = + ExprCreatorUtil::CreateBinaryOperator(SemaRef, AE->getBase(), + AE->getIdx(), + BinaryOperatorKind::BO_Add); + if (TermCondInfo.HasBinaryOp) { + // p[i] == 0 ==> IsCheckPositive = True + // p[i] != 0 ==> IsCheckPositive = False + // !(p[i] == 0) ==> IsCheckPositive = False + // !(p[i] != 0) ==> IsCheckPositive = True + TermCondInfo.IsCheckPositive = + TermCondInfo.IsCheckPositive != TermCondInfo.HasUnaryNot; + + } else { + // p[i] ==> p[i] != 0 ==> HasUnaryNot = False + // IsCheckPositive = False, + // IsComparedValNull = True + + // !p[i] ==> p[i] == 0 ==> HasUnaryNot = True + // IsCheckPositive = True, + // IsComparedValNull = True + TermCondInfo.IsCheckPositive = TermCondInfo.HasUnaryNot; + TermCondInfo.IsComparedValNull = true; + } + + } else if (auto *BO = dyn_cast(E)) { + BinaryOperatorKind BinOp = BO->getOpcode(); + + // *p == 0, 0 != *p + if (BinOp == BO_EQ || BinOp == BO_NE) { + // *p == 0 ==> HasBinaryOp = True + // *p != 0 ==> HasBinaryOp = True + // (c = *p) != 0 ==> HasBinaryOp = True + TermCondInfo.HasBinaryOp = true; + + // *p == 0 ==> IsCheckPositive = True + // *p != 0 ==> IsCheckPositive = False + TermCondInfo.IsCheckPositive = BinOp == BO_EQ; + + Expr *LHS = BO->getLHS(); + Expr *RHS = BO->getRHS(); + + // 0 == *p ==> LHSVal = 0 + // 'a' != *p ==> LHSVal = 'a' + // a != *p ==> LHSVal = nullptr + // *p == *q ==> LHSVal = nullptr; + Optional LHSVal = LHS->getIntegerConstantExpr(Ctx); + + // *p == 0 ==> RHSVal = 0 + // *p != 'a' ==> RHSVal = 'a' + // *p != a ==> RHSVal = nullptr + // *p == *q ==> RHSVal = nullptr + Optional RHSVal = RHS->getIntegerConstantExpr(Ctx); + + // 1 == 2 ==> DerefExpr = nullptr + // *p != q ==> DerefExpr = nullptr + if (LHSVal == RHSVal) { + TermCondInfo.DerefExpr = nullptr; + return; + } + + llvm::APSInt Zero(Ctx.getTargetInfo().getIntWidth(), 0); + if (LHSVal) { + // 0 == *p ==> IsComparedValNull = True + // 'a' != *p ==> IsComparedValNull = False + TermCondInfo.IsComparedValNull = + llvm::APSInt::compareValues(*LHSVal, Zero) == 0; + + // 0 == *p ==> FillTermCondInfo(*p); + // 'a' != *p ==> FillTermCondInfo(*p); + FillTermCondInfo(RHS, TermCondInfo); + + } else if (RHSVal) { + // *p == 0 ==> IsComparedValNull = True + // *p != 'a' ==> IsComparedValNull = False + TermCondInfo.IsComparedValNull = + llvm::APSInt::compareValues(*RHSVal, Zero) == 0; + + // *p == 0 ==> FillTermCondInfo(*p); + // *p != 'a' ==> FillTermCondInfo(*p); + FillTermCondInfo(LHS, TermCondInfo); + } + + // (c = *p) != 0 + } else if (BinOp == BO_Assign) { + // (c = *p) ==> RHS = *p + Expr *RHS = BO->getRHS(); + FillTermCondInfo(RHS, TermCondInfo); + } } - return nullptr; } const VarDecl *BoundsWideningUtil::GetNullTermPtrInExpr(Expr *E) const { @@ -1430,6 +1594,25 @@ const VarDecl *BoundsWideningUtil::GetNullTermPtrInExpr(Expr *E) const { return nullptr; } +Expr *BoundsWideningUtil::StripCasts(const Expr *TermCond) const { + Expr *E = const_cast(TermCond); + + // According to C11 standard section 6.5.13, the logical AND Operator shall + // yield 1 if both of its operands compare unequal to 0; otherwise, it yields + // 0. The result has type int. An IntegralCast is generated for "if (e1 && + // e2)" Here we strip off the IntegralCast. + if (auto *CE = dyn_cast(E)) { + if (CE->getCastKind() == CastKind::CK_IntegralCast) + E = CE->getSubExpr(); + } + + if (auto *CE = dyn_cast(E)) + if (CE->getCastKind() == CastKind::CK_LValueToRValue) + E = CE->getSubExpr(); + + return IgnoreCasts(E); +} + Expr *BoundsWideningUtil::IgnoreCasts(const Expr *E) const { return Lex.IgnoreValuePreservingOperations(Ctx, const_cast(E)); } diff --git a/clang/test/CheckedC/inferred-bounds/widened-bounds-complex-conditionals.c b/clang/test/CheckedC/inferred-bounds/widened-bounds-complex-conditionals.c new file mode 100644 index 000000000000..61a329b68e91 --- /dev/null +++ b/clang/test/CheckedC/inferred-bounds/widened-bounds-complex-conditionals.c @@ -0,0 +1,603 @@ +// Tests for datafow analysis for bounds widening of null-terminated arrays in +// presence of complex conditionals. +// +// RUN: %clang_cc1 -fdump-widened-bounds -verify \ +// RUN: -verify-ignore-unexpected=note -verify-ignore-unexpected=warning %s \ +// RUN: | FileCheck %s + +// expected-no-diagnostics + +#include +#include + +int a; + +void f1(_Nt_array_ptr p : count(0)) { + + if (*p) { + a = 1; + } else { + a = 2; + } + +// CHECK: Function: f1 +// CHECK: Block: B7 (Entry), Pred: Succ: B6 + +// CHECK: Block: B6, Pred: B7, Succ: B5, B4 +// CHECK: Widened bounds before stmt: *p +// CHECK: p: bounds(p, p + 0) + +// CHECK: Block: B5, Pred: B6, Succ: B3 +// CHECK: Widened bounds before stmt: a = 1 +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B4, Pred: B6, Succ: B3 +// CHECK: Widened bounds before stmt: a = 2 +// CHECK: p: bounds(p, p + 0) + + if (!*p) { + a = 3; + } else { + a = 4; + } + +// CHECK: Block: B3, Pred: B4, B5, Succ: B2, B1 +// CHECK: Widened bounds before stmt: !*p +// CHECK: p: bounds(p, p + 0) + +// CHECK: Block: B2, Pred: B3, Succ: B0 +// CHECK: Widened bounds before stmt: a = 3 +// CHECK: p: bounds(p, p + 0) + +// CHECK: Block: B1, Pred: B3, Succ: B0 +// CHECK: Widened bounds before stmt: a = 4 +// CHECK: p: bounds(p, p + 1) +} + +void f2(_Nt_array_ptr p : count(i), int i) { + + if (*(p + i) == 0) { + a = 1; + } else { + a = 2; + } + +// CHECK: Function: f2 +// CHECK: Block: B13 (Entry), Pred: Succ: B12 + +// CHECK: Block: B12, Pred: B13, Succ: B11, B10 +// CHECK: Widened bounds before stmt: *(p + i) == 0 +// CHECK: p: bounds(p, p + i) + +// CHECK: Block: B11, Pred: B12, Succ: B9 +// CHECK: Widened bounds before stmt: a = 1 +// CHECK: p: bounds(p, p + i) + +// CHECK: Block: B10, Pred: B12, Succ: B9 +// CHECK: Widened bounds before stmt: a = 2 +// CHECK: p: bounds(p, p + i + 1) + + if (*(i + p) != 0) { + a = 3; + } else { + a = 4; + } + +// CHECK: Block: B9, Pred: B10, B11, Succ: B8, B7 +// CHECK: Widened bounds before stmt: *(i + p) != 0 +// CHECK: p: bounds(p, p + i) + +// CHECK: Block: B8, Pred: B9, Succ: B6 +// CHECK: Widened bounds before stmt: a = 3 +// CHECK: p: bounds(p, i + p + 1) + +// CHECK: Block: B7, Pred: B9, Succ: B6 +// CHECK: Widened bounds before stmt: a = 4 +// CHECK: p: bounds(p, p + i) + + if ('a' == *(p + i)) { + a = 5; + } else { + a = 6; + } + +// CHECK: Block: B6, Pred: B7, B8, Succ: B5, B4 +// CHECK: Widened bounds before stmt: 'a' == *(p + i) +// CHECK: p: bounds(p, p + i) + +// CHECK: Block: B5, Pred: B6, Succ: B3 +// CHECK: Widened bounds before stmt: a = 5 +// CHECK: p: bounds(p, p + i + 1) + +// CHECK: Block: B4, Pred: B6, Succ: B3 +// CHECK: Widened bounds before stmt: a = 6 +// CHECK: p: bounds(p, p + i) + + if ('a' != *(i + p)) { + a = 7; + } else { + a = 8; + } + +// CHECK: Block: B3, Pred: B4, B5, Succ: B2, B1 +// CHECK: Widened bounds before stmt: 'a' != *(i + p) +// CHECK: p: bounds(p, p + i) + +// CHECK: Block: B2, Pred: B3, Succ: B0 +// CHECK: Widened bounds before stmt: a = 7 +// CHECK: p: bounds(p, p + i) + +// CHECK: Block: B1, Pred: B3, Succ: B0 +// CHECK: Widened bounds before stmt: a = 8 +// CHECK: p: bounds(p, i + p + 1) +} + +void f3(_Nt_array_ptr p : bounds(p, p)) { + + if (*p) { + a = 1; + } else if (*p == 'a') { + a = 2; + } else { + a = 3; + } + +// CHECK: Function: f3 +// CHECK: Block: B16 (Entry), Pred: Succ: B15 + +// CHECK: Block: B15, Pred: B16, Succ: B14, B13 +// CHECK: Widened bounds before stmt: *p +// CHECK: p: bounds(p, p) + +// CHECK: Block: B14, Pred: B15, Succ: B10 +// CHECK: Widened bounds before stmt: a = 1 +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B13, Pred: B15, Succ: B12, B11 +// CHECK: Widened bounds before stmt: *p == 'a' +// CHECK: p: bounds(p, p) + +// CHECK: Block: B12, Pred: B13, Succ: B10 +// CHECK: Widened bounds before stmt: a = 2 +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B11, Pred: B13, Succ: B10 +// CHECK: Widened bounds before stmt: a = 3 +// CHECK: p: bounds(p, p) + + if (!*p) { + a = 4; + } else if (!*p) { + a = 5; + } else { + a = 6; + } + +// CHECK: Block: B10, Pred: B11, B12, B14, Succ: B9, B8 +// CHECK: Widened bounds before stmt: !*p +// CHECK: p: bounds(p, p) + +// CHECK: Block: B9, Pred: B10, Succ: B5 +// CHECK: Widened bounds before stmt: a = 4 +// CHECK: p: bounds(p, p) + +// CHECK: Block: B8, Pred: B10, Succ: B7, B6 +// CHECK: Widened bounds before stmt: !*p +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B7, Pred: B8, Succ: B5 +// CHECK: Widened bounds before stmt: a = 5 +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B6, Pred: B8, Succ: B5 +// CHECK: Widened bounds before stmt: a = 6 +// CHECK: p: bounds(p, p + 1) + + if (!*p) { + a = 7; + } else if (*(p + 1) != '\0') { + a = 8; + } else { + a = 9; + } + +// CHECK: Block: B5, Pred: B6, B7, B9, Succ: B4, B3 +// CHECK: Widened bounds before stmt: !*p +// CHECK: p: bounds(p, p) + +// CHECK: Block: B4, Pred: B5, Succ: B0 +// CHECK: Widened bounds before stmt: a = 7 +// CHECK: p: bounds(p, p) + +// CHECK: Block: B3, Pred: B5, Succ: B2, B1 +// CHECK: Widened bounds before stmt: *(p + 1) != '\x00' +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B2, Pred: B3, Succ: B0 +// CHECK: Widened bounds before stmt: a = 8 +// CHECK: p: bounds(p, p + 1 + 1) + +// CHECK: Block: B1, Pred: B3, Succ: B0 +// CHECK: Widened bounds before stmt: a = 9 +// CHECK: p: bounds(p, p + 1) +} + +void f4(_Nt_array_ptr p : bounds(p, p)) { + if (!!*p) { + a = 1; + } + +// CHECK: Function: f4 +// CHECK: Block: B13 (Entry), Pred: Succ: B12 + +// CHECK: Block: B12, Pred: B13, Succ: B11, B10 +// CHECK: Widened bounds before stmt: !!*p +// CHECK: p: bounds(p, p) + +// CHECK: Block: B11, Pred: B12, Succ: B10 +// CHECK: Widened bounds before stmt: a = 1 +// CHECK: p: bounds(p, p + 1) + + if (!!!*p) { + a = 2; + } + +// CHECK: Block: B10, Pred: B11, B12, Succ: B9, B8 +// CHECK: Widened bounds before stmt: !!!*p +// CHECK: p: bounds(p, p) + +// CHECK: Block: B9, Pred: B10, Succ: B8 +// CHECK: Widened bounds before stmt: a = 2 +// CHECK: p: bounds(p, p) + + if (!(*p == 0)) { + a = 3; + } + +// CHECK: Block: B8, Pred: B9, B10, Succ: B7, B6 +// CHECK: Widened bounds before stmt: !(*p == 0) +// CHECK: p: bounds(p, p) + +// CHECK: Block: B7, Pred: B8, Succ: B6 +// CHECK: Widened bounds before stmt: a = 3 +// CHECK: p: bounds(p, p + 1) + + if (!(*p != 0)) { + a = 4; + } + +// CHECK: Block: B6, Pred: B7, B8, Succ: B5, B4 +// CHECK: Widened bounds before stmt: !(*p != 0) +// CHECK: p: bounds(p, p) + +// CHECK: Block: B5, Pred: B6, Succ: B4 +// CHECK: Widened bounds before stmt: a = 4 +// CHECK: p: bounds(p, p) + + if (!!(*p != 0)) { + a = 5; + } + +// CHECK: Block: B4, Pred: B5, B6, Succ: B3, B2 +// CHECK: Widened bounds before stmt: !!(*p != 0) +// CHECK: p: bounds(p, p) + +// CHECK: Block: B3, Pred: B4, Succ: B2 +// CHECK: Widened bounds before stmt: a = 5 +// CHECK: p: bounds(p, p + 1) + + if (!!!(!*p == 0)) { + a = 6; + } + +// CHECK: Block: B2, Pred: B3, B4, Succ: B1, B0 +// CHECK: Widened bounds before stmt: !!!(!*p == 0) +// CHECK: p: bounds(p, p) + +// CHECK: Block: B1, Pred: B2, Succ: B0 +// CHECK: Widened bounds before stmt: a = 6 +// CHECK: p: bounds(p, p) +} + +void f5(_Nt_array_ptr p : bounds(p, p + 1)) { + char c; + + if ((c = *(p + 1)) == 'a') { + a = 1; + } + +// CHECK: Function: f5 +// CHECK: Block: B10 (Entry), Pred: Succ: B9 + +// CHECK: Block: B9, Pred: B10, Succ: B8, B7 +// CHECK: Widened bounds before stmt: char c; +// CHECK: p: bounds(p, p + 1) + +// CHECK: Widened bounds before stmt: (c = *(p + 1)) == 'a' +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B8, Pred: B9, Succ: B7 +// CHECK: Widened bounds before stmt: a = 1 +// CHECK: p: bounds(p, p + 1 + 1) + + if (0 != (c = *(p + 1))) { + a = 2; + } + +// CHECK: Block: B7, Pred: B8, B9, Succ: B6, B5 +// CHECK: Widened bounds before stmt: 0 != (c = *(p + 1)) +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B6, Pred: B7, Succ: B5 +// CHECK: Widened bounds before stmt: a = 2 +// CHECK: p: bounds(p, p + 1 + 1) + + if ((c = *(p + 1))) { + a = 3; + } + +// CHECK: Block: B5, Pred: B6, B7, Succ: B4, B3 +// CHECK: Widened bounds before stmt: c = *(p + 1) +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B4, Pred: B5, Succ: B3 +// CHECK: Widened bounds before stmt: a = 3 +// CHECK: p: bounds(p, p + 1 + 1) + + if ((c = !*(p + 1))) { + a = 4; + } + +// CHECK: Block: B3, Pred: B4, B5, Succ: B2, B1 +// CHECK: Widened bounds before stmt: c = !*(p + 1) +// CHECK: p: bounds(p, p + 1) + +// CHECK: Block: B2, Pred: B3, Succ: B1 +// CHECK: Widened bounds before stmt: a = 4 +// CHECK: p: bounds(p, p + 1) +} + +void f6(_Nt_array_ptr p : count(0), + _Nt_array_ptr q : count(0)) { + + if (*p == a) { + a = 1; + } + +// CHECK: Function: f6 +// CHECK: Block: B22 (Entry), Pred: Succ: B21 + +// CHECK: Block: B21, Pred: B22, Succ: B20, B19 +// CHECK: Widened bounds before stmt: *p == a +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B20, Pred: B21, Succ: B19 +// CHECK: Widened bounds before stmt: a = 1 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + + if (*p == *q) { + a = 2; + } + +// CHECK: Block: B19, Pred: B20, B21, Succ: B18, B17 +// CHECK: Widened bounds before stmt: *p == *q +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B18, Pred: B19, Succ: B17 +// CHECK: Widened bounds before stmt: a = 2 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + + if ((*p == 'a') && (*q != 'b')) { + a = 3; + } + +// CHECK: Block: B17, Pred: B18, B19, Succ: B16, B14 +// CHECK: Widened bounds before stmt: *p == 'a' +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B16, Pred: B17, Succ: B15, B14 +// CHECK: Widened bounds before stmt: *q != 'b' +// CHECK: p: bounds(p, p + 1) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B15, Pred: B16, Succ: B14 +// CHECK: Widened bounds before stmt: a = 3 +// CHECK: p: bounds(p, p + 1) +// CHECK: q: bounds(q, q + 0) + + if (*p && !*q) { + a = 4; + } else if (!*p && *q) { + a = 5; + } + +// CHECK: Block: B14, Pred: B15, B16, B17, Succ: B13, B11 +// CHECK: Widened bounds before stmt: *p +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B13, Pred: B14, Succ: B12, B11 +// CHECK: Widened bounds before stmt: !*q +// CHECK: p: bounds(p, p + 1) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B12, Pred: B13, Succ: B8 +// CHECK: Widened bounds before stmt: a = 4 +// CHECK: p: bounds(p, p + 1) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B11, Pred: B13, B14, Succ: B10, B8 +// CHECK: Widened bounds before stmt: !*p +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B10, Pred: B11, Succ: B9, B8 +// CHECK: Widened bounds before stmt: *q +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B9, Pred: B10, Succ: B8 +// CHECK: Widened bounds before stmt: a = 5 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 1) + + if (*p != 0 && *(p + 1) && 'a' == *(p + 2)) { + a = 6; + } + +// CHECK: Block: B8, Pred: B9, B10, B11, B12, Succ: B7, B4 +// CHECK: Widened bounds before stmt: *p != 0 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B7, Pred: B8, Succ: B6, B4 +// CHECK: Widened bounds before stmt: *(p + 1) +// CHECK: p: bounds(p, p + 1) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B6, Pred: B7, Succ: B5, B4 +// CHECK: Widened bounds before stmt: 'a' == *(p + 2) +// CHECK: p: bounds(p, p + 1 + 1) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B5, Pred: B6, Succ: B4 +// CHECK: Widened bounds before stmt: a = 6 +// CHECK: p: bounds(p, p + 2 + 1) +// CHECK: q: bounds(q, q + 0) + + char c, d; + if (((c = *p) != 0) && 'a' == (d = *q)) { + a = 7; + } + +// CHECK: Block: B4, Pred: B5, B6, B7, B8, Succ: B3, B1 +// CHECK: Widened bounds before stmt: char c; +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Widened bounds before stmt: char d; +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Widened bounds before stmt: (c = *p) != 0 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B3, Pred: B4, Succ: B2, B1 +// CHECK: Widened bounds before stmt: 'a' == (d = *q) +// CHECK: p: bounds(p, p + 1) +// CHECK: q: bounds(q, q + 0) + +// CHECK: Block: B2, Pred: B3, Succ: B1 +// CHECK: Widened bounds before stmt: a = 7 +// CHECK: p: bounds(p, p + 1) +// CHECK: q: bounds(q, q + 1) +} + +void f7(char p _Nt_checked[1], char q _Nt_checked[2]) { + if (p[0] != '\0' && 'a' == 1[q] && p[1] == 'b') { + a = 1; + } + +// CHECK: Function: f7 +// CHECK: Block: B17 (Entry), Pred: Succ: B16 + +// CHECK: Block: B16, Pred: B17, Succ: B15, B12 +// CHECK: Widened bounds before stmt: p[0] != '\x00' +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Block: B15, Pred: B16, Succ: B14, B12 +// CHECK: Widened bounds before stmt: 'a' == 1[q] +// CHECK: p: bounds(p, p + 0 + 1) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Block: B14, Pred: B15, Succ: B13, B12 +// CHECK: Widened bounds before stmt: p[1] == 'b' +// CHECK: p: bounds(p, p + 0 + 1) +// CHECK: q: bounds(q, q + 1 + 1) + +// CHECK: Block: B13, Pred: B14, Succ: B12 +// CHECK: Widened bounds before stmt: a = 1 +// CHECK: p: bounds(p, p + 1 + 1) +// CHECK: q: bounds(q, q + 1 + 1) + + char c; + if ((c = q[0]) && (c = q[1])) { + a = 2; + } + +// CHECK: Block: B12, Pred: B13, B14, B15, B16, Succ: B11, B9 +// CHECK: Widened bounds before stmt: char c; +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Widened bounds before stmt: (c = q[0]) +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Block: B11, Pred: B12, Succ: B10, B9 +// CHECK: Widened bounds before stmt: (c = q[1]) +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Block: B10, Pred: B11, Succ: B9 +// CHECK: Widened bounds before stmt: a = 2 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 1 + 1) + + if (!p[0]) { + a = 3; + } else if (!q[1]) { + a = 4; + } else if (p[0]) { + a = 5; + } else if (q[1]) { + a = 6; + } + +// CHECK: Block: B9, Pred: B10, B11, B12, Succ: B8, B7 +// CHECK: Widened bounds before stmt: !p[0] +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Block: B8, Pred: B9, Succ: B1 +// CHECK: Widened bounds before stmt: a = 3 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Block: B7, Pred: B9, Succ: B6, B5 +// CHECK: Widened bounds before stmt: !q[1] +// CHECK: p: bounds(p, p + 0 + 1) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Block: B6, Pred: B7, Succ: B1 +// CHECK: Widened bounds before stmt: a = 4 +// CHECK: p: bounds(p, p + 0 + 1) +// CHECK: q: bounds(q, q + 1) + +// CHECK: Block: B5, Pred: B7, Succ: B4, B3 +// CHECK: Widened bounds before stmt: p[0] +// CHECK: p: bounds(p, p + 0 + 1) +// CHECK: q: bounds(q, q + 1 + 1) + +// CHECK: Block: B4, Pred: B5, Succ: B1 +// CHECK: Widened bounds before stmt: a = 5 +// CHECK: p: bounds(p, p + 0 + 1) +// CHECK: q: bounds(q, q + 1 + 1) + +// CHECK: Block: B3, Pred: B5, Succ: B2, B1 +// CHECK: Widened bounds before stmt: q[1] +// CHECK: p: bounds(p, p + 0 + 1) +// CHECK: q: bounds(q, q + 1 + 1) + +// CHECK: Block: B2, Pred: B3, Succ: B1 +// CHECK: Widened bounds before stmt: a = 6 +// CHECK: p: bounds(p, p + 0 + 1) +// CHECK: q: bounds(q, q + 1 + 1) +} From 1a2798b12b8783da51f12fb3ca191cbab80e5dd4 Mon Sep 17 00:00:00 2001 From: Mandeep Singh Grang Date: Mon, 9 Aug 2021 16:04:32 -0700 Subject: [PATCH 2/8] Fixed comment --- clang/include/clang/Sema/BoundsWideningAnalysis.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/include/clang/Sema/BoundsWideningAnalysis.h b/clang/include/clang/Sema/BoundsWideningAnalysis.h index 6d868360920c..ff0abaa84060 100644 --- a/clang/include/clang/Sema/BoundsWideningAnalysis.h +++ b/clang/include/clang/Sema/BoundsWideningAnalysis.h @@ -333,8 +333,7 @@ namespace clang { // The last statement of the block. This is nullptr if the block is empty. const Stmt *LastStmt = nullptr; - // The terminating condition that dereferences a pointer. This is nullptr - // if the terminating condition does not dereference a pointer. + // Various information about the terminating condition of the block. TermCondInfoTy TermCondInfo; // The In set of the last statment of each block. From 2388d7f20ba7180c271c36a2378aadac29c18baf Mon Sep 17 00:00:00 2001 From: Mandeep Singh Grang Date: Wed, 11 Aug 2021 12:16:12 -0700 Subject: [PATCH 3/8] Addressed review comments --- .../clang/Sema/BoundsWideningAnalysis.h | 39 +---- clang/lib/Sema/BoundsWideningAnalysis.cpp | 162 +++++++++--------- 2 files changed, 87 insertions(+), 114 deletions(-) diff --git a/clang/include/clang/Sema/BoundsWideningAnalysis.h b/clang/include/clang/Sema/BoundsWideningAnalysis.h index ff0abaa84060..2362993380a0 100644 --- a/clang/include/clang/Sema/BoundsWideningAnalysis.h +++ b/clang/include/clang/Sema/BoundsWideningAnalysis.h @@ -71,29 +71,12 @@ namespace clang { // if (*(p + i) == 0) ==> DerefExpr = p + i Expr *DerefExpr; - // Whether the value that an expression is compared against is null. For - // example: - // if (*p == 0) ==> IsComparedValNull = True - // if (*p == 'a') ==> IsComparedValNull = False - bool IsComparedValNull; - - // Whether the operator of the conditional is positive. For example: - // if (*p == 0) ==> IsCheckPositive = True - // if (*p != 0) ==> IsCheckPositive = False - bool IsCheckPositive; - - // Whether the terminating condition is an expression involving a binary - // operator. For example: - // if (*p == 0) ==> HasBinaryOp = True - // if (!*p) ==> HasBinaryOp = False - bool HasBinaryOp; - - // Whether the terminating condition is an expression involving a unary not - // operator. For example: - // if (!*p) ==> HasUnaryNot = True - // if (!(*p == 0)) ==> HasUnaryNot = True - // if (*p == 0) ==> HasUnaryNot = False - bool HasUnaryNot; + // Whether the terminating condition checks for a null value. For example: + // if (*p == 0) ==> IsCheckNull = True + // if (*p != 0) ==> IsCheckNull = False + // if (*p == 'a') ==> IsCheckNull = False + // if (*p != 'a') ==> IsCheckNull = True + bool IsCheckNull; }; } // end namespace clang @@ -204,7 +187,8 @@ namespace clang { // @param[out] TermCondInfo is the struct that is filled with various // information about the terminating condition. void FillTermCondInfo(const Expr *TermCond, - TermCondInfoTy &TermCondInfo) const; + TermCondInfoTy &TermCondInfo, + bool HasUnaryNot) const; // Get the variable in an expression that is a pointer to a null-terminated // array. @@ -568,13 +552,6 @@ namespace clang { const Stmt *CurrStmt, VarSetTy PtrsWithAffectedBounds) const; - // Check whether the terminating condition of a block tests for a - // null-terminator. - // @param[in] EB is the current ElevatedCFGBlock. - // @return Returns true if the terminating condition of block EB tests for - // a null-terminator, false otherwise. - bool DoesTermCondCheckNull(ElevatedCFGBlock *EB) const; - // Update the bounds in StmtOut with the adjusted bounds for the current // statement, if they exist. // @param[in] EB is the current ElevatedCFGBlock. diff --git a/clang/lib/Sema/BoundsWideningAnalysis.cpp b/clang/lib/Sema/BoundsWideningAnalysis.cpp index a817baf5a837..1a29b6ece609 100644 --- a/clang/lib/Sema/BoundsWideningAnalysis.cpp +++ b/clang/lib/Sema/BoundsWideningAnalysis.cpp @@ -361,9 +361,8 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( // Check if the edge from pred to the current block is a true edge. bool IsEdgeTrue = BWUtil.IsTrueEdge(PredBlock, CurrBlock); - // Check if the terminating condition of the pred block tests for a null - // terminator. - bool TermCondChecksNull = DoesTermCondCheckNull(PredEB); + // Does the terminating condition of the pred block tests for a null value. + bool TermCondChecksNull = PredEB->TermCondInfo.IsCheckNull; // Get the In of the last statement in the pred block. If the pred // block does not have any statements then InOfLastStmtOfPred is set to @@ -931,25 +930,6 @@ void BoundsWideningAnalysis::CheckStmtInvertibility(ElevatedCFGBlock *EB, } } -bool BoundsWideningAnalysis::DoesTermCondCheckNull( - ElevatedCFGBlock *EB) const { - - // *p == 0 - // ==> IsComparedValNull = True, IsCheckPositive = True, return True - - // *p != 0 - // ==> IsComparedValNull = True, IsCheckPositive = False, return False - - // *p == 'a' - // ==> IsComparedValNull = False, IsCheckPositive = True, return False - - // *p != 'a' - // ==> IsComparedValNull = False, IsCheckPositive = False, return True - - return EB->TermCondInfo.IsComparedValNull == - EB->TermCondInfo.IsCheckPositive; -} - void BoundsWideningAnalysis::UpdateAdjustedBounds( ElevatedCFGBlock *EB, const Stmt *CurrStmt, BoundsMapTy &StmtOut) const { @@ -1402,63 +1382,64 @@ TermCondInfoTy BoundsWideningUtil::GetTermCondInfo( TermCondInfoTy TermCondInfo; // Initialize fields of TermCondInfo. TermCondInfo.DerefExpr = nullptr; - TermCondInfo.IsComparedValNull = false; - TermCondInfo.IsCheckPositive = false; - TermCondInfo.HasBinaryOp = false; - TermCondInfo.HasUnaryNot = false; + TermCondInfo.IsCheckNull = false; - FillTermCondInfo(TermCond, TermCondInfo); + FillTermCondInfo(TermCond, TermCondInfo, /*HasUnaryNot*/ false); return TermCondInfo; } void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, - TermCondInfoTy &TermCondInfo) const { + TermCondInfoTy &TermCondInfo, + bool HasUnaryNot) const { Expr *E = StripCasts(TermCond); + // *p, !*p, !(*p == 0), etc. if (auto *UO = dyn_cast(E)) { UnaryOperatorKind UnaryOp = UO->getOpcode(); + // *p, LHS of *p == 0, etc. if (UnaryOp == UO_Deref) { Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); + // If the expression is a binary expression then we set + // TermCondInfo.DerefExpr to the binary expression just to signal that we + // have visited a biary expression. If TermCondInfo.DerefExpr is nullptr + // then it means we did not visit a binary expression. + if (!TermCondInfo.DerefExpr) { + // If we do not have a binary operator then we do the following + // normalizations: + // *p ==> *p != 0 + // !*p ==> *p == 0 + // In the above normalizations the compared value would always be null. + // Whether the check is null or not is determined by the + // presence/absence of the unary not operator as follows: + // *p == 0 ==> HasUnaryNot = True, IsCheckNull = True + // *p != 0 ==> HasUnaryNot = False, IsCheckNull = False + TermCondInfo.IsCheckNull = HasUnaryNot; + } + // *p ==> DerefExpr = p - // *(p + i) ==> DerefExpr = p + i TermCondInfo.DerefExpr = SubExpr; - if (TermCondInfo.HasBinaryOp) { - // *p == 0 ==> HasUnaryNot = False, IsCheckPositive = True - // *p != 0 ==> HasUnaryNot = True, IsCheckPositive = False - // !(*p == 0) ==> HasUnaryNot = True, IsCheckPositive = False - // !(*p != 0) ==> HasUnaryNot = False, IsCheckPositive = True - TermCondInfo.IsCheckPositive = - TermCondInfo.IsCheckPositive != TermCondInfo.HasUnaryNot; - - } else { - // *p ==> *p != 0 ==> HasUnaryNot = False - // IsCheckPositive = False, - // IsComparedValNull = True - - // !*p ==> *p == 0 ==> HasUnaryNot = True - // IsCheckPositive = True, - // IsComparedValNull = True - TermCondInfo.IsCheckPositive = TermCondInfo.HasUnaryNot; - TermCondInfo.IsComparedValNull = true; - } - + // !*p, !!!*p, !(*p == 0), etc. } else if (UnaryOp == UO_LNot) { Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); + // *p ==> *p != 0 ==> HasUnaryNot = False, IsCheckNull = False + // !*p ==> *p == 0 ==> HasUnaryNot = True, IsCheckNull = True + TermCondInfo.IsCheckNull = HasUnaryNot; + // *p ==> HasUnaryNot = False // !*p ==> HasUnaryNot = True // !!*p ==> HasUnaryNot = False // !!!*p ==> HasUnaryNot = True - TermCondInfo.HasUnaryNot = !TermCondInfo.HasUnaryNot; + HasUnaryNot = !HasUnaryNot; // !*p ==> FillTermCondInfo(*p); // !!*p ==> FillTermCondInfo(!*p); // !!!*p ==> FillTermCondInfo(!!*p); // !(*p == 0) ==> FillTermCondInfo(*p == 0); - FillTermCondInfo(SubExpr, TermCondInfo); + FillTermCondInfo(SubExpr, TermCondInfo, HasUnaryNot); } // If dereference expression contains an array access. An array access can @@ -1466,44 +1447,34 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // always present the normalized view: A[i]. In this case getBase() returns // "A" and getIdx() returns "i". } else if (auto *AE = dyn_cast(E)) { + // If the expression is a binary expression then we set + // TermCondInfo.DerefExpr to the binary expression just to signal that we + // have visited a biary expression. If TermCondInfo.DerefExpr is nullptr + // then it means we did not visit a binary expression. + if (!TermCondInfo.DerefExpr) { + // If we do not have a binary operator then we do the following + // normalizations: + // p[i] ==> p[i] != 0 + // !p[i] ==> p[i] == 0 + // In the above normalizations the compared value would always be null. + // Whether the check is null or not is determined by the + // presence/absence of the unary not operator as follows: + // p[i] == 0 ==> HasUnaryNot = True, IsCheckNull = True + // p[i] != 0 ==> HasUnaryNot = False, IsCheckNull = False + TermCondInfo.IsCheckNull = HasUnaryNot; + } + // p[i] ==> DerefExpr = p + i TermCondInfo.DerefExpr = ExprCreatorUtil::CreateBinaryOperator(SemaRef, AE->getBase(), AE->getIdx(), BinaryOperatorKind::BO_Add); - if (TermCondInfo.HasBinaryOp) { - // p[i] == 0 ==> IsCheckPositive = True - // p[i] != 0 ==> IsCheckPositive = False - // !(p[i] == 0) ==> IsCheckPositive = False - // !(p[i] != 0) ==> IsCheckPositive = True - TermCondInfo.IsCheckPositive = - TermCondInfo.IsCheckPositive != TermCondInfo.HasUnaryNot; - - } else { - // p[i] ==> p[i] != 0 ==> HasUnaryNot = False - // IsCheckPositive = False, - // IsComparedValNull = True - - // !p[i] ==> p[i] == 0 ==> HasUnaryNot = True - // IsCheckPositive = True, - // IsComparedValNull = True - TermCondInfo.IsCheckPositive = TermCondInfo.HasUnaryNot; - TermCondInfo.IsComparedValNull = true; - } } else if (auto *BO = dyn_cast(E)) { BinaryOperatorKind BinOp = BO->getOpcode(); // *p == 0, 0 != *p if (BinOp == BO_EQ || BinOp == BO_NE) { - // *p == 0 ==> HasBinaryOp = True - // *p != 0 ==> HasBinaryOp = True - // (c = *p) != 0 ==> HasBinaryOp = True - TermCondInfo.HasBinaryOp = true; - - // *p == 0 ==> IsCheckPositive = True - // *p != 0 ==> IsCheckPositive = False - TermCondInfo.IsCheckPositive = BinOp == BO_EQ; Expr *LHS = BO->getLHS(); Expr *RHS = BO->getRHS(); @@ -1527,33 +1498,58 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, return; } + // *p == 0 ==> BinOp == BO_EQ, HasUnaryNot = False + // ==> IsCheckPositive = True + // *p != 0 ==> BinOp != BO_EQ, HasUnaryNot = False + // ==> IsCheckPositive = False + // !*p == 0 ==> BinOp == BO_EQ, HasUnaryNot = True + // ==> IsCheckPositive = False + // !*p != 0 ==> BinOp != BO_EQ, HasUnaryNot = True + // ==> IsCheckPositive = True + bool IsCheckPositive = (BinOp == BO_EQ && !HasUnaryNot) || + (BinOp != BO_EQ && HasUnaryNot); + llvm::APSInt Zero(Ctx.getTargetInfo().getIntWidth(), 0); if (LHSVal) { // 0 == *p ==> IsComparedValNull = True // 'a' != *p ==> IsComparedValNull = False - TermCondInfo.IsComparedValNull = + bool IsComparedValNull = llvm::APSInt::compareValues(*LHSVal, Zero) == 0; + TermCondInfo.IsCheckNull = IsCheckPositive == + IsComparedValNull; + + // We are setting DerefExpr here to the RHS just to signal to unary + // operator logic above that a binary expression has been visited. + TermCondInfo.DerefExpr = RHS; + // 0 == *p ==> FillTermCondInfo(*p); // 'a' != *p ==> FillTermCondInfo(*p); - FillTermCondInfo(RHS, TermCondInfo); + FillTermCondInfo(RHS, TermCondInfo, HasUnaryNot); } else if (RHSVal) { // *p == 0 ==> IsComparedValNull = True // *p != 'a' ==> IsComparedValNull = False - TermCondInfo.IsComparedValNull = + bool IsComparedValNull = llvm::APSInt::compareValues(*RHSVal, Zero) == 0; + TermCondInfo.IsCheckNull = IsCheckPositive == + IsComparedValNull; + + // We are setting DerefExpr here to the LHS just to signal to unary + // operator logic above that a binary expression has been visited. + TermCondInfo.DerefExpr = LHS; + // *p == 0 ==> FillTermCondInfo(*p); // *p != 'a' ==> FillTermCondInfo(*p); - FillTermCondInfo(LHS, TermCondInfo); + FillTermCondInfo(LHS, TermCondInfo, HasUnaryNot); } // (c = *p) != 0 } else if (BinOp == BO_Assign) { // (c = *p) ==> RHS = *p Expr *RHS = BO->getRHS(); - FillTermCondInfo(RHS, TermCondInfo); + FillTermCondInfo(RHS, TermCondInfo, HasUnaryNot); } } } From ee6c6775dc698bac3f56b9a5c224d81403720b2f Mon Sep 17 00:00:00 2001 From: Mandeep Singh Grang Date: Wed, 11 Aug 2021 12:25:15 -0700 Subject: [PATCH 4/8] Update comments --- clang/lib/Sema/BoundsWideningAnalysis.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/clang/lib/Sema/BoundsWideningAnalysis.cpp b/clang/lib/Sema/BoundsWideningAnalysis.cpp index 1a29b6ece609..2220aaaecf9a 100644 --- a/clang/lib/Sema/BoundsWideningAnalysis.cpp +++ b/clang/lib/Sema/BoundsWideningAnalysis.cpp @@ -752,8 +752,7 @@ void BoundsWideningAnalysis::GetVarsAndBoundsInPtrDeref( return; // Get information about the terminating condition such as the dereference - // expression, whether the condition check is positive or not (== vs !=), - // whether the compared value is null or not (== '0' vs == 'a'). + // expression and whether the condition tests for a null value. EB->TermCondInfo = BWUtil.GetTermCondInfo(TermCond); // If the terminating condition does not contain a dereference (or an array @@ -1401,10 +1400,11 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, if (UnaryOp == UO_Deref) { Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); - // If the expression is a binary expression then we set - // TermCondInfo.DerefExpr to the binary expression just to signal that we - // have visited a biary expression. If TermCondInfo.DerefExpr is nullptr - // then it means we did not visit a binary expression. + // In the logic for a binary expression below we set + // TermCondInfo.DerefExpr to either the LHS or the RHS of the binary + // expression just to signal that we have visited a biary expression. If + // TermCondInfo.DerefExpr is nullptr here then it means we did not visit + // a binary expression. if (!TermCondInfo.DerefExpr) { // If we do not have a binary operator then we do the following // normalizations: @@ -1447,10 +1447,10 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // always present the normalized view: A[i]. In this case getBase() returns // "A" and getIdx() returns "i". } else if (auto *AE = dyn_cast(E)) { - // If the expression is a binary expression then we set - // TermCondInfo.DerefExpr to the binary expression just to signal that we - // have visited a biary expression. If TermCondInfo.DerefExpr is nullptr - // then it means we did not visit a binary expression. + // In the logic for a binary expression below we set TermCondInfo.DerefExpr + // to either the LHS or the RHS of the binary expression just to signal + // that we have visited a biary expression. If TermCondInfo.DerefExpr is + // nullptr here then it means we did not visit a binary expression. if (!TermCondInfo.DerefExpr) { // If we do not have a binary operator then we do the following // normalizations: @@ -1473,9 +1473,8 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, } else if (auto *BO = dyn_cast(E)) { BinaryOperatorKind BinOp = BO->getOpcode(); - // *p == 0, 0 != *p + // *p == 0, 0 != *p, etc. if (BinOp == BO_EQ || BinOp == BO_NE) { - Expr *LHS = BO->getLHS(); Expr *RHS = BO->getRHS(); From b427e29b34d4fc290c857832842cb7d0578e496b Mon Sep 17 00:00:00 2001 From: Mandeep Singh Grang Date: Wed, 11 Aug 2021 15:22:45 -0700 Subject: [PATCH 5/8] Addressed offline review comments --- .../clang/Sema/BoundsWideningAnalysis.h | 16 +-- clang/lib/Sema/BoundsWideningAnalysis.cpp | 98 +++++++++---------- 2 files changed, 53 insertions(+), 61 deletions(-) diff --git a/clang/include/clang/Sema/BoundsWideningAnalysis.h b/clang/include/clang/Sema/BoundsWideningAnalysis.h index 2362993380a0..a7209b1d58cc 100644 --- a/clang/include/clang/Sema/BoundsWideningAnalysis.h +++ b/clang/include/clang/Sema/BoundsWideningAnalysis.h @@ -71,12 +71,13 @@ namespace clang { // if (*(p + i) == 0) ==> DerefExpr = p + i Expr *DerefExpr; - // Whether the terminating condition checks for a null value. For example: - // if (*p == 0) ==> IsCheckNull = True - // if (*p != 0) ==> IsCheckNull = False - // if (*p == 'a') ==> IsCheckNull = False - // if (*p != 'a') ==> IsCheckNull = True - bool IsCheckNull; + // Whether the terminating condition asserts nullness of the element. For + // example: + // if (*p == 0) ==> DoesCondAssertNullness = True + // if (*p != 0) ==> DoesCondAssertNullness = False + // if (*p == 'a') ==> DoesCondAssertNullness = False + // if (*p != 'a') ==> DoesCondAssertNullness = True + bool DoesCondAssertNullness; }; } // end namespace clang @@ -187,8 +188,7 @@ namespace clang { // @param[out] TermCondInfo is the struct that is filled with various // information about the terminating condition. void FillTermCondInfo(const Expr *TermCond, - TermCondInfoTy &TermCondInfo, - bool HasUnaryNot) const; + TermCondInfoTy &TermCondInfo) const; // Get the variable in an expression that is a pointer to a null-terminated // array. diff --git a/clang/lib/Sema/BoundsWideningAnalysis.cpp b/clang/lib/Sema/BoundsWideningAnalysis.cpp index 2220aaaecf9a..a4caea0a5ae3 100644 --- a/clang/lib/Sema/BoundsWideningAnalysis.cpp +++ b/clang/lib/Sema/BoundsWideningAnalysis.cpp @@ -362,7 +362,7 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( bool IsEdgeTrue = BWUtil.IsTrueEdge(PredBlock, CurrBlock); // Does the terminating condition of the pred block tests for a null value. - bool TermCondChecksNull = PredEB->TermCondInfo.IsCheckNull; + bool TermCondAssertsNullness = PredEB->TermCondInfo.DoesCondAssertNullness; // Get the In of the last statement in the pred block. If the pred // block does not have any statements then InOfLastStmtOfPred is set to @@ -438,23 +438,23 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( if (!IsSwitchCase) { // if (*p != 0) { - // IsEdgeTrue = True, TermCondChecksNull = False + // IsEdgeTrue = True, TermCondAssertsNullness = False // ==> widen bounds of p // // } else { - // IsEdgeTrue = False, TermCondChecksNull = False + // IsEdgeTrue = False, TermCondAssertsNullness = False // ==> do not widen bounds of p // } // if (*p == 0) { - // IsEdgeTrue = True, TermCondChecksNull = True + // IsEdgeTrue = True, TermCondAssertsNullness = True // ==> do not widen bounds of p // // } else { - // IsEdgeTrue = False, TermCondChecksNull = True + // IsEdgeTrue = False, TermCondAssertsNullness = True // ==> widen bounds of p // } - if (IsEdgeTrue == TermCondChecksNull) { + if (IsEdgeTrue == TermCondAssertsNullness) { PrunedOutSet[V] = BoundsOfVInStmtIn; continue; } @@ -752,7 +752,7 @@ void BoundsWideningAnalysis::GetVarsAndBoundsInPtrDeref( return; // Get information about the terminating condition such as the dereference - // expression and whether the condition tests for a null value. + // expression and whether the condition asserts nullness of the element. EB->TermCondInfo = BWUtil.GetTermCondInfo(TermCond); // If the terminating condition does not contain a dereference (or an array @@ -1381,15 +1381,14 @@ TermCondInfoTy BoundsWideningUtil::GetTermCondInfo( TermCondInfoTy TermCondInfo; // Initialize fields of TermCondInfo. TermCondInfo.DerefExpr = nullptr; - TermCondInfo.IsCheckNull = false; + TermCondInfo.DoesCondAssertNullness = false; - FillTermCondInfo(TermCond, TermCondInfo, /*HasUnaryNot*/ false); + FillTermCondInfo(TermCond, TermCondInfo); return TermCondInfo; } void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, - TermCondInfoTy &TermCondInfo, - bool HasUnaryNot) const { + TermCondInfoTy &TermCondInfo) const { Expr *E = StripCasts(TermCond); // *p, !*p, !(*p == 0), etc. @@ -1411,11 +1410,10 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // *p ==> *p != 0 // !*p ==> *p == 0 // In the above normalizations the compared value would always be null. - // Whether the check is null or not is determined by the - // presence/absence of the unary not operator as follows: - // *p == 0 ==> HasUnaryNot = True, IsCheckNull = True - // *p != 0 ==> HasUnaryNot = False, IsCheckNull = False - TermCondInfo.IsCheckNull = HasUnaryNot; + // Whether the condition asserts nullness will be determined in the + // logic for UO_LNot after we return from the current recursive. So for + // now simply set TermCondInfo.DoesCondAssertNullness to false. + TermCondInfo.DoesCondAssertNullness = false; } // *p ==> DerefExpr = p @@ -1425,21 +1423,18 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, } else if (UnaryOp == UO_LNot) { Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); - // *p ==> *p != 0 ==> HasUnaryNot = False, IsCheckNull = False - // !*p ==> *p == 0 ==> HasUnaryNot = True, IsCheckNull = True - TermCondInfo.IsCheckNull = HasUnaryNot; - - // *p ==> HasUnaryNot = False - // !*p ==> HasUnaryNot = True - // !!*p ==> HasUnaryNot = False - // !!!*p ==> HasUnaryNot = True - HasUnaryNot = !HasUnaryNot; - // !*p ==> FillTermCondInfo(*p); // !!*p ==> FillTermCondInfo(!*p); // !!!*p ==> FillTermCondInfo(!!*p); // !(*p == 0) ==> FillTermCondInfo(*p == 0); - FillTermCondInfo(SubExpr, TermCondInfo, HasUnaryNot); + FillTermCondInfo(SubExpr, TermCondInfo); + + // *p ==> DoesCondAssertNullness = False + // !*p ==> DoesCondAssertNullness = True + // !!*p ==> DoesCondAssertNullness = False + // !!!*p ==> DoesCondAssertNullness = True + TermCondInfo.DoesCondAssertNullness = + !TermCondInfo.DoesCondAssertNullness; } // If dereference expression contains an array access. An array access can @@ -1457,11 +1452,10 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // p[i] ==> p[i] != 0 // !p[i] ==> p[i] == 0 // In the above normalizations the compared value would always be null. - // Whether the check is null or not is determined by the - // presence/absence of the unary not operator as follows: - // p[i] == 0 ==> HasUnaryNot = True, IsCheckNull = True - // p[i] != 0 ==> HasUnaryNot = False, IsCheckNull = False - TermCondInfo.IsCheckNull = HasUnaryNot; + // Whether the condition asserts nullness will be determined in the + // logic for UO_LNot after we return from the current recursive. So for + // now simply set TermCondInfo.DoesCondAssertNullness to false. + TermCondInfo.DoesCondAssertNullness = false; } // p[i] ==> DerefExpr = p + i @@ -1497,58 +1491,56 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, return; } - // *p == 0 ==> BinOp == BO_EQ, HasUnaryNot = False - // ==> IsCheckPositive = True - // *p != 0 ==> BinOp != BO_EQ, HasUnaryNot = False - // ==> IsCheckPositive = False - // !*p == 0 ==> BinOp == BO_EQ, HasUnaryNot = True - // ==> IsCheckPositive = False - // !*p != 0 ==> BinOp != BO_EQ, HasUnaryNot = True - // ==> IsCheckPositive = True - bool IsCheckPositive = (BinOp == BO_EQ && !HasUnaryNot) || - (BinOp != BO_EQ && HasUnaryNot); + bool IsComparedValNull = false; llvm::APSInt Zero(Ctx.getTargetInfo().getIntWidth(), 0); if (LHSVal) { // 0 == *p ==> IsComparedValNull = True // 'a' != *p ==> IsComparedValNull = False - bool IsComparedValNull = + IsComparedValNull = llvm::APSInt::compareValues(*LHSVal, Zero) == 0; - TermCondInfo.IsCheckNull = IsCheckPositive == - IsComparedValNull; - // We are setting DerefExpr here to the RHS just to signal to unary // operator logic above that a binary expression has been visited. TermCondInfo.DerefExpr = RHS; // 0 == *p ==> FillTermCondInfo(*p); // 'a' != *p ==> FillTermCondInfo(*p); - FillTermCondInfo(RHS, TermCondInfo, HasUnaryNot); + FillTermCondInfo(RHS, TermCondInfo); } else if (RHSVal) { // *p == 0 ==> IsComparedValNull = True // *p != 'a' ==> IsComparedValNull = False - bool IsComparedValNull = + IsComparedValNull = llvm::APSInt::compareValues(*RHSVal, Zero) == 0; - TermCondInfo.IsCheckNull = IsCheckPositive == - IsComparedValNull; - // We are setting DerefExpr here to the LHS just to signal to unary // operator logic above that a binary expression has been visited. TermCondInfo.DerefExpr = LHS; // *p == 0 ==> FillTermCondInfo(*p); // *p != 'a' ==> FillTermCondInfo(*p); - FillTermCondInfo(LHS, TermCondInfo, HasUnaryNot); + FillTermCondInfo(LHS, TermCondInfo); } + // *p == 0 ==> BinOp == BO_EQ, IsComparedValNull = True, + // DoesCondAssertNullness(prev value) = False + // ==> DoesCondAssertNullness = True + + // *p != 'a' ==> BinOp != BO_EQ, IsComparedValNull = False, + // DoesCondAssertNullness(prev value) = False + // ==> DoesCondAssertNullness = True + TermCondInfo.DoesCondAssertNullness = + (BinOp == BO_EQ && IsComparedValNull && + !TermCondInfo.DoesCondAssertNullness) || + (BinOp != BO_EQ && !IsComparedValNull && + !TermCondInfo.DoesCondAssertNullness); + // (c = *p) != 0 } else if (BinOp == BO_Assign) { // (c = *p) ==> RHS = *p Expr *RHS = BO->getRHS(); - FillTermCondInfo(RHS, TermCondInfo, HasUnaryNot); + FillTermCondInfo(RHS, TermCondInfo); } } } From d031dfa37d67f720ada82d6c0bca3b997ea478e8 Mon Sep 17 00:00:00 2001 From: Mandeep Singh Grang Date: Wed, 11 Aug 2021 18:06:44 -0700 Subject: [PATCH 6/8] Renamed variable --- .../clang/Sema/BoundsWideningAnalysis.h | 13 ++--- clang/lib/Sema/BoundsWideningAnalysis.cpp | 58 +++++++++---------- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/clang/include/clang/Sema/BoundsWideningAnalysis.h b/clang/include/clang/Sema/BoundsWideningAnalysis.h index a7209b1d58cc..da9d48f43f39 100644 --- a/clang/include/clang/Sema/BoundsWideningAnalysis.h +++ b/clang/include/clang/Sema/BoundsWideningAnalysis.h @@ -71,13 +71,12 @@ namespace clang { // if (*(p + i) == 0) ==> DerefExpr = p + i Expr *DerefExpr; - // Whether the terminating condition asserts nullness of the element. For - // example: - // if (*p == 0) ==> DoesCondAssertNullness = True - // if (*p != 0) ==> DoesCondAssertNullness = False - // if (*p == 'a') ==> DoesCondAssertNullness = False - // if (*p != 'a') ==> DoesCondAssertNullness = True - bool DoesCondAssertNullness; + // Whether the terminating condition tests for a null value. For example: + // if (*p == 0) ==> IsCheckNull = True + // if (*p != 0) ==> IsCheckNull = False + // if (*p == 'a') ==> IsCheckNull = False + // if (*p != 'a') ==> IsCheckNull = True + bool IsCheckNull; }; } // end namespace clang diff --git a/clang/lib/Sema/BoundsWideningAnalysis.cpp b/clang/lib/Sema/BoundsWideningAnalysis.cpp index a4caea0a5ae3..66947b51456d 100644 --- a/clang/lib/Sema/BoundsWideningAnalysis.cpp +++ b/clang/lib/Sema/BoundsWideningAnalysis.cpp @@ -362,7 +362,7 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( bool IsEdgeTrue = BWUtil.IsTrueEdge(PredBlock, CurrBlock); // Does the terminating condition of the pred block tests for a null value. - bool TermCondAssertsNullness = PredEB->TermCondInfo.DoesCondAssertNullness; + bool DoesTermCondCheckNull = PredEB->TermCondInfo.IsCheckNull; // Get the In of the last statement in the pred block. If the pred // block does not have any statements then InOfLastStmtOfPred is set to @@ -438,23 +438,23 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( if (!IsSwitchCase) { // if (*p != 0) { - // IsEdgeTrue = True, TermCondAssertsNullness = False + // IsEdgeTrue = True, DoesTermCondCheckNull = False // ==> widen bounds of p // // } else { - // IsEdgeTrue = False, TermCondAssertsNullness = False + // IsEdgeTrue = False, DoesTermCondCheckNull = False // ==> do not widen bounds of p // } // if (*p == 0) { - // IsEdgeTrue = True, TermCondAssertsNullness = True + // IsEdgeTrue = True, DoesTermCondCheckNull = True // ==> do not widen bounds of p // // } else { - // IsEdgeTrue = False, TermCondAssertsNullness = True + // IsEdgeTrue = False, DoesTermCondCheckNull = True // ==> widen bounds of p // } - if (IsEdgeTrue == TermCondAssertsNullness) { + if (IsEdgeTrue == DoesTermCondCheckNull) { PrunedOutSet[V] = BoundsOfVInStmtIn; continue; } @@ -752,7 +752,7 @@ void BoundsWideningAnalysis::GetVarsAndBoundsInPtrDeref( return; // Get information about the terminating condition such as the dereference - // expression and whether the condition asserts nullness of the element. + // expression and whether the condition tests for a null value. EB->TermCondInfo = BWUtil.GetTermCondInfo(TermCond); // If the terminating condition does not contain a dereference (or an array @@ -1381,7 +1381,7 @@ TermCondInfoTy BoundsWideningUtil::GetTermCondInfo( TermCondInfoTy TermCondInfo; // Initialize fields of TermCondInfo. TermCondInfo.DerefExpr = nullptr; - TermCondInfo.DoesCondAssertNullness = false; + TermCondInfo.IsCheckNull = false; FillTermCondInfo(TermCond, TermCondInfo); return TermCondInfo; @@ -1410,10 +1410,10 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // *p ==> *p != 0 // !*p ==> *p == 0 // In the above normalizations the compared value would always be null. - // Whether the condition asserts nullness will be determined in the - // logic for UO_LNot after we return from the current recursive. So for - // now simply set TermCondInfo.DoesCondAssertNullness to false. - TermCondInfo.DoesCondAssertNullness = false; + // Whether the condition tests for a null value will be determined in + // the logic for UO_LNot after we return from the current recursive + // call. So for now simply set TermCondInfo.IsCheckNull to false. + TermCondInfo.IsCheckNull = false; } // *p ==> DerefExpr = p @@ -1429,12 +1429,12 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // !(*p == 0) ==> FillTermCondInfo(*p == 0); FillTermCondInfo(SubExpr, TermCondInfo); - // *p ==> DoesCondAssertNullness = False - // !*p ==> DoesCondAssertNullness = True - // !!*p ==> DoesCondAssertNullness = False - // !!!*p ==> DoesCondAssertNullness = True - TermCondInfo.DoesCondAssertNullness = - !TermCondInfo.DoesCondAssertNullness; + // *p ==> IsCheckNull = False + // !*p ==> IsCheckNull = True + // !!*p ==> IsCheckNull = False + // !!!*p ==> IsCheckNull = True + TermCondInfo.IsCheckNull = + !TermCondInfo.IsCheckNull; } // If dereference expression contains an array access. An array access can @@ -1452,10 +1452,10 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // p[i] ==> p[i] != 0 // !p[i] ==> p[i] == 0 // In the above normalizations the compared value would always be null. - // Whether the condition asserts nullness will be determined in the - // logic for UO_LNot after we return from the current recursive. So for - // now simply set TermCondInfo.DoesCondAssertNullness to false. - TermCondInfo.DoesCondAssertNullness = false; + // Whether the condition tests for a null value will be determined in + // the logic for UO_LNot after we return from the current recursive + // call. So for now simply set TermCondInfo.IsCheckNull to false. + TermCondInfo.IsCheckNull = false; } // p[i] ==> DerefExpr = p + i @@ -1524,17 +1524,17 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, } // *p == 0 ==> BinOp == BO_EQ, IsComparedValNull = True, - // DoesCondAssertNullness(prev value) = False - // ==> DoesCondAssertNullness = True + // IsCheckNull(prev value) = False + // ==> IsCheckNull = True // *p != 'a' ==> BinOp != BO_EQ, IsComparedValNull = False, - // DoesCondAssertNullness(prev value) = False - // ==> DoesCondAssertNullness = True - TermCondInfo.DoesCondAssertNullness = + // IsCheckNull(prev value) = False + // ==> IsCheckNull = True + TermCondInfo.IsCheckNull = (BinOp == BO_EQ && IsComparedValNull && - !TermCondInfo.DoesCondAssertNullness) || + !TermCondInfo.IsCheckNull) || (BinOp != BO_EQ && !IsComparedValNull && - !TermCondInfo.DoesCondAssertNullness); + !TermCondInfo.IsCheckNull); // (c = *p) != 0 } else if (BinOp == BO_Assign) { From 129b0112fa34769ee6570133faa60a89ea958f6b Mon Sep 17 00:00:00 2001 From: Mandeep Singh Grang Date: Thu, 12 Aug 2021 09:20:29 -0700 Subject: [PATCH 7/8] Simplified code and added more tests --- clang/lib/Sema/BoundsWideningAnalysis.cpp | 177 ++++++++---------- .../widened-bounds-complex-conditionals.c | 63 +++++++ 2 files changed, 138 insertions(+), 102 deletions(-) diff --git a/clang/lib/Sema/BoundsWideningAnalysis.cpp b/clang/lib/Sema/BoundsWideningAnalysis.cpp index 66947b51456d..544a2ec3b488 100644 --- a/clang/lib/Sema/BoundsWideningAnalysis.cpp +++ b/clang/lib/Sema/BoundsWideningAnalysis.cpp @@ -361,7 +361,7 @@ BoundsMapTy BoundsWideningAnalysis::PruneOutSet( // Check if the edge from pred to the current block is a true edge. bool IsEdgeTrue = BWUtil.IsTrueEdge(PredBlock, CurrBlock); - // Does the terminating condition of the pred block tests for a null value. + // Does the terminating condition of the pred block test for a null value. bool DoesTermCondCheckNull = PredEB->TermCondInfo.IsCheckNull; // Get the In of the last statement in the pred block. If the pred @@ -1394,35 +1394,25 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // *p, !*p, !(*p == 0), etc. if (auto *UO = dyn_cast(E)) { UnaryOperatorKind UnaryOp = UO->getOpcode(); + Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); // *p, LHS of *p == 0, etc. if (UnaryOp == UO_Deref) { - Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); - - // In the logic for a binary expression below we set - // TermCondInfo.DerefExpr to either the LHS or the RHS of the binary - // expression just to signal that we have visited a biary expression. If - // TermCondInfo.DerefExpr is nullptr here then it means we did not visit - // a binary expression. - if (!TermCondInfo.DerefExpr) { - // If we do not have a binary operator then we do the following - // normalizations: - // *p ==> *p != 0 - // !*p ==> *p == 0 - // In the above normalizations the compared value would always be null. - // Whether the condition tests for a null value will be determined in - // the logic for UO_LNot after we return from the current recursive - // call. So for now simply set TermCondInfo.IsCheckNull to false. - TermCondInfo.IsCheckNull = false; - } + // We do the following normalizations: + // *p ==> *p != 0 + // !*p ==> *p == 0 + // In the above normalizations the compared value would always be null. + // Whether the condition tests for a null value will be determined in + // the logic for UO_LNot after we return from the current recursive + // call. // *p ==> DerefExpr = p + // *(p + 1) ==> DerefExpr = p + 1 + // **p ==> DerefExpr = *p TermCondInfo.DerefExpr = SubExpr; // !*p, !!!*p, !(*p == 0), etc. } else if (UnaryOp == UO_LNot) { - Expr *SubExpr = IgnoreCasts(UO->getSubExpr()); - // !*p ==> FillTermCondInfo(*p); // !!*p ==> FillTermCondInfo(!*p); // !!!*p ==> FillTermCondInfo(!!*p); @@ -1433,8 +1423,7 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // !*p ==> IsCheckNull = True // !!*p ==> IsCheckNull = False // !!!*p ==> IsCheckNull = True - TermCondInfo.IsCheckNull = - !TermCondInfo.IsCheckNull; + TermCondInfo.IsCheckNull = !TermCondInfo.IsCheckNull; } // If dereference expression contains an array access. An array access can @@ -1442,21 +1431,13 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, // always present the normalized view: A[i]. In this case getBase() returns // "A" and getIdx() returns "i". } else if (auto *AE = dyn_cast(E)) { - // In the logic for a binary expression below we set TermCondInfo.DerefExpr - // to either the LHS or the RHS of the binary expression just to signal - // that we have visited a biary expression. If TermCondInfo.DerefExpr is - // nullptr here then it means we did not visit a binary expression. - if (!TermCondInfo.DerefExpr) { - // If we do not have a binary operator then we do the following - // normalizations: - // p[i] ==> p[i] != 0 - // !p[i] ==> p[i] == 0 - // In the above normalizations the compared value would always be null. - // Whether the condition tests for a null value will be determined in - // the logic for UO_LNot after we return from the current recursive - // call. So for now simply set TermCondInfo.IsCheckNull to false. - TermCondInfo.IsCheckNull = false; - } + // We do the following normalizations: + // p[i] ==> p[i] != 0 + // !p[i] ==> p[i] == 0 + // In the above normalizations the compared value would always be null. + // Whether the condition tests for a null value will be determined in + // the logic for UO_LNot after we return from the current recursive + // call. // p[i] ==> DerefExpr = p + i TermCondInfo.DerefExpr = @@ -1467,81 +1448,73 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, } else if (auto *BO = dyn_cast(E)) { BinaryOperatorKind BinOp = BO->getOpcode(); - // *p == 0, 0 != *p, etc. - if (BinOp == BO_EQ || BinOp == BO_NE) { - Expr *LHS = BO->getLHS(); + // (c = *p) != 0 + if (BinOp == BO_Assign) { + // (c = *p) ==> RHS = *p Expr *RHS = BO->getRHS(); + FillTermCondInfo(RHS, TermCondInfo); + return; + } - // 0 == *p ==> LHSVal = 0 - // 'a' != *p ==> LHSVal = 'a' - // a != *p ==> LHSVal = nullptr - // *p == *q ==> LHSVal = nullptr; - Optional LHSVal = LHS->getIntegerConstantExpr(Ctx); - - // *p == 0 ==> RHSVal = 0 - // *p != 'a' ==> RHSVal = 'a' - // *p != a ==> RHSVal = nullptr - // *p == *q ==> RHSVal = nullptr - Optional RHSVal = RHS->getIntegerConstantExpr(Ctx); - - // 1 == 2 ==> DerefExpr = nullptr - // *p != q ==> DerefExpr = nullptr - if (LHSVal == RHSVal) { - TermCondInfo.DerefExpr = nullptr; - return; - } - - bool IsComparedValNull = false; + // We only handle *p == 0, 0 != *p, etc. + if (BinOp != BO_EQ && BinOp != BO_NE) + return; - llvm::APSInt Zero(Ctx.getTargetInfo().getIntWidth(), 0); - if (LHSVal) { - // 0 == *p ==> IsComparedValNull = True - // 'a' != *p ==> IsComparedValNull = False - IsComparedValNull = - llvm::APSInt::compareValues(*LHSVal, Zero) == 0; + Expr *LHS = BO->getLHS(); + Expr *RHS = BO->getRHS(); - // We are setting DerefExpr here to the RHS just to signal to unary - // operator logic above that a binary expression has been visited. - TermCondInfo.DerefExpr = RHS; + // 0 == *p ==> LHSVal = 0 + // 'a' != *p ==> LHSVal = 'a' + // a != *p ==> LHSVal = nullptr + // *p == *q ==> LHSVal = nullptr; + Optional LHSVal = LHS->getIntegerConstantExpr(Ctx); - // 0 == *p ==> FillTermCondInfo(*p); - // 'a' != *p ==> FillTermCondInfo(*p); - FillTermCondInfo(RHS, TermCondInfo); + // *p == 0 ==> RHSVal = 0 + // *p != 'a' ==> RHSVal = 'a' + // *p != a ==> RHSVal = nullptr + // *p == *q ==> RHSVal = nullptr + Optional RHSVal = RHS->getIntegerConstantExpr(Ctx); - } else if (RHSVal) { - // *p == 0 ==> IsComparedValNull = True - // *p != 'a' ==> IsComparedValNull = False - IsComparedValNull = - llvm::APSInt::compareValues(*RHSVal, Zero) == 0; + // 1 == 2 ==> DerefExpr = nullptr + // *p != q ==> DerefExpr = nullptr + if ((LHSVal && RHSVal) || (!LHSVal && !RHSVal)) + return; - // We are setting DerefExpr here to the LHS just to signal to unary - // operator logic above that a binary expression has been visited. - TermCondInfo.DerefExpr = LHS; + bool IsComparedValNull = false; - // *p == 0 ==> FillTermCondInfo(*p); - // *p != 'a' ==> FillTermCondInfo(*p); - FillTermCondInfo(LHS, TermCondInfo); - } + llvm::APSInt Zero(Ctx.getTargetInfo().getIntWidth(), 0); + if (LHSVal) { + // 0 == *p ==> IsComparedValNull = True + // 'a' != *p ==> IsComparedValNull = False + IsComparedValNull = + llvm::APSInt::compareValues(*LHSVal, Zero) == 0; - // *p == 0 ==> BinOp == BO_EQ, IsComparedValNull = True, - // IsCheckNull(prev value) = False - // ==> IsCheckNull = True - - // *p != 'a' ==> BinOp != BO_EQ, IsComparedValNull = False, - // IsCheckNull(prev value) = False - // ==> IsCheckNull = True - TermCondInfo.IsCheckNull = - (BinOp == BO_EQ && IsComparedValNull && - !TermCondInfo.IsCheckNull) || - (BinOp != BO_EQ && !IsComparedValNull && - !TermCondInfo.IsCheckNull); - - // (c = *p) != 0 - } else if (BinOp == BO_Assign) { - // (c = *p) ==> RHS = *p - Expr *RHS = BO->getRHS(); + // 0 == *p ==> FillTermCondInfo(*p); + // 'a' != *p ==> FillTermCondInfo(*p); FillTermCondInfo(RHS, TermCondInfo); + + } else if (RHSVal) { + // *p == 0 ==> IsComparedValNull = True + // *p != 'a' ==> IsComparedValNull = False + IsComparedValNull = + llvm::APSInt::compareValues(*RHSVal, Zero) == 0; + + // *p == 0 ==> FillTermCondInfo(*p); + // *p != 'a' ==> FillTermCondInfo(*p); + FillTermCondInfo(LHS, TermCondInfo); } + + // *p == 0 ==> BinOp == BO_EQ, IsComparedValNull = True, + // IsCheckNull(prev value) = False + // ==> IsCheckNull = True + + // *p != 'a' ==> BinOp != BO_EQ, IsComparedValNull = False, + // IsCheckNull(prev value) = False + // ==> IsCheckNull = True + TermCondInfo.IsCheckNull = (BinOp == BO_EQ && IsComparedValNull && + !TermCondInfo.IsCheckNull) || + (BinOp != BO_EQ && !IsComparedValNull && + !TermCondInfo.IsCheckNull); } } diff --git a/clang/test/CheckedC/inferred-bounds/widened-bounds-complex-conditionals.c b/clang/test/CheckedC/inferred-bounds/widened-bounds-complex-conditionals.c index 61a329b68e91..d65a11af3950 100644 --- a/clang/test/CheckedC/inferred-bounds/widened-bounds-complex-conditionals.c +++ b/clang/test/CheckedC/inferred-bounds/widened-bounds-complex-conditionals.c @@ -601,3 +601,66 @@ void f7(char p _Nt_checked[1], char q _Nt_checked[2]) { // CHECK: p: bounds(p, p + 0 + 1) // CHECK: q: bounds(q, q + 1 + 1) } + +void f8(_Nt_array_ptr<_Ptr> p : count(0), + _Nt_array_ptr<_Ptr> q : bounds(*q, *q)) { + + if (**p) { + a = 1; + } + +// CHECK: Function: f8 +// CHECK: Block: B9 (Entry), Pred: Succ: B8 + +// CHECK: Block: B8, Pred: B9, Succ: B7, B6 +// CHECK: Widened bounds before stmt: **p +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(*q, *q) + +// CHECK: Block: B7, Pred: B8, Succ: B6 +// CHECK: Widened bounds before stmt: a = 1 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(*q, *q) + + if (!**p) { + a = 2; + } + +// CHECK: Block: B6, Pred: B7, B8, Succ: B5, B4 +// CHECK: Widened bounds before stmt: !**p +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(*q, *q) + +// CHECK: Block: B5, Pred: B6, Succ: B4 +// CHECK: Widened bounds before stmt: a = 2 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(*q, *q) + + if (**q) { + a = 3; + } + +// CHECK: Block: B4, Pred: B5, B6, Succ: B3, B2 +// CHECK: Widened bounds before stmt: **q +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(*q, *q) + +// CHECK: Block: B3, Pred: B4, Succ: B2 +// CHECK: Widened bounds before stmt: a = 3 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(*q, *q + 1) + + if (!**q) { + a = 4; + } + +// CHECK: Block: B2, Pred: B3, B4, Succ: B1, B0 +// CHECK: Widened bounds before stmt: !**q +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(*q, *q) + +// CHECK: Block: B1, Pred: B2, Succ: B0 +// CHECK: Widened bounds before stmt: a = 4 +// CHECK: p: bounds(p, p + 0) +// CHECK: q: bounds(*q, *q) +} From 5627cc44e510d66965afc0a188ba7dd2b3c47e80 Mon Sep 17 00:00:00 2001 From: Mandeep Singh Grang Date: Thu, 12 Aug 2021 11:51:43 -0700 Subject: [PATCH 8/8] Check if LHS or RHS of binary expr contain errors --- clang/lib/Sema/BoundsWideningAnalysis.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/lib/Sema/BoundsWideningAnalysis.cpp b/clang/lib/Sema/BoundsWideningAnalysis.cpp index 544a2ec3b488..34eac909286c 100644 --- a/clang/lib/Sema/BoundsWideningAnalysis.cpp +++ b/clang/lib/Sema/BoundsWideningAnalysis.cpp @@ -1463,6 +1463,9 @@ void BoundsWideningUtil::FillTermCondInfo(const Expr *TermCond, Expr *LHS = BO->getLHS(); Expr *RHS = BO->getRHS(); + if (LHS->containsErrors() || RHS->containsErrors()) + return; + // 0 == *p ==> LHSVal = 0 // 'a' != *p ==> LHSVal = 'a' // a != *p ==> LHSVal = nullptr