[PATCH] D91902: [analyzer] Ignore annotations if func is inlined.

[Haowei Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ce78f54edcf: [analyzer] Ignore annotations if func is inlined. (authored by aabbaabb, committed by haowei).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D91902?vs=309122&id=309968#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91902

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
@@ -315,6 +315,45 @@
   // expected-note at -1 {{Potential leak of handle}}
 }
 
+// Assume this function's declaration that has the release annotation is in one
+// header file while its implementation is in another file. We have to annotate
+// the declaration because it might be used outside the TU.
+// We also want to make sure it is okay to call the function within the same TU.
+zx_status_t test_release_handle(zx_handle_t handle ZX_HANDLE_RELEASE) {
+  return zx_handle_close(handle);
+}
+
+void checkReleaseImplementedFunc() {
+  zx_handle_t a, b;
+  zx_channel_create(0, &a, &b);
+  zx_handle_close(a);
+  test_release_handle(b);
+}
+
+void use_handle(zx_handle_t handle) {
+  // Do nothing.
+}
+
+void test_call_by_value() {
+  zx_handle_t a, b;
+  zx_channel_create(0, &a, &b);
+  zx_handle_close(a);
+  use_handle(b);
+  zx_handle_close(b);
+}
+
+void test_call_by_value_leak() {
+  zx_handle_t a, b;
+  zx_channel_create(0, &a, &b); // expected-note {{Handle allocated through 3rd parameter}}
+  zx_handle_close(a);
+  // Here we are passing handle b as integer value to a function that could be
+  // analyzed by the analyzer, thus the handle should not be considered escaped.
+  // After the function 'use_handle', handle b is still tracked and should be
+  // reported leaked.
+  use_handle(b);
+} // expected-warning {{Potential leak of handle}}
+// expected-note at -1 {{Potential leak of handle}}
+
 // RAII
 
 template <typename T>
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -331,11 +331,6 @@
           return;
         }
       }
-      if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
-          PVD->getType()->isIntegerType()) {
-        // Working around integer by-value escapes.
-        State = State->set<HStateMap>(Handle, HandleState::getEscaped());
-      }
     }
   }
   C.addTransition(State);
@@ -347,6 +342,10 @@
   if (!FuncDecl)
     return;
 
+  // If we analyzed the function body, then ignore the annotations.
+  if (C.wasInlined)
+    return;
+
   ProgramStateRef State = C.getState();
 
   std::vector<std::function<std::string(BugReport & BR)>> Notes;
@@ -417,6 +416,14 @@
         });
         State = State->set<HStateMap>(
             Handle, HandleState::getMaybeAllocated(ResultSymbol));
+      } else if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
+                 PVD->getType()->isIntegerType()) {
+        // Working around integer by-value escapes.
+        // The by-value escape would not be captured in checkPointerEscape.
+        // If the function was not analyzed (otherwise wasInlined should be
+        // true) and there is no annotation on the handle, we assume the handle
+        // is escaped.
+        State = State->set<HStateMap>(Handle, HandleState::getEscaped());
       }
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91902.309968.patch
Type: text/x-patch
Size: 3168 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201207/0e8dd4b4/attachment-0001.bin>