[llvm-branch-commits] [clang] 3ce78f5 - [analyzer] Ignore annotations if func is inlined.

[Haowei Wu via llvm-branch-commits]

Author: Yu Shan
Date: 2020-12-07T11:28:11-08:00
New Revision: 3ce78f54edcfc881377a9e567715caf2f0be2abc

URL: https://github.com/llvm/llvm-project/commit/3ce78f54edcfc881377a9e567715caf2f0be2abc
DIFF: https://github.com/llvm/llvm-project/commit/3ce78f54edcfc881377a9e567715caf2f0be2abc.diff

LOG: [analyzer] Ignore annotations if func is inlined.

When we annotating a function header so that it could be used by other
TU, we also need to make sure the function is parsed correctly within
the same TU. So if we can find the function's implementation,
ignore the annotations, otherwise, false positive would occur.
Move the escape by value case to post call and do not escape the handle
if the function is inlined and we have analyzed the handle.

Differential Revision: https://reviews.llvm.org/D91902

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
index d4901eb0abbb..c246a8db3067 100644
--- a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -331,11 +331,6 @@ void FuchsiaHandleChecker::checkPreCall(const CallEvent &Call,
           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 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
   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 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
         });
         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());
       }
     }
   }

diff  --git a/clang/test/Analysis/fuchsia_handle.cpp b/clang/test/Analysis/fuchsia_handle.cpp
index 911ab7adaaa9..99f0449a4902 100644
--- a/clang/test/Analysis/fuchsia_handle.cpp
+++ b/clang/test/Analysis/fuchsia_handle.cpp
@@ -315,6 +315,45 @@ void checkHandlePtrInStructureLeak() {
   // 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>