summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorArtem Dergachev <artem.dergachev@gmail.com>2019-04-03 18:21:16 +0000
committerArtem Dergachev <artem.dergachev@gmail.com>2019-04-03 18:21:16 +0000
commit3d90e7e8db2c522c87f0fbfcdcb9aeed62680922 (patch)
tree32fcd49ab9ea9d85079d3a5eea2a42046fb843da
parent1362d7ef885d1e8136e19d85c0eeba3477b53020 (diff)
downloadbcm5719-llvm-3d90e7e8db2c522c87f0fbfcdcb9aeed62680922.tar.gz
bcm5719-llvm-3d90e7e8db2c522c87f0fbfcdcb9aeed62680922.zip
Revert "[analyzer] Toning down invalidation a bit".
This reverts commit r352473. The overall idea is great, but it seems to cause unintented consequences when not only Region Store invalidation but also pointer escape mechanism was accidentally affected. Based on discussions in https://reviews.llvm.org/D58121#1452483 and https://reviews.llvm.org/D57230#1434161 Differential Revision: https://reviews.llvm.org/D57230 llvm-svn: 357620
-rw-r--r--clang/lib/StaticAnalyzer/Core/CallEvent.cpp22
-rw-r--r--clang/test/Analysis/call-invalidation.cpp5
-rw-r--r--clang/test/Analysis/cxx-uninitialized-object.cpp5
-rw-r--r--clang/test/Analysis/malloc.c20
-rw-r--r--clang/test/Analysis/taint-generic.c1
-rw-r--r--clang/test/Analysis/taint-tester.c2
6 files changed, 28 insertions, 27 deletions
diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index 8dc6646e0eb..11dda7c3acb 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -303,23 +303,11 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount,
for (unsigned Idx = 0, Count = getNumArgs(); Idx != Count; ++Idx) {
// Mark this region for invalidation. We batch invalidate regions
// below for efficiency.
- if (const MemRegion *MR = getArgSVal(Idx).getAsRegion()) {
- bool UseBaseRegion = true;
- if (const auto *FR = MR->getAs<FieldRegion>()) {
- if (const auto *TVR = FR->getSuperRegion()->getAs<TypedValueRegion>()) {
- if (!TVR->getValueType()->isUnionType()) {
- ETraits.setTrait(MR, RegionAndSymbolInvalidationTraits::
- TK_DoNotInvalidateSuperRegion);
- UseBaseRegion = false;
- }
- }
- }
- // todo: factor this out + handle the lower level const pointers.
- if (PreserveArgs.count(Idx))
- ETraits.setTrait(
- UseBaseRegion ? MR->getBaseRegion() : MR,
- RegionAndSymbolInvalidationTraits::TK_PreserveContents);
- }
+ if (PreserveArgs.count(Idx))
+ if (const MemRegion *MR = getArgSVal(Idx).getAsRegion())
+ ETraits.setTrait(MR->getBaseRegion(),
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+ // TODO: Factor this out + handle the lower level const pointers.
ValuesToInvalidate.push_back(getArgSVal(Idx));
diff --git a/clang/test/Analysis/call-invalidation.cpp b/clang/test/Analysis/call-invalidation.cpp
index dade8db9ac2..c107e107054 100644
--- a/clang/test/Analysis/call-invalidation.cpp
+++ b/clang/test/Analysis/call-invalidation.cpp
@@ -132,21 +132,18 @@ void testInvalidationThroughBaseRegionPointer() {
PlainStruct s1;
s1.x = 1;
s1.z = 1;
- s1.y = 1;
clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(s1.z == 1); // expected-warning{{TRUE}}
// Not only passing a structure pointer through const pointer parameter,
// but also passing a field pointer through const pointer parameter
// should preserve the contents of the structure.
useAnythingConst(&(s1.y));
- clang_analyzer_eval(s1.y == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
// FIXME: Should say "UNKNOWN", because it is not uncommon to
// modify a mutable member variable through const pointer.
clang_analyzer_eval(s1.z == 1); // expected-warning{{TRUE}}
useAnything(&(s1.y));
- clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
- clang_analyzer_eval(s1.y == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(s1.x == 1); // expected-warning{{UNKNOWN}}
}
diff --git a/clang/test/Analysis/cxx-uninitialized-object.cpp b/clang/test/Analysis/cxx-uninitialized-object.cpp
index 93a02a48382..07006bea478 100644
--- a/clang/test/Analysis/cxx-uninitialized-object.cpp
+++ b/clang/test/Analysis/cxx-uninitialized-object.cpp
@@ -358,7 +358,7 @@ template <class T>
void wontInitialize(const T &);
class PassingToUnknownFunctionTest1 {
- int a, b; // expected-note{{uninitialized field 'this->b'}}
+ int a, b;
public:
PassingToUnknownFunctionTest1() {
@@ -368,7 +368,8 @@ public:
}
PassingToUnknownFunctionTest1(int) {
- mayInitialize(a); // expected-warning{{1 uninitialized field at the end of the constructor call}}
+ mayInitialize(a);
+ // All good!
}
PassingToUnknownFunctionTest1(int, int) {
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index cbc21b492e5..5288e21a282 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1758,8 +1758,8 @@ void constEscape(const void *ptr);
void testConstEscapeThroughAnotherField() {
struct IntAndPtr s;
s.p = malloc(sizeof(int));
- constEscape(&(s.x));
-} // expected-warning {{Potential leak of memory pointed to by 's.p'}}
+ constEscape(&(s.x)); // could free s->p!
+} // no-warning
// PR15623
int testNoCheckerDataPropogationFromLogicalOpOperandToOpResult(void) {
@@ -1812,6 +1812,22 @@ void testLivenessBug(struct B *in_b) {
livenessBugRealloc(b->a);
}
+struct ListInfo {
+ struct ListInfo *next;
+};
+
+struct ConcreteListItem {
+ struct ListInfo li;
+ int i;
+};
+
+void list_add(struct ListInfo *list, struct ListInfo *item);
+
+void testCStyleListItems(struct ListInfo *list) {
+ struct ConcreteListItem *x = malloc(sizeof(struct ConcreteListItem));
+ list_add(list, &x->li); // will free 'x'.
+}
+
// ----------------------------------------------------------------------------
// False negatives.
diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c
index 42e390dddef..cdac02bf9e3 100644
--- a/clang/test/Analysis/taint-generic.c
+++ b/clang/test/Analysis/taint-generic.c
@@ -238,7 +238,6 @@ void testUnion() {
int sock = socket(AF_INET, SOCK_STREAM, 0);
read(sock, &tainted.y, sizeof(tainted.y));
- tainted.x = 0;
// FIXME: overlapping regions aren't detected by isTainted yet
__builtin_memcpy(buffer, tainted.y, tainted.x);
}
diff --git a/clang/test/Analysis/taint-tester.c b/clang/test/Analysis/taint-tester.c
index b072eb84d43..3a8cc1825a0 100644
--- a/clang/test/Analysis/taint-tester.c
+++ b/clang/test/Analysis/taint-tester.c
@@ -51,7 +51,7 @@ void taintTracking(int x) {
scanf("%d", &xy.y);
scanf("%d", &xy.x);
int tx = xy.x; // expected-warning + {{tainted}}
- int ty = xy.y; // expected-warning + {{tainted}}
+ int ty = xy.y; // FIXME: This should be tainted as well.
char ntz = xy.z;// no warning
// Now, scanf scans both.
scanf("%d %d", &xy.y, &xy.x);
OpenPOWER on IntegriCloud