[cfe-commits] r109939 - in /cfe/trunk: include/clang/AST/Attr.h include/clang/Basic/Attr.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Parse/AttributeList.h lib/AST/AttrImpl.cpp lib/Checker/MallocChecker.cpp lib/Lex/PPMacroExpansion.cpp lib/Parse/AttributeList.cpp lib/Sema/SemaDeclAttr.cpp test/Analysis/malloc.c
Ted Kremenek
kremenek at apple.com
Fri Jul 30 18:52:12 PDT 2010
Author: kremenek
Date: Fri Jul 30 20:52:11 2010
New Revision: 109939
URL: http://llvm.org/viewvc/llvm-project?rev=109939&view=rev
Log:
After a lengthy design discussion, add support for "ownership attributes" for malloc/free checking. Patch by Andrew McGregor!
Modified:
cfe/trunk/include/clang/AST/Attr.h
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Parse/AttributeList.h
cfe/trunk/lib/AST/AttrImpl.cpp
cfe/trunk/lib/Checker/MallocChecker.cpp
cfe/trunk/lib/Lex/PPMacroExpansion.cpp
cfe/trunk/lib/Parse/AttributeList.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Analysis/malloc.c
Modified: cfe/trunk/include/clang/AST/Attr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Attr.h?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Attr.h (original)
+++ cfe/trunk/include/clang/AST/Attr.h Fri Jul 30 20:52:11 2010
@@ -369,6 +369,121 @@
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:
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Jul 30 20:52:11 2010
@@ -297,6 +297,21 @@
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"];
}
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jul 30 20:52:11 2010
@@ -838,6 +838,8 @@
"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<
Modified: cfe/trunk/include/clang/Parse/AttributeList.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/AttributeList.h?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/AttributeList.h (original)
+++ cfe/trunk/include/clang/Parse/AttributeList.h Fri Jul 30 20:52:11 2010
@@ -97,6 +97,9 @@
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,
Modified: cfe/trunk/lib/AST/AttrImpl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/AttrImpl.cpp?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/lib/AST/AttrImpl.cpp (original)
+++ cfe/trunk/lib/AST/AttrImpl.cpp Fri Jul 30 20:52:11 2010
@@ -48,6 +48,43 @@
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 @@
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);
}
Modified: cfe/trunk/lib/Checker/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/MallocChecker.cpp?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/MallocChecker.cpp (original)
+++ cfe/trunk/lib/Checker/MallocChecker.cpp Fri Jul 30 20:52:11 2010
@@ -24,15 +24,17 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
// 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 @@
// 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 @@
}
// 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 @@
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::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);
+ }
+ }
+}
Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
+++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Fri Jul 30 20:52:11 2010
@@ -506,6 +506,9 @@
.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)
Modified: cfe/trunk/lib/Parse/AttributeList.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/AttributeList.cpp?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/AttributeList.cpp (original)
+++ cfe/trunk/lib/Parse/AttributeList.cpp Fri Jul 30 20:52:11 2010
@@ -118,6 +118,9 @@
.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)
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Jul 30 20:52:11 2010
@@ -346,10 +346,175 @@
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 @@
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 @@
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 @@
// 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;
}
}
Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=109939&r1=109938&r2=109939&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Fri Jul 30 20:52:11 2010
@@ -4,22 +4,136 @@
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.
More information about the cfe-commits
mailing list