]> granicus.if.org Git - clang/commitdiff
[analyzer] In getSVal() API, disable auto-detection of void type as char type.
authorArtem Dergachev <artem.dergachev@gmail.com>
Tue, 12 Dec 2017 02:27:55 +0000 (02:27 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Tue, 12 Dec 2017 02:27:55 +0000 (02:27 +0000)
This is a follow-up from r314910. When a checker developer attempts to
dereference a location in memory through ProgramState::getSVal(Loc) or
ProgramState::getSVal(const MemRegion *), without specifying the second
optional QualType parameter for the type of the value he tries to find at this
location, the type is auto-detected from location type. If the location
represents a value beyond a void pointer, we thought that auto-detecting the
type as 'char' is a good idea. However, in most practical cases, the correct
behavior would be to specify the type explicitly, as it is available from other
sources, and the few cases where we actually need to take a 'char' are
workarounds rather than an intended behavior. Therefore, try to fail with an
easy-to-understand assertion when asked to read from a void pointer location.

Differential Revision: https://reviews.llvm.org/D38801

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

include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
lib/StaticAnalyzer/Core/RegionStore.cpp

index e3a2164b11ff08d376c14a68b633c6c0253b2298..dd2564b0a3c3ea7c6a9513cce837deff525392d8 100644 (file)
@@ -308,8 +308,12 @@ public:
 
   /// \brief Return the value bound to the specified location.
   /// Returns UnknownVal() if none found.
-  SVal getSVal(const MemRegion* R) const;
+  SVal getSVal(const MemRegion* R, QualType T = QualType()) const;
 
+  /// \brief Return the value bound to the specified location, assuming
+  /// that the value is a scalar integer or an enumeration or a pointer.
+  /// Returns UnknownVal() if none found or the region is not known to hold
+  /// a value of such type.
   SVal getSValAsScalarOrLoc(const MemRegion *R) const;
 
   /// \brief Visits the symbols reachable from the given SVal using the provided
@@ -758,9 +762,10 @@ inline SVal ProgramState::getRawSVal(Loc LV, QualType T) const {
   return getStateManager().StoreMgr->getBinding(getStore(), LV, T);
 }
 
-inline SVal ProgramState::getSVal(const MemRegion* R) const {
+inline SVal ProgramState::getSVal(const MemRegion* R, QualType T) const {
   return getStateManager().StoreMgr->getBinding(getStore(),
-                                                loc::MemRegionVal(R));
+                                                loc::MemRegionVal(R),
+                                                T);
 }
 
 inline BasicValueFactory &ProgramState::getBasicVals() const {
index 07285d27ed9e1b2329738e6668c3d40f7c91dc06..20a46843e23e206c93fe1f50c9f22f8321197686 100644 (file)
@@ -179,7 +179,7 @@ bool CallAndMessageChecker::uninitRefOrPointer(
 
   if (const MemRegion *SValMemRegion = V.getAsRegion()) {
     const ProgramStateRef State = C.getState();
-    const SVal PSV = State->getSVal(SValMemRegion);
+    const SVal PSV = State->getSVal(SValMemRegion, C.getASTContext().CharTy);
     if (PSV.isUndef()) {
       if (ExplodedNode *N = C.generateErrorNode()) {
         LazyInit_BT(BD, BT);
index 883c6a663291d8fa6cef7f354b5a1460e3cb8b89..43966656cd8d0e4e6e986dd673f78937e8623ee0 100644 (file)
@@ -466,7 +466,7 @@ bool GenericTaintChecker::checkPre(const CallExpr *CE, CheckerContext &C) const{
 }
 
 Optional<SVal> GenericTaintChecker::getPointedToSVal(CheckerContext &C,
-                                            const Expr* Arg) {
+                                                     const Expr *Arg) {
   ProgramStateRef State = C.getState();
   SVal AddrVal = State->getSVal(Arg->IgnoreParens(), C.getLocationContext());
   if (AddrVal.isUnknownOrUndef())
@@ -476,9 +476,18 @@ Optional<SVal> GenericTaintChecker::getPointedToSVal(CheckerContext &C,
   if (!AddrLoc)
     return None;
 
-  const PointerType *ArgTy =
-    dyn_cast<PointerType>(Arg->getType().getCanonicalType().getTypePtr());
-  return State->getSVal(*AddrLoc, ArgTy ? ArgTy->getPointeeType(): QualType());
+  QualType ArgTy = Arg->getType().getCanonicalType();
+  if (!ArgTy->isPointerType())
+    return None;
+
+  QualType ValTy = ArgTy->getPointeeType();
+
+  // Do not dereference void pointers. Treat them as byte pointers instead.
+  // FIXME: we might want to consider more than just the first byte.
+  if (ValTy->isVoidType())
+    ValTy = C.getASTContext().CharTy;
+
+  return State->getSVal(*AddrLoc, ValTy);
 }
 
 ProgramStateRef
index ecb32cc378d0a7b2b7c52c91dd1241fdc1a6d272..7f2a481c6b0d36342c7f312293595344db4631bd 100644 (file)
@@ -1405,10 +1405,7 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
         T = Ctx.VoidTy;
     }
     assert(!T.isNull() && "Unable to auto-detect binding type!");
-    if (T->isVoidType()) {
-      // When trying to dereference a void pointer, read the first byte.
-      T = Ctx.CharTy;
-    }
+    assert(!T->isVoidType() && "Attempting to dereference a void pointer!");
     MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
   }