[cfe-dev] Ownership attribute for malloc etc. checking

[Jordy Rose]

I do think this is pretty cool stuff -- like Ted said, being able to
generalize the leak-checking code would be nice (perhaps even unifying
malloc/free and Cocoa retain counting someday).

I have a number of minor suggestions, such as asking the attribute which
arguments matter instead of iterating over them all and seeing if they're
important, and following the LLVM comment style (complete, capitalized
sentences ending in [.?!] whenever possible). Also using "!" instead of
"not". *grin*

I think you have a logic error dealing with held memory. You're allowing a
double-free if the current free is a hold, but I think what you want is to
allow /uses/ if the /previous/ free was a hold. Perhaps the RefState kind
needs a new enum value, Relinquished or something.

It'd be nice to write a couple of tests for this new attribute. It's
probably sufficiently new to warrant its own test file in the test/Analysis
directory.

Secretly I would love for the module name /not/ to be checked at all, only
associated with a pointer when ownership_returns is encountered. It would
then be expected that any non-escaping marked pointer is cleaned up by the
end of the function body. This would let people define their own modules
without any extra configuration at all.

const char * __attribute__((ownership_returns(my_app_intern)))
intern(const char *);
void __attribute__((ownership_takes(my_app_intern))) release(const char
*);

void useInput (const char *input) {
  const char *str = intern(input);
  // do complicated stuff with str
  release(str);
}

The downside, of course, is that /any/ use of the pointer as a function
argument would count as escaping, making it not very useful. Perhaps there
could be heuristics for this sort of thing -- a non-builtin module name
/must/ be balanced or explicitly escape, or else be passed to a free-like
function. For things like "malloc" and "fopen", we probably have to be more
lenient.

If this is extended to work with multiple owners, you'll also run into the
problem of two kinds of "hold" -- in both cases, there's a new owner, but
the current owner doesn't necessarily give up ownership. But that's a
problem for another day.

Last, as a small note, I don't think ownership_returns should require a
size argument at all -- it falls down for something as simple as this:

struct Node *allocNode() {
  return (struct Node *)malloc(sizeof(struct Node));
}

I see you're using MallocMemAux(); if you use the second form, which takes
a size SVal instead of an Expr*, then you can handle cases where the size
is unknown.

Overall, I like it! Perhaps clean it up a bit, but having a general
"ownership checker" seems like a great idea.

Jordy