r301224 - [analyzer] Improve suppression for inlined defensive checks before operator &.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 24 12:30:34 PDT 2017
Author: dergachev
Date: Mon Apr 24 14:30:33 2017
New Revision: 301224
URL: http://llvm.org/viewvc/llvm-project?rev=301224&view=rev
Log:
[analyzer] Improve suppression for inlined defensive checks before operator &.
Null dereferences are suppressed if the lvalue was constrained to 0 for the
first time inside a sub-function that was inlined during analysis, because
such constraint is a valid defensive check that does not, by itself,
indicate that null pointer case is anyhow special for the caller.
If further operations on the lvalue are performed, the symbolic lvalue is
collapsed to concrete null pointer, and we need to track where does the null
pointer come from.
Improve such tracking for lvalue operations involving operator &.
rdar://problem/27876009
Differential Revision: https://reviews.llvm.org/D31982
Added:
cfe/trunk/test/Analysis/null-deref-offsets.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c
cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp
cfe/trunk/test/Analysis/uninit-const.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=301224&r1=301223&r2=301224&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Mon Apr 24 14:30:33 2017
@@ -961,6 +961,26 @@ bool bugreporter::trackNullOrUndefValue(
const Expr *Inner = nullptr;
if (const Expr *Ex = dyn_cast<Expr>(S)) {
Ex = Ex->IgnoreParenCasts();
+
+ // Performing operator `&' on an lvalue expression is essentially a no-op.
+ // Then, if we are taking addresses of fields or elements, these are also
+ // unlikely to matter.
+ // FIXME: There's a hack in our Store implementation that always computes
+ // field offsets around null pointers as if they are always equal to 0.
+ // The idea here is to report accesses to fields as null dereferences
+ // even though the pointer value that's being dereferenced is actually
+ // the offset of the field rather than exactly 0.
+ // See the FIXME in StoreManager's getLValueFieldOrIvar() method.
+ // This code interacts heavily with this hack; otherwise the value
+ // would not be null at all for most fields, so we'd be unable to track it.
+ if (const auto *Op = dyn_cast<UnaryOperator>(Ex))
+ if (Op->getOpcode() == UO_AddrOf && Op->getSubExpr()->isLValue()) {
+ Ex = Op->getSubExpr()->IgnoreParenCasts();
+ while (const auto *ME = dyn_cast<MemberExpr>(Ex)) {
+ Ex = ME->getBase()->IgnoreParenCasts();
+ }
+ }
+
if (ExplodedGraph::isInterestingLValueExpr(Ex) || CallEvent::isCallStmt(Ex))
Inner = Ex;
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=301224&r1=301223&r2=301224&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Mon Apr 24 14:30:33 2017
@@ -404,9 +404,15 @@ SVal StoreManager::getLValueFieldOrIvar(
case loc::ConcreteIntKind:
// While these seem funny, this can happen through casts.
- // FIXME: What we should return is the field offset. For example,
- // add the field offset to the integer value. That way funny things
+ // FIXME: What we should return is the field offset, not base. For example,
+ // add the field offset to the integer value. That way things
// like this work properly: &(((struct foo *) 0xa)->f)
+ // However, that's not easy to fix without reducing our abilities
+ // to catch null pointer dereference. Eg., ((struct foo *)0x0)->f = 7
+ // is a null dereference even though we're dereferencing offset of f
+ // rather than null. Coming up with an approach that computes offsets
+ // over null pointers properly while still being able to catch null
+ // dereferences might be worth it.
return Base;
default:
@@ -431,7 +437,7 @@ SVal StoreManager::getLValueElement(Qual
// If the base is an unknown or undefined value, just return it back.
// FIXME: For absolute pointer addresses, we just return that value back as
// well, although in reality we should return the offset added to that
- // value.
+ // value. See also the similar FIXME in getLValueFieldOrIvar().
if (Base.isUnknownOrUndef() || Base.getAs<loc::ConcreteInt>())
return Base;
Modified: cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c?rev=301224&r1=301223&r2=301224&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c (original)
+++ cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c Mon Apr 24 14:30:33 2017
@@ -1,7 +1,7 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config suppress-inlined-defensive-checks=true -verify %s
// Perform inline defensive checks.
-void idc(int *p) {
+void idc(void *p) {
if (p)
;
}
@@ -139,3 +139,42 @@ void idcTrackZeroThroughDoubleAssignemnt
int z = y;
idcTriggerZeroValueThroughCall(z);
}
+
+struct S {
+ int f1;
+ int f2;
+};
+
+void idcTrackZeroValueThroughUnaryPointerOperators(struct S *s) {
+ idc(s);
+ *(&(s->f1)) = 7; // no-warning
+}
+
+void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset1(struct S *s) {
+ idc(s);
+ int *x = &(s->f2);
+ *x = 7; // no-warning
+}
+
+void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
+ idc(s);
+ int *x = &(s->f2) - 1;
+ // FIXME: Should not warn.
+ *x = 7; // expected-warning{{Dereference of null pointer}}
+}
+
+void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {
+ idc(s);
+ int *x = &(s->f1);
+ *x = 7; // no-warning
+}
+
+
+struct S2 {
+ int a[1];
+};
+
+void idcTrackZeroValueThroughUnaryPointerOperatorsWithArrayField(struct S2 *s) {
+ idc(s);
+ *(&(s->a[0])) = 7; // no-warning
+}
Modified: cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp?rev=301224&r1=301223&r2=301224&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp (original)
+++ cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp Mon Apr 24 14:30:33 2017
@@ -70,4 +70,17 @@ int *retNull() {
void test(int *p1, int *p2) {
idc(p1);
Foo f(p1);
-}
\ No newline at end of file
+}
+
+struct Bar {
+ int x;
+};
+void idcBar(Bar *b) {
+ if (b)
+ ;
+}
+void testRefToField(Bar *b) {
+ idcBar(b);
+ int &x = b->x; // no-warning
+ x = 5;
+}
Added: cfe/trunk/test/Analysis/null-deref-offsets.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-offsets.c?rev=301224&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/null-deref-offsets.c (added)
+++ cfe/trunk/test/Analysis/null-deref-offsets.c Mon Apr 24 14:30:33 2017
@@ -0,0 +1,34 @@
+// RUN: %clang_analyze_cc1 -w -triple i386-apple-darwin10 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+ int x, y;
+ int z[2];
+};
+
+void testOffsets(struct S *s) {
+ if (s != 0)
+ return;
+
+ // FIXME: Here we are testing the hack that computes offsets to null pointers
+ // as 0 in order to find null dereferences of not-exactly-null pointers,
+ // such as &(s->y) below, which is equal to 4 rather than 0 in run-time.
+
+ // These are indeed null.
+ clang_analyzer_eval(s == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(&(s->x) == 0); // expected-warning{{TRUE}}
+
+ // FIXME: These should ideally be true.
+ clang_analyzer_eval(&(s->y) == 4); // expected-warning{{FALSE}}
+ clang_analyzer_eval(&(s->z[0]) == 8); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(&(s->z[1]) == 12); // expected-warning{{UNKNOWN}}
+
+ // FIXME: These should ideally be false.
+ clang_analyzer_eval(&(s->y) == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(&(s->z[0]) == 0); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(&(s->z[1]) == 0); // expected-warning{{UNKNOWN}}
+
+ // But this should still be a null dereference.
+ s->y = 5; // expected-warning{{Access to field 'y' results in a dereference of a null pointer (loaded from variable 's')}}
+}
Modified: cfe/trunk/test/Analysis/uninit-const.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-const.cpp?rev=301224&r1=301223&r2=301224&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/uninit-const.cpp (original)
+++ cfe/trunk/test/Analysis/uninit-const.cpp Mon Apr 24 14:30:33 2017
@@ -122,7 +122,7 @@ void f1(void) {
}
void f_uninit(void) {
- int x;
+ int x; // expected-note {{'x' declared without an initial value}}
doStuff_uninit(&x); // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note at -1 {{1st function call argument is a pointer to uninitialized value}}
}
More information about the cfe-commits
mailing list