On Mar 8, 2013, at 15:06 , Anton Yartsev <[email protected]> wrote:
> On 09.03.2013 2:51, Anton Yartsev wrote: >> On 08.03.2013 23:25, Jordan Rose wrote: >>> >>> On Mar 8, 2013, at 11:22 , Anna Zaks <[email protected]> wrote: >>> >>>> Anton, >>>> >>>> We've briefly discussed this with Jordan. Below are the cases we should >>>> handle: >>>> >>>> Case 1: The selector starts with "dataWithBytesNoCopy" or >>>> "initWithBytesNoCopy" or "initWithCharactersNoCopy" and "freeWhenDone" is >>>> not set to "0". >>>> We should assume that the call preforms hold action from malloc family. >>>> (So the pointer should not escape and we should model this during >>>> ObjCMsgCall processing) >>> >>> To explain the rationalization here, Anna pointed out that while it's >>> likely that methods with a "freeWhenDone:" or "...NoCopy:" selector piece >>> all behave as ownership-holders, we can't actually prove it. In particular, >>> if/when MallocChecker gains the ability to reason about custom allocators, >>> someone could very well use "...NoCopy" to mean "I will free this using my >>> custom deallocator", and we don't want to produce a bogus allocator >>> mismatch bug in that case. >>> >>> By the way, by "the selector starts with __", we mean the existing logic of >>> "the first selector piece is __", not "the first selector piece starts with >>> __". >>> >>> Thanks for bearing with us. >>> Jordan >> Aaaa, got it! >> Updated the patch. >> >> > Case 1: The selector starts with "dataWithBytesNoCopy" or >> > "initWithBytesNoCopy" or "initWithCharactersNoCopy" and "freeWhenDone" is >> > not set to "0". >> > We should assume that the call preforms hold action from malloc family. >> > (So the pointer should not escape and we should model this during >> > ObjCMsgCall processing) >> > >> > Case 2: The selector contains "freeWhenDone", which is set to '0'. >> > The call results in no escape of the pointer. >> > >> > Case 3: Otherwise (this is not Case 1 nor Case 3) and the selector ends >> > with "NoCopy" >> > The pointer should escape. >> > >> > Please, work off your previous patch, but change the logic to handle these >> > cases and include the tests. Also, I think it's >> > important to include a commit message with your patch so that there would >> > be no ambiguity in what the patch is trying to accomplish. >> > Usually, the commit message and the tests would resolve that. >> I'll put this cases in a commit message. >> -- >> Anton > Or even with a better name for doesNotFreeMemory() and comment. Thanks, Anton; committed in r176744! We thought of another test case and so I had to restructure it a bit, but I think after rebasing we can move forward with your main patch. Jordan
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
