llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Balazs Benics (steakhal)
<details>
<summary>Changes</summary>
When doing a base to derived cast, and we should add a cast info recording that
fact.
This information will be then used for example inside
`CXXInstanceCall::getRuntimeDefinition` (CallEvent.cpp), where for virtual
calls, it will query the associated dynamic type for the `*this` region.
Note that inside `ExprEngine::defaultEvalCall`, if the runtime definition is
found but it might have other definitions, then we will split the path and have
one where we inline the candidate and one other doing conservative call
evaluation. This ensures that if we inlined the wrong definition, we still have
a path where conservative evaluation happened. In addition to this, remember
that once a function was conservatively evaluated on a path, then further calls
to the same function will always result in conservative evaluation, see
`ExprEngine::BifurcateCall` and the `DynamicDispatchBifurcationMap`.
As a consequence of these rules, we can have execution paths where a function
call might be resolved to different implemetations, depending on the sequence
of events, like here:
```C++
void top(Base *p) {
p->fun(); // Will split into 2 paths:
// 1) Inlines the virtual `Base::fun()`.
// 2) Conservative eval calls, and remembers to always conservatively eval
this function.
// Perform a base to derived cast, and just discard the result.
(void)static_cast<Derived *>(p);
p->fun(); // Will split path (1) into 2:
// 2.1) Inlines the virtual `Derived::fun()`.
// 2.2) Conservative eval calls, and remembers to always conservatively eval
this function.
// Now, we have exactly 3 paths at this point.
}
```
Fixes #<!-- -->62663
---
Full diff: https://github.com/llvm/llvm-project/pull/69057.diff
3 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
(+17-10)
- (added) clang/test/Analysis/cast-trust-base-to-derived.cpp (+105)
- (modified) clang/test/Analysis/osobject-retain-release.cpp (+7-5)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
index eda8302595ba4fb..6539f3c1e09e321 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
@@ -20,6 +20,7 @@
//
//===----------------------------------------------------------------------===//
+#include "clang/AST/OperationKinds.h"
#include "clang/AST/ParentMap.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/Builtins.h"
@@ -392,10 +393,6 @@ void DynamicTypePropagation::checkPostCall(const CallEvent
&Call,
}
}
-/// TODO: Handle explicit casts.
-/// Handle C++ casts.
-///
-/// Precondition: the cast is between ObjCObjectPointers.
ExplodedNode *DynamicTypePropagation::dynamicTypePropagationOnCasts(
const CastExpr *CE, ProgramStateRef &State, CheckerContext &C) const {
// We only track type info for regions.
@@ -403,8 +400,19 @@ ExplodedNode
*DynamicTypePropagation::dynamicTypePropagationOnCasts(
if (!ToR)
return C.getPredecessor();
- if (isa<ExplicitCastExpr>(CE))
+ if (CE->getCastKind() == CK_BaseToDerived) {
+ bool CastSucceeds = true;
+ QualType FromTy = CE->getSubExpr()->getType();
+ QualType ToTy = CE->getType();
+ ToR = ToR->StripCasts(/*StripBaseAndDerivedCasts=*/true);
+ State = setDynamicTypeAndCastInfo(State, ToR, FromTy, ToTy, CastSucceeds);
+ return C.addTransition(State);
+ }
+
+ // TODO: Handle the rest of explicit casts, inluding the regular C++ casts.
+ if (isa<ExplicitCastExpr>(CE)) {
return C.getPredecessor();
+ }
if (const Type *NewTy = getBetterObjCType(CE, C)) {
State = setDynamicTypeInfo(State, ToR, QualType(NewTy, 0));
@@ -609,9 +617,13 @@ storeWhenMoreInformative(ProgramStateRef &State, SymbolRef
Sym,
/// symbol and the destination type of the cast are unrelated, report an error.
void DynamicTypePropagation::checkPostStmt(const CastExpr *CE,
CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ ExplodedNode *AfterTypeProp = dynamicTypePropagationOnCasts(CE, State, C);
+
if (CE->getCastKind() != CK_BitCast)
return;
+ ASTContext &ASTCtxt = C.getASTContext();
QualType OriginType = CE->getSubExpr()->getType();
QualType DestType = CE->getType();
@@ -621,11 +633,6 @@ void DynamicTypePropagation::checkPostStmt(const CastExpr
*CE,
if (!OrigObjectPtrType || !DestObjectPtrType)
return;
- ProgramStateRef State = C.getState();
- ExplodedNode *AfterTypeProp = dynamicTypePropagationOnCasts(CE, State, C);
-
- ASTContext &ASTCtxt = C.getASTContext();
-
// This checker detects the subtyping relationships using the assignment
// rules. In order to be able to do this the kindofness must be stripped
// first. The checker treats every type as kindof type anyways: when the
diff --git a/clang/test/Analysis/cast-trust-base-to-derived.cpp
b/clang/test/Analysis/cast-trust-base-to-derived.cpp
new file mode 100644
index 000000000000000..e717e5617867ca9
--- /dev/null
+++ b/clang/test/Analysis/cast-trust-base-to-derived.cpp
@@ -0,0 +1,105 @@
+// RUN: %clang_analyze_cc1 %s -verify \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=alpha.security.taint.TaintPropagation \
+// RUN: -analyzer-checker=debug.ExprInspection
+
+// See issue https://github.com/llvm/llvm-project/issues/62663
+
+template <typename T> void clang_analyzer_dump(T);
+void clang_analyzer_warnIfReached();
+void clang_analyzer_numTimesReached();
+void clang_analyzer_isTainted(int);
+
+extern int scanf(const char *format, ...);
+
+class ActionHandler {
+public:
+ virtual ~ActionHandler() = default;
+ virtual void onAction(int x, int &) {
+ clang_analyzer_dump(x + 1); // expected-warning {{101}}
+ }
+};
+
+class MyHandler final : public ActionHandler {
+public:
+ void onAction(int x, int &) override {
+ clang_analyzer_dump(x + 2); // expected-warning {{202}}
+ }
+};
+
+void trust_static_types(ActionHandler *p) {
+ // This variable will help to see if conservative call evaluation happened
or not.
+ int invalidation_detector;
+
+ // At this point we don't know anything about the dynamic type of `*p`, thus
the `onAction` call might be resolved to the default implementation, matching
the static type.
+ invalidation_detector = 1000;
+ p->onAction(100, invalidation_detector);
+ clang_analyzer_dump(invalidation_detector);
+ // expected-warning@-1 {{1000}} on this path we trusted the type, and
resolved the call to `ActionHandler::onAction(int, int&)`
+ // expected-warning@-2 {{conj}} on this path we conservatively evaluated the
previous call
+
+ clang_analyzer_numTimesReached(); // expected-warning {{2}} we only have
that 2 paths here
+ // 1) inlined `ActionHandler::onAction(int, int&)`
+ // 2) conservatively eval called
+
+ // Trust that the `*p` refers to an object with `MyHandler` static type (or
to some other sub-class).
+ auto *q = static_cast<MyHandler *>(p);
+ (void)q; // Discard the result of the cast. We already learned the type `p`
might refer to.
+
+ invalidation_detector = 2000;
+ p->onAction(200, invalidation_detector);
+ clang_analyzer_dump(invalidation_detector);
+ // expected-warning@-1 {{2000}} on this path we trusted the type, and
resolved the call to `MyHandler::onAction(int, int&)`
+ // expected-warning@-2 {{conj}} on this path we conservatively evaluated the
previous call
+
+ clang_analyzer_numTimesReached(); // expected-warning {{3}} we only have 3
paths here, not 4
+ // 1) inlined 2 different callees
+ // 2) inlined only 1st
+ // 3) non were inlined
+ // 4) inlined only the second: This can't happen because if we conservative
called a specific function on a path, we will always evaluate it like that.
+ // See ExprEngine::BifurcateCall and DynamicDispatchBifurcationMap.
+}
+
+// -------
+
+class Base {
+public:
+ virtual void OnRecvCancel(int port) = 0;
+};
+class Handler final : public Base {
+public:
+ void OnRecvCancel(int port) override {
+ clang_analyzer_dump(100 + port); // expected-warning {{+ 150}}
+ clang_analyzer_isTainted(port); // expected-warning {{YES}}
+ }
+};
+class PParent {
+public:
+ bool OnMessageReceived();
+};
+class Actor {
+public:
+ explicit Actor(Base* aRequest) : m(aRequest) {}
+protected:
+ Base* m;
+};
+
+class Parent : public Actor, public PParent {
+public:
+ explicit Parent(Base* aRequest) : Actor(aRequest) {}
+ bool RecvCancel(int port) {
+ clang_analyzer_dump(200 + port); // expected-warning {{+ 200}}
+ clang_analyzer_isTainted(port); // expected-warning {{YES}}
+ Handler* foo = (Handler*)m;
+ foo->OnRecvCancel(port + 50);
+ return true;
+ }
+};
+
+bool PParent::OnMessageReceived() {
+ int port;
+ scanf("%i", &port);
+ clang_analyzer_isTainted(port); // expected-warning {{YES}}
+ Parent* foo = static_cast<Parent*>(this);
+ return foo->RecvCancel(port);
+}
diff --git a/clang/test/Analysis/osobject-retain-release.cpp
b/clang/test/Analysis/osobject-retain-release.cpp
index 2ae5752f4402374..b4085813a72a1aa 100644
--- a/clang/test/Analysis/osobject-retain-release.cpp
+++ b/clang/test/Analysis/osobject-retain-release.cpp
@@ -492,11 +492,13 @@ void check_required_cast() {
void check_cast_behavior(OSObject *obj) {
OSArray *arr1 = OSDynamicCast(OSArray, obj);
- clang_analyzer_eval(arr1 == obj); // expected-warning{{TRUE}}
- // expected-note@-1{{TRUE}}
- // expected-note@-2{{Assuming 'arr1' is
not equal to 'obj'}}
- // expected-warning@-3{{FALSE}}
- // expected-note@-4 {{FALSE}}
+ clang_analyzer_eval(arr1 == obj); // #check_cast_behavior_1
+ // expected-warning@#check_cast_behavior_1 {{TRUE}}
+ // expected-note@#check_cast_behavior_1 {{TRUE}}
+ // expected-note@#check_cast_behavior_1{{Assuming 'arr1' is equal to 'obj'}}
+ // expected-warning@#check_cast_behavior_1 {{FALSE}}
+ // expected-note@#check_cast_behavior_1 {{FALSE}}
+ // expected-note@#check_cast_behavior_1{{Assuming 'arr1' is not equal to
'obj'}}
OSArray *arr2 = OSRequiredCast(OSArray, obj);
clang_analyzer_eval(arr2 == obj); // expected-warning{{TRUE}}
// expected-note@-1{{TRUE}}
``````````
</details>
https://github.com/llvm/llvm-project/pull/69057
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
