Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -961,6 +961,26 @@ const Expr *Inner = nullptr; if (const Expr *Ex = dyn_cast(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(Ex)) + if (Op->getOpcode() == UO_AddrOf && Op->getSubExpr()->isLValue()) { + Ex = Op->getSubExpr()->IgnoreParenCasts(); + while (const auto *ME = dyn_cast(Ex)) { + Ex = ME->getBase()->IgnoreParenCasts(); + } + } + if (ExplodedGraph::isInterestingLValueExpr(Ex) || CallEvent::isCallStmt(Ex)) Inner = Ex; } Index: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp +++ cfe/trunk/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()) return Base; Index: cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c =================================================================== --- cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c +++ cfe/trunk/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: cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp =================================================================== --- cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp +++ cfe/trunk/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: cfe/trunk/test/Analysis/null-deref-offsets.c =================================================================== --- cfe/trunk/test/Analysis/null-deref-offsets.c +++ cfe/trunk/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: cfe/trunk/test/Analysis/uninit-const.cpp =================================================================== --- cfe/trunk/test/Analysis/uninit-const.cpp +++ cfe/trunk/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}} }