diff options
| -rw-r--r-- | clang/include/clang/AST/Expr.h | 5 | ||||
| -rw-r--r-- | clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h | 4 | ||||
| -rw-r--r-- | clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 45 | ||||
| -rw-r--r-- | clang/test/Analysis/atomics.c | 95 |
4 files changed, 147 insertions, 2 deletions
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 80710e90211..9179c7736a9 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -4859,9 +4859,12 @@ public: } AtomicOp getOp() const { return Op; } - unsigned getNumSubExprs() { return NumSubExprs; } + unsigned getNumSubExprs() const { return NumSubExprs; } Expr **getSubExprs() { return reinterpret_cast<Expr **>(SubExprs); } + const Expr * const *getSubExprs() const { + return reinterpret_cast<Expr * const *>(SubExprs); + } bool isVolatile() const { return getPtr()->getType()->getPointeeType().isVolatileQualified(); diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index ec96560a2f0..50d85057041 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -392,6 +392,10 @@ public: void VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred, ExplodedNodeSet &Dst); + /// VisitMemberExpr - Transfer function for builtin atomic expressions + void VisitAtomicExpr(const AtomicExpr *E, ExplodedNode *Pred, + ExplodedNodeSet &Dst); + /// Transfer function logic for ObjCAtSynchronizedStmts. void VisitObjCAtSynchronizedStmt(const ObjCAtSynchronizedStmt *S, ExplodedNode *Pred, ExplodedNodeSet &Dst); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 85cdaf2513d..83d4b6ab041 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -902,7 +902,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, case Stmt::CUDAKernelCallExprClass: case Stmt::OpaqueValueExprClass: case Stmt::AsTypeExprClass: - case Stmt::AtomicExprClass: // Fall through. // Cases we intentionally don't evaluate, since they don't need @@ -1247,6 +1246,12 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, Bldr.addNodes(Dst); break; + case Stmt::AtomicExprClass: + Bldr.takeNodes(Pred); + VisitAtomicExpr(cast<AtomicExpr>(S), Pred, Dst); + Bldr.addNodes(Dst); + break; + case Stmt::ObjCIvarRefExprClass: Bldr.takeNodes(Pred); VisitLvalObjCIvarRefExpr(cast<ObjCIvarRefExpr>(S), Pred, Dst); @@ -2070,6 +2075,44 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred, getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, M, *this); } +void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred, + ExplodedNodeSet &Dst) { + ExplodedNodeSet AfterPreSet; + getCheckerManager().runCheckersForPreStmt(AfterPreSet, Pred, AE, *this); + + // For now, treat all the arguments to C11 atomics as escaping. + // FIXME: Ideally we should model the behavior of the atomics precisely here. + + ExplodedNodeSet AfterInvalidateSet; + StmtNodeBuilder Bldr(AfterPreSet, AfterInvalidateSet, *currBldrCtx); + + for (ExplodedNodeSet::iterator I = AfterPreSet.begin(), E = AfterPreSet.end(); + I != E; ++I) { + ProgramStateRef State = (*I)->getState(); + const LocationContext *LCtx = (*I)->getLocationContext(); + + SmallVector<SVal, 8> ValuesToInvalidate; + for (unsigned SI = 0, Count = AE->getNumSubExprs(); SI != Count; SI++) { + const Expr *SubExpr = AE->getSubExprs()[SI]; + SVal SubExprVal = State->getSVal(SubExpr, LCtx); + ValuesToInvalidate.push_back(SubExprVal); + } + + State = State->invalidateRegions(ValuesToInvalidate, AE, + currBldrCtx->blockCount(), + LCtx, + /*CausedByPointerEscape*/true, + /*Symbols=*/nullptr); + + SVal ResultVal = UnknownVal(); + State = State->BindExpr(AE, LCtx, ResultVal); + Bldr.generateNode(AE, *I, State, nullptr, + ProgramPoint::PostStmtKind); + } + + getCheckerManager().runCheckersForPostStmt(Dst, AfterInvalidateSet, AE, *this); +} + namespace { class CollectReachableSymbolsCallback final : public SymbolVisitor { InvalidatedSymbols Symbols; diff --git a/clang/test/Analysis/atomics.c b/clang/test/Analysis/atomics.c new file mode 100644 index 00000000000..f0f5ff07684 --- /dev/null +++ b/clang/test/Analysis/atomics.c @@ -0,0 +1,95 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s + +// Tests for c11 atomics. Many of these tests currently yield unknown +// because we don't fully model the atomics and instead imprecisely +// treat their arguments as escaping. + +typedef unsigned int uint32_t; +typedef enum memory_order { + memory_order_relaxed = __ATOMIC_RELAXED, + memory_order_consume = __ATOMIC_CONSUME, + memory_order_acquire = __ATOMIC_ACQUIRE, + memory_order_release = __ATOMIC_RELEASE, + memory_order_acq_rel = __ATOMIC_ACQ_REL, + memory_order_seq_cst = __ATOMIC_SEQ_CST +} memory_order; + +void clang_analyzer_eval(int); + +struct RefCountedStruct { + uint32_t refCount; + void *ptr; +}; + +void test_atomic_fetch_add(struct RefCountedStruct *s) { + s->refCount = 1; + + uint32_t result = __c11_atomic_fetch_add((volatile _Atomic(uint32_t) *)&s->refCount,- 1, memory_order_relaxed); + + // When we model atomics fully this should (probably) be FALSE. It should never + // be TRUE (because the operation mutates the passed in storage). + clang_analyzer_eval(s->refCount == 1); // expected-warning {{UNKNOWN}} + + // When fully modeled this should be TRUE + clang_analyzer_eval(result == 1); // expected-warning {{UNKNOWN}} +} + +void test_atomic_load(struct RefCountedStruct *s) { + s->refCount = 1; + + uint32_t result = __c11_atomic_load((volatile _Atomic(uint32_t) *)&s->refCount, memory_order_relaxed); + + // When we model atomics fully this should (probably) be TRUE. + clang_analyzer_eval(s->refCount == 1); // expected-warning {{UNKNOWN}} + + // When fully modeled this should be TRUE + clang_analyzer_eval(result == 1); // expected-warning {{UNKNOWN}} +} + +void test_atomic_store(struct RefCountedStruct *s) { + s->refCount = 1; + + __c11_atomic_store((volatile _Atomic(uint32_t) *)&s->refCount, 2, memory_order_relaxed); + + // When we model atomics fully this should (probably) be FALSE. It should never + // be TRUE (because the operation mutates the passed in storage). + clang_analyzer_eval(s->refCount == 1); // expected-warning {{UNKNOWN}} +} + +void test_atomic_exchange(struct RefCountedStruct *s) { + s->refCount = 1; + + uint32_t result = __c11_atomic_exchange((volatile _Atomic(uint32_t) *)&s->refCount, 2, memory_order_relaxed); + + // When we model atomics fully this should (probably) be FALSE. It should never + // be TRUE (because the operation mutates the passed in storage). + clang_analyzer_eval(s->refCount == 1); // expected-warning {{UNKNOWN}} + + // When fully modeled this should be TRUE + clang_analyzer_eval(result == 1); // expected-warning {{UNKNOWN}} +} + + +void test_atomic_compare_exchange_strong(struct RefCountedStruct *s) { + s->refCount = 1; + uint32_t expected = 2; + uint32_t desired = 3; + _Bool result = __c11_atomic_compare_exchange_strong((volatile _Atomic(uint32_t) *)&s->refCount, &expected, desired, memory_order_relaxed, memory_order_relaxed); + + // For now we expect both expected and refCount to be invalidated by the + // call. In the future we should model more precisely. + clang_analyzer_eval(s->refCount == 3); // expected-warning {{UNKNOWN}} + clang_analyzer_eval(expected == 2); // expected-warning {{UNKNOWN}} +} + +void test_atomic_compare_exchange_weak(struct RefCountedStruct *s) { + s->refCount = 1; + uint32_t expected = 2; + uint32_t desired = 3; + _Bool result = __c11_atomic_compare_exchange_weak((volatile _Atomic(uint32_t) *)&s->refCount, &expected, desired, memory_order_relaxed, memory_order_relaxed); + + // For now we expect both expected and refCount to be invalidated by the + // call. In the future we should model more precisely. + clang_analyzer_eval(s->refCount == 3); // expected-warning {{UNKNOWN}} + clang_analyzer_eval(expected == 2); // expected-warning {{UNKNOWN}} +} |

