Allow 'Environment::getSVal()' to allow an optional way for checkers to do a direct lookup to values bound to expressions, without

resulting to lazy logic.  This is critical for the OSAtomicChecker that does a simulated load on any arbitrary expression.

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

5 files changed, 43 insertions, 7 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h b/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
index 732a40cb21..193056e6b0 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
@@ -51,9 +51,10 @@ public:
   iterator end() const { return ExprBindings.end(); }
 
 
-  /// GetSVal - Fetches the current binding of the expression in the
+  /// getSVal - Fetches the current binding of the expression in the
   ///  Environment.
-  SVal getSVal(const Stmt* Ex, SValBuilder& svalBuilder) const;
+  SVal getSVal(const Stmt* Ex, SValBuilder& svalBuilder,
+	       bool useOnlyDirectBindings = false) const;
 
   /// Profile - Profile the contents of an Environment object for use
   ///  in a FoldingSet.
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/GRState.h b/include/clang/StaticAnalyzer/Core/PathSensitive/GRState.h
index 827373870f..a957c897b9 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/GRState.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/GRState.h
@@ -260,7 +260,7 @@ public:
   const llvm::APSInt *getSymVal(SymbolRef sym) const;
 
   /// Returns the SVal bound to the statement 'S' in the state's environment.
-  SVal getSVal(const Stmt* S) const;
+  SVal getSVal(const Stmt* S, bool useOnlyDirectBindings = false) const;
   
   SVal getSValAsScalarOrLoc(const Stmt *Ex) const;
 
@@ -683,8 +683,9 @@ inline const llvm::APSInt *GRState::getSymVal(SymbolRef sym) const {
   return getStateManager().getSymVal(this, sym);
 }
 
-inline SVal GRState::getSVal(const Stmt* Ex) const {
-  return Env.getSVal(Ex, *getStateManager().svalBuilder);
+inline SVal GRState::getSVal(const Stmt* Ex, bool useOnlyDirectBindings) const{
+  return Env.getSVal(Ex, *getStateManager().svalBuilder,
+		     useOnlyDirectBindings);
 }
 
 inline SVal GRState::getSValAsScalarOrLoc(const Stmt *S) const {
diff --git a/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp b/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
index 6a4f1926f0..7262bc3640 100644
--- a/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
@@ -127,7 +127,12 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
 
     ExplodedNode *N = *I;
     const GRState *stateLoad = N->getState();
-    SVal theValueVal_untested = stateLoad->getSVal(theValueExpr);
+
+    // Use direct bindings from the environment since we are forcing a load
+    // from a location that the Environment would typically not be used
+    // to bind a value.
+    SVal theValueVal_untested = stateLoad->getSVal(theValueExpr, true);
+
     SVal oldValueVal_untested = stateLoad->getSVal(oldValueExpr);
 
     // FIXME: Issue an error.
diff --git a/lib/StaticAnalyzer/Core/Environment.cpp b/lib/StaticAnalyzer/Core/Environment.cpp
index 0d43c37587..a00f9dc10b 100644
--- a/lib/StaticAnalyzer/Core/Environment.cpp
+++ b/lib/StaticAnalyzer/Core/Environment.cpp
@@ -27,7 +27,17 @@ SVal Environment::lookupExpr(const Stmt* E) const {
   return UnknownVal();
 }
 
-SVal Environment::getSVal(const Stmt *E, SValBuilder& svalBuilder) const {
+SVal Environment::getSVal(const Stmt *E, SValBuilder& svalBuilder,
+			  bool useOnlyDirectBindings) const {
+
+  if (useOnlyDirectBindings) {
+    // This branch is rarely taken, but can be exercised by
+    // checkers that explicitly bind values to arbitrary
+    // expressions.  It is crucial that we do not ignore any
+    // expression here, and do a direct lookup.
+    return lookupExpr(E);
+  }
+
   for (;;) {
     switch (E->getStmtClass()) {
       case Stmt::AddrLabelExprClass:
diff --git a/test/Analysis/OSAtomic_mac.cpp b/test/Analysis/OSAtomic_mac.cpp
new file mode 100644
index 0000000000..8ad7b3c3da
--- /dev/null
+++ b/test/Analysis/OSAtomic_mac.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,osx -analyzer-store=region -verify -fblocks   -analyzer-opt-analyze-nested-blocks %s
+
+// Test handling of OSAtomicCompareAndSwap when C++ inserts "no-op" casts and we
+// do a forced load and binding to the environment on an expression that would regularly
+// not have an environment binding.  This previously triggered a crash (<rdar://problem/9339920>).
+// NOTE: It is critical that the function called is OSAtomicCompareAndSwapIntBarrier.
+bool OSAtomicCompareAndSwapIntBarrier( int __oldValue, int __newValue, volatile int *__theValue ) ;
+static int _rdar9339920_x = 0;
+int rdar9339920_aux();
+
+int rdar9339920_test() {
+  int rdar9339920_x = rdar9339920_aux();
+  if (rdar9339920_x != _rdar9339920_x) {
+    if (OSAtomicCompareAndSwapIntBarrier(_rdar9339920_x, rdar9339920_x, &_rdar9339920_x))
+      return 1;
+  }
+  return 0;
+}
+