From: Jordan Rose Date: Sat, 1 Sep 2012 17:39:00 +0000 (+0000) Subject: [analyzer] Always derive a CallEvent's return type from its origin expr. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5699f62df144545702b91e91836a63db4e5f2627;p=clang [analyzer] Always derive a CallEvent's return type from its origin expr. 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 --- diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h index 510293a104..d0d1585545 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -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 index 84ea8c709d..0000000000 --- a/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp +++ /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 > { -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(callee)) { - const FunctionDecl *FD = FT->getDecl(); - actualResultTy = FD->getResultType(); - } - else if (const BlockDataRegion *BD = dyn_cast(callee)) { - const BlockTextRegion *BR = BD->getCodeRegion(); - const BlockPointerType *BT=BR->getLocationType()->getAs(); - const FunctionType *FT = BT->getPointeeType()->getAs(); - actualResultTy = FT->getResultType(); - } - - // Can this happen? - if (actualResultTy.isNull()) - return; - - // For now, ignore references. - if (actualResultTy->getAs()) - 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(); -} diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 7fe51d3a4c..ba39ccc456 100644 --- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -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 diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index 7949637ec6..92b5a16d70 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -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">; diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp index 32a66c3b27..aed0f55982 100644 --- a/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -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(BlockTy->getPointeeType())->getResultType(); -} - - SVal CXXConstructorCall::getCXXThisVal() const { if (Data) return loc::MemRegionVal(static_cast(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(); diff --git a/test/Analysis/reference.cpp b/test/Analysis/reference.cpp index e46016b762..4a2cbb8e25 100644 --- a/test/Analysis/reference.cpp +++ b/test/Analysis/reference.cpp @@ -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 // ------------------------------------