[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 239469.
xazax.hun added a comment.

- Minor refactoring.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73151/new/

https://reviews.llvm.org/D73151

Files:
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.cpp


Index: clang/test/Analysis/fuchsia_handle.cpp
===================================================================
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -63,6 +63,14 @@
     zx_handle_close(sa);
 }
 
+void handleDieBeforeErrorSymbol() {
+  zx_handle_t sa, sb;
+  zx_status_t status = zx_channel_create(0, &sa, &sb);
+  if (status < 0)
+    return;
+  __builtin_trap();
+}
+
 void checkNoCrash01() {
   zx_handle_t sa, sb;
   zx_channel_create(0, &sa, &sb);
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -130,6 +130,9 @@
   static HandleState getEscaped() {
     return HandleState(Kind::Escaped, nullptr);
   }
+  static HandleState getWithoutError(HandleState S) {
+    return HandleState(S.K, nullptr);
+  }
 
   SymbolRef getErrorSym() const { return ErrorSym; }
 
@@ -149,6 +152,10 @@
       CASE(Kind::Released)
       CASE(Kind::Escaped)
     }
+    if (ErrorSym) {
+      OS << " ErrorSym: ";
+      ErrorSym->dumpToStream(OS);
+    }
   }
 
   LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); }
@@ -160,7 +167,7 @@
 
 class FuchsiaHandleChecker
     : public Checker<check::PostCall, check::PreCall, check::DeadSymbols,
-                     check::PointerEscape, eval::Assume> {
+                     check::LiveSymbols, check::PointerEscape, eval::Assume> {
   BugType LeakBugType{this, "Fuchsia handle leak", "Fuchsia Handle Error",
                       /*SuppressOnSink=*/true};
   BugType DoubleReleaseBugType{this, "Fuchsia handle double release",
@@ -172,6 +179,7 @@
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+  void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
                              bool Assumption) const;
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
@@ -381,6 +389,10 @@
   SmallVector<SymbolRef, 2> LeakedSyms;
   HStateMapTy TrackedHandles = State->get<HStateMap>();
   for (auto &CurItem : TrackedHandles) {
+    SymbolRef ErrorSym = CurItem.second.getErrorSym();
+    if (ErrorSym && SymReaper.isDead(ErrorSym))
+      State = State->set<HStateMap>(
+          CurItem.first, HandleState::getWithoutError(CurItem.second));
     if (!SymReaper.isDead(CurItem.first))
       continue;
     if (CurItem.second.isAllocated() || CurItem.second.maybeAllocated())
@@ -395,6 +407,15 @@
   C.addTransition(State, N);
 }
 
+void FuchsiaHandleChecker::checkLiveSymbols(ProgramStateRef State,
+                                            SymbolReaper &SymReaper) const {
+  HStateMapTy TrackedHandles = State->get<HStateMap>();
+  for (auto &CurItem : TrackedHandles) {
+    if (CurItem.second.getErrorSym())
+      SymReaper.markLive(CurItem.first);
+  }
+}
+
 // Acquiring a handle is not always successful. In Fuchsia most functions
 // return a status code that determines the status of the handle.
 // When we split the path based on this status code we know that on one


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73151.239469.patch
Type: text/x-patch
Size: 3346 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200122/affcdd93/attachment-0001.bin>