After a lengthy design discussion, add support for "ownership attributes" for malloc/free checking. Patch by Andrew McGregor!

llvm-svn: 109939

clang/include/clang/AST/Attr.h

@@ -369,6 +369,121 @@ public:
   static bool classof(const NonNullAttr *A) { return true; }
 };
 
+/// OwnershipAttr
+/// Ownership attributes are used to annotate pointers that own a resource
+/// in order for the analyzer to check correct allocation and deallocation.
+/// There are three attributes, ownership_returns, ownership_holds and
+/// ownership_takes, represented by subclasses of OwnershipAttr
+class OwnershipAttr: public AttrWithString {
+ protected:
+  unsigned* ArgNums;
+  unsigned Size;
+public:
+  enum OwnershipKind { Holds, Takes, Returns, None };
+  OwnershipKind OKind;
+  attr::Kind AKind;
+public:
+  OwnershipAttr(attr::Kind AK, ASTContext &C, unsigned* arg_nums, unsigned size,
+                       llvm::StringRef module, OwnershipKind kind);
+
+
+  virtual void Destroy(ASTContext &C);
+
+  OwnershipKind getKind() const {
+    return OKind;
+  }
+  bool isKind(const OwnershipKind k) const {
+    return OKind == k;
+  }
+
+  /// Ownership attributes have a 'module', which is the name of a kind of
+  /// resource that can be checked.
+  /// The Malloc checker uses the module 'malloc'.
+  llvm::StringRef getModule() const {
+    return getString();
+  }
+  void setModule(ASTContext &C, llvm::StringRef module) {
+    ReplaceString(C, module);
+  }
+  bool isModule(const char *m) const {
+    return getModule().equals(m);
+  }
+
+  typedef const unsigned *iterator;
+  iterator begin() const {
+    return ArgNums;
+  }
+  iterator end() const {
+    return ArgNums + Size;
+  }
+  unsigned size() const {
+    return Size;
+  }
+
+  virtual Attr *clone(ASTContext &C) const;
+
+  static bool classof(const Attr *A) {
+    return true;
+  }
+  static bool classof(const OwnershipAttr *A) {
+    return true;
+  }
+};
+
+class OwnershipTakesAttr: public OwnershipAttr {
+public:
+  OwnershipTakesAttr(ASTContext &C, unsigned* arg_nums, unsigned size,
+                     llvm::StringRef module);
+
+  virtual Attr *clone(ASTContext &C) const;
+
+  static bool classof(const Attr *A) {
+    return A->getKind() == attr::OwnershipTakes;
+  }
+  static bool classof(const OwnershipTakesAttr *A) {
+    return true;
+  }
+  static bool classof(const OwnershipAttr *A) {
+    return A->OKind == Takes;
+  }
+};
+
+class OwnershipHoldsAttr: public OwnershipAttr {
+public:
+  OwnershipHoldsAttr(ASTContext &C, unsigned* arg_nums, unsigned size,
+                     llvm::StringRef module);
+
+  virtual Attr *clone(ASTContext &C) const;
+
+  static bool classof(const Attr *A) {
+    return A->getKind() == attr::OwnershipHolds;
+  }
+  static bool classof(const OwnershipHoldsAttr *A) {
+    return true;
+  }
+  static bool classof(const OwnershipAttr *A) {
+    return A->OKind == Holds;
+  }
+};
+
+class OwnershipReturnsAttr: public OwnershipAttr {
+public:
+  OwnershipReturnsAttr(ASTContext &C, unsigned* arg_nums, unsigned size,
+                     llvm::StringRef module);
+
+  virtual Attr *clone(ASTContext &C) const;
+
+  static bool classof(const Attr *A) {
+    return A->getKind() == attr::OwnershipReturns;
+  }
+  static bool classof(const OwnershipReturnsAttr *A) {
+    return true;
+  }
+  static bool classof(const OwnershipAttr *A) {
+    return A->OKind == Returns;
+  }
+};
+
 class FormatAttr : public AttrWithString {
   int formatIdx, firstArg;
 public:

clang/include/clang/Basic/Attr.td

@@ -297,6 +297,21 @@ def Overloadable : Attr {
   let Spellings = ["overloadable"];
 }
 
+def OwnershipReturns : Attr {
+  let Spellings = ["ownership_returns"];
+  let Args = [StringArgument<"Module">, IntArgument<"SizeIdx">];
+}
+
+def OwnershipTakes : Attr {
+  let Spellings = ["ownership_takes"];
+  let Args = [StringArgument<"Module">, IntArgument<"PtrIdx">];
+}
+
+def OwnershipHolds : Attr {
+  let Spellings = ["ownership_holds"];
+  let Args = [StringArgument<"Module">, IntArgument<"PtrIdx">];
+}
+
 def Packed : Attr {
   let Spellings = ["packed"];
 }

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -838,6 +838,8 @@ def err_attribute_requires_objc_interface : Error<
   "attribute may only be applied to an Objective-C interface">;
 def err_nonnull_pointers_only : Error<
   "nonnull attribute only applies to pointer arguments">;
+def err_ownership_type : Error<
+  "%0 attribute only applies to %1 arguments">;
 def err_format_strftime_third_parameter : Error<
   "strftime format attribute requires 3rd parameter to be 0">;
 def err_format_attribute_requires_variadic : Error<

clang/include/clang/Parse/AttributeList.h

@@ -97,6 +97,9 @@ public:
     AT_ns_returns_retained,     // Clang-specific.
     AT_objc_gc,
     AT_overloadable,       // Clang-specific.
+    AT_ownership_holds,    // Clang-specific.
+    AT_ownership_returns,  // Clang-specific.
+    AT_ownership_takes,    // Clang-specific.
     AT_packed,
     AT_pure,
     AT_regparm,

clang/lib/AST/AttrImpl.cpp

@@ -48,6 +48,43 @@ NonNullAttr::NonNullAttr(ASTContext &C, unsigned* arg_nums, unsigned size)
   memcpy(ArgNums, arg_nums, sizeof(*ArgNums)*size);
 }
 
+OwnershipAttr::OwnershipAttr(attr::Kind AK, ASTContext &C, unsigned* arg_nums,
+                                           unsigned size,
+                                           llvm::StringRef module,
+                                           OwnershipKind kind) :
+  AttrWithString(AK, C, module), ArgNums(0), Size(0), OKind(kind) {
+  if (size == 0)
+    return;
+  assert(arg_nums);
+  ArgNums = new (C) unsigned[size];
+  Size = size;
+  AKind = AK;
+  OKind = kind;
+  memcpy(ArgNums, arg_nums, sizeof(*ArgNums) * size);
+}
+
+
+void OwnershipAttr::Destroy(ASTContext &C) {
+  if (ArgNums)
+    C.Deallocate(ArgNums);
+}
+
+OwnershipTakesAttr::OwnershipTakesAttr(ASTContext &C, unsigned* arg_nums,
+                                       unsigned size, llvm::StringRef module) :
+  OwnershipAttr(attr::OwnershipTakes, C, arg_nums, size, module, Takes) {
+}
+
+OwnershipHoldsAttr::OwnershipHoldsAttr(ASTContext &C, unsigned* arg_nums,
+                                       unsigned size, llvm::StringRef module) :
+  OwnershipAttr(attr::OwnershipHolds, C, arg_nums, size, module, Holds) {
+}
+
+OwnershipReturnsAttr::OwnershipReturnsAttr(ASTContext &C, unsigned* arg_nums,
+                                           unsigned size,
+                                           llvm::StringRef module) :
+  OwnershipAttr(attr::OwnershipReturns, C, arg_nums, size, module, Returns) {
+}
+
 #define DEF_SIMPLE_ATTR_CLONE(ATTR)                                     \
   Attr *ATTR##Attr::clone(ASTContext &C) const {                        \
     return ::new (C) ATTR##Attr;                                        \
@@ -147,6 +184,22 @@ Attr *NonNullAttr::clone(ASTContext &C) const {
   return ::new (C) NonNullAttr(C, ArgNums, Size);
 }
 
+Attr *OwnershipAttr::clone(ASTContext &C) const {
+  return ::new (C) OwnershipAttr(AKind, C, ArgNums, Size, getModule(), OKind);
+}
+
+Attr *OwnershipReturnsAttr::clone(ASTContext &C) const {
+  return ::new (C) OwnershipReturnsAttr(C, ArgNums, Size, getModule());
+}
+
+Attr *OwnershipTakesAttr::clone(ASTContext &C) const {
+  return ::new (C) OwnershipTakesAttr(C, ArgNums, Size, getModule());
+}
+
+Attr *OwnershipHoldsAttr::clone(ASTContext &C) const {
+  return ::new (C) OwnershipHoldsAttr(C, ArgNums, Size, getModule());
+}
+
 Attr *FormatAttr::clone(ASTContext &C) const {
   return ::new (C) FormatAttr(C, getType(), formatIdx, firstArg);
 }

clang/lib/Checker/MallocChecker.cpp

@@ -24,15 +24,17 @@ using namespace clang;
 namespace {
 
 class RefState {
-  enum Kind { AllocateUnchecked, AllocateFailed, Released, Escaped } K;
+  enum Kind { AllocateUnchecked, AllocateFailed, Released, Escaped, Relinquished } K;
   const Stmt *S;
 
 public:
   RefState(Kind k, const Stmt *s) : K(k), S(s) {}
 
   bool isAllocated() const { return K == AllocateUnchecked; }
+  bool isFailed() const { return K == AllocateFailed; }
   bool isReleased() const { return K == Released; }
   bool isEscaped() const { return K == Escaped; }
+  bool isRelinquished() const { return K == Relinquished; }
 
   bool operator==(const RefState &X) const {
     return K == X.K && S == X.S;
@@ -46,6 +48,7 @@ public:
   }
   static RefState getReleased(const Stmt *s) { return RefState(Released, s); }
   static RefState getEscaped(const Stmt *s) { return RefState(Escaped, s); }
+  static RefState getRelinquished(const Stmt *s) { return RefState(Relinquished, s); }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.AddInteger(K);
@@ -59,12 +62,13 @@ class MallocChecker : public CheckerVisitor<MallocChecker> {
   BuiltinBug *BT_DoubleFree;
   BuiltinBug *BT_Leak;
   BuiltinBug *BT_UseFree;
+  BuiltinBug *BT_UseRelinquished;
   BuiltinBug *BT_BadFree;
   IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc;
 
 public:
   MallocChecker() 
-    : BT_DoubleFree(0), BT_Leak(0), BT_UseFree(0), BT_BadFree(0),
+    : BT_DoubleFree(0), BT_Leak(0), BT_UseFree(0), BT_UseRelinquished(0), BT_BadFree(0),
       II_malloc(0), II_free(0), II_realloc(0), II_calloc(0) {}
   static void *getTag();
   bool EvalCallExpr(CheckerContext &C, const CallExpr *CE);
@@ -73,9 +77,13 @@ public:
   void PreVisitReturnStmt(CheckerContext &C, const ReturnStmt *S);
   const GRState *EvalAssume(const GRState *state, SVal Cond, bool Assumption);
   void VisitLocation(CheckerContext &C, const Stmt *S, SVal l);
+  virtual void PreVisitBind(CheckerContext &C, const Stmt *AssignE,
+                            const Stmt *StoreE, SVal location,
+                            SVal val);
 
 private:
   void MallocMem(CheckerContext &C, const CallExpr *CE);
+  void MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att);
   const GRState *MallocMemAux(CheckerContext &C, const CallExpr *CE,
                               const Expr *SizeEx, SVal Init,
                               const GRState *state) {
@@ -86,8 +94,9 @@ private:
                               const GRState *state);
 
   void FreeMem(CheckerContext &C, const CallExpr *CE);
+  void FreeMemAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att);
   const GRState *FreeMemAux(CheckerContext &C, const CallExpr *CE,
-                            const GRState *state);
+                            const GRState *state, unsigned Num, bool Hold);
 
   void ReallocMem(CheckerContext &C, const CallExpr *CE);
   void CallocMem(CheckerContext &C, const CallExpr *CE);
@@ -156,7 +165,31 @@ bool MallocChecker::EvalCallExpr(CheckerContext &C, const CallExpr *CE) {
     return true;
   }
 
-  return false;
+  // Check all the attributes, if there are any.
+  // There can be multiple of these attributes.
+  bool rv = false;
+  if (FD->hasAttrs()) {
+    for (const Attr *attr = FD->getAttrs(); attr; attr = attr->getNext()) {
+      if(const OwnershipAttr* Att = dyn_cast<OwnershipAttr>(attr)) {
+        switch (Att->getKind()) {
+        case OwnershipAttr::Returns: {
+          MallocMemReturnsAttr(C, CE, Att);
+          rv = true;
+          break;
+        }
+        case OwnershipAttr::Takes:
+        case OwnershipAttr::Holds: {
+          FreeMemAttr(C, CE, Att);
+          rv = true;
+          break;
+        }
+        default:
+          break;
+        }
+      }
+    }
+  }
+  return rv;
 }
 
 void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) {
@@ -165,6 +198,23 @@ void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) {
   C.addTransition(state);
 }
 
+void MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE,
+                                         const OwnershipAttr* Att) {
+  if (!Att->isModule("malloc"))
+    return;
+
+  const unsigned *I = Att->begin(), *E = Att->end();
+  if (I != E) {
+    const GRState *state =
+        MallocMemAux(C, CE, CE->getArg(*I), UndefinedVal(), C.getState());
+    C.addTransition(state);
+    return;
+  }
+  const GRState *state = MallocMemAux(C, CE, UnknownVal(), UndefinedVal(),
+                                        C.getState());
+  C.addTransition(state);
+}
+
 const GRState *MallocChecker::MallocMemAux(CheckerContext &C,  
                                            const CallExpr *CE,
                                            SVal Size, SVal Init,
@@ -196,25 +246,52 @@ const GRState *MallocChecker::MallocMemAux(CheckerContext &C,
 }
 
 void MallocChecker::FreeMem(CheckerContext &C, const CallExpr *CE) {
-  const GRState *state = FreeMemAux(C, CE, C.getState());
+  const GRState *state = FreeMemAux(C, CE, C.getState(), 0, false);
 
   if (state)
     C.addTransition(state);
 }
 
+void MallocChecker::FreeMemAttr(CheckerContext &C, const CallExpr *CE,
+                                     const OwnershipAttr* Att) {
+  if (!Att->isModule("malloc"))
+    return;
+
+  for (const unsigned *I = Att->begin(), *E = Att->end(); I != E; ++I) {
+    const GRState *state =
+        FreeMemAux(C, CE, C.getState(), *I, Att->isKind(OwnershipAttr::Holds));
+  if (state)
+    C.addTransition(state);
+  }
+}
+
 const GRState *MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
-                                         const GRState *state) {
-  const Expr *ArgExpr = CE->getArg(0);
+                                         const GRState *state, unsigned Num,
+                                         bool Hold) {
+  const Expr *ArgExpr = CE->getArg(Num);
   SVal ArgVal = state->getSVal(ArgExpr);
 
-  // If ptr is NULL, no operation is preformed.
-  if (ArgVal.isZeroConstant())
+  DefinedOrUnknownSVal location = cast<DefinedOrUnknownSVal>(ArgVal);
+
+  // Check for null dereferences.
+  if (!isa<Loc>(location))
     return state;
-  
+
+  // FIXME: Technically using 'Assume' here can result in a path
+  //  bifurcation.  In such cases we need to return two states, not just one.
+  const GRState *notNullState, *nullState;
+  llvm::tie(notNullState, nullState) = state->Assume(location);
+
+  // The explicit NULL case, no operation is performed.
+  if (nullState && !notNullState)
+    return nullState;
+
+  assert(notNullState);
+
   // Unknown values could easily be okay
   // Undefined values are handled elsewhere
   if (ArgVal.isUnknownOrUndef())
-    return state;
+    return notNullState;
 
   const MemRegion *R = ArgVal.getAsRegion();
   
@@ -253,7 +330,7 @@ const GRState *MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
   // Various cases could lead to non-symbol values here.
   // For now, ignore them.
   if (!SR)
-    return state;
+    return notNullState;
 
   SymbolRef Sym = SR->getSymbol();
   
@@ -263,14 +340,15 @@ const GRState *MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
   // called on a pointer that does not get its pointee directly from malloc(). 
   // Full support of this requires inter-procedural analysis.
   if (!RS)
-    return state;
+    return notNullState;
 
   // Check double free.
   if (RS->isReleased()) {
     ExplodedNode *N = C.GenerateSink();
     if (N) {
       if (!BT_DoubleFree)
-        BT_DoubleFree = new BuiltinBug("Double free",
+        BT_DoubleFree
+          = new BuiltinBug("Double free",
                          "Try to free a memory block that has been released");
       // FIXME: should find where it's freed last time.
       BugReport *R = new BugReport(*BT_DoubleFree, 
@@ -281,7 +359,9 @@ const GRState *MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
   }
 
   // Normal free.
-  return state->set<RegionState>(Sym, RefState::getReleased(CE));
+  if (Hold)
+    return notNullState->set<RegionState>(Sym, RefState::getRelinquished(CE));
+  return notNullState->set<RegionState>(Sym, RefState::getReleased(CE));
 }
 
 bool MallocChecker::SummarizeValue(llvm::raw_ostream& os, SVal V) {
@@ -446,13 +526,13 @@ void MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE) {
                                       ValMgr.makeIntValWithPtrWidth(0, false));
 
     if (const GRState *stateSizeZero = stateNotEqual->Assume(SizeZero, true)) {
-      const GRState *stateFree = FreeMemAux(C, CE, stateSizeZero);
+      const GRState *stateFree = FreeMemAux(C, CE, stateSizeZero, 0, false);
       if (stateFree)
         C.addTransition(stateFree->BindExpr(CE, UndefinedVal(), true));
     }
 
     if (const GRState *stateSizeNotZero=stateNotEqual->Assume(SizeZero,false)) {
-      const GRState *stateFree = FreeMemAux(C, CE, stateSizeNotZero);
+      const GRState *stateFree = FreeMemAux(C, CE, stateSizeNotZero, 0, false);
       if (stateFree) {
         // FIXME: We should copy the content of the original buffer.
         const GRState *stateRealloc = MallocMemAux(C, CE, CE->getArg(1), 
@@ -581,3 +661,62 @@ void MallocChecker::VisitLocation(CheckerContext &C, const Stmt *S, SVal l) {
       }
   }
 }
+
+void MallocChecker::PreVisitBind(CheckerContext &C,
+                                 const Stmt *AssignE,
+                                 const Stmt *StoreE,
+                                 SVal location,
+                                 SVal val) {
+  // The PreVisitBind implements the same algorithm as already used by the 
+  // Objective C ownership checker: if the pointer escaped from this scope by 
+  // assignment, let it go.  However, assigning to fields of a stack-storage 
+  // structure does not transfer ownership.
+
+  const GRState *state = C.getState();
+  DefinedOrUnknownSVal l = cast<DefinedOrUnknownSVal>(location);
+
+  // Check for null dereferences.
+  if (!isa<Loc>(l))
+    return;
+
+  // Before checking if the state is null, check if 'val' has a RefState.
+  // Only then should we check for null and bifurcate the state.
+  SymbolRef Sym = val.getLocSymbolInBase();
+  if (Sym) {
+    if (const RefState *RS = state->get<RegionState>(Sym)) {
+      // If ptr is NULL, no operation is performed.
+      const GRState *notNullState, *nullState;
+      llvm::tie(notNullState, nullState) = state->Assume(l);
+
+      // Generate a transition for 'nullState' to record the assumption
+      // that the state was null.
+      if (nullState)
+        C.addTransition(nullState);
+
+      if (!notNullState)
+        return;
+
+      if (RS->isAllocated()) {
+        // Something we presently own is being assigned somewhere.
+        const MemRegion *AR = location.getAsRegion();
+        if (!AR)
+          return;
+        AR = AR->StripCasts()->getBaseRegion();
+        do {
+          // If it is on the stack, we still own it.
+          if (AR->hasStackNonParametersStorage())
+            break;
+
+          // If the state can't represent this binding, we still own it.
+          if (notNullState == (notNullState->bindLoc(cast<Loc>(location), UnknownVal())))
+            break;
+
+          // We no longer own this pointer.
+          notNullState = notNullState->set<RegionState>(Sym, RefState::getRelinquished(StoreE));
+        }
+        while (false);
+      }
+      C.addTransition(notNullState);
+    }
+  }
+}

clang/lib/Lex/PPMacroExpansion.cpp

@@ -506,6 +506,9 @@ static bool HasFeature(const Preprocessor &PP, const IdentifierInfo *II) {
            .Case("cxx_static_assert", LangOpts.CPlusPlus0x)
            .Case("objc_nonfragile_abi", LangOpts.ObjCNonFragileABI)
            .Case("objc_weak_class", LangOpts.ObjCNonFragileABI)
+           .Case("ownership_holds", true)
+           .Case("ownership_returns", true)
+           .Case("ownership_takes", true)
          //.Case("cxx_concepts", false)
          //.Case("cxx_lambdas", false)
          //.Case("cxx_nullptr", false)

clang/lib/Parse/AttributeList.cpp

@@ -118,6 +118,9 @@ AttributeList::Kind AttributeList::getKind(const IdentifierInfo *Name) {
     .Case("ns_returns_retained", AT_ns_returns_retained)
     .Case("cf_returns_not_retained", AT_cf_returns_not_retained)
     .Case("cf_returns_retained", AT_cf_returns_retained)
+    .Case("ownership_returns", AT_ownership_returns)
+    .Case("ownership_holds", AT_ownership_holds)
+    .Case("ownership_takes", AT_ownership_takes)
     .Case("reqd_work_group_size", AT_reqd_wg_size)
     .Case("init_priority", AT_init_priority)
     .Case("no_instrument_function", AT_no_instrument_function)

clang/lib/Sema/SemaDeclAttr.cpp

@@ -346,10 +346,175 @@ static void HandleNonNullAttr(Decl *d, const AttributeList &Attr, Sema &S) {
 
   unsigned* start = &NonNullArgs[0];
   unsigned size = NonNullArgs.size();
-  std::sort(start, start + size);
+  llvm::array_pod_sort(start, start + size);
   d->addAttr(::new (S.Context) NonNullAttr(S.Context, start, size));
 }
 
+static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
+  // This attribute must be applied to a function declaration.
+  // The first argument to the attribute must be a string,
+  // the name of the resource, for example "malloc".
+  // The following arguments must be argument indexes, the arguments must be
+  // of integer type for Returns, otherwise of pointer type.
+  // The difference between Holds and Takes is that a pointer may still be used
+  // after being held.  free() should be __attribute((ownership_takes)), whereas a list
+  // append function may well be __attribute((ownership_holds)).
+
+  if (!AL.getParameterName()) {
+    S.Diag(AL.getLoc(), diag::err_attribute_argument_n_not_string)
+        << AL.getName()->getName() << 1;
+    return;
+  }
+  // Figure out our Kind, and check arguments while we're at it.
+  OwnershipAttr::OwnershipKind K = OwnershipAttr::None;
+  if (AL.getName()->getName().equals("ownership_takes")) {
+    K = OwnershipAttr::Takes;
+    if (AL.getNumArgs() < 1) {
+      S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << 2;
+      return;
+    }
+  } else if (AL.getName()->getName().equals("ownership_holds")) {
+    K = OwnershipAttr::Holds;
+    if (AL.getNumArgs() < 1) {
+      S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << 2;
+      return;
+    }
+  } else if (AL.getName()->getName().equals("ownership_returns")) {
+    K = OwnershipAttr::Returns;
+    if (AL.getNumArgs() > 1) {
+      S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments)
+          << AL.getNumArgs() + 1;
+      return;
+    }
+  }
+  // This should never happen given how we are called.
+  if (K == OwnershipAttr::None) {
+    return;
+  }
+
+  if (!isFunction(d) || !hasFunctionProto(d)) {
+    S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type) << AL.getName()
+        << 0 /*function*/;
+    return;
+  }
+
+  unsigned NumArgs = getFunctionOrMethodNumArgs(d);
+
+  llvm::StringRef Module = AL.getParameterName()->getName();
+
+  // Normalize the argument, __foo__ becomes foo.
+  if (Module.startswith("__") && Module.endswith("__"))
+    Module = Module.substr(2, Module.size() - 4);
+
+  llvm::SmallVector<unsigned, 10> OwnershipArgs;
+
+  for (AttributeList::arg_iterator I = AL.arg_begin(), E = AL.arg_end(); I != E; ++I) {
+
+    Expr *IdxExpr = static_cast<Expr *>(*I);
+    llvm::APSInt ArgNum(32);
+    if (IdxExpr->isTypeDependent() || IdxExpr->isValueDependent()
+        || !IdxExpr->isIntegerConstantExpr(ArgNum, S.Context)) {
+      S.Diag(AL.getLoc(), diag::err_attribute_argument_not_int)
+          << AL.getName()->getName() << IdxExpr->getSourceRange();
+      continue;
+    }
+
+    unsigned x = (unsigned) ArgNum.getZExtValue();
+
+    if (x > NumArgs || x < 1) {
+      S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
+          << AL.getName()->getName() << x << IdxExpr->getSourceRange();
+      continue;
+    }
+    --x;
+    switch (K) {
+    case OwnershipAttr::Takes:
+    case OwnershipAttr::Holds: {
+      // Is the function argument a pointer type?
+      QualType T = getFunctionOrMethodArgType(d, x);
+      if (!T->isAnyPointerType() && !T->isBlockPointerType()) {
+        // FIXME: Should also highlight argument in decl.
+        S.Diag(AL.getLoc(), diag::err_ownership_type)
+            << ((K==OwnershipAttr::Takes)?"ownership_takes":"ownership_holds")
+            << "pointer"
+            << IdxExpr->getSourceRange();
+        continue;
+      }
+      break;
+    }
+    case OwnershipAttr::Returns: {
+      if (AL.getNumArgs() > 1) {
+          // Is the function argument an integer type?
+          Expr *IdxExpr = static_cast<Expr *>(AL.getArg(0));
+          llvm::APSInt ArgNum(32);
+          if (IdxExpr->isTypeDependent() || IdxExpr->isValueDependent()
+              || !IdxExpr->isIntegerConstantExpr(ArgNum, S.Context)) {
+            S.Diag(AL.getLoc(), diag::err_ownership_type)
+                << "ownership_returns" << "integer"
+                << IdxExpr->getSourceRange();
+            return;
+          }
+      }
+      break;
+    }
+    // Should never happen, here to silence a warning.
+    default: {
+      return;
+    }
+    } // switch
+
+    // Check we don't have a conflict with another ownership attribute.
+    if (d->hasAttrs()) {
+      for (const Attr *attr = d->getAttrs(); attr; attr = attr->getNext()) {
+        if (const OwnershipAttr* Att = dyn_cast<OwnershipAttr>(attr)) {
+          // Two ownership attributes of the same kind can't conflict,
+          // except returns attributes.
+          if (K == OwnershipAttr::Returns || Att->getKind() != K) {
+            for (const unsigned *I = Att->begin(), *E = Att->end(); I != E; ++I) {
+              if (x == *I) {
+                S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible)
+                    << AL.getName()->getName() << "ownership_*";
+              }
+            }
+          }
+        }
+      }
+    }
+    OwnershipArgs.push_back(x);
+  }
+
+  unsigned* start = OwnershipArgs.data();
+  unsigned size = OwnershipArgs.size();
+  llvm::array_pod_sort(start, start + size);
+  switch (K) {
+  case OwnershipAttr::Takes: {
+    if (OwnershipArgs.empty()) {
+      S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << 2;
+      return;
+    }
+    d->addAttr(::new (S.Context) OwnershipTakesAttr(S.Context, start, size,
+                                                    Module));
+    break;
+  }
+  case OwnershipAttr::Holds: {
+    if (OwnershipArgs.empty()) {
+      S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << 2;
+      return;
+    }
+    d->addAttr(::new (S.Context) OwnershipHoldsAttr(S.Context, start, size,
+                                                    Module));
+    break;
+  }
+  case OwnershipAttr::Returns: {
+    d->addAttr(::new (S.Context) OwnershipReturnsAttr(S.Context, start, size,
+                                                      Module));
+    break;
+  }
+  default:
+    break;
+  }
+}
+
 static bool isStaticVarOrStaticFunciton(Decl *D) {
   if (VarDecl *VD = dyn_cast<VarDecl>(D))
     return VD->getStorageClass() == VarDecl::Static;
@@ -379,7 +544,7 @@ static void HandleWeakRefAttr(Decl *d, const AttributeList &Attr, Sema &S) {
     Ctx = Ctx->getLookupContext();
     if (!isa<TranslationUnitDecl>(Ctx) && !isa<NamespaceDecl>(Ctx) ) {
       S.Diag(Attr.getLoc(), diag::err_attribute_weakref_not_global_context) <<
-	dyn_cast<NamedDecl>(d)->getNameAsString();
+    dyn_cast<NamedDecl>(d)->getNameAsString();
       return;
     }
   }
@@ -2009,6 +2174,10 @@ static void ProcessDeclAttribute(Scope *scope, Decl *D,
   case AttributeList::AT_mode:        HandleModeAttr        (D, Attr, S); break;
   case AttributeList::AT_malloc:      HandleMallocAttr      (D, Attr, S); break;
   case AttributeList::AT_nonnull:     HandleNonNullAttr     (D, Attr, S); break;
+  case AttributeList::AT_ownership_returns:
+  case AttributeList::AT_ownership_takes:
+  case AttributeList::AT_ownership_holds:
+      HandleOwnershipAttr     (D, Attr, S); break;
   case AttributeList::AT_noreturn:    HandleNoReturnAttr    (D, Attr, S); break;
   case AttributeList::AT_nothrow:     HandleNothrowAttr     (D, Attr, S); break;
   case AttributeList::AT_override:    HandleOverrideAttr    (D, Attr, S); break;
@@ -2087,7 +2256,7 @@ void Sema::ProcessDeclAttributeList(Scope *S, Decl *D, const AttributeList *Attr
   // but that looks really pointless. We reject it.
   if (D->hasAttr<WeakRefAttr>() && !D->hasAttr<AliasAttr>()) {
     Diag(AttrList->getLoc(), diag::err_attribute_weakref_without_alias) <<
-	dyn_cast<NamedDecl>(D)->getNameAsString();
+    dyn_cast<NamedDecl>(D)->getNameAsString();
     return;
   }
 }

clang/test/Analysis/malloc.c

@@ -4,22 +4,136 @@ void *malloc(size_t);
 void free(void *);
 void *realloc(void *ptr, size_t size);
 void *calloc(size_t nmemb, size_t size);
+void __attribute((ownership_returns(malloc))) *my_malloc(size_t);
+void __attribute((ownership_takes(malloc, 1))) my_free(void *);
+void __attribute((ownership_returns(malloc, 1))) *my_malloc2(size_t);
+void __attribute((ownership_holds(malloc, 1))) my_hold(void *);
+
+// Duplicate attributes are silly, but not an error.
+// Duplicate attribute has no extra effect.
+// If two are of different kinds, that is an error and reported as such. 
+void __attribute((ownership_holds(malloc, 1)))
+__attribute((ownership_holds(malloc, 1)))
+__attribute((ownership_holds(malloc, 3))) my_hold2(void *, void *, void *);
+void *my_malloc3(size_t);
+void *myglobalpointer;
+struct stuff {
+  void *somefield;
+};
+struct stuff myglobalstuff;
 
 void f1() {
   int *p = malloc(12);
   return; // expected-warning{{Allocated memory never released. Potential memory leak.}}
 }
 
-void f1_b() {
-  int *p = malloc(12); // expected-warning{{Allocated memory never released. Potential memory leak.}}
-}
-
 void f2() {
   int *p = malloc(12);
   free(p);
   free(p); // expected-warning{{Try to free a memory block that has been released}}
 }
 
+// ownership attributes tests
+void naf1() {
+  int *p = my_malloc3(12);
+  return; // no-warning
+}
+
+void n2af1() {
+  int *p = my_malloc2(12);
+  return; // expected-warning{{Allocated memory never released. Potential memory leak.}}
+}
+
+void af1() {
+  int *p = my_malloc(12);
+  return; // expected-warning{{Allocated memory never released. Potential memory leak.}}
+}
+
+void af1_b() {
+  int *p = my_malloc(12); // expected-warning{{Allocated memory never released. Potential memory leak.}}
+}
+
+void af1_c() {
+  myglobalpointer = my_malloc(12); // no-warning
+}
+
+void af1_d() {
+  struct stuff mystuff;
+  mystuff.somefield = my_malloc(12); // expected-warning{{Allocated memory never released. Potential memory leak.}}
+}
+
+// Test that we can pass out allocated memory via pointer-to-pointer.
+void af1_e(void **pp) {
+  *pp = my_malloc(42); // no-warning
+}
+
+void af1_f(struct stuff *somestuff) {
+  somestuff->somefield = my_malloc(12); // no-warning
+}
+
+// Allocating memory for a field via multiple indirections to our arguments is OK.
+void af1_g(struct stuff **pps) {
+  *pps = my_malloc(sizeof(struct stuff)); // no-warning
+  (*pps)->somefield = my_malloc(42); // no-warning
+}
+
+void af2() {
+  int *p = my_malloc(12);
+  my_free(p);
+  free(p); // expected-warning{{Try to free a memory block that has been released}}
+}
+
+void af2b() {
+  int *p = my_malloc(12);
+  free(p);
+  my_free(p); // expected-warning{{Try to free a memory block that has been released}}
+}
+
+void af2c() {
+  int *p = my_malloc(12);
+  free(p);
+  my_hold(p); // expected-warning{{Try to free a memory block that has been released}}
+}
+
+void af2d() {
+  int *p = my_malloc(12);
+  free(p);
+  my_hold2(0, 0, p); // expected-warning{{Try to free a memory block that has been released}}
+}
+
+// No leak if malloc returns null.
+void af2e() {
+  int *p = my_malloc(12);
+  if (!p)
+    return; // no-warning
+  free(p); // no-warning
+}
+
+// This case would inflict a double-free elsewhere.
+// However, this case is considered an analyzer bug since it causes false-positives.
+void af3() {
+  int *p = my_malloc(12);
+  my_hold(p);
+  free(p); // no-warning
+}
+
+// This case would inflict a double-free elsewhere.
+// However, this case is considered an analyzer bug since it causes false-positives.
+int * af4() {
+  int *p = my_malloc(12);
+  my_free(p);
+  return p; // no-warning
+}
+
+// This case is (possibly) ok, be conservative
+int * af5() {
+  int *p = my_malloc(12);
+  my_hold(p);
+  return p; // no-warning
+}
+
+
+
 // This case tests that storing malloc'ed memory to a static variable which is
 // then returned is not leaked.  In the absence of known contracts for functions
 // or inter-procedural analysis, this is a conservative answer.