diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -386,7 +386,7 @@ /// to the user. This method allows to rest the location which should be used /// for uniquing reports. For example, memory leaks checker, could set this to /// the allocation site, rather then the location where the bug is reported. - PathSensitiveBugReport(BugType &bt, StringRef desc, + PathSensitiveBugReport(const BugType &bt, StringRef desc, const ExplodedNode *errorNode, PathDiagnosticLocation LocationToUnique, const Decl *DeclToUnique) diff --git a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp @@ -199,6 +199,28 @@ REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState) +static const ExplodedNode *getAcquireSite(const ExplodedNode *N, SymbolRef Sym, + CheckerContext &Ctx) { + ProgramStateRef State = N->getState(); + // When bug type is handle leak, exploded node N does not have state info for + // leaking handle. Get the predecessor of N instead. + if (!State->get(Sym)) + N = N->getFirstPred(); + + const ExplodedNode *Pred = N; + while (N) { + State = N->getState(); + if (!State->get(Sym)) { + const HandleState *HState = Pred->getState()->get(Sym); + if (HState && (HState->isAllocated() || HState->maybeAllocated())) + return N; + } + Pred = N; + N = N->getFirstPred(); + } + return nullptr; +} + /// Returns the symbols extracted from the argument or null if it cannot be /// found. SymbolRef getFuchsiaHandleSymbol(QualType QT, SVal Arg, ProgramStateRef State) { @@ -282,6 +304,7 @@ ProgramStateRef State = C.getState(); + std::vector> Notes; SymbolRef ResultSymbol = nullptr; if (const auto *TypeDefTy = FuncDecl->getReturnType()->getAs()) if (TypeDefTy->getDecl()->getName() == ErrorTypeName) @@ -310,14 +333,45 @@ if (HState && HState->isReleased()) { reportDoubleRelease(Handle, Call.getArgSourceRange(Arg), C); return; - } else + } else { + Notes.push_back([Handle](BugReport &BR) { + auto *PathBR = static_cast(&BR); + if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) { + return "Handle released here."; + } else + return ""; + }); State = State->set(Handle, HandleState::getReleased()); + } } else if (hasFuchsiaAttr(PVD)) { + Notes.push_back([Handle](BugReport &BR) { + auto *PathBR = static_cast(&BR); + if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) { + return "Handle allocated here."; + } else + return ""; + }); State = State->set( Handle, HandleState::getMaybeAllocated(ResultSymbol)); } } - C.addTransition(State); + const NoteTag *T = nullptr; + if (!Notes.empty()) { + T = C.getNoteTag( + [this, Notes{std::move(Notes)}](BugReport &BR) -> std::string { + if (&BR.getBugType() != &UseAfterReleaseBugType && + &BR.getBugType() != &LeakBugType && + &BR.getBugType() != &DoubleReleaseBugType) + return ""; + for (auto &Note : Notes) { + std::string Text = Note(BR); + if (!Text.empty()) + return Text; + } + return ""; + }); + } + C.addTransition(State, T); } void FuchsiaHandleChecker::checkDeadSymbols(SymbolReaper &SymReaper, @@ -353,6 +407,7 @@ ProgramStateRef FuchsiaHandleChecker::evalAssume(ProgramStateRef State, SVal Cond, bool Assumption) const { + // TODO: add notes about successes/fails for APIs. ConstraintManager &Cmr = State->getConstraintManager(); HStateMapTy TrackedHandles = State->get(); for (auto &CurItem : TrackedHandles) { @@ -453,7 +508,22 @@ if (!ErrorNode) return; - auto R = std::make_unique(Type, Msg, ErrorNode); + std::unique_ptr R; + if (Type.isSuppressOnSink()) { + const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C); + if (AcquireNode) { + PathDiagnosticLocation LocUsedForUniqueing = + PathDiagnosticLocation::createBegin( + AcquireNode->getStmtForDiagnostics(), C.getSourceManager(), + AcquireNode->getLocationContext()); + + R = std::make_unique( + Type, Msg, ErrorNode, LocUsedForUniqueing, + AcquireNode->getLocationContext()->getDecl()); + } + } + if (!R) + R = std::make_unique(Type, Msg, ErrorNode); if (Range) R->addRange(*Range); R->markInteresting(Sym); diff --git a/clang/test/Analysis/fuchsia_handle.cpp b/clang/test/Analysis/fuchsia_handle.cpp --- a/clang/test/Analysis/fuchsia_handle.cpp +++ b/clang/test/Analysis/fuchsia_handle.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -analyzer-output=text \ +// RUN: -verify %s typedef __typeof__(sizeof(int)) size_t; typedef int zx_status_t; @@ -116,33 +117,76 @@ void checkLeak01(int tag) { zx_handle_t sa, sb; - if (zx_channel_create(0, &sa, &sb)) - return; + if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated here}} + return; // expected-note@-1 {{Assuming the condition is false}} + // expected-note@-2 {{Taking false branch}} use1(&sa); - if (tag) + if (tag) // expected-note {{Assuming 'tag' is 0}} zx_handle_close(sa); + // expected-note@-2 {{Taking false branch}} use2(sb); // expected-warning {{Potential leak of handle}} + // expected-note@-1 {{Potential leak of handle}} + zx_handle_close(sb); +} + +void checkReportLeakOnOnePath(int tag) { + zx_handle_t sa, sb; + if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated here}} + return; // expected-note@-1 {{Assuming the condition is false}} + // expected-note@-2 {{Taking false branch}} zx_handle_close(sb); + switch(tag) { // expected-note {{Control jumps to the 'default' case at line}} + case 0: + use2(sa); + return; + case 1: + use2(sa); + return; + case 2: + use2(sa); + return; + case 3: + use2(sa); + return; + case 4: + use2(sa); + return; + default: + use2(sa); + return; // expected-warning {{Potential leak of handle}} + // expected-note@-1 {{Potential leak of handle}} + } } void checkDoubleRelease01(int tag) { zx_handle_t sa, sb; zx_channel_create(0, &sa, &sb); - if (tag) - zx_handle_close(sa); + // expected-note@-1 {{Handle allocated here}} + if (tag) // expected-note {{Assuming 'tag' is not equal to 0}} + zx_handle_close(sa); // expected-note {{Handle released here}} + // expected-note@-2 {{Taking true branch}} zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}} + // expected-note@-1 {{Releasing a previously released handle}} zx_handle_close(sb); } void checkUseAfterFree01(int tag) { zx_handle_t sa, sb; zx_channel_create(0, &sa, &sb); + // expected-note@-1 {{Handle allocated here}} + // expected-note@-2 {{Handle allocated here}} + // expected-note@+2 {{Taking true branch}} + // expected-note@+1 {{Taking false branch}} if (tag) { - zx_handle_close(sa); + // expected-note@-1 {{Assuming 'tag' is not equal to 0}} + zx_handle_close(sa); // expected-note {{Handle released here}} use1(&sa); // expected-warning {{Using a previously released handle}} + // expected-note@-1 {{Using a previously released handle}} } - zx_handle_close(sb); + // expected-note@-6 {{Assuming 'tag' is 0}} + zx_handle_close(sb); // expected-note {{Handle released here}} use2(sb); // expected-warning {{Using a previously released handle}} + // expected-note@-1 {{Using a previously released handle}} } void checkMemberOperatorIndices() {