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 @@ -314,6 +314,17 @@ // Function returns an open handle. if (hasFuchsiaAttr(FuncDecl)) { SymbolRef RetSym = Call.getReturnValue().getAsSymbol(); + Notes.push_back([RetSym, FuncDecl](BugReport &BR) -> std::string { + auto *PathBR = static_cast(&BR); + if (auto IsInteresting = PathBR->getInterestingnessKind(RetSym)) { + std::string SBuf; + llvm::raw_string_ostream OS(SBuf); + OS << "Function '" << FuncDecl->getNameAsString() + << "' returns an open handle"; + return OS.str(); + } else + return ""; + }); State = State->set(RetSym, HandleState::getMaybeAllocated(nullptr)); } @@ -322,6 +333,7 @@ if (Arg >= FuncDecl->getNumParams()) break; const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg); + unsigned ParamDiagIdx = PVD->getFunctionScopeIndex() + 1; SymbolRef Handle = getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State); if (!Handle) @@ -335,20 +347,28 @@ reportDoubleRelease(Handle, Call.getArgSourceRange(Arg), C); return; } else { - Notes.push_back([Handle](BugReport &BR) { + Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string { auto *PathBR = static_cast(&BR); if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) { - return "Handle released here."; + std::string SBuf; + llvm::raw_string_ostream OS(SBuf); + OS << "Handle released through " << ParamDiagIdx + << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter"; + return OS.str(); } else return ""; }); State = State->set(Handle, HandleState::getReleased()); } } else if (hasFuchsiaAttr(PVD)) { - Notes.push_back([Handle](BugReport &BR) { + Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string { auto *PathBR = static_cast(&BR); if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) { - return "Handle allocated here."; + std::string SBuf; + llvm::raw_string_ostream OS(SBuf); + OS << "Handle allocated through " << ParamDiagIdx + << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter"; + return OS.str(); } else return ""; }); 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 @@ -26,6 +26,9 @@ zx_status_t zx_handle_close( zx_handle_t handle ZX_HANDLE_RELEASE); +ZX_HANDLE_ACQUIRE +zx_handle_t return_handle(); + void escape1(zx_handle_t *in); void escape2(zx_handle_t in); void (*escape3)(zx_handle_t) = escape2; @@ -117,7 +120,7 @@ void checkLeak01(int tag) { zx_handle_t sa, sb; - if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated here}} + if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated through 2nd parameter}} return; // expected-note@-1 {{Assuming the condition is false}} // expected-note@-2 {{Taking false branch}} use1(&sa); @@ -129,9 +132,15 @@ zx_handle_close(sb); } +void checkLeakFromReturn01(int tag) { + zx_handle_t sa = return_handle(); // expected-note {{Function 'return_handle' returns an open handle}} + (void)sa; +} // expected-note {{Potential leak of handle}} + // expected-warning@-1 {{Potential leak of handle}} + void checkReportLeakOnOnePath(int tag) { zx_handle_t sa, sb; - if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated here}} + if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated through 2nd parameter}} return; // expected-note@-1 {{Assuming the condition is false}} // expected-note@-2 {{Taking false branch}} zx_handle_close(sb); @@ -161,9 +170,9 @@ void checkDoubleRelease01(int tag) { zx_handle_t sa, sb; zx_channel_create(0, &sa, &sb); - // expected-note@-1 {{Handle allocated here}} + // expected-note@-1 {{Handle allocated through 2nd parameter}} if (tag) // expected-note {{Assuming 'tag' is not equal to 0}} - zx_handle_close(sa); // expected-note {{Handle released here}} + zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}} // 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}} @@ -173,18 +182,18 @@ 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@-1 {{Handle allocated through 2nd parameter}} + // expected-note@-2 {{Handle allocated through 3rd parameter}} // expected-note@+2 {{Taking true branch}} // expected-note@+1 {{Taking false branch}} if (tag) { // expected-note@-1 {{Assuming 'tag' is not equal to 0}} - zx_handle_close(sa); // expected-note {{Handle released here}} + zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}} use1(&sa); // expected-warning {{Using a previously released handle}} // expected-note@-1 {{Using a previously released handle}} } // expected-note@-6 {{Assuming 'tag' is 0}} - zx_handle_close(sb); // expected-note {{Handle released here}} + zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}} use2(sb); // expected-warning {{Using a previously released handle}} // expected-note@-1 {{Using a previously released handle}} }