haowei created this revision.
Herald added a subscriber: xazax.hun.
This commit improves the bug reporting of MagentaHandleChecker introduced in
https://reviews.llvm.org/D35968 , https://reviews.llvm.org/D36022 and
https://reviews.llvm.org/D36023.
After this commit, the allocation and release sites of the handle that causes a
bug will be marked in the generated bug report. If the handle is declared as a
local variable, the naming of the handle will also be marked in the generated
bug report as well.
https://reviews.llvm.org/D36024
Files:
lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
test/Analysis/mxhandle.c
Index: test/Analysis/mxhandle.c
===================================================================
--- test/Analysis/mxhandle.c
+++ test/Analysis/mxhandle.c
@@ -210,16 +210,16 @@
void checkLeak01() {
mx_handle_t sa, sb;
mx_channel_create(0, &sa, &sb);
-} // expected-warning {{Potential leak of handle}}
+} // expected-warning 2 {{Potential leak of handle pointed to by}}
void checkLeak02(int tag) {
mx_handle_t sa, sb;
mx_channel_create(0, &sa, &sb);
if (tag) {
mx_handle_close(sa);
}
- mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+ mx_handle_close(sb); // expected-warning {{Potential leak of handle pointed to by 'sa'}}
}
void checkLeak03(mx_handle_t handle) {
@@ -273,24 +273,24 @@
mx_channel_create(0, &sa, &sb);
useHandle01(sa);
- mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+ mx_handle_close(sb); // expected-warning {{Potential leak of handle pointed to by 'sa'}}
}
void checkLeak08() {
mx_handle_t sa, sb;
mx_channel_create(0, &sa, &sb);
useHandle02(&sa);
- mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+ mx_handle_close(sb); // expected-warning {{Potential leak of handle pointed to by 'sa'}}
}
void checkDoubleRelease01(int tag) {
mx_handle_t sa, sb;
mx_channel_create(0, &sa, &sb);
if (tag) {
mx_handle_close(sa);
}
- mx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+ mx_handle_close(sa); // expected-warning {{Releasing a previously released handle pointed to by 'sa'}}
mx_handle_close(sb);
}
@@ -300,7 +300,27 @@
if (tag) {
mx_handle_close(sa);
}
- useHandle01(sa); // expected-warning {{Using a previously released handle}}
+ useHandle01(sa); // expected-warning {{Using a previously released handle pointed to by 'sa'}}
mx_handle_close(sa);
mx_handle_close(sb);
}
+
+void checkSuppressWarning01() MX_SYSCALL_PARAM_ATTR(suppress_warning) {
+ mx_handle_t sa, sb;
+ if (mx_channel_create(0, &sa, &sb) < 0) {
+ return;
+ }
+ mx_handle_close(sa); // Should not report any bugs here
+}
+
+void checkSuppressWarning02() {
+ mx_handle_t sa, sb;
+ if (mx_channel_create(0, &sa, &sb) < 0) {
+ return;
+ }
+ mx_handle_close(sa); // Should not report any bugs here
+}
+
+void checkSuppressWarning03() MX_SYSCALL_PARAM_ATTR(suppress_warning) {
+ checkSuppressWarning02();
+}
Index: lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
@@ -235,6 +235,7 @@
static const char *HANDLE_TYPE_NAME = "mx_handle_t";
static const char *SYSCALL_RETURN_TYPE_NAME = "mx_status_t";
static const int MAX_HANDLE_IN_ARRAY = 64;
+typedef std::pair<const ExplodedNode *, const MemRegion *> AllocInfo;
class MagentaHandleChecker
: public Checker<check::DeadSymbols, check::PointerEscape, eval::Call,
@@ -281,6 +282,8 @@
const SourceRange *Range, const std::unique_ptr<BugType> &Type,
StringRef Msg) const;
// Utility functions
+ AllocInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
+ CheckerContext &Ctx) const;
// Generate function spec info based on inline annotations in declaration
bool generateSpecForFuncionDecl(const FunctionDecl *FuncDecl) const;
// Extract magenta related annotation string from declarations
@@ -331,12 +334,70 @@
}
return false;
}
+
+ bool shouldReport(CheckerContext &Ctx) const;
+};
+
+class MagentaBugVisitor : public BugReporterVisitorImpl<MagentaBugVisitor> {
+private:
+ SymbolRef Sym;
+
+public:
+ MagentaBugVisitor(SymbolRef sym) : Sym(sym) {}
+ void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(Sym); }
+ std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR);
};
} // end anonymous namespace
// Handle -> HandleState map
REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState)
+std::shared_ptr<PathDiagnosticPiece>
+MagentaBugVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN,
+ BugReporterContext &BRC, BugReport &BR) {
+ ProgramStateRef State = N->getState();
+ ProgramStateRef StatePref = PrevN->getState();
+ const HandleState *HSCurrent = State->get<HStateMap>(Sym);
+ const HandleState *HSPrev = StatePref->get<HStateMap>(Sym);
+ const Stmt *S = PathDiagnosticLocation::getStmt(N);
+ if (!S)
+ return nullptr;
+
+ SmallString<200> buf;
+ llvm::raw_svector_ostream os(buf);
+ if (!HSPrev && HSCurrent && HSCurrent->isAllocated()) {
+ PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+ N->getLocationContext());
+ const MemRegion *Region = HSCurrent->getRegion();
+ if (Region && Region->canPrintPretty()) {
+ os << "Handle ";
+ Region->printPretty(os);
+ os << " allocated here.";
+ } else {
+ os << "Handle allocated here.";
+ }
+ return std::make_shared<PathDiagnosticEventPiece>(Pos, os.str());
+ }
+
+ if (HSPrev && HSCurrent && HSPrev->isAllocated() && HSCurrent->isReleased()) {
+ PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+ N->getLocationContext());
+ const MemRegion *Region = HSCurrent->getRegion();
+ if (Region && Region->canPrintPretty()) {
+ os << "Handle ";
+ Region->printPretty(os);
+ os << " released here.";
+ } else {
+ os << "Handle released here.";
+ }
+ return std::make_shared<PathDiagnosticEventPiece>(Pos, os.str());
+ }
+ return nullptr;
+}
+
MagentaHandleChecker::MagentaHandleChecker() {
// Initialize the bug types.
LeakBugType.reset(
@@ -1034,16 +1095,38 @@
StringRef Msg) const {
if (!ErrorNode || !Sym)
return;
- auto R = llvm::make_unique<BugReport>(*Type, Msg, ErrorNode);
+ PathDiagnosticLocation LocUsedForUniqueing;
+ const ExplodedNode *AllocNode = nullptr;
+ const MemRegion *Region = nullptr;
+ std::tie(AllocNode, Region) = getAllocationSite(ErrorNode, Sym, C);
+ const Stmt *AllocStmt = PathDiagnosticLocation::getStmt(AllocNode);
+ if (AllocStmt) {
+ LocUsedForUniqueing = PathDiagnosticLocation::createBegin(
+ AllocStmt, C.getSourceManager(), AllocNode->getLocationContext());
+ }
+ SmallString<200> buf;
+ llvm::raw_svector_ostream os(buf);
+ if (Region && Region->canPrintPretty()) {
+ os << Msg << " pointed to by ";
+ Region->printPretty(os);
+ } else {
+ os << Msg;
+ }
+ auto R = llvm::make_unique<BugReport>(
+ *Type, os.str(), ErrorNode, LocUsedForUniqueing,
+ AllocNode->getLocationContext()->getDecl());
if (Range)
R->addRange(*Range);
R->markInteresting(Sym);
+ R->addVisitor(llvm::make_unique<MagentaBugVisitor>(Sym));
C.emitReport(std::move(R));
}
void MagentaHandleChecker::reportLeaks(ArrayRef<SymbolRef> LeakedHandles,
CheckerContext &C,
ExplodedNode *ErrorNode) const {
+ if (!shouldReport(C))
+ return;
for (SymbolRef LeakedHandle : LeakedHandles) {
reportBug(LeakedHandle, ErrorNode, C, nullptr, LeakBugType,
"Potential leak of handle");
@@ -1053,6 +1136,8 @@
void MagentaHandleChecker::reportDoubleRelease(SymbolRef HandleSym,
const SourceRange &Range,
CheckerContext &C) const {
+ if (!shouldReport(C))
+ return;
ExplodedNode *ErrNode = C.generateErrorNode();
if (!ErrNode)
return;
@@ -1063,13 +1148,87 @@
void MagentaHandleChecker::reportUseAfterFree(SymbolRef HandleSym,
const SourceRange &Range,
CheckerContext &C) const {
+ if (!shouldReport(C))
+ return;
ExplodedNode *ErrNode = C.generateErrorNode();
if (!ErrNode)
return;
reportBug(HandleSym, ErrNode, C, &Range, UseAfterFreeBugType,
"Using a previously released handle");
}
+// Looks for functions that are marked as "DONOTREPORT" in the call stack. If
+// there exists one, return false to stop reporting a bug.
+bool MagentaHandleChecker::shouldReport(CheckerContext &Ctx) const {
+ const StackFrameContext *SFCtx = Ctx.getStackFrame();
+ // Reverse lookup call stack. Similar to StackFrameContext.dump()
+ for (const LocationContext *CurLCtx = SFCtx; CurLCtx;
+ CurLCtx = CurLCtx->getParent()) {
+ const StackFrameContext *CurSFCtx = dyn_cast<StackFrameContext>(CurLCtx);
+ if (!CurSFCtx)
+ continue;
+
+ const FunctionDecl *FD =
+ dyn_cast_or_null<FunctionDecl>(CurSFCtx->getDecl());
+ if (!FD)
+ continue;
+ // Top-level functions are not evaluated by evalCall. So their annotations
+ // are not processed. Process the annotation here.
+ if (!FuncKindMap.count(FD)) {
+ if (!generateSpecForFuncionDecl(FD)) {
+ FuncKindMap[FD] = UNANNOTATED_FUNC;
+ } else {
+ FuncSpec FS = FuncDeclMap[FD];
+ if (FS.RetAction == DONOTREPORT) {
+ FuncKindMap[FD] = DONOTREPORT_FUNC;
+ }
+ }
+ }
+ if (FuncKindMap.count(FD) && FuncKindMap[FD] == DONOTREPORT_FUNC)
+ return false;
+ }
+ return true;
+}
+
+// Reverse traverse the exploded graph to look for allocation side information
+// for handle "Sym".
+AllocInfo MagentaHandleChecker::getAllocationSite(const ExplodedNode *N,
+ SymbolRef Sym,
+ CheckerContext &C) const {
+ const LocationContext *LeakContext = N->getLocationContext();
+ // Walk the ExplodedGraph backwards and find the first node that referred to
+ // the tracked symbol.
+ const ExplodedNode *AllocNode = N;
+ const MemRegion *ReferenceRegion = nullptr;
+
+ 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<HStateMap>(Sym)) {
+ N = N->pred_empty() ? nullptr : *(N->pred_begin());
+ }
+
+ while (N) {
+ State = N->getState();
+ if (!State->get<HStateMap>(Sym))
+ break;
+ // Find the most recent expression bound to the symbol in the current
+ // context.
+ if (!ReferenceRegion) {
+ const HandleState *HS = State->get<HStateMap>(Sym);
+ if (HS)
+ ReferenceRegion = HS->getRegion();
+ }
+ // Allocation node, is the last node in the current or parent context in
+ // which the symbol was tracked.
+ const LocationContext *NContext = N->getLocationContext();
+ if (NContext == LeakContext || NContext->isParentOf(LeakContext))
+ AllocNode = N;
+ N = N->pred_empty() ? nullptr : *(N->pred_begin());
+ }
+ return AllocInfo(AllocNode, ReferenceRegion);
+}
+
void ento::registerMagentaHandleChecker(CheckerManager &mgr) {
mgr.registerChecker<MagentaHandleChecker>();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
