Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -159,6 +159,167 @@ return std::move(P); } +namespace { + +/// Put a diagnostic on return statement of the inlined function \p F +/// iff the region of interest \p R was passed to \p F, +/// but not stored into inside \p F. +class NoStoreFuncVisitor final + : public BugReporterVisitorImpl { + + const SubRegion *RegionOfInterest; + +public: + NoStoreFuncVisitor(const SubRegion *R) : RegionOfInterest(R) {} + + void Profile(llvm::FoldingSetNodeID &ID) const override { + static int Tag = 0; + ID.AddPointer(&Tag); + } + + std::shared_ptr VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR) override { + const LocationContext *Ctx = N->getLocationContext(); + const StackFrameContext *SCtx = Ctx->getCurrentStackFrame(); + ProgramStateRef State = N->getState(); + auto CallExitLoc = N->getLocationAs(); + + if (!CallExitLoc) + return nullptr; + + CallEventRef<> Call = + BRC.getStateManager().getCallEventManager().getCaller(SCtx, State); + + const LocationContext *ContextAtCallsite = SCtx->getParent(); + for (unsigned I = 0, E = Call->getNumArgs(); I != E; ++I) { + const ParmVarDecl *PVD = Call->parameters()[I]; + const MemRegion *R = Call->getArgSVal(I).getAsRegion(); + if (RegionOfInterest->isSubRegionOf(R) && + !isPointerToConst(PVD->getType()) && + !wasRegionModifiedInsideFunction(RegionOfInterest, N, + ContextAtCallsite)) + return notModifiedDiagnostics(Ctx, BRC, *CallExitLoc, Call, PVD, R); + } + + return nullptr; + } + +private: + /// \p ValueAtReturn Value of \p R at the time of return. + /// \return Whether \p R was modified inside the function + /// starting at \p N. + bool + wasRegionModifiedInsideFunction(const MemRegion *R, + const ExplodedNode *ReturnNode, + const LocationContext *ContextAtCallsite) { + + ProgramStateRef ReturnState = ReturnNode->getState(); + SVal ValueAtReturn = ReturnState->getSVal(R); + const ExplodedNode *N = ReturnNode; + do { + if (N->getLocationAs() && + N->getLocationContext() == ContextAtCallsite) + return false; + + // Writing into region of interest. + if (auto PS = N->getLocationAs()) + if (auto *BO = PS->getStmtAs()) + if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf( + N->getSVal(BO->getLHS()).getAsRegion())) + return true; + + // SVal after the state is possibly different. + SVal ValueAtN = N->getState()->getSVal(R); + if (!ReturnState->areEqual(ValueAtN, ValueAtReturn).isConstrainedTrue() + && (!ValueAtN.isUndef() || !ValueAtReturn.isUndef())) + return true; + + N = N->getFirstPred(); + } while (N); + return false; + } + + /// \return whether \p Ty points to a const type, or is a const reference. + bool isPointerToConst(QualType Ty) { + return !Ty->getPointeeType().isNull() && + Ty->getPointeeType().getCanonicalType().isConstQualified(); + } + + std::shared_ptr + notModifiedDiagnostics(const LocationContext *Ctx, BugReporterContext &BRC, + CallExitBegin &CallExitLoc, + CallEventRef<> Call, + const ParmVarDecl *PVD, const MemRegion *ArgRegion) { + + PathDiagnosticLocation L; + if (const ReturnStmt *RS = CallExitLoc.getReturnStmt()) { + L = PathDiagnosticLocation::createBegin(RS, BRC.getSourceManager(), Ctx); + } else { + L = PathDiagnosticLocation( + Call->getRuntimeDefinition().getDecl()->getSourceRange().getEnd(), + BRC.getSourceManager()); + } + + SmallString<256> sbuf; + llvm::raw_svector_ostream os(sbuf); + os << "Returning without writing to '"; + + SmallString<128> RegionPrettyName; + const PrintingPolicy &PP = BRC.getASTContext().getPrintingPolicy(); + bool out = prettyPrintRegionName(PVD, ArgRegion, RegionPrettyName, PP); + if (!out) { + + // Fallback if we have failed to pretty-print. + PVD->printQualifiedName(os); + } else { + os << RegionPrettyName; + } + os << "'"; + return std::make_shared(L, os.str()); + } + + /// Pretty-print region to \p RegionPrettyName + /// \return whether printing has succeeded + bool prettyPrintRegionName(const ParmVarDecl *PVD, const MemRegion *ArgRegion, + SmallString<128> &RegionPrettyName, + const PrintingPolicy &PP) { + + llvm::raw_svector_ostream os(RegionPrettyName); + SmallVector Regions; + const MemRegion *R = RegionOfInterest; + Regions.push_back(R); + while (R != ArgRegion) { + R = dyn_cast(R)->getSuperRegion(); + Regions.push_back(R); + } + + const char *Sep = PVD->getType()->isReferenceType() ? "." : "->"; + for (auto I = Regions.rbegin(), E = Regions.rend(); I != E; ++I) { + R = *I; + if (R == ArgRegion) { + PVD->printQualifiedName(os); + } else if (auto *FR = dyn_cast(R)) { + os << Sep; + FR->getDecl()->getDeclName().print(os, PP); + Sep = "."; + + } else if (auto *CXXR = dyn_cast(R)) { + + // Just keep going up to the base region. + continue; + } else { + + // Pattern matching failed, use dumber printing. + return false; + } + } + return true; + } +}; + +} // namespace namespace { /// Emits an extra note at the return statement of an interesting stack frame. @@ -1128,6 +1289,9 @@ if (R) { // Mark both the variable region and its contents as interesting. SVal V = LVState->getRawSVal(loc::MemRegionVal(R)); + assert(isa(R)); + report.addVisitor( + llvm::make_unique(R->getAs())); report.markInteresting(R); report.markInteresting(V); Index: test/Analysis/diagnostics/no-store-func-path-notes.c =================================================================== --- /dev/null +++ test/Analysis/diagnostics/no-store-func-path-notes.c @@ -0,0 +1,178 @@ +// RUN: %clang_analyze_cc1 -x c -analyzer-checker=core,unix -analyzer-output=text -verify %s + +typedef __typeof(sizeof(int)) size_t; +void *memset(void *__s, int __c, size_t __n); + +int initializer1(int *p, int x) { + if (x) { // expected-note{{Taking false branch}} + *p = 1; + return 0; + } else { + return 1; // expected-note {{Returning without writing to 'p'}} + } +} + +int param_not_initialized_by_func() { + int p; // expected-note {{'p' declared without an initial value}} + int out = initializer1(&p, 0); // expected-note{{Calling 'initializer1'}} + // expected-note@-1{{Returning from 'initializer1'}} + return p; // expected-note{{Undefined or garbage value returned to caller}} + // expected-warning@-1{{Undefined or garbage value returned to caller}} +} + +int param_initialized_properly() { + int p; + int out = initializer1(&p, 1); + return p; //no-warning +} + +static int global; + +int initializer2(int **p, int x) { + if (x) { // expected-note{{Taking false branch}} + *p = &global; + return 0; + } else { + return 1; // expected-note {{Returning without writing to 'p'}} + } +} + +int param_not_written_into_by_func() { + int *p = 0; // expected-note{{'p' initialized to a null pointer value}} + int out = initializer2(&p, 0); // expected-note{{Calling 'initializer2'}} + // expected-note@-1{{Returning from 'initializer2'}} + return *p; // expected-warning{{Dereference of null pointer (loaded from variable 'p')}} + // expected-note@-1{{Dereference of null pointer (loaded from variable 'p')}} +} + +void initializer3(int *p, int param) { + if (param) // expected-note{{Taking false branch}} + *p = 0; +} // expected-note{{Returning without writing to 'p'}} + +int param_written_into_by_void_func() { + int p; // expected-note{{'p' declared without an initial value}} + initializer3(&p, 0); // expected-note{{Calling 'initializer3'}} + // expected-note@-1{{Returning from 'initializer3'}} + return p; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +void initializer4(int *p, int param) { + if (param) // expected-note{{Taking false branch}} + *p = 0; +} // expected-note{{Returning without writing to 'p'}} + +void initializer5(int *p, int param) { + if (!param) // expected-note{{Taking false branch}} + *p = 0; +} // expected-note{{Returning without writing to 'p'}} + +int multi_init_tries_func() { + int p; // expected-note{{'p' declared without an initial value}} + initializer4(&p, 0); // expected-note{{Calling 'initializer4'}} + // expected-note@-1{{Returning from 'initializer4'}} + initializer5(&p, 1); // expected-note{{Calling 'initializer5'}} + // expected-note@-1{{Returning from 'initializer5'}} + return p; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +int initializer6(const int *p) { + return 0; +} + +int no_msg_on_const() { + int p; // expected-note{{'p' declared without an initial value}} + initializer6(&p); + return p; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +typedef struct { + int x; +} S; + +int initializer7(S *s, int param) { + if (param) { // expected-note{{Taking false branch}} + s->x = 0; + return 0; + } + return 1; // expected-note{{Returning without writing to 's->x'}} +} + +int initialize_struct_field() { + S local; + initializer7(&local, 0); // expected-note{{Calling 'initializer7'}} + // expected-note@-1{{Returning from 'initializer7'}} + return local.x; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +void nullwriter(int **p) { + *p = 0; // expected-note{{Null pointer value stored to 'p'}} +} // no extra note + +int usage() { + int x = 0; + int *p = &x; + nullwriter(&p); // expected-note{{Calling 'nullwriter'}} + // expected-note@-1{{Returning from 'nullwriter'}} + return *p; // expected-warning{{Dereference of null pointer (loaded from variable 'p')}} + // expected-note@-1{{Dereference of null pointer (loaded from variable 'p')}} +} + +typedef struct { + int x; + int y; +} A; + +void partial_initializer(A *a) { + a->x = 0; +} // expected-note{{Returning without writing to 'a->y'}} + +int use_partial_initializer() { + A a; + partial_initializer(&a); // expected-note{{Calling 'partial_initializer'}} + // expected-note@-1{{Returning from 'partial_initializer'}} + return a.y; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +typedef struct { + int x; + int y; +} B; + +typedef struct { + B b; +} C; + +void partial_nested_initializer(C *c) { + c->b.x = 0; +} // expected-note{{Returning without writing to 'c->b.y'}} + +int use_partial_nested_initializer() { + B localB; + C localC; + localC.b = localB; + partial_nested_initializer(&localC); // expected-note{{Calling 'partial_nested_initializer'}} + // expected-note@-1{{Returning from 'partial_nested_initializer'}} + return localC.b.y; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +void test_subregion_assignment(C* c) { + B b; + c->b = b; +} + +int use_subregion_assignment() { + C c; + test_subregion_assignment(&c); // expected-note{{Calling 'test_subregion_assignment'}} + // expected-note@-1{{Returning from 'test_subregion_assignment'}} + return c.b.x; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +// TODO: add test for arrays with explanations. Index: test/Analysis/diagnostics/no-store-func-path-notes.cpp =================================================================== --- /dev/null +++ test/Analysis/diagnostics/no-store-func-path-notes.cpp @@ -0,0 +1,106 @@ +// RUN: %clang_analyze_cc1 -x c++ -analyzer-checker=core -analyzer-output=text -verify %s + +int initializer1(int &p, int x) { + if (x) { // expected-note{{Taking false branch}} + p = 1; + return 0; + } else { + return 1; // expected-note {{Returning without writing to 'p'}} + } +} + +int param_not_initialized_by_func() { + int p; // expected-note {{'p' declared without an initial value}} + int out = initializer1(p, 0); // expected-note{{Calling 'initializer1'}} + // expected-note@-1{{Returning from 'initializer1'}} + return p; // expected-note{{Undefined or garbage value returned to caller}} + // expected-warning@-1{{Undefined or garbage value returned to caller}} +} + +struct S { + int initialize(int *p, int param) { + if (param) { //expected-note{{Taking false branch}} + *p = 1; + return 1; + } + return 0; // expected-note{{Returning without writing to 'p'}} + } +}; + +int use(S *s) { + int p; //expected-note{{'p' declared without an initial value}} + s->initialize(&p, 0); //expected-note{{Calling 'S::initialize'}} + //expected-note@-1{{Returning from 'S::initialize'}} + return p; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +int initializer2(const int &p) { + return 0; +} + +int no_msg_const_ref() { + int p; //expected-note{{'p' declared without an initial value}} + initializer2(p); + return p; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +void nested() {} +void init_in_nested_func(int **x) { + *x = 0; // expected-note{{Null pointer value stored to 'y'}} + nested(); +} // no-note + +int call_init_nested() { + int x = 0; + int *y = &x; + init_in_nested_func(&y); // expected-note{{Calling 'init_in_nested_func'}} + // expected-note@-1{{Returning from 'init_in_nested_func'}} + return *y; //expected-warning{{Dereference of null pointer (loaded from variable 'y')}} + //expected-note@-1{{Dereference of null pointer (loaded from variable 'y')}} +} + +struct A { + int x; + int y; +}; + +void partial_init_by_reference(A &a) { + a.x = 0; +} // expected-note {{Returning without writing to 'a.y'}} + +int use_partial_init_by_reference() { + A a; + partial_init_by_reference(a); // expected-note{{Calling 'partial_init_by_reference'}} + // expected-note@-1{{Returning from 'partial_init_by_reference'}} + return a.y; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +struct B : A { +}; + +void partially_init_inherited_struct(B *b) { + b->x = 0; +} // expected-note{{Returning without writing to 'b->y'}} + +int use_partially_init_inherited_struct() { + B b; + partially_init_inherited_struct(&b); // expected-note{{Calling 'partially_init_inherited_struct'}} + // expected-note@-1{{Returning from 'partially_init_inherited_struct'}} + return b.y; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + +struct C { + int x; + int y; + C(int pX, int pY) : x(pX) {} +}; + +int use_constructor() { + C c(0, 0); + return c.y; // expected-note{{Undefined or garbage value returned to caller}} + // expected-warning@-1{{Undefined or garbage value returned to caller}} +} Index: test/Analysis/diagnostics/no-store-func-path-notes.m =================================================================== --- /dev/null +++ test/Analysis/diagnostics/no-store-func-path-notes.m @@ -0,0 +1,46 @@ +// RUN: %clang_analyze_cc1 -x objective-c -analyzer-checker=core -analyzer-output=text -Wno-objc-root-class -fblocks -verify %s + +@interface I +- (int)initVar:(int *)var param:(int)param; +@end + +@implementation I +- (int)initVar:(int *)var param:(int)param { + if (param) { // expected-note{{Taking false branch}} + *var = 1; + return 0; + } + return 1; // expected-note{{Returning without writing to 'var'}} +} +@end + +int foo(I *i) { + int x; //expected-note{{'x' declared without an initial value}} + int out = [i initVar:&x param:0]; //expected-note{{Calling 'initVar:param:'}} + //expected-note@-1{{Returning from 'initVar:param:'}} + if (out) // expected-note{{Taking true branch}} + return x; //expected-warning{{Undefined or garbage value returned to caller}} + //expected-note@-1{{Undefined or garbage value returned to caller}} + return 0; +} + +int initializer1(int *p, int x) { + if (x) { // expected-note{{Taking false branch}} + *p = 1; + return 0; + } else { + return 1; // expected-note {{Returning without writing to 'p'}} + } +} + +int initFromBlock() { + __block int z; + ^{ // expected-note {{Calling anonymous block}} + int p; // expected-note{{'p' declared without an initial value}} + initializer1(&p, 0); // expected-note{{Calling 'initializer1'}} + // expected-note@-1{{Returning from 'initializer1'}} + z = p; // expected-warning{{Assigned value is garbage or undefined}} + // expected-note@-1{{Assigned value is garbage or undefined}} + }(); + return z; +} Index: test/Analysis/diagnostics/undef-value-param.c =================================================================== --- test/Analysis/diagnostics/undef-value-param.c +++ test/Analysis/diagnostics/undef-value-param.c @@ -12,7 +12,7 @@ if (c) //expected-note@-1{{Assuming 'c' is not equal to 0}} //expected-note@-2{{Taking true branch}} - return; + return; // expected-note{{Returning without writing to 'x'}} *x = 5; } @@ -51,7 +51,7 @@ if (x <= 0) //expected-note {{Taking true branch}} //expected-note@-1 {{Assuming 'x' is <= 0}} - return; + return; //expected-note{{Returning without writing to 'X->f1'}} X->f1 = getValidPtr(); } double testPassingParentRegionStruct(int x) { @@ -293,12 +293,12 @@ // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line15 -// CHECK-NEXT: col9 +// CHECK-NEXT: col12 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line15 -// CHECK-NEXT: col14 +// CHECK-NEXT: col17 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: @@ -309,6 +309,20 @@ // CHECK-NEXT: kindevent // CHECK-NEXT: location // CHECK-NEXT: +// CHECK-NEXT: line15 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Returning without writing to 'x' +// CHECK-NEXT: message +// CHECK-NEXT: Returning without writing to 'x' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: // CHECK-NEXT: line22 // CHECK-NEXT: col5 // CHECK-NEXT: file0 @@ -1046,6 +1060,20 @@ // CHECK-NEXT: kindevent // CHECK-NEXT: location // CHECK-NEXT: +// CHECK-NEXT: line54 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Returning without writing to 'X->f1' +// CHECK-NEXT: message +// CHECK-NEXT: Returning without writing to 'X->f1' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: // CHECK-NEXT: line60 // CHECK-NEXT: col3 // CHECK-NEXT: file0 Index: test/Analysis/diagnostics/undef-value-param.m =================================================================== --- test/Analysis/diagnostics/undef-value-param.m +++ test/Analysis/diagnostics/undef-value-param.m @@ -69,7 +69,7 @@ //expected-note@-1{{Assuming 'err' is not equal to 0}} //expected-note@-2{{Taking true branch}} CFRelease(ref); - return; + return; // expected-note{{Returning without writing to 'storeRef'}} } *storeRef = ref; } @@ -831,6 +831,54 @@ // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line71 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line71 +// CHECK-NEXT: col13 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line72 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line72 +// CHECK-NEXT: col10 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line72 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Returning without writing to 'storeRef' +// CHECK-NEXT: message +// CHECK-NEXT: Returning without writing to 'storeRef' +// CHECK-NEXT: +// CHECK-NEXT: // CHECK-NEXT: kindevent // CHECK-NEXT: location // CHECK-NEXT: