[analyzer] Add rudimentary handling of AtomicExpr.

This proposed patch adds crude handling of atomics to the static analyzer.
Rather than ignore AtomicExprs, as we now do, this patch causes the analyzer
to escape the arguments. This is imprecise -- and we should model the
expressions fully in the future -- but it is less wrong than ignoring their
effects altogether.

This is rdar://problem/25353187

Differential Revision: http://reviews.llvm.org/D21667

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@274816 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index 80710e90211333abb208e7d59fab91c85ce41774..9179c7736a9a483d54117e8e227d26b4babaad26 100644 (file)
--- a/include/clang/AST/Expr.h
+++ b/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/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index ec96560a2f0be09f87d99e3b887e744652666645..50d85057041dd0b5edc24d58f51af4c94f1ff8bb 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/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/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 85cdaf2513dcf45dba07e4694b8bd15af4b18eac..83d4b6ab0416389c96d40c5a2e99455b313d5b62 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/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/test/Analysis/atomics.c b/test/Analysis/atomics.c
new file mode 100644 (file)
index 0000000..f0f5ff0
--- /dev/null
+++ b/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}}
+}