baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: NoQ.
baloghadamsoftware added a project: clang.
Herald added subscribers: ASDenysPetrov, martong, steakhal, Charusso,
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs,
szepet, xazax.hun, whisperity, mgorny.
Herald added a reviewer: Szelethus.
There are cases when a checker cannot know in advance the nature of the return
value they try to retrieve from a `CallEvent`. If it is a class instance then
it must use `getReturnValueUnderConstruction()`. If it is not, then it must use
`getReturnValue()`, because the former function returns `None` since only class
instances have construction context.
Simlarly, arguments which are class instances passed by value are
copy-constructed into the parameter. To retrieve them the checker needs to
invoke `getParameterLocation()` instead of `getArgSVal()`. However, for basic
types and class instances passed by pointer of reference only the latter can be
used to track the value of the argument.
To save checkers from doing these branching all over the time this patch
introduces two methods to `CallEvent`: `getReturnObject()` to get the correct
return value and `getArgObject()` to get the correct argument.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81718
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/unittests/StaticAnalyzer/CMakeLists.txt
clang/unittests/StaticAnalyzer/TestParameterLocation.cpp
clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
Index: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
===================================================================
--- clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
+++ clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
@@ -23,18 +23,46 @@
: public Checker<check::PostCall> {
public:
void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
- // Only calls with origin expression are checked. These are `returnC()`
- // and C::C().
- if (!Call.getOriginExpr())
+ const IdentifierInfo *ID = Call.getCalleeIdentifier();
+ if (!ID)
return;
- // Since `returnC` returns an object by value, the invocation results
- // in an object of type `C` constructed into variable `c`. Thus the
- // return value of `CallEvent::getReturnValueUnderConstruction()` must
- // be non-empty and has to be a `MemRegion`.
- Optional<SVal> RetVal = Call.getReturnValueUnderConstruction();
- ASSERT_TRUE(RetVal);
- ASSERT_TRUE(RetVal->getAsRegion());
+ SVal RetVal = Call.getReturnValue();
+ Optional<SVal> RetValUC = Call.getReturnValueUnderConstruction();
+ SVal RetObj = Call.getReturnObject();
+
+ if (ID->getName() == "returnC") {
+ // Since `returnC` returns an object by value, the invocation results
+ // in an object of type `C` constructed into variable `c`. Thus the
+ // return value of `CallEvent::getReturnValueUnderConstruction()` must
+ // be non-empty and has to be a `MemRegion`.
+ ASSERT_TRUE(RetValUC);
+ ASSERT_TRUE(RetValUC->getAsRegion());
+
+ // In this case `CallEvent::getReturnValue()` returns a `LazyCompoundVal`.
+ ASSERT_TRUE(RetVal.getAs<nonloc::LazyCompoundVal>().hasValue());
+
+ // The result of `CallEvent::getReturnObject()` is the same as the result
+ // of `CallEvent::getReturnValueUnderConstruction()`.
+ ASSERT_TRUE(RetObj == *RetValUC);
+ return;
+ }
+
+ if (ID->getName() == "returnVoidPtr") {
+ // Since `returnVoidPtr` returns a basic value (pointer), it has no
+ // consturcion context.
+ ASSERT_FALSE(RetValUC);
+
+ // In this case `CallEvent::getReturnValue()` returns a `MemRegion`.
+ ASSERT_TRUE(RetVal.getAsRegion());
+
+ // The result of `CallEvent::getReturnObject()` is the same as the result
+ // of `CallEvent::getReturnValueUnderConstruction()`.
+ ASSERT_TRUE(RetObj == RetVal);
+ return;
+ }
+
+ llvm_unreachable("Unknown callee.");
}
};
@@ -64,8 +92,13 @@
return c;
}
+ void *returnVoidPtr(int m) {
+ return &m;
+ }
+
void foo() {
C c = returnC(1);
+ void *vp = returnVoidPtr(1);
})"));
}
Index: clang/unittests/StaticAnalyzer/TestParameterLocation.cpp
===================================================================
--- /dev/null
+++ clang/unittests/StaticAnalyzer/TestParameterLocation.cpp
@@ -0,0 +1,143 @@
+//===- unittests/StaticAnalyzer/TestParameterLocation.cpp -----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+class TestParameterLocationChecker : public Checker<check::PostCall> {
+public:
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+ const IdentifierInfo *ID = Call.getCalleeIdentifier();
+ if (!ID)
+ return;
+
+ SVal Arg0 = Call.getArgSVal(0);
+ const ParamVarRegion *Param0Loc =
+ Call.getParameterLocation(0, C.blockCount());
+ SVal Arg0Obj = Call.getArgObject(0, C.blockCount());
+
+ if (ID->getName() == "passC") {
+ // Since `PassC` takes an object by value, the object is copy-constructed
+ // into the location of the parameter.
+ ASSERT_TRUE(Param0Loc);
+
+ ASSERT_FALSE(Arg0.isConstant() || Arg0.getAsSymbol() ||
+ Arg0.getAsRegion());
+
+ ASSERT_TRUE(Arg0Obj.getAsRegion());
+ ASSERT_TRUE(Arg0Obj.getAsRegion() == Param0Loc);
+ return;
+ }
+
+ if (ID->getName() == "passCPtr" || ID->getName() == "passCRef" ||
+ ID->getName() == "passCConstPtr" || ID->getName() == "passCConstRef" ||
+ ID->getName() == "passVoidPtr") {
+ // Since these functions take their arguments by pointer or reference,
+ // no copy-construction happens, but the region of the object is passed.
+ ASSERT_TRUE(Param0Loc);
+
+ ASSERT_TRUE(Arg0.getAsRegion());
+
+ ASSERT_TRUE(Arg0Obj.getAsRegion());
+ ASSERT_TRUE(Arg0Obj == Arg0);
+ return;
+ }
+
+ if (ID->getName() == "passInt") {
+ // Since `PassInt` takes an integer by value, it is copied into the
+ // parameter but not copy-constructed since it is a basic value.
+ ASSERT_TRUE(Param0Loc);
+
+ ASSERT_TRUE(Arg0.isConstant());
+
+ ASSERT_TRUE(Arg0Obj.isConstant());
+ ASSERT_TRUE(Arg0Obj == Arg0);
+ return;
+ }
+
+ llvm_unreachable("Unknown callee.");
+ }
+};
+
+void addTestParameterLocationChecker(
+ AnalysisASTConsumer &AnalysisConsumer, AnalyzerOptions &AnOpts) {
+ AnOpts.CheckersAndPackages =
+ {{"test.TestParameterLocation", true}};
+ AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+ Registry.addChecker<TestParameterLocationChecker>(
+ "test.TestParameterLocation", "", "");
+ });
+}
+
+TEST(TestParameterLocationChecker,
+ ParameterLocationChecker) {
+ EXPECT_TRUE(runCheckerOnCode<addTestParameterLocationChecker>(
+ R"(class C {
+ public:
+ C(int nn): n(nn) {}
+ operator int() const { return n; }
+ virtual ~C() {}
+ private:
+ int n;
+ };
+
+ int passC(C c) {
+ return c;
+ }
+
+ int passCConstPtr(const C *ccp) {
+ return *ccp;
+ }
+
+ int passCConstRef(const C &ccr) {
+ return ccr;
+ }
+
+ int passCPtr(C *cp) {
+ return *cp;
+ }
+
+ int passCRef(C &cr) {
+ return cr;
+ }
+
+ int passVoidPtr(void *vp) {
+ return *((int*)vp);
+ }
+
+ int passInt(int k) {
+ return k;
+ }
+
+ void foo() {
+ int m = 1;
+ C cc(m);
+ void *vp = &m;
+ m = passC(cc);
+ m = passCPtr(&cc);
+ m = passCRef(cc);
+ m = passCConstPtr(&cc);
+ m = passCConstRef(cc);
+ m = passVoidPtr(vp);
+ m = passInt(m);
+ })"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang
Index: clang/unittests/StaticAnalyzer/CMakeLists.txt
===================================================================
--- clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -12,6 +12,7 @@
RegisterCustomCheckersTest.cpp
StoreTest.cpp
SymbolReaperTest.cpp
+ TestParameterLocation.cpp
TestReturnValueUnderConstruction.cpp
)
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -221,6 +221,14 @@
return PVR;
}
+SVal CallEvent::getArgObject(unsigned Index, unsigned BlockCount) const {
+ SVal Arg = getArgSVal(Index);
+ if (Arg.isConstant() || Arg.getAsSymbol() || Arg.getAsRegion())
+ return Arg;
+
+ return loc::MemRegionVal(getParameterLocation(Index, BlockCount));
+}
+
/// Returns true if a type is a pointer-to-const or reference-to-const
/// with no further indirection.
static bool isPointerToConst(QualType Ty) {
@@ -531,6 +539,14 @@
return RetVal;
}
+SVal CallEvent::getReturnObject() const {
+ Optional<SVal> RetValUnderConstr = getReturnValueUnderConstruction();
+ if (RetValUnderConstr.hasValue())
+ return *RetValUnderConstr;
+
+ return getReturnValue();
+}
+
ArrayRef<ParmVarDecl*> AnyFunctionCall::parameters() const {
const FunctionDecl *D = getDecl();
if (!D)
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -407,6 +407,11 @@
const ParamVarRegion *getParameterLocation(unsigned Index,
unsigned BlockCount) const;
+ /// Returns the argument value if it is a symbol or a region (see
+ /// getArgSVal()). Otherwise it returns the parameter location (see
+ /// getParameterLocation()).
+ SVal getArgObject(unsigned Index, unsigned BlockCount) const;
+
/// Returns true if on the current path, the argument was constructed by
/// calling a C++ constructor over it. This is an internal detail of the
/// analysis which doesn't necessarily represent the program semantics:
@@ -444,6 +449,12 @@
/// can be retrieved from its construction context.
Optional<SVal> getReturnValueUnderConstruction() const;
+
+ /// If the call returns a C++ record type then this function retrieves and
+ /// returns its return value from its construction context. Otherwise it
+ /// simply returns its return value.
+ SVal getReturnObject() const;
+
// Iterator access to formal parameters and their types.
private:
struct GetTypeFn {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
