r189492 - [analyzer] Add support for testing the presence of weak functions.

Jordan Rose jordan_rose at apple.com
Wed Aug 28 10:07:04 PDT 2013


Author: jrose
Date: Wed Aug 28 12:07:04 2013
New Revision: 189492

URL: http://llvm.org/viewvc/llvm-project?rev=189492&view=rev
Log:
[analyzer] Add support for testing the presence of weak functions.

When casting the address of a FunctionTextRegion to bool, or when adding
constraints to such an address, use a stand-in symbol to represent the
presence or absence of the function if the function is weakly linked.
This is groundwork for possible simple availability testing checks, and
can already catch mistakes involving inverted null checks for
weakly-linked functions.

Currently, the implementation reuses the "extent" symbols, originally created
for tracking the size of a malloc region. Since FunctionTextRegions cannot
be dereferenced, the extent symbol will never be used for anything else.
Still, this probably deserves a refactoring in the future.

This patch does not attempt to support testing the presence of weak
/variables/ (global variables), which would likely require much more of
a change and a generalization of "region structure metadata", like the
current "extents", vs. "region contents metadata", like CStringChecker's
"string length".

Patch by Richard <tarka.t.otter at googlemail.com>!

Added:
    cfe/trunk/test/Analysis/weak-functions.c
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.h
    cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=189492&r1=189491&r2=189492&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Wed Aug 28 12:07:04 2013
@@ -405,15 +405,18 @@ SVal SValBuilder::evalCast(SVal val, Qua
       return val;
     if (val.isConstant())
       return makeTruthVal(!val.isZeroConstant(), castTy);
-    if (SymbolRef Sym = val.getAsSymbol()) {
+    if (SymbolRef Sym = val.getAsSymbol(true)) {
       BasicValueFactory &BVF = getBasicValueFactory();
       // FIXME: If we had a state here, we could see if the symbol is known to
       // be zero, but we don't.
       return makeNonLoc(Sym, BO_NE, BVF.getValue(0, Sym->getType()), castTy);
     }
+    // Loc values are not always true, they could be weakly linked functions.
+    if (Optional<Loc> L = val.getAs<Loc>())
+      return evalCastFromLoc(*L, castTy);
 
-    assert(val.getAs<Loc>() || val.getAs<nonloc::LocAsInteger>());
-    return makeTruthVal(true, castTy);
+    Loc L = val.castAs<nonloc::LocAsInteger>().getLoc();
+    return evalCastFromLoc(L, castTy);
   }
 
   // For const casts, casts to void, just propagate the value.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp?rev=189492&r1=189491&r2=189492&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp Wed Aug 28 12:07:04 2013
@@ -68,51 +68,20 @@ bool SimpleConstraintManager::canReasonA
 ProgramStateRef SimpleConstraintManager::assume(ProgramStateRef state,
                                                DefinedSVal Cond,
                                                bool Assumption) {
-  if (Optional<NonLoc> NV = Cond.getAs<NonLoc>())
-    return assume(state, *NV, Assumption);
-  return assume(state, Cond.castAs<Loc>(), Assumption);
-}
-
-ProgramStateRef SimpleConstraintManager::assume(ProgramStateRef state, Loc cond,
-                                               bool assumption) {
-  state = assumeAux(state, cond, assumption);
-  if (NotifyAssumeClients && SU)
-    return SU->processAssume(state, cond, assumption);
-  return state;
-}
+  // If we have a Loc value, cast it to a bool NonLoc first.
+  if (Optional<Loc> LV = Cond.getAs<Loc>()) {
+    SValBuilder &SVB = state->getStateManager().getSValBuilder();
+    QualType T;
+    const MemRegion *MR = LV->getAsRegion();
+    if (const TypedRegion *TR = dyn_cast_or_null<TypedRegion>(MR))
+      T = TR->getLocationType();
+    else
+      T = SVB.getContext().VoidPtrTy;
 
-ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef state,
-                                                  Loc Cond, bool Assumption) {
-  switch (Cond.getSubKind()) {
-  default:
-    assert (false && "'Assume' not implemented for this Loc.");
-    return state;
-
-  case loc::MemRegionKind: {
-    // FIXME: Should this go into the storemanager?
-    const MemRegion *R = Cond.castAs<loc::MemRegionVal>().getRegion();
-
-    // FIXME: now we only find the first symbolic region.
-    if (const SymbolicRegion *SymR = R->getSymbolicBase()) {
-      const llvm::APSInt &zero = getBasicVals().getZeroWithPtrWidth();
-      if (Assumption)
-        return assumeSymNE(state, SymR->getSymbol(), zero, zero);
-      else
-        return assumeSymEQ(state, SymR->getSymbol(), zero, zero);
-    }
-
-    // FALL-THROUGH.
+    Cond = SVB.evalCast(*LV, SVB.getContext().BoolTy, T).castAs<DefinedSVal>();
   }
 
-  case loc::GotoLabelKind:
-    return Assumption ? state : NULL;
-
-  case loc::ConcreteIntKind: {
-    bool b = Cond.castAs<loc::ConcreteInt>().getValue() != 0;
-    bool isFeasible = b ? Assumption : !Assumption;
-    return isFeasible ? state : NULL;
-  }
-  } // end switch
+  return assume(state, Cond.castAs<NonLoc>(), Assumption);
 }
 
 ProgramStateRef SimpleConstraintManager::assume(ProgramStateRef state,
@@ -216,8 +185,8 @@ ProgramStateRef SimpleConstraintManager:
   }
 
   case nonloc::LocAsIntegerKind:
-    return assumeAux(state, Cond.castAs<nonloc::LocAsInteger>().getLoc(),
-                     Assumption);
+    return assume(state, Cond.castAs<nonloc::LocAsInteger>().getLoc(),
+                  Assumption);
   } // end switch
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.h?rev=189492&r1=189491&r2=189492&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.h (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.h Wed Aug 28 12:07:04 2013
@@ -36,8 +36,6 @@ public:
   ProgramStateRef assume(ProgramStateRef state, DefinedSVal Cond,
                         bool Assumption);
 
-  ProgramStateRef assume(ProgramStateRef state, Loc Cond, bool Assumption);
-
   ProgramStateRef assume(ProgramStateRef state, NonLoc Cond, bool Assumption);
 
   ProgramStateRef assumeSymRel(ProgramStateRef state,
@@ -87,10 +85,6 @@ protected:
   bool canReasonAbout(SVal X) const;
 
   ProgramStateRef assumeAux(ProgramStateRef state,
-                                Loc Cond,
-                                bool Assumption);
-
-  ProgramStateRef assumeAux(ProgramStateRef state,
                                 NonLoc Cond,
                                 bool Assumption);
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=189492&r1=189491&r2=189492&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Wed Aug 28 12:07:04 2013
@@ -137,6 +137,32 @@ SVal SimpleSValBuilder::evalCastFromLoc(
   if (castTy->isUnionType())
     return UnknownVal();
 
+  // Casting a Loc to a bool will almost always be true,
+  // unless this is a weak function or a symbolic region.
+  if (castTy->isBooleanType()) {
+    switch (val.getSubKind()) {
+      case loc::MemRegionKind: {
+        const MemRegion *R = val.castAs<loc::MemRegionVal>().getRegion();
+        if (const FunctionTextRegion *FTR = dyn_cast<FunctionTextRegion>(R))
+          if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(FTR->getDecl()))
+            if (FD->isWeak())
+              // FIXME: Currently we are using an extent symbol here,
+              // because there are no generic region address metadata
+              // symbols to use, only content metadata.
+              return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR));
+
+        if (const SymbolicRegion *SymR = R->getSymbolicBase())
+          return nonloc::SymbolVal(SymR->getSymbol());
+
+        // FALL-THROUGH
+      }
+
+      case loc::GotoLabelKind:
+        // Labels and non symbolic memory regions are always true.
+        return makeTruthVal(true, castTy);
+    }
+  }
+
   if (castTy->isIntegralOrEnumerationType()) {
     unsigned BitWidth = Context.getTypeSize(castTy);
 
@@ -668,7 +694,7 @@ SVal SimpleSValBuilder::evalBinOpLL(Prog
     if (Optional<loc::ConcreteInt> rInt = rhs.getAs<loc::ConcreteInt>()) {
       // If one of the operands is a symbol and the other is a constant,
       // build an expression for use by the constraint manager.
-      if (SymbolRef lSym = lhs.getAsLocSymbol())
+      if (SymbolRef lSym = lhs.getAsLocSymbol(true))
         return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy);
 
       // Special case comparisons to NULL.
@@ -676,19 +702,14 @@ SVal SimpleSValBuilder::evalBinOpLL(Prog
       // build constraints. The address of any non-symbolic region is guaranteed
       // to be non-NULL.
       if (rInt->isZeroConstant()) {
-        switch (op) {
-        default:
-          break;
-        case BO_Sub:
+        if (op == BO_Sub)
           return evalCastFromLoc(lhs, resultTy);
-        case BO_EQ:
-        case BO_LT:
-        case BO_LE:
-          return makeTruthVal(false, resultTy);
-        case BO_NE:
-        case BO_GT:
-        case BO_GE:
-          return makeTruthVal(true, resultTy);
+
+        if (BinaryOperator::isComparisonOp(op)) {
+          QualType boolType = getContext().BoolTy;
+          NonLoc l = evalCastFromLoc(lhs, boolType).castAs<NonLoc>();
+          NonLoc r = makeTruthVal(false, boolType).castAs<NonLoc>();
+          return evalBinOpNN(state, op, l, r, resultTy);
         }
       }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp?rev=189492&r1=189491&r2=189492&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp Wed Aug 28 12:07:04 2013
@@ -435,6 +435,9 @@ bool SymbolReaper::isLiveRegion(const Me
   if (isa<MemSpaceRegion>(MR))
     return true;
 
+  if (isa<CodeTextRegion>(MR))
+    return true;
+
   return false;
 }
 

Added: cfe/trunk/test/Analysis/weak-functions.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/weak-functions.c?rev=189492&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/weak-functions.c (added)
+++ cfe/trunk/test/Analysis/weak-functions.c Wed Aug 28 12:07:04 2013
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection,unix.Malloc,unix.cstring,alpha.unix.cstring,unix.API,osx.API,osx.cocoa.RetainCount -Wno-null-dereference -analyzer-store=region -fblocks -verify %s
+#define NULL 0
+void clang_analyzer_eval(int);
+void myFunc();
+void myWeakFunc() __attribute__((weak_import));
+
+void testWeakFuncIsNull()
+{
+  clang_analyzer_eval(myFunc == NULL);  // expected-warning{{FALSE}}
+  clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{UNKNOWN}}
+  if (myWeakFunc == NULL) {
+    clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{TRUE}}
+  } else {
+    clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{FALSE}}
+  }
+}
+
+void testWeakFuncIsNot()
+{
+  clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{UNKNOWN}}
+  if (!myWeakFunc) {
+    clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{TRUE}}
+  } else {
+    clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{FALSE}}
+  }
+}
+
+void testWeakFuncIsTrue()
+{
+    clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{UNKNOWN}}
+    if (myWeakFunc) {
+        clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{FALSE}}
+    } else {
+        clang_analyzer_eval(myWeakFunc == NULL);  // expected-warning{{TRUE}}
+    }
+}
+
+//===----------------------------------------------------------------------===
+// func.c
+//===----------------------------------------------------------------------===
+void f(void) __attribute__((weak_import));
+void g(void (*fp)(void)) __attribute__((weak_import));
+
+void f(void) {
+  void (*p)(void);
+  p = f;
+  p = &f;
+  p();
+  (*p)();
+}
+
+void g(void (*fp)(void));
+
+void f2() {
+  g(f);
+}
+
+void f3(void (*f)(void), void (*g)(void)) {
+  clang_analyzer_eval(!f); // expected-warning{{UNKNOWN}}
+  f();
+  clang_analyzer_eval(!f); // expected-warning{{FALSE}}
+
+  clang_analyzer_eval(!g); // expected-warning{{UNKNOWN}}
+  (*g)();
+  clang_analyzer_eval(!g); // expected-warning{{FALSE}}
+}
+
+//===----------------------------------------------------------------------===
+// free.c
+//===----------------------------------------------------------------------===
+void free(void *) __attribute__((weak_import));
+
+void t10 () {
+  free((void*)&t10); // expected-warning {{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+}
+
+//===----------------------------------------------------------------------===
+// string.c : strnlen()
+//===----------------------------------------------------------------------===
+typedef typeof(sizeof(int)) size_t;
+size_t strlen(const char *s) __attribute__((weak_import));
+
+size_t strlen_fn() {
+  return strlen((char*)&strlen_fn); // expected-warning{{Argument to string length function is the address of the function 'strlen_fn', which is not a null-terminated string}}
+}
+
+//===----------------------------------------------------------------------===
+// unix-fns.c : dispatch_once
+//===----------------------------------------------------------------------===
+typedef void (^dispatch_block_t)(void);
+typedef long dispatch_once_t;
+void dispatch_once(dispatch_once_t *predicate, dispatch_block_t block) __attribute__((weak_import));
+
+void test_dispatch_once() {
+  dispatch_once_t pred = 0;
+  do { if (__builtin_expect(*(&pred), ~0l) != ~0l) dispatch_once((&pred), (^() {})); } while (0); // expected-warning{{Call to 'dispatch_once' uses the local variable 'pred' for the predicate value}}
+}
+void test_dispatch_once_neg() {
+  static dispatch_once_t pred = 0;
+  do { if (__builtin_expect(*(&pred), ~0l) != ~0l) dispatch_once((&pred), (^() {})); } while (0); // no-warning
+}
+
+//===----------------------------------------------------------------------===
+// retain-release-path-notes.m
+//===----------------------------------------------------------------------===
+typedef struct CFType *CFTypeRef;
+CFTypeRef CFCreateSomething() __attribute__((weak_import));
+CFTypeRef CFGetSomething() __attribute__((weak_import));
+
+CFTypeRef CFCopyRuleViolation () {
+  CFTypeRef object = CFGetSomething();
+  return object; // expected-warning{{Object with a +0 retain count returned to caller where a +1 (owning) retain count is expected}}
+}
+
+CFTypeRef CFGetRuleViolation () {
+  CFTypeRef object = CFCreateSomething(); // expected-warning{{Potential leak of an object stored into 'object'}}
+  return object; }





More information about the cfe-commits mailing list