[cfe-commits] r147569 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/ lib/StaticAnalyzer/Checkers/ lib/StaticAnalyzer/Core/ test/Analysis/

Anna Zaks ganna at apple.com
Wed Jan 4 15:54:01 PST 2012


Author: zaks
Date: Wed Jan  4 17:54:01 2012
New Revision: 147569

URL: http://llvm.org/viewvc/llvm-project?rev=147569&view=rev
Log:
[analyzer] Be less pessimistic about invalidation of global variables
as a result of a call.

Problem:
Global variables, which come in from system libraries should not be
invalidated by all calls. Also, non-system globals should not be
invalidated by system calls.

Solution:
The following solution to invalidation of globals seems flexible enough
for taint (does not invalidate stdin) and should not lead to too
many false positives. We split globals into 3 classes:

* immutable - values are preserved by calls (unless the specific
global is passed in as a parameter):
     A :  Most system globals and const scalars

* invalidated by functions defined in system headers:
     B: errno

* invalidated by all other functions (note, these functions may in
turn contain system calls):
     B: errno
     C: all other globals (which are not in A nor B)

Added:
    cfe/trunk/test/Analysis/global-region-invalidation.c
    cfe/trunk/test/Analysis/system-header-simulator.h
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
    cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
    cfe/trunk/test/Analysis/misc-ps.c

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Wed Jan  4 17:54:01 2012
@@ -470,13 +470,6 @@
                     const ProgramPointTag *tag, bool isLoad);
 
   bool InlineCall(ExplodedNodeSet &Dst, const CallExpr *CE, ExplodedNode *Pred);
-  
-  
-public:
-  /// Returns true if calling the specific function or method would possibly
-  /// cause global variables to be invalidated.
-  bool doesInvalidateGlobals(const CallOrObjCMessage &callOrMessage) const;
-  
 };
 
 } // end ento namespace

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Wed Jan  4 17:54:01 2012
@@ -73,12 +73,16 @@
     StackArgumentsSpaceRegionKind,
     HeapSpaceRegionKind,
     UnknownSpaceRegionKind,
-    NonStaticGlobalSpaceRegionKind,
     StaticGlobalSpaceRegionKind,
-    BEG_GLOBAL_MEMSPACES = NonStaticGlobalSpaceRegionKind,
-    END_GLOBAL_MEMSPACES = StaticGlobalSpaceRegionKind,
+    GlobalInternalSpaceRegionKind,
+    GlobalSystemSpaceRegionKind,
+    GlobalImmutableSpaceRegionKind,
+    BEG_NON_STATIC_GLOBAL_MEMSPACES = GlobalInternalSpaceRegionKind,
+    END_NON_STATIC_GLOBAL_MEMSPACES = GlobalImmutableSpaceRegionKind,
+    BEG_GLOBAL_MEMSPACES = StaticGlobalSpaceRegionKind,
+    END_GLOBAL_MEMSPACES = GlobalImmutableSpaceRegionKind,
     BEG_MEMSPACES = GenericMemSpaceRegionKind,
-    END_MEMSPACES = StaticGlobalSpaceRegionKind,
+    END_MEMSPACES = GlobalImmutableSpaceRegionKind,
     // Untyped regions.
     SymbolicRegionKind,
     AllocaRegionKind,
@@ -150,7 +154,7 @@
   static bool classof(const MemRegion*) { return true; }
 };
 
-/// MemSpaceRegion - A memory region that represents and "memory space";
+/// MemSpaceRegion - A memory region that represents a "memory space";
 ///  for example, the set of global variables, the stack frame, etc.
 class MemSpaceRegion : public MemRegion {
 protected:
@@ -187,7 +191,11 @@
     return k >= BEG_GLOBAL_MEMSPACES && k <= END_GLOBAL_MEMSPACES;
   }
 };
-  
+
+/// \class The region of the static variables within the current CodeTextRegion
+/// scope.
+/// Currently, only the static locals are placed there, so we know that these
+/// variables do not get invalidated by calls to other functions.
 class StaticGlobalSpaceRegion : public GlobalsSpaceRegion {
   friend class MemRegionManager;
 
@@ -207,22 +215,86 @@
     return R->getKind() == StaticGlobalSpaceRegionKind;
   }
 };
-  
+
+/// \class The region for all the non-static global variables.
+///
+/// This class is further split into subclasses for efficient implementation of
+/// invalidating a set of related global values as is done in
+/// RegionStoreManager::invalidateRegions (instead of finding all the dependent
+/// globals, we invalidate the whole parent region).
 class NonStaticGlobalSpaceRegion : public GlobalsSpaceRegion {
   friend class MemRegionManager;
   
-  NonStaticGlobalSpaceRegion(MemRegionManager *mgr)
-    : GlobalsSpaceRegion(mgr, NonStaticGlobalSpaceRegionKind) {}
+protected:
+  NonStaticGlobalSpaceRegion(MemRegionManager *mgr, Kind k)
+    : GlobalsSpaceRegion(mgr, k) {}
   
 public:
 
   void dumpToStream(raw_ostream &os) const;
 
   static bool classof(const MemRegion *R) {
-    return R->getKind() == NonStaticGlobalSpaceRegionKind;
+    Kind k = R->getKind();
+    return k >= BEG_NON_STATIC_GLOBAL_MEMSPACES &&
+           k <= END_NON_STATIC_GLOBAL_MEMSPACES;
   }
 };
-  
+
+/// \class The region containing globals which are defined in system/external
+/// headers and are considered modifiable by system calls (ex: errno).
+class GlobalSystemSpaceRegion : public NonStaticGlobalSpaceRegion {
+  friend class MemRegionManager;
+
+  GlobalSystemSpaceRegion(MemRegionManager *mgr)
+    : NonStaticGlobalSpaceRegion(mgr, GlobalSystemSpaceRegionKind) {}
+
+public:
+
+  void dumpToStream(raw_ostream &os) const;
+
+  static bool classof(const MemRegion *R) {
+    return R->getKind() == GlobalSystemSpaceRegionKind;
+  }
+};
+
+/// \class The region containing globals which are considered not to be modified
+/// or point to data which could be modified as a result of a function call
+/// (system or internal). Ex: Const global scalars would be modeled as part of
+/// this region. This region also includes most system globals since they have
+/// low chance of being modified.
+class GlobalImmutableSpaceRegion : public NonStaticGlobalSpaceRegion {
+  friend class MemRegionManager;
+
+  GlobalImmutableSpaceRegion(MemRegionManager *mgr)
+    : NonStaticGlobalSpaceRegion(mgr, GlobalImmutableSpaceRegionKind) {}
+
+public:
+
+  void dumpToStream(raw_ostream &os) const;
+
+  static bool classof(const MemRegion *R) {
+    return R->getKind() == GlobalImmutableSpaceRegionKind;
+  }
+};
+
+/// \class The region containing globals which can be modified by calls to
+/// "internally" defined functions - (for now just) functions other then system
+/// calls.
+class GlobalInternalSpaceRegion : public NonStaticGlobalSpaceRegion {
+  friend class MemRegionManager;
+
+  GlobalInternalSpaceRegion(MemRegionManager *mgr)
+    : NonStaticGlobalSpaceRegion(mgr, GlobalInternalSpaceRegionKind) {}
+
+public:
+
+  void dumpToStream(raw_ostream &os) const;
+
+  static bool classof(const MemRegion *R) {
+    return R->getKind() == GlobalInternalSpaceRegionKind;
+  }
+};
+
 class HeapSpaceRegion : public MemSpaceRegion {
   virtual void anchor();
   friend class MemRegionManager;
@@ -927,7 +999,10 @@
   llvm::BumpPtrAllocator& A;
   llvm::FoldingSet<MemRegion> Regions;
 
-  NonStaticGlobalSpaceRegion *globals;
+  GlobalInternalSpaceRegion *InternalGlobals;
+  GlobalSystemSpaceRegion *SystemGlobals;
+  GlobalImmutableSpaceRegion *ImmutableGlobals;
+
   
   llvm::DenseMap<const StackFrameContext *, StackLocalsSpaceRegion *> 
     StackLocalsSpaceRegions;
@@ -942,7 +1017,8 @@
 
 public:
   MemRegionManager(ASTContext &c, llvm::BumpPtrAllocator& a)
-    : C(c), A(a), globals(0), heap(0), unknown(0), code(0) {}
+    : C(c), A(a), InternalGlobals(0), SystemGlobals(0), ImmutableGlobals(0),
+      heap(0), unknown(0), code(0) {}
 
   ~MemRegionManager();
 
@@ -962,7 +1038,9 @@
 
   /// getGlobalsRegion - Retrieve the memory region associated with
   ///  global variables.
-  const GlobalsSpaceRegion *getGlobalsRegion(const CodeTextRegion *R = 0);
+  const GlobalsSpaceRegion *getGlobalsRegion(
+      MemRegion::Kind K = MemRegion::GlobalInternalSpaceRegionKind,
+      const CodeTextRegion *R = 0);
 
   /// getHeapRegion - Retrieve the memory region associated with the
   ///  generic "heap".
@@ -1059,11 +1137,6 @@
   const BlockDataRegion *getBlockDataRegion(const BlockTextRegion *bc,
                                             const LocationContext *lc = NULL);
 
-  bool isGlobalsRegion(const MemRegion* R) {
-    assert(R);
-    return R == globals;
-  }
-  
 private:
   template <typename RegionTy, typename A1>
   RegionTy* getRegion(const A1 a1);

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h Wed Jan  4 17:54:01 2012
@@ -19,6 +19,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/PointerUnion.h"
 
 namespace clang {
@@ -233,6 +234,16 @@
     return ActualCallE && isa<CXXMemberCallExpr>(ActualCallE);
   }
 
+  /// Check if the callee is declared in the system header.
+  bool isInSystemHeader() const {
+    if (const Decl *FD = getDecl()) {
+      const SourceManager &SM =
+        State->getStateManager().getContext().getSourceManager();
+      return SM.isInSystemHeader(FD->getLocation());
+    }
+    return false;
+  }
+
   const Expr *getOriginExpr() const {
     if (!CallE)
       return Msg.getOriginExpr();

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Wed Jan  4 17:54:01 2012
@@ -34,8 +34,8 @@
 
 namespace ento {
 
+class CallOrObjCMessage;
 class ProgramStateManager;
-
 typedef ConstraintManager* (*ConstraintManagerCreator)(ProgramStateManager&,
                                                        SubEngine&);
 typedef StoreManager* (*StoreManagerCreator)(ProgramStateManager&);
@@ -219,9 +219,9 @@
   ///  cleared from the store. The regions are provided as a continuous array
   ///  from Begin to End. Optionally invalidates global regions as well.
   const ProgramState *invalidateRegions(ArrayRef<const MemRegion *> Regions,
-                                   const Expr *E, unsigned BlockCount,
-                                   StoreManager::InvalidatedSymbols *IS = 0,
-                                   bool invalidateGlobals = false) const;
+                               const Expr *E, unsigned BlockCount,
+                               StoreManager::InvalidatedSymbols *IS = 0,
+                               const CallOrObjCMessage *Call = 0) const;
 
   /// enterStackFrame - Returns the state for entry to the given stack frame,
   ///  preserving the current state.
@@ -384,7 +384,7 @@
   invalidateRegionsImpl(ArrayRef<const MemRegion *> Regions,
                         const Expr *E, unsigned BlockCount,
                         StoreManager::InvalidatedSymbols &IS,
-                        bool invalidateGlobals) const;
+                        const CallOrObjCMessage *Call) const;
 };
 
 class ProgramStateSet {

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h Wed Jan  4 17:54:01 2012
@@ -29,6 +29,7 @@
 
 namespace ento {
 
+class CallOrObjCMessage;
 class ProgramState;
 class ProgramStateManager;
 class SubRegionMap;
@@ -180,8 +181,8 @@
   ///   symbols to mark the values of invalidated regions.
   /// \param[in,out] IS A set to fill with any symbols that are no longer
   ///   accessible. Pass \c NULL if this information will not be used.
-  /// \param[in] invalidateGlobals If \c true, any non-static global regions
-  ///   are invalidated as well.
+  /// \param[in] Call The call expression which will be used to determine which
+  ///   globals should get invalidated.
   /// \param[in,out] Regions A vector to fill with any regions being
   ///   invalidated. This should include any regions explicitly invalidated
   ///   even if they do not currently have bindings. Pass \c NULL if this
@@ -190,7 +191,7 @@
                                      ArrayRef<const MemRegion *> Regions,
                                      const Expr *E, unsigned Count,
                                      InvalidatedSymbols &IS,
-                                     bool invalidateGlobals,
+                                     const CallOrObjCMessage *Call,
                                      InvalidatedRegions *Invalidated) = 0;
 
   /// enterStackFrame - Let the StoreManager to do something when execution

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Wed Jan  4 17:54:01 2012
@@ -407,8 +407,7 @@
   default: {
     const MemSpaceRegion *MS = MR->getMemorySpace();
     
-    switch (MS->getKind()) {
-    case MemRegion::StackLocalsSpaceRegionKind: {
+    if (isa<StackLocalsSpaceRegion>(MS)) {
       const VarRegion *VR = dyn_cast<VarRegion>(MR);
       const VarDecl *VD;
       if (VR)
@@ -422,7 +421,8 @@
         os << "the address of a local stack variable";
       return true;
     }
-    case MemRegion::StackArgumentsSpaceRegionKind: {
+
+    if (isa<StackArgumentsSpaceRegion>(MS)) {
       const VarRegion *VR = dyn_cast<VarRegion>(MR);
       const VarDecl *VD;
       if (VR)
@@ -436,8 +436,8 @@
         os << "the address of a parameter";
       return true;
     }
-    case MemRegion::NonStaticGlobalSpaceRegionKind:
-    case MemRegion::StaticGlobalSpaceRegionKind: {
+
+    if (isa<GlobalsSpaceRegion>(MS)) {
       const VarRegion *VR = dyn_cast<VarRegion>(MR);
       const VarDecl *VD;
       if (VR)
@@ -454,9 +454,8 @@
         os << "the address of a global variable";
       return true;
     }
-    default:
-      return false;
-    }
+
+    return false;
   }
   }
 }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Jan  4 17:54:01 2012
@@ -24,7 +24,6 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/ADT/ImmutableList.h"
@@ -158,27 +157,6 @@
   return state;
 }
 
-bool
-ExprEngine::doesInvalidateGlobals(const CallOrObjCMessage &callOrMessage) const
-{
-  if (callOrMessage.isFunctionCall() && !callOrMessage.isCXXCall()) {
-    SVal calleeV = callOrMessage.getFunctionCallee();
-    if (const FunctionTextRegion *codeR =
-          dyn_cast_or_null<FunctionTextRegion>(calleeV.getAsRegion())) {
-      
-      const FunctionDecl *fd = codeR->getDecl();
-      if (const IdentifierInfo *ii = fd->getIdentifier()) {
-        StringRef fname = ii->getName();
-        if (fname == "strlen")
-          return false;
-      }
-    }
-  }
-  
-  // The conservative answer: invalidates globals.
-  return true;
-}
-
 //===----------------------------------------------------------------------===//
 // Top-level transfer function logic (Dispatcher).
 //===----------------------------------------------------------------------===//

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Jan  4 17:54:01 2012
@@ -294,15 +294,17 @@
     if (ObjTy->isRecordType()) {
       regionsToInvalidate.push_back(EleReg);
       // Invalidate the regions.
+      // TODO: Pass the call to new information as the last argument, to limit
+      // the globals which will get invalidated.
       state = state->invalidateRegions(regionsToInvalidate,
-                                       CNE, blockCount, 0,
-                                       /* invalidateGlobals = */ true);
+                                       CNE, blockCount, 0, 0);
       
     } else {
       // Invalidate the regions.
+      // TODO: Pass the call to new information as the last argument, to limit
+      // the globals which will get invalidated.
       state = state->invalidateRegions(regionsToInvalidate,
-                                       CNE, blockCount, 0,
-                                       /* invalidateGlobals = */ true);
+                                       CNE, blockCount, 0, 0);
 
       if (CNE->hasInitializer()) {
         SVal V = state->getSVal(*CNE->constructor_arg_begin());

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed Jan  4 17:54:01 2012
@@ -200,7 +200,7 @@
   //  global variables.
   return State->invalidateRegions(RegionsToInvalidate,
                                   Call.getOriginExpr(), Count,
-                                  &IS, doesInvalidateGlobals(Call));
+                                  &IS, &Call);
 
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Jan  4 17:54:01 2012
@@ -19,6 +19,7 @@
 #include "clang/Analysis/Support/BumpVector.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/RecordLayout.h"
+#include "clang/Basic/SourceManager.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
@@ -459,10 +460,6 @@
   os << superRegion << "->" << *getDecl();
 }
 
-void NonStaticGlobalSpaceRegion::dumpToStream(raw_ostream &os) const {
-  os << "NonStaticGlobalSpaceRegion";
-}
-
 void ObjCIvarRegion::dumpToStream(raw_ostream &os) const {
   os << "ivar{" << superRegion << ',' << *getDecl() << '}';
 }
@@ -491,6 +488,22 @@
   os << "StaticGlobalsMemSpace{" << CR << '}';
 }
 
+void NonStaticGlobalSpaceRegion::dumpToStream(raw_ostream &os) const {
+  os << "NonStaticGlobalSpaceRegion";
+}
+
+void GlobalInternalSpaceRegion::dumpToStream(raw_ostream &os) const {
+  os << "GlobalInternalSpaceRegion";
+}
+
+void GlobalSystemSpaceRegion::dumpToStream(raw_ostream &os) const {
+  os << "GlobalSystemSpaceRegion";
+}
+
+void GlobalImmutableSpaceRegion::dumpToStream(raw_ostream &os) const {
+  os << "GlobalImmutableSpaceRegion";
+}
+
 //===----------------------------------------------------------------------===//
 // MemRegionManager methods.
 //===----------------------------------------------------------------------===//
@@ -542,10 +555,18 @@
 }
 
 const GlobalsSpaceRegion
-*MemRegionManager::getGlobalsRegion(const CodeTextRegion *CR) {
-  if (!CR)
-    return LazyAllocate(globals);
+*MemRegionManager::getGlobalsRegion(MemRegion::Kind K,
+                                    const CodeTextRegion *CR) {
+  if (!CR) {
+    if (K == MemRegion::GlobalSystemSpaceRegionKind)
+      return LazyAllocate(SystemGlobals);
+    if (K == MemRegion::GlobalImmutableSpaceRegionKind)
+      return LazyAllocate(ImmutableGlobals);
+    assert(K == MemRegion::GlobalInternalSpaceRegionKind);
+    return LazyAllocate(InternalGlobals);
+  }
 
+  assert(K == MemRegion::StaticGlobalSpaceRegionKind);
   StaticGlobalSpaceRegion *&R = StaticsGlobalSpaceRegions[CR];
   if (R)
     return R;
@@ -570,7 +591,6 @@
 //===----------------------------------------------------------------------===//
 // Constructing regions.
 //===----------------------------------------------------------------------===//
-
 const StringRegion* MemRegionManager::getStringRegion(const StringLiteral* Str){
   return getSubRegion<StringRegion>(Str, getGlobalsRegion());
 }
@@ -579,9 +599,31 @@
                                                 const LocationContext *LC) {
   const MemRegion *sReg = 0;
 
-  if (D->hasGlobalStorage() && !D->isStaticLocal())
-    sReg = getGlobalsRegion();
-  else {
+  if (D->hasGlobalStorage() && !D->isStaticLocal()) {
+
+    // First handle the globals defined in system headers.
+    if (C.getSourceManager().isInSystemHeader(D->getLocation())) {
+      // Whitelist the system globals which often DO GET modified, assume the
+      // rest are immutable.
+      if (D->getName().find("errno") != StringRef::npos)
+        sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
+      else
+        sReg = getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind);
+
+    // Treat other globals as GlobalInternal unless they are constants.
+    } else {
+      QualType GQT = D->getType();
+      const Type *GT = GQT.getTypePtrOrNull();
+      // TODO: We could walk the complex types here and see if everything is
+      // constified.
+      if (GT && GQT.isConstQualified() && GT->isArithmeticType())
+        sReg = getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind);
+      else
+        sReg = getGlobalsRegion();
+    }
+  
+  // Finally handle static locals.  
+  } else {
     // FIXME: Once we implement scope handling, we will need to properly lookup
     // 'D' to the proper LocationContext.
     const DeclContext *DC = D->getDeclContext();
@@ -599,13 +641,15 @@
         assert(D->isStaticLocal());
         const Decl *D = STC->getDecl();
         if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
-          sReg = getGlobalsRegion(getFunctionTextRegion(FD));
+          sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
+                                  getFunctionTextRegion(FD));
         else if (const BlockDecl *BD = dyn_cast<BlockDecl>(D)) {
           const BlockTextRegion *BTR =
             getBlockTextRegion(BD,
                      C.getCanonicalType(BD->getSignatureAsWritten()->getType()),
                      STC->getAnalysisDeclContext());
-          sReg = getGlobalsRegion(BTR);
+          sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
+                                  BTR);
         }
         else {
           // FIXME: For ObjC-methods, we need a new CodeTextRegion.  For now

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp Wed Jan  4 17:54:01 2012
@@ -136,20 +136,20 @@
 ProgramState::invalidateRegions(ArrayRef<const MemRegion *> Regions,
                                 const Expr *E, unsigned Count,
                                 StoreManager::InvalidatedSymbols *IS,
-                                bool invalidateGlobals) const {
+                                const CallOrObjCMessage *Call) const {
   if (!IS) {
     StoreManager::InvalidatedSymbols invalidated;
     return invalidateRegionsImpl(Regions, E, Count,
-                                 invalidated, invalidateGlobals);
+                                 invalidated, Call);
   }
-  return invalidateRegionsImpl(Regions, E, Count, *IS, invalidateGlobals);
+  return invalidateRegionsImpl(Regions, E, Count, *IS, Call);
 }
 
 const ProgramState *
 ProgramState::invalidateRegionsImpl(ArrayRef<const MemRegion *> Regions,
                                     const Expr *E, unsigned Count,
                                     StoreManager::InvalidatedSymbols &IS,
-                                    bool invalidateGlobals) const {
+                                    const CallOrObjCMessage *Call) const {
   ProgramStateManager &Mgr = getStateManager();
   SubEngine* Eng = Mgr.getOwningEngine();
  
@@ -157,14 +157,14 @@
     StoreManager::InvalidatedRegions Invalidated;
     const StoreRef &newStore
       = Mgr.StoreMgr->invalidateRegions(getStore(), Regions, E, Count, IS,
-                                        invalidateGlobals, &Invalidated);
+                                        Call, &Invalidated);
     const ProgramState *newState = makeWithStore(newStore);
     return Eng->processRegionChanges(newState, &IS, Regions, Invalidated);
   }
 
   const StoreRef &newStore =
     Mgr.StoreMgr->invalidateRegions(getStore(), Regions, E, Count, IS,
-                                    invalidateGlobals, NULL);
+                                    Call, NULL);
   return makeWithStore(newStore);
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Jan  4 17:54:01 2012
@@ -20,6 +20,7 @@
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/AnalysisContext.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
@@ -235,11 +236,16 @@
   //===-------------------------------------------------------------------===//
   // Binding values to regions.
   //===-------------------------------------------------------------------===//
+  RegionBindings invalidateGlobalRegion(MemRegion::Kind K,
+                                        const Expr *Ex,
+                                        unsigned Count,
+                                        RegionBindings B,
+                                        InvalidatedRegions *Invalidated);
 
   StoreRef invalidateRegions(Store store, ArrayRef<const MemRegion *> Regions,
                              const Expr *E, unsigned Count,
                              InvalidatedSymbols &IS,
-                             bool invalidateGlobals,
+                             const CallOrObjCMessage *Call,
                              InvalidatedRegions *Invalidated);
 
 public:   // Made public for helper classes.
@@ -718,15 +724,39 @@
   B = RM.addBinding(B, baseR, BindingKey::Direct, V);
 }
 
+RegionBindings RegionStoreManager::invalidateGlobalRegion(MemRegion::Kind K,
+                                                          const Expr *Ex,
+                                                          unsigned Count,
+                                                          RegionBindings B,
+                                            InvalidatedRegions *Invalidated) {
+  // Bind the globals memory space to a new symbol that we will use to derive
+  // the bindings for all globals.
+  const GlobalsSpaceRegion *GS = MRMgr.getGlobalsRegion(K);
+  SVal V =
+      svalBuilder.getConjuredSymbolVal(/* SymbolTag = */ (void*) GS, Ex,
+          /* symbol type, doesn't matter */ Ctx.IntTy,
+          Count);
+
+  B = removeBinding(B, GS);
+  B = addBinding(B, BindingKey::Make(GS, BindingKey::Default), V);
+
+  // Even if there are no bindings in the global scope, we still need to
+  // record that we touched it.
+  if (Invalidated)
+    Invalidated->push_back(GS);
+
+  return B;
+}
+
 StoreRef RegionStoreManager::invalidateRegions(Store store,
                                             ArrayRef<const MemRegion *> Regions,
                                                const Expr *Ex, unsigned Count,
                                                InvalidatedSymbols &IS,
-                                               bool invalidateGlobals,
+                                               const CallOrObjCMessage *Call,
                                               InvalidatedRegions *Invalidated) {
   invalidateRegionsWorker W(*this, StateMgr,
                             RegionStoreManager::GetRegionBindings(store),
-                            Ex, Count, IS, Invalidated, invalidateGlobals);
+                            Ex, Count, IS, Invalidated, false);
 
   // Scan the bindings and generate the clusters.
   W.GenerateClusters();
@@ -741,20 +771,20 @@
   // Return the new bindings.
   RegionBindings B = W.getRegionBindings();
 
-  if (invalidateGlobals) {
-    // Bind the non-static globals memory space to a new symbol that we will
-    // use to derive the bindings for all non-static globals.
-    const GlobalsSpaceRegion *GS = MRMgr.getGlobalsRegion();
-    SVal V =
-      svalBuilder.getConjuredSymbolVal(/* SymbolTag = */ (void*) GS, Ex,
-                                  /* symbol type, doesn't matter */ Ctx.IntTy,
-                                  Count);
-    B = addBinding(B, BindingKey::Make(GS, BindingKey::Default), V);
-
-    // Even if there are no bindings in the global scope, we still need to
-    // record that we touched it.
-    if (Invalidated)
-      Invalidated->push_back(GS);
+  // For all globals which are not static nor immutable: determine which global
+  // regions should be invalidated and invalidate them.
+  // TODO: This could possibly be more precise with modules.
+  //
+  // System calls invalidate only system globals.
+  if (Call && Call->isInSystemHeader()) {
+    B = invalidateGlobalRegion(MemRegion::GlobalSystemSpaceRegionKind,
+                               Ex, Count, B, Invalidated);
+  // Internal calls might invalidate both system and internal globals.
+  } else {
+    B = invalidateGlobalRegion(MemRegion::GlobalSystemSpaceRegionKind,
+                               Ex, Count, B, Invalidated);
+    B = invalidateGlobalRegion(MemRegion::GlobalInternalSpaceRegionKind,
+                               Ex, Count, B, Invalidated);
   }
 
   return StoreRef(B.getRootWithoutRetain(), *this);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Wed Jan  4 17:54:01 2012
@@ -101,8 +101,10 @@
     case MemRegion::StackArgumentsSpaceRegionKind:
     case MemRegion::HeapSpaceRegionKind:
     case MemRegion::UnknownSpaceRegionKind:
-    case MemRegion::NonStaticGlobalSpaceRegionKind:
-    case MemRegion::StaticGlobalSpaceRegionKind: {
+    case MemRegion::StaticGlobalSpaceRegionKind:
+    case MemRegion::GlobalInternalSpaceRegionKind:
+    case MemRegion::GlobalSystemSpaceRegionKind:
+    case MemRegion::GlobalImmutableSpaceRegionKind: {
       llvm_unreachable("Invalid region cast");
     }
 

Added: cfe/trunk/test/Analysis/global-region-invalidation.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/global-region-invalidation.c?rev=147569&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/global-region-invalidation.c (added)
+++ cfe/trunk/test/Analysis/global-region-invalidation.c Wed Jan  4 17:54:01 2012
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -disable-free -analyzer-eagerly-assume -analyzer-checker=core,deadcode,experimental.security.taint,debug.TaintTest -verify %s
+
+// Note, we do need to include headers here, since the analyzer checks if the function declaration is located in a system header.
+#include "system-header-simulator.h"
+
+// Test that system header does not invalidate the internal global.
+int size_rdar9373039 = 1;
+int rdar9373039() {
+  int x;
+  int j = 0;
+
+  for (int i = 0 ; i < size_rdar9373039 ; ++i)
+    x = 1;
+
+  // strlen doesn't invalidate the value of 'size_rdar9373039'.
+  int extra = (2 + strlen ("Clang") + ((4 - ((unsigned int) (2 + strlen ("Clang")) % 4)) % 4)) + (2 + strlen ("1.0") + ((4 - ((unsigned int) (2 + strlen ("1.0")) % 4)) % 4));
+
+  for (int i = 0 ; i < size_rdar9373039 ; ++i)
+    j += x; // no-warning
+
+  return j;
+}
+
+// Test stdin does not get invalidated by a system call nor by an internal call.
+void foo();
+int stdinTest() {
+  int i = 0;
+  fscanf(stdin, "%d", &i);
+  foo();
+  int m = i; // expected-warning + {{tainted}}
+  fscanf(stdin, "%d", &i);
+  int j = i; // expected-warning + {{tainted}}
+  return m + j; // expected-warning + {{tainted}}
+}
+
+// Test errno gets invalidated by a system call.
+int testErrnoSystem() {
+  int i;
+  int *p = 0;
+  fscanf(stdin, "%d", &i);
+  if (errno == 0) {
+    fscanf(stdin, "%d", &i); // errno gets invalidated here.
+    return 5 / errno; // no-warning
+  }
+  return 0;
+}
+
+// Test that errno gets invalidated by internal calls.
+int testErrnoInternal() {
+  int i;
+  int *p = 0;
+  fscanf(stdin, "%d", &i);
+  if (errno == 0) {
+    foo(); // errno gets invalidated here.
+    return 5 / errno; // no-warning
+  }
+  return 0;
+}
+
+// Test that const integer does not get invalidated.
+const int x = 0;
+int constIntGlob() {
+  const int *m = &x;
+    foo();
+  return 3 / *m; // expected-warning {{Division by zero}}
+}
+
+extern const int x;
+int constIntGlobExtern() {
+  if (x == 0) {
+    foo();
+    return 5 / x; // expected-warning {{Division by zero}}
+  }
+  return 0;
+}

Modified: cfe/trunk/test/Analysis/misc-ps.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.c?rev=147569&r1=147568&r2=147569&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps.c (original)
+++ cfe/trunk/test/Analysis/misc-ps.c Wed Jan  4 17:54:01 2012
@@ -1,24 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -disable-free -analyzer-eagerly-assume -analyzer-checker=core -analyzer-checker=deadcode -verify %s
 
-unsigned long strlen(const char *);
-
 int size_rdar9373039 = 1;
-int rdar9373039() {
-  int x;
-  int j = 0;
-
-  for (int i = 0 ; i < size_rdar9373039 ; ++i)
-    x = 1;
-    
-  // strlen doesn't invalidate the value of 'size_rdar9373039'.
-  int extra = (2 + strlen ("Clang") + ((4 - ((unsigned int) (2 + strlen ("Clang")) % 4)) % 4)) + (2 + strlen ("1.0") + ((4 - ((unsigned int) (2 + strlen ("1.0")) % 4)) % 4));
-
-  for (int i = 0 ; i < size_rdar9373039 ; ++i)
-    j += x; // no-warning
-
-  return j;
-}
-
 int foo_rdar9373039(const char *);
 
 int rdar93730392() {

Added: cfe/trunk/test/Analysis/system-header-simulator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/system-header-simulator.h?rev=147569&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/system-header-simulator.h (added)
+++ cfe/trunk/test/Analysis/system-header-simulator.h Wed Jan  4 17:54:01 2012
@@ -0,0 +1,10 @@
+#pragma clang system_header
+
+typedef struct _FILE FILE;
+extern FILE *stdin;
+int fscanf(FILE *restrict stream, const char *restrict format, ...);
+
+// Note, on some platforms errno macro gets replaced with a function call.
+extern int errno;
+
+unsigned long strlen(const char *);





More information about the cfe-commits mailing list