On 07.03.2013 21:29, Jordan Rose wrote:
On Mar 7, 2013, at 4:47 , Anton Yartsev <[email protected]> wrote:
On 06.03.2013 21:46, Anna Zaks wrote:
CC-ing the patch author!
Also,
Could you split out the ObjC NoCopy + FreeWhenDone change into a separate
patch. It does not seem to be directly related to the other changes. Also, I am
not 100% sure what changes we are making there. One part is refactoring,
however, you've also removed the check for the message calls from the
doesNotFreeMemory(). Do we expect any behavior changes from this?
(Sorry if I've missed something; the patch is getting big.)
Splitted ObjC NoCopy + FreeWhenDone change, kept changes in doesNotFreeMemory().
The logic of doesNotFreeMemory() was broken - it treated all 'NoCopy' and
'FreeWhenDone==1' methods as freeing memory and unknown to us. This lead to
removal of RefState from the State and impossibility for further alloc/dealloc
matching analysis.
Can we just do the Objective-C part first? Can you send that patch too?
Attached. I'll update the main patch after this one gets in.
Have not found any suitable place in lib/Frontend/CompilerInvocation.cpp
Did you mean to update clang/lib/Driver/Tools.cpp ?
Whoops, I grepped for "unix" and found the comment there. You're right, and
that's the right change.
--
Anton
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp (revision 176639)
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp (working copy)
@@ -168,12 +168,14 @@
private:
void initIdentifierInfo(ASTContext &C) const;
+ ///@{
/// Check if this is one of the functions which can allocate/reallocate memory
/// pointed to by one of its arguments.
bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const;
bool isFreeFunction(const FunctionDecl *FD, ASTContext &C) const;
bool isAllocationFunction(const FunctionDecl *FD, ASTContext &C) const;
-
+ bool isObjCNoCopyFreeWhenDone(const ObjCMethodCall &Call) const;
+ ///@}
static ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
const CallExpr *CE,
const OwnershipAttr* Att);
@@ -439,6 +441,24 @@
return false;
}
+bool MallocChecker::isObjCNoCopyFreeWhenDone(const ObjCMethodCall &Call) const {
+ // If the first selector ends with NoCopy, assume that the memory will be
+ // released with 'free' by the new object.
+ // Ex: [NSData dataWithBytesNoCopy:bytes length:10];
+ // Unless 'freeWhenDone' param set to 0.
+ Selector S = Call.getSelector();
+
+ if (!S.getNameForSlot(0).endswith("NoCopy"))
+ return false;
+
+ for (unsigned i = 1; i < S.getNumArgs(); ++i)
+ if (S.getNameForSlot(i).equals("freeWhenDone"))
+ if (Call.getArgSVal(i).isConstant(0))
+ return false;
+
+ return true;
+}
+
void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
if (C.wasInlined)
return;
@@ -495,40 +515,23 @@
C.addTransition(State);
}
-static bool isFreeWhenDoneSetToZero(const ObjCMethodCall &Call) {
- Selector S = Call.getSelector();
- for (unsigned i = 1; i < S.getNumArgs(); ++i)
- if (S.getNameForSlot(i).equals("freeWhenDone"))
- if (Call.getArgSVal(i).isConstant(0))
- return true;
-
- return false;
-}
-
void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call,
CheckerContext &C) const {
+
if (C.wasInlined)
return;
- // If the first selector is dataWithBytesNoCopy, assume that the memory will
- // be released with 'free' by the new object.
- // Ex: [NSData dataWithBytesNoCopy:bytes length:10];
- // Unless 'freeWhenDone' param set to 0.
- // TODO: Check that the memory was allocated with malloc.
- bool ReleasedAllocatedMemory = false;
- Selector S = Call.getSelector();
- if ((S.getNameForSlot(0) == "dataWithBytesNoCopy" ||
- S.getNameForSlot(0) == "initWithBytesNoCopy" ||
- S.getNameForSlot(0) == "initWithCharactersNoCopy") &&
- !isFreeWhenDoneSetToZero(Call)){
- unsigned int argIdx = 0;
- ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(argIdx),
- Call.getOriginExpr(), C.getState(), true,
- ReleasedAllocatedMemory,
- /* RetNullOnFailure*/ true);
+ if (!isObjCNoCopyFreeWhenDone(Call))
+ return;
- C.addTransition(State);
- }
+ bool ReleasedAllocatedMemory;
+ unsigned int argIdx = 0;
+ ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(argIdx),
+ Call.getOriginExpr(), C.getState(), true,
+ ReleasedAllocatedMemory,
+ /* RetNullOnFailure*/ true);
+
+ C.addTransition(State);
}
ProgramStateRef MallocChecker::MallocMemReturnsAttr(CheckerContext &C,
@@ -1378,37 +1381,22 @@
if (!Call->isInSystemHeader() || Call->hasNonZeroCallbackArg())
return false;
- Selector S = Msg->getSelector();
-
- // Whitelist the ObjC methods which do free memory.
- // - Anything containing 'freeWhenDone' param set to 1.
- // Ex: dataWithBytesNoCopy:length:freeWhenDone.
- for (unsigned i = 1; i < S.getNumArgs(); ++i) {
- if (S.getNameForSlot(i).equals("freeWhenDone")) {
- if (Call->getArgSVal(i).isConstant(1))
- return false;
- else
- return true;
- }
- }
-
- // If the first selector ends with NoCopy, assume that the ownership is
- // transferred as well.
- // Ex: [NSData dataWithBytesNoCopy:bytes length:10];
- StringRef FirstSlot = S.getNameForSlot(0);
- if (FirstSlot.endswith("NoCopy"))
- return false;
-
// If the first selector starts with addPointer, insertPointer,
// or replacePointer, assume we are dealing with NSPointerArray or similar.
// This is similar to C++ containers (vector); we still might want to check
// that the pointers get freed by following the container itself.
+ StringRef FirstSlot = Msg->getSelector().getNameForSlot(0);
if (FirstSlot.startswith("addPointer") ||
FirstSlot.startswith("insertPointer") ||
FirstSlot.startswith("replacePointer")) {
return false;
}
+ // Return true for methods with the first selector ending with NoCopy and
+ // 'freeWhenDone' param set to 1. This methods are known to free memory but
+ // we reason about them. isObjCNoCopyFreeWhenDone() can be used to detect
+ // this methods.
+
// Otherwise, assume that the method does not free memory.
// Most framework methods do not free memory.
return true;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
