NoQ updated this revision to Diff 95890.
NoQ marked 5 inline comments as done.
NoQ added a comment.
Fix comments!
https://reviews.llvm.org/D31982
Files:
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
lib/StaticAnalyzer/Core/Store.cpp
test/Analysis/explain-svals.cpp
test/Analysis/inlining/inline-defensive-checks.c
test/Analysis/inlining/inline-defensive-checks.cpp
test/Analysis/null-deref-offsets.c
test/Analysis/uninit-const.cpp
Index: test/Analysis/uninit-const.cpp
===================================================================
--- test/Analysis/uninit-const.cpp
+++ test/Analysis/uninit-const.cpp
@@ -122,7 +122,7 @@
}
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@-1 {{1st function call argument is a pointer to uninitialized value}}
}
Index: test/Analysis/null-deref-offsets.c
===================================================================
--- /dev/null
+++ test/Analysis/null-deref-offsets.c
@@ -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')}}
+}
Index: test/Analysis/inlining/inline-defensive-checks.cpp
===================================================================
--- test/Analysis/inlining/inline-defensive-checks.cpp
+++ test/Analysis/inlining/inline-defensive-checks.cpp
@@ -70,4 +70,17 @@
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;
+}
Index: test/Analysis/inlining/inline-defensive-checks.c
===================================================================
--- test/Analysis/inlining/inline-defensive-checks.c
+++ test/Analysis/inlining/inline-defensive-checks.c
@@ -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 @@
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
+}
Index: test/Analysis/explain-svals.cpp
===================================================================
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -81,9 +81,10 @@
namespace {
class C {
+public:
int x[10];
+ int y;
-public:
void test_5(int i) {
clang_analyzer_explain(this); // expected-warning-re{{{{^pointer to 'this' object$}}}}
clang_analyzer_explain(&x[i]); // expected-warning-re{{{{^pointer to element of type 'int' with index 'argument 'i'' of field 'x' of 'this' object$}}}}
Index: lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -404,9 +404,15 @@
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 @@
// 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;
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -961,6 +961,26 @@
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
