[PATCH] D58366: [analyzer] MIGChecker: Make use of the server routine annotation.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

The attribute that's added in D58365 <https://reviews.llvm.org/D58365> allows us to avoid making unreliable guesses on whether the current function is a MIG server routine.


Repository:
  rC Clang

https://reviews.llvm.org/D58366

Files:
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/test/Analysis/mig.cpp


Index: clang/test/Analysis/mig.cpp
===================================================================
--- clang/test/Analysis/mig.cpp
+++ clang/test/Analysis/mig.cpp
@@ -12,6 +12,9 @@
 
 kern_return_t vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
 
+#define MIG_SERVER_ROUTINE __attribute__((mig_server_routine))
+
+MIG_SERVER_ROUTINE
 kern_return_t basic_test(mach_port_name_t port, vm_address_t address, vm_size_t size) {
   vm_deallocate(port, address, size);
   if (size > 10) {
@@ -21,6 +24,7 @@
 }
 
 // Make sure we don't crash when they forgot to write the return statement.
+MIG_SERVER_ROUTINE
 kern_return_t no_crash(mach_port_name_t port, vm_address_t address, vm_size_t size) {
   vm_deallocate(port, address, size);
 }
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -55,7 +55,6 @@
          VR->getStackFrame()->inTopFrame();
 }
 
-// This function will probably be replaced with looking up annotations.
 static bool isInMIGCall(const LocationContext *LC) {
   const StackFrameContext *SFC;
   // Find the top frame.
@@ -64,17 +63,15 @@
     LC = SFC->getParent();
   }
 
-  const auto *FD = dyn_cast<FunctionDecl>(SFC->getDecl());
+  const FunctionDecl *FD = dyn_cast<FunctionDecl>(SFC->getDecl());
   if (!FD)
     return false;
 
-  // FIXME: This is an unreliable (even if surprisingly reliable) heuristic.
-  // The real solution here is to make MIG annotate its callbacks in
-  // autogenerated headers so that we didn't need to think hard if it's
-  // actually a MIG callback.
-  QualType T = FD->getReturnType();
-  return T.getCanonicalType()->isIntegerType() &&
-         T.getAsString() == "kern_return_t";
+  // Even though there's a Sema warning when the return type of an annotated
+  // function is not a kern_return_t, this warning isn't an error, so we need
+  // an extra sanity check here.
+  return FD->hasAttr<MIGServerRoutineAttr>() &&
+         FD->getReturnType().getCanonicalType()->isSignedIntegerType();
 }
 
 void MIGChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58366.187296.patch
Type: text/x-patch
Size: 2231 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190219/2fdab309/attachment-0001.bin>