baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, dcoughlin.
baloghadamsoftware added a project: clang.
Herald added subscribers: gamesh411, donat.nagy, mikhail.ramalho, a.sidorin,
szepet.
This patch is a fix for bug 40625 <https://bugs.llvm.org/show_bug.cgi?id=40625>.
`FindLastStoreBRVisitor` tries to find the first node in the exploded graph
where the current value was assigned to a region. This node is called the
"store site". It is identified by a pair of `Pred` and `Succ` nodes where
`Succ` already has the binding for the value while `Pred` does not have it.
However the visitor mistakenly identifies a node pair as the store site where
the value is a `LazyCompoundVal` and `Pred` does not have a store yet but
`Succ` has it. In this case the `LazyCompoundVal` is different in the `Pred`
node because it also contains the store which is different in the two nodes.
This error may lead to crashes (a declaration is cast to a parameter
declaration without check) or misleading bug path notes.
In this patch we fix this problem by checking for unequal `LazyCompoundVals`:
if their region is equal, and their store is the same as the store of their
nodes we consider them as equal when looking for the store site.
Repository:
rC Clang
https://reviews.llvm.org/D58067
Files:
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
test/Analysis/PR40625.cpp
test/Analysis/const_array.cpp
test/Analysis/uninit-vals.m
Index: test/Analysis/uninit-vals.m
===================================================================
--- test/Analysis/uninit-vals.m
+++ test/Analysis/uninit-vals.m
@@ -394,11 +394,11 @@
struct {
int : 4;
int y : 4;
- } a, b, c;
+ } a, b, c; // expected-note{{'c' initialized here}}
a.y = 2;
- b = a; // expected-note{{Value assigned to 'c'}}
+ b = a;
clang_analyzer_eval(b.y == 2); // expected-warning{{TRUE}}
// expected-note@-1{{TRUE}}
@@ -411,11 +411,11 @@
struct {
int x : 4;
int : 4;
- } a, b, c;
+ } a, b, c; // expected-note{{'c' initialized here}}
a.x = 1;
- b = a; // expected-note{{Value assigned to 'c'}}
+ b = a;
clang_analyzer_eval(b.x == 1); // expected-warning{{TRUE}}
// expected-note@-1{{TRUE}}
Index: test/Analysis/const_array.cpp
===================================================================
--- test/Analysis/const_array.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,alpha.core.CallAndMessageUnInitRefArg %s -verify
-
-// expect-no-diagnostics
-
-const int arr[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
-
-void f(const int *begin, const int *end) {
- int sum = 0;
- for (const int *p = begin; p != end; ++p) {
- sum += *p;
- }
-}
-
-typedef const int intarray[10];
-
-void g(const intarray &arrr) {
- f(arrr, arrr+sizeof(arrr));
-}
-
-void h() {
- g(arr);
-}
Index: test/Analysis/PR40625.cpp
===================================================================
--- /dev/null
+++ test/Analysis/PR40625.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,alpha.core.CallAndMessageUnInitRefArg %s -verify
+
+void f(const int *end);
+
+void g(const int (&arrr)[10]) {
+ f(arrr+sizeof(arrr)); // expected-warning{{1st function call argument is a pointer to uninitialized value}}
+ // FIXME: This is a false positive that should be fixed. Until then this
+ // tests the crash fix in FindLastStoreBRVisitor (beside
+ // uninit-vals.m).
+}
+
+void h() {
+ int arr[10];
+
+ g(arr);
+}
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -153,6 +153,27 @@
return E;
}
+/// Two values match if either they are equal, or for LazyCompoundVals their
+/// regions are equal and their stores are equal to the stores of their
+/// exploded nodes.
+bool matchesValue(const ExplodedNode *LeftNode, SVal LeftVal,
+ const ExplodedNode *RightNode, SVal RightVal) {
+ if (LeftVal == RightVal)
+ return true;
+
+ const auto LLCV = LeftVal.getAs<nonloc::LazyCompoundVal>();
+ if (!LLCV)
+ return false;
+
+ const auto RLCV = RightVal.getAs<nonloc::LazyCompoundVal>();
+ if (!RLCV)
+ return false;
+
+ return LLCV->getRegion() == RLCV->getRegion() &&
+ LLCV->getStore() == LeftNode->getState()->getStore() &&
+ RLCV->getStore() == RightNode->getState()->getStore();
+}
+
//===----------------------------------------------------------------------===//
// Definitions for bug reporter visitors.
//===----------------------------------------------------------------------===//
@@ -1177,7 +1198,7 @@
if (Succ->getState()->getSVal(R) != V)
return nullptr;
- if (Pred->getState()->getSVal(R) == V) {
+ if (matchesValue(Pred, Pred->getState()->getSVal(R), Succ, V)) {
Optional<PostStore> PS = Succ->getLocationAs<PostStore>();
if (!PS || PS->getLocationValue() != R)
return nullptr;
@@ -1198,6 +1219,7 @@
// UndefinedVal.)
if (Optional<CallEnter> CE = Succ->getLocationAs<CallEnter>()) {
if (const auto *VR = dyn_cast<VarRegion>(R)) {
+
const auto *Param = cast<ParmVarDecl>(VR->getDecl());
ProgramStateManager &StateMgr = BRC.getStateManager();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
