[analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

[xazax.hun]

Summary

This check is based on https://reviews.llvm.org/D36022 but it takes a bit different approach. It does less state splitting and tries to avoid the evalCall callback. The state machine is also a bit different, now the escaped and untracked states are merged.

[xazax.hun]

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

68

Whoops, the spelling of the attribute is wrong here, will fix in a followup patch.

[NoQ]

Yay, i see the "Schrödinger handle" pattern: until the return value of release is checked, the handle is believed to be both dead and alive at the same time.

We usually use this pattern because a lot of code that's already written doesn't check their return values. But do you really need this for a brand-new API? Maybe aggressively assume that the release may always fail and fix all the places in your code where the return value is not checked before use?

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

100–101

So you plan to unhardcode these as well eventually?

124

For now it doesn't seem to be used at all.

142–147

This macro has literally saved exactly as many lines as it has caused :)

157–169

We've recently dumbed down this idiom to

class FuchsiaHandleChecker : ... {
  LeakBugType{this, "Fuchsia handle leak",
              "Fuchsia Handle Error", /*SuppressOnSink=*/true};
  ...
};

:)

208

This is never necessary, just call all the methods directly on QT - it has an overloaded operator ->() for this.

209

Ok, so what's the deal with arrays here? If the function receives an array of handles, do you ultimately plan to return multiple symbols - one for each element of the array?

Also, in the generic non-Fuchsia case, what if the handle itself is a pointer?

213

Here QT->getAs<TypedefType>() respectively.

303–304

Why not generate a fatal error instead?

337–339

Ok, so you're basically saying that whenever the code under analysis does anything to check the return value, this callback will fire immediately? Thus guaranteeing that whenever the Schrödinger state is accessed, it's either already collapsed or indeed not checked yet?

This sounds right but i wish i had your confidence :D

My usual approach to Schrödinger states will be to do the same thing but in checkDeadSymbols(). I.e., when the return value symbol dies, see if it's constrained and collapse the state accordingly. If the access occurs earlier than symbol death, update the state according to the current constraints and forget about the symbol. If current constraints are insufficient, conservatively mark it as a successful release (as that's how non-paranoid code under analysis would behave).

Regardless of the approach, you still need to update the state upon access as if the release has succeeded (when the current state is a Schrödinger state). But with your approach you can avoid checking the constraints upon access, and instead blindly assume that the symbol is underconstrained.

I like this! Can we prove that it works by adding an assertion that the return value symbol is underconstrained upon every access to the potentially Schrödinger state?

344

If you want this check to be universally applicable, you will probably also need an annotation that explains which statuses indicate an error.

349

I request comments about this path. When does this happen?

367

I strongly recommend testing this (i.e., by annotating an overloaded member operator with the use-annotation). This code looks correct to me but it's just too easy to make a mistake here :/

clang/test/Analysis/fuchsia_handle.c

2

core,!

77

This test doesn't really test the pointer escape mechanism because both handles are closed correctly, so there's no leak in this test regardless of escapes.

[xazax.hun]

Thanks for the review!

I am not very well versed in Fuchsia's syscalls yet but my understanding is that not all of the syscalls can fail so we do not need all the users to check for errors. But this is something I will verify with the rest of the team, so please treat my answer with a grain of salt for now. An alternative would be to introduce an annotation to tell which APIs will never fail, but I am afraid that is also sometimes subjective. For example it is pretty much possible to have no available port in a system but is very unlikely to happen so some non mission critical applications might not check for errors there. But this is also something that I will follow up with the team.

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

100–101

I have no specific plans at this point. I do not see these types changing frequently, so I have no urge to unhardcode them. Are there any downsides I am not being aware of?

124

Wow, indeed! This is from an eariler iteration :)

142–147

Good point! Initially I had a separate escaped state, but now I guess I should just expand them :)

157–169

+1 :)

208

getPointeeOrArrayElementType returns a Type *, so I cannot really continue to use QualType and I am not interested in the qualifiers at all.

209

I do not have a real plan with arrays just yet. Creating eagerly a symbol for all the elements might look a bit wasteful but also users are probably not expected to have large arrays of handles? Probably I should remove that code for now.

I would expect non-Fuchsia checkers to introduce their own getHandleSymbol logic. Does that make sense? Maybe I should rename this to getFuchsiaHandleSymbol.

303–304

The motivation was to not to reduce coverage, but I have no strong feelings about making this a sink. But in case of leaks I certainly prefer non-fatal errors.

337–339

Yeah, you got my intention right! I was also thinking about doing this in checkDeadSymbols, but I was wondering if that is ultimately the right thing to do. It might be possible to write code where the symbol for the error code becomes dead too late, so we keep the Schrödinger state longer than necessary. Redoing this when we access the handle makes sense though! I think that could probably perform better compared to my approach with evalAssume.

My only problem at this point do not really know when is a handle in a Schrödinger state. Do you recommend specifying a distinct state for that?

344

I think the main reason why we use annotations for release/acquire/use because of the potentially large number of calls that deals with handles. The status for success, however, is not a big amount of information, so I think this is easier to be hard coded. It is also less prone to changes.

349

Oh, this is to model that maybe a handle allocation failed. I will definitely add a comment and also make the code more narrow to that specific case!

367

I do agree, I also hate this part.

clang/test/Analysis/fuchsia_handle.c

2

Whoops! :)

77

Indeed! I will make one of the handles "leak".

[xazax.hun]

• Addressed some of the review comments. More changes are planned :)

[NoQ]

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

208

Whoops ^.^"

209

But, i mean, why do you even consider arrays in this code? Why not simply QT = QT->getPointeeType()?

[xazax.hun]

• Explicitly model "maybe" states.
• Fix some escaping issues.
• Address most review comments.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

19–20

Btw, this state machine is fairly common. Both MallocChecker and SimpleStreamChecker already follow this same model. Do you have any thoughts on re-using it in an abstract manner?

Something like this maybe?:

template <typename TraitID>
class SimpleStreamStateMachine {
public:
  SimpleStreamStateMachine(void(*pointerEscapePolicy)(CheckerContext C, Other Customizations));

  ProgramStateRef makeOpened(ProgramStateRef State, SymbolRef Key) {
    return State->set<TraitID>(Key, TraitID::ValueTy::makeOpened());
  }
  // makeReleased and so on pre-defined for all users,
  // allowing customization when necessary.
};

class SimpleStreamStateMachineBookkeeping : Checker {
public:
  checkDeadSymbols() {
    // Perform the state cleanup for all concrete machines
    // ever instantiated, in the only possible way, probably
    // invoke callbacks for leaks.
  }
  checkPointerEscape() {
    // Invoke the passed-down policy for each concrete
    // state machine.
  }
  // Other callbacks are implemented in the dependent checker.
};

And then:

REGISTER_MAP_WITH_PROGRAMSTATE(MyTrait, SymbolRef, MyState);

class MyChecker : Checker {
  SimpleStreamStateMachine<MyStateTy> StM {
      getStateMachine<SimpleStreamStateMachine<MyStateTy>>(
          &MyChecker::checkPointerEscapeImpl, /*Other Customizations*/)};
  void checkPointerEscapeImpl(...);

public:
  checkPostCall(...) {
    C.addTransition(StM.makeOpened(C.getState(), Call.getRetVal()));
  }
};

'Cause i have this pipe dream that we make a lot of such abstract state machines and then we'll never need to write more of them and that'll make it cheaper to introduce non-trivial operations over the program state such as replacing values or advanced widening because we'll only have to implement them in the few state machines rather than in many checkers.

[xazax.hun]

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

19–20

Hmm, indeed, lots of those checks following a very similar idea. I think a good litmus test would be to check if the customization points of such abstractions are strong enough to handle all sorts of error handling mechanisms that would potentially be handled by the checkers including: return values (like your example), output arguments, errno/other global error state. Also, checkers sometimes have slightly different state machines see the AllocatedOfSizeZero in MallocChecker, and sometimes they also have more companion data in the state like allocation family (this one should be easy to solve though).

That being said I would prefer such experiments to happen in a separate patch :)

[xazax.hun]

• Make the patch standalone and the check on by default for the fuchsia platform.

[xazax.hun]

• Fix member operator modeling.
• Added new lines to the end of files.
• Added documentation.
• Minor typo fixes in tests.

[xazax.hun]

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

337–339

Hmm, evalAssume does not get a CheckerContext, so it cannot really use SValBuilder to do more complicated queries, like check if a symbol is greater or equal to zero rather than just equal to.

I wonder, is this intentional? Should I just use the checkDeadSymbol method instead or should I add CheckerContext to evalAssume?

[xazax.hun]

• Handle cases where the number of args/params mismatch.

[xazax.hun]

Ok, now I have some real world experience with the results of the check.
The false positive ratio for double free and use after free seems to be quite good but the handle leak part is almost unusable at this point. The main problem is somewhat anticipated, we are not doing a great job notifying the checkers about escaped non-pointer symbols. The question is, what should we do about this? Should the PointerEscape callback be renamed? Currently, the list of symbols will also contain non-pointer symbols, so in some cases that callback does solve the escaping issue for integers. But it is not consistent.
So I see two ways forward:

1. Have a separate SymbolEscape callback.
2. Rename PointerEscape, and fix it to be triggered for all kinds of symbols.

I feel like the 2. is a better solution. Of course, that change might have a performance impact as well.

Any thoughts?

[NoQ]

I feel like the 2. is a better solution. Of course, that change might have a performance impact as well.

Yes, i'm all for '2.'. There's no need to make this callback more complicated than it already is.

As for performance, it's messy and suffers from a deeper problem: the number of escaped symbols is potentially infinite. The following false positive illustrates that well:

void invalidate(int **x);

void foo(int **x) {
  int *y = *x;
  if (*y == 0) {
    // **x should be invalidated here!
    invalidate(x);
  }
  // Should not warn about division by zero!
  1 / *y;
}

Therefore one does not simply compose a list of escaped symbols. We need something similar to SymbolReaper but for invalidation/escapes. And then we'll talk about performance.

[xazax.hun]

• Fix some problems discovered during some real world stress test
• Add more tests
• The ASCII art is not updated yet, but will do so at some point.

[NoQ]

LGTM! Yeah, please update the ASCII-art, it's great and every checker should have it.

clang/docs/analyzer/checkers.rst

1341 ↗

(On Diff #233691)

Maybe explain what Fuchsia is so that people don't wonder if they should turn it on? (:

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

345

Let's add a high-level comment about what's going on here.

[xazax.hun]

Thanks for the reviews!

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

 
 let ParentPackage = Fuchsia in {
 
 def FuchsiaLockChecker : Checker<"Lock">,
   HelpText<"Check for the correct usage of locking APIs.">,
   Dependencies<[PthreadLockBase]>,
   Documentation<HasDocumentation>;
 
+def FuchsiaHandleChecker : Checker<"FuchsiaHandleChecker">,
+  HelpText<"A Checker that detect leaks related to Fuchsia handles">,
+  Documentation<HasDocumentation>;
+
 } // end fuchsia

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

public:
   /// @param Tag The tag to uniquely identify the creation site. If null,
   ///        the default tag for the checker will be used.
   ExplodedNode *
   generateNonFatalErrorNode(ProgramStateRef State = nullptr,
                             const ProgramPointTag *Tag = nullptr) {
     return addTransition(State, (Tag ? Tag : Location.getTag()));
   }
 
+  /// Generate a transition to a node that will be used to report
+  /// an error. This node will not be a sink. That is, exploration will
+  /// continue along this path.
+  ///
+  /// @param State The state of the generated node.
+  /// @param Pred The transition will be generated from the specified Pred node
+  ///             to the newly generated node.
+  /// @param Tag The tag to uniquely identify the creation site. If null,
+  ///        the default tag for the checker will be used.
+  ExplodedNode *
+  generateNonFatalErrorNode(ProgramStateRef State,
+                            ExplodedNode *Pred,
+                            const ProgramPointTag *Tag = nullptr) {
+    return addTransition(State, Pred, (Tag ? Tag : Location.getTag()));
+  }
+
   /// Emit the diagnostics report.
   void emitReport(std::unique_ptr<BugReport> R) {
     Changed = true;
     Eng.getBugReporter().emitReport(std::move(R));
   }
 
   /// Produce a program point tag that displays an additional path note
   /// to the user. This is a lightweight alternative to the

clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

add_clang_library(clangStaticAnalyzerCheckers
   DereferenceChecker.cpp
   DirectIvarAssignment.cpp
   DivZeroChecker.cpp
   DynamicTypePropagation.cpp
   DynamicTypeChecker.cpp
   EnumCastOutOfRangeChecker.cpp
   ExprInspectionChecker.cpp
   FixedAddressChecker.cpp
+  FuchsiaHandleChecker.cpp
   GCDAntipatternChecker.cpp
   GenericTaintChecker.cpp
   GTestChecker.cpp
   IdenticalExprChecker.cpp
   InnerPointerChecker.cpp
   IteratorChecker.cpp
   IvarInvalidationChecker.cpp
   LLVMConventionsChecker.cpp

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

+//=== FuchsiaHandleChecker.cpp - Find handle leaks/double closes -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This checker checks if the handle of Fuchsia is properly used according to
+// following rules.
+//   - If a handle is acquired, it should be released before execution
+//        ends.
+//   - If a handle is released, it should not be released again.
+//   - If a handle is released, it should not be used for other purposes
+//        such as I/O.
+//
+// In this checker, each tracked handle is associated with a state. When the
+// handle variable is passed to different function calls or syscalls, its state
+// changes. The state changes can be generally represented by following ASCII
+// Art:
+//
+//    +-----------------------------------------------------------+
+//    |                                                           |
+//    |                        release_func failed                |
+//    |                           +---------+                     |
+//    |                           |         |                     |
+//    |                           |         |                     |
+//    |                         +-+---------v-+                   |
+//    |  acquire_func succeeded |             |    Escape         |
+//    |       +----------------->  Allocated  +-------------------+
+//    |       |                 |             |
+//    |       |                 +-----+------++
+//    |       |                       |      |
+//    |       |         release_func  |      |
+//    |       |          succeeded    |      +--+
+//    |       |                       |         | handle   +--------+
+//    |       |                  +----v-----+   | dies     |        |
+//    |       |       Escape     |          |   +----------> Leaked |
+// +--v-------+--+  +------------+ Released |              |(REPORT)|
+// |             |  |            |          |              +--------+
+// | Not tracked <--+            +----+---+-+
+// |             |  |                 |   |        As argument by value
+// +------+------+  |    release_func |   +------+ in function call
+//        |         |                 |          | or by reference in
+//        |         |                 |          | use_func call
+//        |         |            +----v-----+    |     +-----------+
+//        +---------+            |          |    |     |           |
+//        acquire_func failed    | Double   |    +-----> Use after |
+//                               | released |          | released  |
+//                               | (REPORT) |          | (REPORT)  |
+//                               +----------+          +-----------+
+//
+// acquire_func represents the functions or syscalls that may acquire a handle.
+// release_func represents the functions or syscalls that may release a handle.
+// use_func represents the functions or syscall that requires an open handle.
+//
+// If a tracked handle dies in "Released" or "Not Tracked" state, we assume it
+// is properly used. Otherwise a bug and will be reported.
+//
+// Note that, the analyzer does not always know for sure if a function failed
+// or succeeded. In those cases we use the states MaybeReleased and
+// MaybeAllocated. Thus, the diagramm above captures the intent, not
+// implementation details.
+//
+// Due to the fact that the number of handle related syscalls in Fuchsia
+// is large, we adopt the annotation attributes to descript syscalls'
+// operations(acquire/release/use) on handles instead of hardcoding
+// everything in the checker.
+//
+// We use following annotation attributes for handle related syscalls or
+// functions:
+//  1. __attribute__((acquire_handle)) |handle will be acquired
+//  2. __attribute__((release_handle)) |handle will be released
+//  3. __attribute__((use_handle)) |handle will not transit to
+//     escaped state, it also needs to be open.
+//
+// For example, an annotated syscall:
+//   zx_status_t zx_channel_create(
+//   uint32_t options,
+//   zx_handle_t* out0 __attribute__((acquire_handle)) ,
+//   zx_handle_t* out1 __attribute__((acquire_handle)));
+// denotes a syscall which will acquire two handles and save them to 'out0' and
+// 'out1' when succeeded.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/Type.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+static const StringRef HandleTypeName = "zx_handle_t";
+static const StringRef ErrorTypeName = "zx_status_t";
+
+class HandleState {
+private:
+  enum class Kind { MaybeAllocated, Allocated, MaybeReleased, Released } K;
+  SymbolRef ErrorSym;
+  HandleState(Kind K, SymbolRef ErrorSym) : K(K), ErrorSym(ErrorSym) {}
+
+public:
+  bool operator==(const HandleState &Other) const {
+    return K == Other.K && ErrorSym == Other.ErrorSym;
+  }
+  bool isAllocated() const { return K == Kind::Allocated; }
+  bool maybeAllocated() const { return K == Kind::MaybeAllocated; }
+  bool isReleased() const { return K == Kind::Released; }
+  bool maybeReleased() const { return K == Kind::MaybeReleased; }
+
+  static HandleState getMaybeAllocated(SymbolRef ErrorSym) {
+    return HandleState(Kind::MaybeAllocated, ErrorSym);
+  }
+  static HandleState getAllocated(ProgramStateRef State, HandleState S) {
+    assert(S.maybeAllocated() || S.maybeReleased());
+    assert(State->getConstraintManager()
+               .isNull(State, S.getErrorSym())
+               .isConstrained());
+    return HandleState(Kind::Allocated, nullptr);
+  }
+  static HandleState getMaybeReleased(SymbolRef ErrorSym) {
+    return HandleState(Kind::MaybeReleased, ErrorSym);
+  }
+  static HandleState getReleased(ProgramStateRef State, HandleState S) {
+    assert(S.maybeReleased());
+    assert(State->getConstraintManager()
+               .isNull(State, S.getErrorSym())
+               .isConstrained());
+    return HandleState(Kind::Released, nullptr);
+  }
+
+  SymbolRef getErrorSym() const { return ErrorSym; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+    ID.AddInteger(static_cast<int>(K));
+    ID.AddPointer(ErrorSym);
+  }
+
+  LLVM_DUMP_METHOD void dump(raw_ostream &OS) const {
+    switch (K) {
+#define CASE(ID)                                                               \
+  case ID:                                                                     \
+    OS << #ID;                                                                 \
+    break;
+      CASE(Kind::MaybeAllocated)
+      CASE(Kind::Allocated)
+      CASE(Kind::MaybeReleased)
+      CASE(Kind::Released)
+    }
+  }
+
+  LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); }
+};
+
+class FuchsiaHandleChecker
+    : public Checker<check::PostCall, check::PreCall, check::DeadSymbols,
+                     check::PointerEscape, eval::Assume> {
+  BugType LeakBugType{this, "Fuchsia handle leak", "Fuchsia Handle Error",
+                      /*SuppressOnSink=*/true};
+  BugType DoubleReleaseBugType{this, "Fuchsia handle double release",
+                               "Fuchsia Handle Error"};
+  BugType UseAfterFreeBugType{this, "Fuchsia handle use after release",
+                              "Fuchsia Handle Error"};
+
+public:
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+                             bool Assumption) const;
+  ProgramStateRef checkPointerEscape(ProgramStateRef State,
+                                     const InvalidatedSymbols &Escaped,
+                                     const CallEvent *Call,
+                                     PointerEscapeKind Kind) const;
+
+  ExplodedNode *reportLeaks(ArrayRef<SymbolRef> LeakedHandles,
+                            CheckerContext &C, ExplodedNode *Pred) const;
+
+  void reportDoubleRelease(SymbolRef HandleSym, const SourceRange &Range,
+                           CheckerContext &C) const;
+
+  void reportUseAfterFree(SymbolRef HandleSym, const SourceRange &Range,
+                          CheckerContext &C) const;
+
+  void reportBug(SymbolRef Sym, ExplodedNode *ErrorNode, CheckerContext &C,
+                 const SourceRange *Range, const BugType &Type,
+                 StringRef Msg) const;
+
+  void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
+                  const char *Sep) const override;
+};
+} // end anonymous namespace
+
+REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState)
+
+/// Returns the symbols extracted from the argument or null if it cannot be
+/// found.
+SymbolRef getFuchsiaHandleSymbol(QualType QT, SVal Arg, ProgramStateRef State) {
+  int PtrToHandleLevel = 0;
+  while (QT->isAnyPointerType() || QT->isReferenceType()) {
+    ++PtrToHandleLevel;
+    QT = QT->getPointeeType();
+  }
+  if (const auto *HandleType = QT->getAs<TypedefType>()) {
+    if (HandleType->getDecl()->getName() != HandleTypeName)
+      return nullptr;
+    if (PtrToHandleLevel > 1) {
+      // Not supported yet.
+      return nullptr;
+    }
+
+    if (PtrToHandleLevel == 0) {
+      return Arg.getAsSymbol();
+    } else {
+      assert(PtrToHandleLevel == 1);
+      if (Optional<Loc> ArgLoc = Arg.getAs<Loc>())
+        return State->getSVal(*ArgLoc).getAsSymbol();
+    }
+  }
+  return nullptr;
+}
+
+void FuchsiaHandleChecker::checkPreCall(const CallEvent &Call,
+                                        CheckerContext &C) const {
+  const FunctionDecl *FuncDecl = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+  if (!FuncDecl)
+    return;
+
+  ProgramStateRef State = C.getState();
+
+  for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) {
+    Optional<unsigned> ParamIdx = Call.getAdjustedParameterIndex(Arg);
+    if (!ParamIdx)
+      continue;
+
+    const ParmVarDecl *PVD = FuncDecl->getParamDecl(*ParamIdx);
+    SymbolRef HandleSym =
+        getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State);
+    if (!HandleSym)
+      continue;
+
+    // Handled in checkPostCall.
+    if (PVD->hasAttr<ReleaseHandleAttr>() || PVD->hasAttr<AcquireHandleAttr>())
+      continue;
+
+    const HandleState *HState = State->get<HStateMap>(HandleSym);
+    if (!HState)
+        continue;
+
+    if (PVD->hasAttr<UseHandleAttr>() || PVD->getType()->isIntegerType()) {
+      if (HState->isReleased() || HState->maybeReleased()) {
+        reportUseAfterFree(HandleSym, Call.getArgSourceRange(Arg), C);
+        return;
+      }
+    }
+    if (!PVD->hasAttr<UseHandleAttr>() &&
+               PVD->getType()->isIntegerType()) {
+      // Working around integer by-value escapes.
+      State = State->remove<HStateMap>(HandleSym);
+    }
+  }
+  C.addTransition(State);
+}
+
+void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
+                                         CheckerContext &C) const {
+  const FunctionDecl *FuncDecl = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+  if (!FuncDecl)
+    return;
+
+  ProgramStateRef State = C.getState();
+
+  SymbolRef ResultSymbol = nullptr;
+  if (const auto *TypeDefTy = FuncDecl->getReturnType()->getAs<TypedefType>())
+    if (TypeDefTy->getDecl()->getName() == ErrorTypeName)
+      ResultSymbol = Call.getReturnValue().getAsSymbol();
+
+  // Function returns an open handle.
+  if (FuncDecl->hasAttr<AcquireHandleAttr>()) {
+    SymbolRef RetSym = Call.getReturnValue().getAsSymbol();
+    State =
+        State->set<HStateMap>(RetSym, HandleState::getMaybeAllocated(nullptr));
+  }
+
+  for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) {
+    Optional<unsigned> ParamIdx = Call.getAdjustedParameterIndex(Arg);
+    if (!ParamIdx)
+      continue;
+
+    const ParmVarDecl *PVD = FuncDecl->getParamDecl(*ParamIdx);
+    SymbolRef HandleSym =
+        getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State);
+    if (!HandleSym)
+      continue;
+
+    const HandleState *HState = State->get<HStateMap>(HandleSym);
+    if (PVD->hasAttr<ReleaseHandleAttr>()) {
+      if (HState && (HState->isReleased() || HState->maybeReleased())) {
+        reportDoubleRelease(HandleSym, Call.getArgSourceRange(Arg), C);
+        return;
+      } else
+        State = State->set<HStateMap>(
+            HandleSym, HandleState::getMaybeReleased(ResultSymbol));
+    } else if (PVD->hasAttr<AcquireHandleAttr>()) {
+      State = State->set<HStateMap>(
+          HandleSym, HandleState::getMaybeAllocated(ResultSymbol));
+    }
+  }
+  C.addTransition(State);
+}
+
+void FuchsiaHandleChecker::checkDeadSymbols(SymbolReaper &SymReaper,
+                                            CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SmallVector<SymbolRef, 2> LeakedSyms;
+  HStateMapTy TrackedHandles = State->get<HStateMap>();
+  for (auto &CurItem : TrackedHandles) {
+    if (!SymReaper.isDead(CurItem.first))
+      continue;
+    if (CurItem.second.isAllocated() || CurItem.second.maybeAllocated())
+      LeakedSyms.push_back(CurItem.first);
+    State = State->remove<HStateMap>(CurItem.first);
+  }
+
+  ExplodedNode *N = C.getPredecessor();
+  if (!LeakedSyms.empty())
+    N = reportLeaks(LeakedSyms, C, N);
+
+  C.addTransition(State, N);
+}
+
+ProgramStateRef FuchsiaHandleChecker::evalAssume(ProgramStateRef State,
+                                                 SVal Cond,
+                                                 bool Assumption) const {
+  ConstraintManager &Cmr = State->getConstraintManager();
+  HStateMapTy TrackedHandles = State->get<HStateMap>();
+  for (auto &CurItem : TrackedHandles) {
+    SymbolRef ErrorSym = CurItem.second.getErrorSym();
+    if (!ErrorSym)
+      continue;
+    ConditionTruthVal ErrorVal = Cmr.isNull(State, ErrorSym);
+    if (ErrorVal.isConstrainedTrue()) {
+      // Allocation/release succeeded.
+      if (CurItem.second.maybeReleased())
+        State = State->set<HStateMap>(
+            CurItem.first, HandleState::getReleased(State, CurItem.second));
+      else if (CurItem.second.maybeAllocated())
+        State = State->set<HStateMap>(
+            CurItem.first, HandleState::getAllocated(State, CurItem.second));
+    } else if (ErrorVal.isConstrainedFalse()) {
+      // Allocation/release failed.
+      if (CurItem.second.maybeReleased())
+        State = State->set<HStateMap>(
+            CurItem.first, HandleState::getAllocated(State, CurItem.second));
+      else if (CurItem.second.maybeAllocated())
+        State = State->remove<HStateMap>(CurItem.first);
+    }
+  }
+  return State;
+}
+
+ProgramStateRef FuchsiaHandleChecker::checkPointerEscape(
+    ProgramStateRef State, const InvalidatedSymbols &Escaped,
+    const CallEvent *Call, PointerEscapeKind Kind) const {
+  const FunctionDecl *FuncDecl =
+      Call ? dyn_cast_or_null<FunctionDecl>(Call->getDecl()) : nullptr;
+  for (const SymbolRef &EscapedSymbol : Escaped) {
+    if (State->contains<HStateMap>(EscapedSymbol)) {
+      bool IsUnescapedUse = false;
+      // Not all calls should escape our symbols.
+      if (FuncDecl && (Kind == PSK_DirectEscapeOnCall ||
+                       Kind == PSK_IndirectEscapeOnCall)) {
+        for (unsigned Arg = 0; Arg < Call->getNumArgs(); ++Arg) {
+          Optional<unsigned> ParamIdx = Call->getAdjustedParameterIndex(Arg);
+          if (!ParamIdx)
+            continue;
+
+          const ParmVarDecl *PVD = FuncDecl->getParamDecl(*ParamIdx);
+          SymbolRef HandleSym = getFuchsiaHandleSymbol(
+              PVD->getType(), Call->getArgSVal(Arg), State);
+          if (!HandleSym)
+            continue;
+          if (HandleSym == EscapedSymbol && PVD->hasAttr<UseHandleAttr>())
+            IsUnescapedUse = true;
+        }
+      }
+      if (!IsUnescapedUse)
+        State = State->remove<HStateMap>(EscapedSymbol);
+    }
+  }
+  return State;
+}
+
+ExplodedNode *
+FuchsiaHandleChecker::reportLeaks(ArrayRef<SymbolRef> LeakedHandles,
+                                  CheckerContext &C, ExplodedNode *Pred) const {
+  ExplodedNode *ErrNode = C.generateNonFatalErrorNode(C.getState(), Pred);
+  for (SymbolRef LeakedHandle : LeakedHandles) {
+    reportBug(LeakedHandle, ErrNode, C, nullptr, LeakBugType,
+              "Potential leak of handle");
+  }
+  return ErrNode;
+}
+
+void FuchsiaHandleChecker::reportDoubleRelease(SymbolRef HandleSym,
+                                               const SourceRange &Range,
+                                               CheckerContext &C) const {
+  ExplodedNode *ErrNode = C.generateErrorNode(C.getState());
+  reportBug(HandleSym, ErrNode, C, &Range, DoubleReleaseBugType,
+            "Releasing a previously released handle");
+}
+
+void FuchsiaHandleChecker::reportUseAfterFree(SymbolRef HandleSym,
+                                              const SourceRange &Range,
+                                              CheckerContext &C) const {
+  ExplodedNode *ErrNode = C.generateErrorNode(C.getState());
+  reportBug(HandleSym, ErrNode, C, &Range, UseAfterFreeBugType,
+            "Using a previously released handle");
+}
+
+void FuchsiaHandleChecker::reportBug(SymbolRef Sym, ExplodedNode *ErrorNode,
+                                     CheckerContext &C,
+                                     const SourceRange *Range,
+                                     const BugType &Type, StringRef Msg) const {
+  if (!ErrorNode)
+    return;
+
+  auto R = std::make_unique<PathSensitiveBugReport>(Type, Msg, ErrorNode);
+  if (Range)
+    R->addRange(*Range);
+  R->markInteresting(Sym);
+  C.emitReport(std::move(R));
+}
+
+void ento::registerFuchsiaHandleChecker(CheckerManager &mgr) {
+  mgr.registerChecker<FuchsiaHandleChecker>();
+}
+
+bool ento::shouldRegisterFuchsiaHandleChecker(const LangOptions &LO) {
+  return true;
+}
+
+void FuchsiaHandleChecker::printState(raw_ostream &Out, ProgramStateRef State,
+                                      const char *NL, const char *Sep) const {
+
+  HStateMapTy StateMap = State->get<HStateMap>();
+
+  if (!StateMap.isEmpty()) {
+    Out << Sep << "FuchsiaHandleChecker :" << NL;
+    for (HStateMapTy::iterator I = StateMap.begin(), E = StateMap.end(); I != E;
+         ++I) {
+      I.getKey()->dumpToStream(Out);
+      Out << " : ";
+      I.getData().dump(Out);
+      Out << NL;
+    }
+  }
+}
 No newline at end of file

clang/test/Analysis/fuchsia_handle.c

+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.FuchsiaHandleChecker -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int zx_status_t;
+typedef __typeof__(sizeof(int)) zx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+
+#if defined(__clang__)
+#define XZ_HANDLE_ACQUIRE  __attribute__((acquire_handle))
+#define XZ_HANDLE_RELEASE  __attribute__((release_handle))
+#define XZ_HANDLE_USE  __attribute__((use_handle))
+#else
+#define XZ_HANDLE_ACQUIRE
+#define XZ_HANDLE_RELEASE
+#define XZ_HANDLE_USE
+#endif
+
+zx_status_t zx_channel_create(
+uint32_t options,
+    XZ_HANDLE_ACQUIRE zx_handle_t* out0,
+    zx_handle_t* out1 XZ_HANDLE_ACQUIRE);
+
+zx_status_t zx_handle_close(
+    zx_handle_t handle XZ_HANDLE_RELEASE);
+
+void escape1(zx_handle_t *in);
+void escape2(zx_handle_t in);
+
+void use1(const zx_handle_t *in XZ_HANDLE_USE);
+void use2(zx_handle_t in XZ_HANDLE_USE);
+
+void checkNoLeak01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, &ay[0], &ay[1]);
+  zx_handle_close(ay[0]);
+  zx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, &ay[0], &ay[1]);
+  for (int i = 0; i < 2; i++)
+    zx_handle_close(ay[i]);
+}
+
+zx_handle_t checkNoLeak04() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  zx_handle_close(sa);
+  return sb; // no warning
+}
+
+zx_handle_t checkNoLeak05(zx_handle_t *out1) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak06() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, &sa, &sb))
+    return;
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+} 
+
+void checkNoLeak07(int tag) {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, &sa, &sb))
+    return;
+  escape1(&sa);
+  escape2(sb);
+}
+
+void checkNoLeak08(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  use1(&sa);
+  if (tag)
+    zx_handle_close(sa);
+  use2(sb); // expected-warning {{Potential leak of handle}}
+  zx_handle_close(sb);
+}
+
+void checkDoubleRelease01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  if (tag)
+    zx_handle_close(sa);
+  zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  zx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  if (tag) {
+    zx_handle_close(sa);
+    use1(&sa); // expected-warning {{Using a previously released handle}}
+  }
+  zx_handle_close(sb);
+  use2(sb); // expected-warning {{Using a previously released handle}}
+}