steakhal created this revision.
steakhal added reviewers: ASDenysPetrov, martong, xazax.hun, NoQ, Szelethus.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Casting a pointer to a suitably large integral type by reinterpret-cast
should result in the same value as by using the `__builtin_bit_cast()`.
The compiler exploits this: https://godbolt.org/z/zMP3sG683
However, the analyzer does not bind the same symbolic value to these
expressions, resulting in weird situations, such as failing equality
checks and even results in crashes: https://godbolt.org/z/oeMP7cj8q
Previously, the `&SymRegion` were not cast to `NonLoc`, which later in
the `evalBinOpLN()` caused a crash when the engine wanted to calculate
the offset at the BinaryOperator expression.
So, in this patch, I'm proposing to add an explicit cast when evaluating
an `LValueToRValueBitCast` cast expression. Other than casting the
resulting value to the given type before binding it to the expression it
does the same thing that we do for `LValueToRValue` casts.
Here is a snippet of code, annotated by the previous and new dump values.
`LocAsInteger` wraps the `SymRegion` in only two cases:
void LValueToRValueBitCast_dumps(void *p, char (*array)[8]) {
clang_analyzer_dump(p); // remained: &SymRegion{reg_$0<void * p>}
clang_analyzer_dump(array); // remained: {{&SymRegion{reg_$1<char (*)[8]
array>}
clang_analyzer_dump((unsigned long)p);
// remained: {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
clang_analyzer_dump(__builtin_bit_cast(unsigned long, p)); <---------
change #1
// previously: {{&SymRegion{reg_$0<void * p>}}}
// now: {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
clang_analyzer_dump((unsigned long)array); // remained:
{{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit integer]}}
clang_analyzer_dump(__builtin_bit_cast(unsigned long, array)); <---------
change #2
// previously: {{&SymRegion{reg_$1<char (*)[8] array>}}}
// now: {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit integer]}}
}
PS: I'm not sure how/when we should get rid of the `LocAsInteger` and
represent this by a `SymbolCast`.
Maybe @ASDenysPetrov or @martong could help me review this.
This is a follow-up of D105017 <https://reviews.llvm.org/D105017>, where I
thought it should be enough to
model `LValueToRValueBitCast` the same way as we do with `LValueToRValueCast`.
I still think the same, but we really should have some `NonLoc` value here, IMO.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136603
Files:
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
clang/test/Analysis/ptr-arith.cpp
Index: clang/test/Analysis/ptr-arith.cpp
===================================================================
--- clang/test/Analysis/ptr-arith.cpp
+++ clang/test/Analysis/ptr-arith.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14
-analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -verify %s -triple
x86_64-pc-linux-gnu \
+// RUN: -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm
template <typename T> void clang_analyzer_dump(T);
@@ -141,3 +142,22 @@
return bits->b; // no-warning
}
} // namespace Bug_55934
+
+void LValueToRValueBitCast_dumps(void *p, char (*array)[8]) {
+ clang_analyzer_dump(p);
+ clang_analyzer_dump(array);
+ // expected-warning@-2 {{&SymRegion{reg_$0<void * p>}}}
+ // expected-warning@-2 {{&SymRegion{reg_$1<char (*)[8] array>}}}
+ clang_analyzer_dump((unsigned long)p);
+ clang_analyzer_dump(__builtin_bit_cast(unsigned long, p));
+ // expected-warning@-2 {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
+ // expected-warning@-2 {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
+ clang_analyzer_dump((unsigned long)array);
+ clang_analyzer_dump(__builtin_bit_cast(unsigned long, array));
+ // expected-warning@-2 {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit
integer]}}
+ // expected-warning@-2 {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit
integer]}}
+}
+
+unsigned long ptr_arithmetic(void *p) {
+ return __builtin_bit_cast(unsigned long, p) + 1; // no-crash
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -288,8 +288,36 @@
ExplodedNodeSet dstPreStmt;
getCheckerManager().runCheckersForPreStmt(dstPreStmt, Pred, CastE, *this);
- if (CastE->getCastKind() == CK_LValueToRValue ||
- CastE->getCastKind() == CK_LValueToRValueBitCast) {
+ if (CastE->getCastKind() == CK_LValueToRValueBitCast) {
+ // Do the same as for 'CK_LValueToRValue' but cast the resulting value to
+ // the appropriate type before binding.
+ for (ExplodedNode *subExprNode : dstPreStmt) {
+ ProgramStateRef state = subExprNode->getState();
+ const LocationContext *LCtx = subExprNode->getLocationContext();
+ Optional<Loc> Location = state->getSVal(Ex, LCtx).getAs<Loc>();
+ if (!Location)
+ return;
+
+ ExplodedNodeSet Tmp;
+ evalLocation(Tmp, CastE, CastE, subExprNode, state, *Location, true);
+ if (Tmp.empty())
+ return;
+
+ // Proceed with the load.
+ StmtNodeBuilder Bldr(Tmp, Dst, *currBldrCtx);
+ for (ExplodedNode *I : Tmp) {
+ state = I->getState();
+
+ SVal V = state->getSVal(*Location, CastE->getType());
+ SVal CastV = getSValBuilder().evalCast(V, CastE->getType(),
QualType{});
+ Bldr.generateNode(CastE, I, state->BindExpr(CastE, LCtx, CastV),
+ nullptr, ProgramPoint::PostLoadKind);
+ }
+ }
+ return;
+ }
+
+ if (CastE->getCastKind() == CK_LValueToRValue) {
for (ExplodedNodeSet::iterator I = dstPreStmt.begin(), E =
dstPreStmt.end();
I!=E; ++I) {
ExplodedNode *subExprNode = *I;
Index: clang/test/Analysis/ptr-arith.cpp
===================================================================
--- clang/test/Analysis/ptr-arith.cpp
+++ clang/test/Analysis/ptr-arith.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -verify %s -triple x86_64-pc-linux-gnu \
+// RUN: -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm
template <typename T> void clang_analyzer_dump(T);
@@ -141,3 +142,22 @@
return bits->b; // no-warning
}
} // namespace Bug_55934
+
+void LValueToRValueBitCast_dumps(void *p, char (*array)[8]) {
+ clang_analyzer_dump(p);
+ clang_analyzer_dump(array);
+ // expected-warning@-2 {{&SymRegion{reg_$0<void * p>}}}
+ // expected-warning@-2 {{&SymRegion{reg_$1<char (*)[8] array>}}}
+ clang_analyzer_dump((unsigned long)p);
+ clang_analyzer_dump(__builtin_bit_cast(unsigned long, p));
+ // expected-warning@-2 {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
+ // expected-warning@-2 {{&SymRegion{reg_$0<void * p>} [as 64 bit integer]}}
+ clang_analyzer_dump((unsigned long)array);
+ clang_analyzer_dump(__builtin_bit_cast(unsigned long, array));
+ // expected-warning@-2 {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit integer]}}
+ // expected-warning@-2 {{&SymRegion{reg_$1<char (*)[8] array>} [as 64 bit integer]}}
+}
+
+unsigned long ptr_arithmetic(void *p) {
+ return __builtin_bit_cast(unsigned long, p) + 1; // no-crash
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -288,8 +288,36 @@
ExplodedNodeSet dstPreStmt;
getCheckerManager().runCheckersForPreStmt(dstPreStmt, Pred, CastE, *this);
- if (CastE->getCastKind() == CK_LValueToRValue ||
- CastE->getCastKind() == CK_LValueToRValueBitCast) {
+ if (CastE->getCastKind() == CK_LValueToRValueBitCast) {
+ // Do the same as for 'CK_LValueToRValue' but cast the resulting value to
+ // the appropriate type before binding.
+ for (ExplodedNode *subExprNode : dstPreStmt) {
+ ProgramStateRef state = subExprNode->getState();
+ const LocationContext *LCtx = subExprNode->getLocationContext();
+ Optional<Loc> Location = state->getSVal(Ex, LCtx).getAs<Loc>();
+ if (!Location)
+ return;
+
+ ExplodedNodeSet Tmp;
+ evalLocation(Tmp, CastE, CastE, subExprNode, state, *Location, true);
+ if (Tmp.empty())
+ return;
+
+ // Proceed with the load.
+ StmtNodeBuilder Bldr(Tmp, Dst, *currBldrCtx);
+ for (ExplodedNode *I : Tmp) {
+ state = I->getState();
+
+ SVal V = state->getSVal(*Location, CastE->getType());
+ SVal CastV = getSValBuilder().evalCast(V, CastE->getType(), QualType{});
+ Bldr.generateNode(CastE, I, state->BindExpr(CastE, LCtx, CastV),
+ nullptr, ProgramPoint::PostLoadKind);
+ }
+ }
+ return;
+ }
+
+ if (CastE->getCastKind() == CK_LValueToRValue) {
for (ExplodedNodeSet::iterator I = dstPreStmt.begin(), E = dstPreStmt.end();
I!=E; ++I) {
ExplodedNode *subExprNode = *I;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
