]> granicus.if.org Git - clang/commitdiff
[analyzer] Always derive a CallEvent's return type from its origin expr.
authorJordan Rose <jordan_rose@apple.com>
Sat, 1 Sep 2012 17:39:00 +0000 (17:39 +0000)
committerJordan Rose <jordan_rose@apple.com>
Sat, 1 Sep 2012 17:39:00 +0000 (17:39 +0000)
Previously, we preferred to get a result type by looking at the callee's
declared result type. This allowed us to handlereferences, which are
represented in the AST as lvalues of their pointee type. (That is, a call
to a function returning 'int &' has type 'int' and value kind 'lvalue'.)

However, this results in us preferring the original type of a function
over a casted type. This is a problem when a function  pointer is casted
to another type, because the conjured result value will have the wrong
type. AdjustedReturnValueChecker is supposed to handle this, but still
doesn't handle the case where there is no "original function" at all,
i.e. where the callee is unknown.

Now, we instead look at the call expression's value kind (lvalue, xvalue,
or prvalue), and adjust the expr's type accordingly. This will have no
effect when the function is inlined, and will conjure the value that will
actually be used when it is not.

This makes AdjustedReturnValueChecker /nearly/ unnecessary; unfortunately,
the cases where it would still be useful are where we need to cast the
result of an inlined function or a checker-evaluated function, and in these
cases we don't know what we're casting /from/ by the time we can do post-
call checks. In light of that, remove AdjustedReturnValueChecker, which
was already not checking quite a few calls.

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

include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp [deleted file]
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Core/CallEvent.cpp
test/Analysis/reference.cpp

index 510293a10461024fda6eb9a4f24fd611b0db5c35..d0d15855459c859629c43d8bf646bb62d7e4b2f4 100644 (file)
@@ -168,8 +168,6 @@ protected:
   /// result of this call.
   virtual void getExtraInvalidatedRegions(RegionList &Regions) const {}
 
-  virtual QualType getDeclaredResultType() const = 0;
-
 public:
   virtual ~CallEvent() {}
 
@@ -357,8 +355,6 @@ protected:
     : CallEvent(D, St, LCtx) {}
   AnyFunctionCall(const AnyFunctionCall &Other) : CallEvent(Other) {}
 
-  virtual QualType getDeclaredResultType() const;
-
 public:
   // This function is overridden by subclasses, but they must return
   // a FunctionDecl.
@@ -453,8 +449,6 @@ protected:
 
   virtual void getExtraInvalidatedRegions(RegionList &Regions) const;
 
-  virtual QualType getDeclaredResultType() const;
-
 public:
   /// \brief Returns the region associated with this instance of the block.
   ///
@@ -772,8 +766,6 @@ protected:
 
   virtual void getExtraInvalidatedRegions(RegionList &Regions) const;
 
-  virtual QualType getDeclaredResultType() const;
-
   /// Check if the selector may have multiple definitions (may have overrides).
   virtual bool canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl,
                                         Selector Sel) const;
diff --git a/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp b/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp
deleted file mode 100644 (file)
index 84ea8c7..0000000
+++ /dev/null
@@ -1,92 +0,0 @@
-//== AdjustedReturnValueChecker.cpp -----------------------------*- C++ -*--==//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This file defines AdjustedReturnValueChecker, a simple check to see if the
-// return value of a function call is different than the one the caller thinks
-// it is.
-//
-//===----------------------------------------------------------------------===//
-
-#include "ClangSACheckers.h"
-#include "clang/StaticAnalyzer/Core/Checker.h"
-#include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
-
-using namespace clang;
-using namespace ento;
-
-namespace {
-class AdjustedReturnValueChecker : 
-    public Checker< check::PostStmt<CallExpr> > {
-public:
-  void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
-};
-}
-
-void AdjustedReturnValueChecker::checkPostStmt(const CallExpr *CE,
-                                               CheckerContext &C) const {
-  
-  // Get the result type of the call.
-  QualType expectedResultTy = CE->getType();
-
-  // Fetch the signature of the called function.
-  ProgramStateRef state = C.getState();
-  const LocationContext *LCtx = C.getLocationContext();
-
-  SVal V = state->getSVal(CE, LCtx);
-  
-  if (V.isUnknown())
-    return;
-  
-  // Casting to void?  Discard the value.
-  if (expectedResultTy->isVoidType()) {
-    C.addTransition(state->BindExpr(CE, LCtx, UnknownVal()));
-    return;
-  }                   
-
-  const MemRegion *callee = state->getSVal(CE->getCallee(), LCtx).getAsRegion();
-  if (!callee)
-    return;
-
-  QualType actualResultTy;
-  
-  if (const FunctionTextRegion *FT = dyn_cast<FunctionTextRegion>(callee)) {
-    const FunctionDecl *FD = FT->getDecl();
-    actualResultTy = FD->getResultType();
-  }
-  else if (const BlockDataRegion *BD = dyn_cast<BlockDataRegion>(callee)) {
-    const BlockTextRegion *BR = BD->getCodeRegion();
-    const BlockPointerType *BT=BR->getLocationType()->getAs<BlockPointerType>();
-    const FunctionType *FT = BT->getPointeeType()->getAs<FunctionType>();
-    actualResultTy = FT->getResultType();
-  }
-
-  // Can this happen?
-  if (actualResultTy.isNull())
-    return;
-
-  // For now, ignore references.
-  if (actualResultTy->getAs<ReferenceType>())
-    return;
-  
-
-  // Are they the same?
-  if (expectedResultTy != actualResultTy) {
-    // FIXME: Do more checking and actual emit an error. At least performing
-    // the cast avoids some assertion failures elsewhere.
-    SValBuilder &svalBuilder = C.getSValBuilder();
-    V = svalBuilder.evalCast(V, expectedResultTy, actualResultTy);
-    C.addTransition(state->BindExpr(CE, LCtx, V));
-  }
-}
-
-void ento::registerAdjustedReturnValueChecker(CheckerManager &mgr) {
-  mgr.registerChecker<AdjustedReturnValueChecker>();
-}
index 7fe51d3a4ce3c0ba6609402e753f7e55c1b0b0a6..ba39ccc4560fc89cf790ae92fbd31d3d1053c58e 100644 (file)
@@ -4,7 +4,6 @@ clang_tablegen(Checkers.inc -gen-clang-sa-checkers
   TARGET ClangSACheckers)
 
 add_clang_library(clangStaticAnalyzerCheckers
-  AdjustedReturnValueChecker.cpp
   AnalyzerStatsChecker.cpp
   ArrayBoundChecker.cpp
   ArrayBoundCheckerV2.cpp
index 7949637ec615b4662ae81cd265201c39758fc1ec..92b5a16d7022f55f875097f0ce71690beeec83e8 100644 (file)
@@ -60,10 +60,6 @@ def CallAndMessageChecker : Checker<"CallAndMessage">,
   HelpText<"Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)">,
   DescFile<"CallAndMessageChecker.cpp">;
 
-def AdjustedReturnValueChecker : Checker<"AdjustedReturnValue">,
-  HelpText<"Check to see if the return value of a function call is different than the caller expects (e.g., from calls through function pointers)">,
-  DescFile<"AdjustedReturnValueChecker.cpp">;
-
 def AttrNonNullChecker : Checker<"AttributeNonNull">,
   HelpText<"Check for null pointers passed as arguments to a function whose arguments are marked with the 'nonnull' attribute">,
   DescFile<"AttrNonNullChecker.cpp">;
index 32a66c3b27020c157469384d34eef140c7b1af43..aed0f55982ac28920a86ca7ed7d9a2318d85d1ff 100644 (file)
@@ -23,10 +23,26 @@ using namespace clang;
 using namespace ento;
 
 QualType CallEvent::getResultType() const {
-  QualType ResultTy = getDeclaredResultType();
+  const Expr *E = getOriginExpr();
+  assert(E && "Calls without origin expressions do not have results");
+  QualType ResultTy = E->getType();
 
-  if (ResultTy.isNull())
-    ResultTy = getOriginExpr()->getType();
+  ASTContext &Ctx = getState()->getStateManager().getContext();
+
+  // A function that returns a reference to 'int' will have a result type
+  // of simply 'int'. Check the origin expr's value kind to recover the
+  // proper type.
+  switch (E->getValueKind()) {
+  case VK_LValue:
+    ResultTy = Ctx.getLValueReferenceType(ResultTy);
+    break;
+  case VK_XValue:
+    ResultTy = Ctx.getRValueReferenceType(ResultTy);
+    break;
+  case VK_RValue:
+    // No adjustment is necessary.
+    break;
+  }
 
   return ResultTy;
 }
@@ -285,14 +301,6 @@ void AnyFunctionCall::getInitialStackFrameContents(
                                D->param_begin(), D->param_end());
 }
 
-QualType AnyFunctionCall::getDeclaredResultType() const {
-  const FunctionDecl *D = getDecl();
-  if (!D)
-    return QualType();
-
-  return D->getResultType();
-}
-
 bool AnyFunctionCall::argumentsMayEscape() const {
   if (hasNonZeroCallbackArg())
     return true;
@@ -501,15 +509,6 @@ void BlockCall::getInitialStackFrameContents(const StackFrameContext *CalleeCtx,
 }
 
 
-QualType BlockCall::getDeclaredResultType() const {
-  const BlockDataRegion *BR = getBlockRegion();
-  if (!BR)
-    return QualType();
-  QualType BlockTy = BR->getCodeRegion()->getLocationType();
-  return cast<FunctionType>(BlockTy->getPointeeType())->getResultType();
-}
-
-
 SVal CXXConstructorCall::getCXXThisVal() const {
   if (Data)
     return loc::MemRegionVal(static_cast<const MemRegion *>(Data));
@@ -566,14 +565,6 @@ ObjCMethodCall::getExtraInvalidatedRegions(RegionList &Regions) const {
     Regions.push_back(R);
 }
 
-QualType ObjCMethodCall::getDeclaredResultType() const {
-  const ObjCMethodDecl *D = getDecl();
-  if (!D)
-    return QualType();
-
-  return D->getResultType();
-}
-
 SVal ObjCMethodCall::getSelfSVal() const {
   const LocationContext *LCtx = getLocationContext();
   const ImplicitParamDecl *SelfDecl = LCtx->getSelfDecl();
index e46016b762d55b525d68b19602c1d9285ed91119..4a2cbb8e25a5e486013e4fa880fca933006cc628 100644 (file)
@@ -121,6 +121,18 @@ void testReferenceAddress(int &x) {
 }
 
 
+void testFunctionPointerReturn(void *opaque) {
+  typedef int &(*RefFn)();
+
+  RefFn getRef = (RefFn)opaque;
+
+  // Don't crash writing to or reading from this reference.
+  int &x = getRef();
+  x = 42;
+  clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
+}
+
+
 // ------------------------------------
 // False negatives
 // ------------------------------------