Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ 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) Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp @@ -163,6 +163,28 @@ LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); } }; +class HandleBugVisitor : public BugReporterVisitor { +private: + SymbolRef HandleSym; + + static void *getTag() { + static int Tag = 0; + return &Tag; + } + +public: + HandleBugVisitor(SymbolRef HandleSym) : HandleSym(HandleSym) {} + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(HandleSym); + ID.AddPointer(getTag()); + } + + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; +}; + class FuchsiaHandleChecker : public Checker { @@ -204,6 +226,71 @@ REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState) +PathDiagnosticPieceRef HandleBugVisitor::VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { + ProgramStateRef State = N->getState(); + ProgramStateRef PrevState = N->getFirstPred()->getState(); + + const Stmt *S = N->getStmtForDiagnostics(); + if (!S) + return nullptr; + + const HandleState *HState = State->get(HandleSym); + const HandleState *PrevHState = PrevState->get(HandleSym); + if (HState == PrevHState) + return nullptr; + + SmallString<200> Buf; + llvm::raw_svector_ostream OS(Buf); + if (!PrevHState && (HState->isAllocated() || HState->maybeAllocated())) { + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + N->getLocationContext()); + OS << "Handle allocated here."; + return std::make_shared(Pos, OS.str()); + } + if ((!PrevHState || PrevHState->isAllocated() || + PrevHState->maybeAllocated()) && + HState && (HState->isReleased() || HState->maybeReleased())) { + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + N->getLocationContext()); + OS << "Handle released here."; + return std::make_shared(Pos, OS.str()); + } + + // TODO: do we want to emit notes for escapes? + // do we want to emit notes for for error checks? I.e. on which branch + // do we consider the acquire/release success or fail. + + return nullptr; +} + +static const ExplodedNode *getAcquireSite(const ExplodedNode *N, SymbolRef Sym, + CheckerContext &Ctx) { + const ExplodedNode *AcquireNode = N; + + 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->pred_empty() ? nullptr : *(N->pred_begin()); + } + + 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())) + AcquireNode = N; + break; + } + Pred = N; + N = N->pred_empty() ? nullptr : *(N->pred_begin()); + } + return AcquireNode; +} + /// Returns the symbols extracted from the argument or null if it cannot be /// found. SymbolRef getFuchsiaHandleSymbol(QualType QT, SVal Arg, ProgramStateRef State) { @@ -429,10 +516,26 @@ 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); + R->addVisitor(std::make_unique(Sym)); C.emitReport(std::move(R)); } Index: clang/test/Analysis/fuchsia_handle.cpp =================================================================== --- clang/test/Analysis/fuchsia_handle.cpp +++ 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; @@ -102,31 +103,73 @@ void checkLeak01(int tag) { zx_handle_t sa, sb; zx_channel_create(0, &sa, &sb); + // expected-note@-1 {{Handle allocated here}} 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; + zx_channel_create(0, &sa, &sb); + // expected-note@-1 {{Handle allocated here}} + 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() {