r357620 - Revert "[analyzer] Toning down invalidation a bit".
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 3 11:21:16 PDT 2019
Author: dergachev
Date: Wed Apr 3 11:21:16 2019
New Revision: 357620
URL: http://llvm.org/viewvc/llvm-project?rev=357620&view=rev
Log:
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
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/test/Analysis/call-invalidation.cpp
cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
cfe/trunk/test/Analysis/malloc.c
cfe/trunk/test/Analysis/taint-generic.c
cfe/trunk/test/Analysis/taint-tester.c
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=357620&r1=357619&r2=357620&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Wed Apr 3 11:21:16 2019
@@ -303,23 +303,11 @@ ProgramStateRef CallEvent::invalidateReg
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));
Modified: cfe/trunk/test/Analysis/call-invalidation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/call-invalidation.cpp?rev=357620&r1=357619&r2=357620&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/call-invalidation.cpp (original)
+++ cfe/trunk/test/Analysis/call-invalidation.cpp Wed Apr 3 11:21:16 2019
@@ -132,21 +132,18 @@ void testInvalidationThroughBaseRegionPo
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}}
}
Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp?rev=357620&r1=357619&r2=357620&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp Wed Apr 3 11:21:16 2019
@@ -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) {
Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=357620&r1=357619&r2=357620&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Wed Apr 3 11:21:16 2019
@@ -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.
Modified: cfe/trunk/test/Analysis/taint-generic.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-generic.c?rev=357620&r1=357619&r2=357620&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/taint-generic.c (original)
+++ cfe/trunk/test/Analysis/taint-generic.c Wed Apr 3 11:21:16 2019
@@ -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);
}
Modified: cfe/trunk/test/Analysis/taint-tester.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-tester.c?rev=357620&r1=357619&r2=357620&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/taint-tester.c (original)
+++ cfe/trunk/test/Analysis/taint-tester.c Wed Apr 3 11:21:16 2019
@@ -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);
More information about the cfe-commits
mailing list