[cfe-commits] r163065 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/reference.cpp
Jordan Rose
jordan_rose at apple.com
Sat Sep 1 10:39:01 PDT 2012
Author: jrose
Date: Sat Sep 1 12:39:00 2012
New Revision: 163065
URL: http://llvm.org/viewvc/llvm-project?rev=163065&view=rev
Log:
[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.
Removed:
cfe/trunk/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/test/Analysis/reference.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h?rev=163065&r1=163064&r2=163065&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h Sat Sep 1 12:39:00 2012
@@ -168,8 +168,6 @@
/// result of this call.
virtual void getExtraInvalidatedRegions(RegionList &Regions) const {}
- virtual QualType getDeclaredResultType() const = 0;
-
public:
virtual ~CallEvent() {}
@@ -357,8 +355,6 @@
: 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 @@
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 @@
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;
Removed: cfe/trunk/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp?rev=163064&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp (removed)
@@ -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>();
-}
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=163065&r1=163064&r2=163065&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Sat Sep 1 12:39:00 2012
@@ -4,7 +4,6 @@
TARGET ClangSACheckers)
add_clang_library(clangStaticAnalyzerCheckers
- AdjustedReturnValueChecker.cpp
AnalyzerStatsChecker.cpp
ArrayBoundChecker.cpp
ArrayBoundCheckerV2.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=163065&r1=163064&r2=163065&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Sat Sep 1 12:39:00 2012
@@ -60,10 +60,6 @@
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">;
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=163065&r1=163064&r2=163065&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Sat Sep 1 12:39:00 2012
@@ -23,10 +23,26 @@
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 @@
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 @@
}
-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 @@
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();
Modified: cfe/trunk/test/Analysis/reference.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=163065&r1=163064&r2=163065&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/reference.cpp (original)
+++ cfe/trunk/test/Analysis/reference.cpp Sat Sep 1 12:39:00 2012
@@ -121,6 +121,18 @@
}
+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
// ------------------------------------
More information about the cfe-commits
mailing list