From 7a80359d95188d12fafad3e93d057171ceeb711d Mon Sep 17 00:00:00 2001 From: George Karpenkov <ekarpenkov@apple.com> Date: Tue, 5 Dec 2017 21:19:59 +0000 Subject: [PATCH] [analyzer] do not crash on cases where an array subscript is an rvalue Array subscript is almost always an lvalue, except for a few cases where it is not, such as a subscript into an Objective-C property, or a return from the function. This commit prevents crashing in such cases. Fixes rdar://34829842 Differential Revision: https://reviews.llvm.org/D40584 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@319834 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/PathSensitive/ExprEngine.h | 6 ++-- lib/StaticAnalyzer/Core/ExprEngine.cpp | 35 +++++++++++++------ test/Analysis/{vector.c => vector.m} | 33 +++++++++++++++++ 3 files changed, 60 insertions(+), 14 deletions(-) rename test/Analysis/{vector.c => vector.m} (58%) diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index ecff5c6623d..712cd6361e1 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -332,9 +332,9 @@ public: void Visit(const Stmt *S, ExplodedNode *Pred, ExplodedNodeSet &Dst); /// VisitArraySubscriptExpr - Transfer function for array accesses. - void VisitLvalArraySubscriptExpr(const ArraySubscriptExpr *Ex, - ExplodedNode *Pred, - ExplodedNodeSet &Dst); + void VisitArraySubscriptExpr(const ArraySubscriptExpr *Ex, + ExplodedNode *Pred, + ExplodedNodeSet &Dst); /// VisitGCCAsmStmt - Transfer function logic for inline asm. void VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred, diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index 31e21232408..3be37e7ae30 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1150,7 +1150,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, case Stmt::ArraySubscriptExprClass: Bldr.takeNodes(Pred); - VisitLvalArraySubscriptExpr(cast<ArraySubscriptExpr>(S), Pred, Dst); + VisitArraySubscriptExpr(cast<ArraySubscriptExpr>(S), Pred, Dst); Bldr.addNodes(Dst); break; @@ -2126,10 +2126,9 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D, } /// VisitArraySubscriptExpr - Transfer function for array accesses -void ExprEngine::VisitLvalArraySubscriptExpr(const ArraySubscriptExpr *A, +void ExprEngine::VisitArraySubscriptExpr(const ArraySubscriptExpr *A, ExplodedNode *Pred, ExplodedNodeSet &Dst){ - const Expr *Base = A->getBase()->IgnoreParens(); const Expr *Idx = A->getIdx()->IgnoreParens(); @@ -2138,18 +2137,32 @@ void ExprEngine::VisitLvalArraySubscriptExpr(const ArraySubscriptExpr *A, ExplodedNodeSet EvalSet; StmtNodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx); - assert(A->isGLValue() || - (!AMgr.getLangOpts().CPlusPlus && - A->getType().isCForbiddenLValueType())); + + bool IsVectorType = A->getBase()->getType()->isVectorType(); + + // The "like" case is for situations where C standard prohibits the type to + // be an lvalue, e.g. taking the address of a subscript of an expression of + // type "void *". + bool IsGLValueLike = A->isGLValue() || + (A->getType().isCForbiddenLValueType() && !AMgr.getLangOpts().CPlusPlus); for (auto *Node : CheckerPreStmt) { const LocationContext *LCtx = Node->getLocationContext(); ProgramStateRef state = Node->getState(); - SVal V = state->getLValue(A->getType(), - state->getSVal(Idx, LCtx), - state->getSVal(Base, LCtx)); - Bldr.generateNode(A, Node, state->BindExpr(A, LCtx, V), nullptr, - ProgramPoint::PostLValueKind); + + if (IsGLValueLike) { + SVal V = state->getLValue(A->getType(), + state->getSVal(Idx, LCtx), + state->getSVal(Base, LCtx)); + Bldr.generateNode(A, Node, state->BindExpr(A, LCtx, V), nullptr, + ProgramPoint::PostLValueKind); + } else if (IsVectorType) { + // FIXME: non-glvalue vector reads are not modelled. + Bldr.generateNode(A, Node, state, nullptr); + } else { + llvm_unreachable("Array subscript should be an lValue when not \ +a vector and not a forbidden lvalue type"); + } } getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, A, *this); diff --git a/test/Analysis/vector.c b/test/Analysis/vector.m similarity index 58% rename from test/Analysis/vector.c rename to test/Analysis/vector.m index 32b568f6b00..e74c487d3a6 100644 --- a/test/Analysis/vector.c +++ b/test/Analysis/vector.m @@ -2,6 +2,7 @@ typedef int __attribute__((ext_vector_type(2))) V; +void clang_analyzer_warnIfReached(); void clang_analyzer_numTimesReached(); void clang_analyzer_eval(int); @@ -26,3 +27,35 @@ V dont_crash_and_dont_split_state(V x, V y) { clang_analyzer_numTimesReached(); // expected-warning{{2}} return z; } + +void test_read() { + V x; + x[0] = 0; + x[1] = 1; + + clang_analyzer_eval(x[0] == 0); // expected-warning{{TRUE}} +} + +V return_vector() { + V z; + z[0] = 0; + z[1] = 0; + return z; +} + +int test_vector_access() { + return return_vector()[0]; // no-crash no-warning +} + +@interface I +@property V v; +@end + +// Do not crash on subscript operations into ObjC properties. +int myfunc(I *i2) { + int out = i2.v[0]; // no-crash no-warning + + // Check that the analysis continues. + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + return out; +} -- GitLab