Since its important to know whether a function frees memory (even if its a reallocating function!), I used two CallDescriptionMaps to merge all CallDescriptions into it. MemFunctionInfoTy no longer makes sense, it may never have, but for now, it would be more of a distraction then anything else.
It may be more precise to call this isCMemCall, because that's the question it answers.
Ugh, no idea whats happening here, I'll be honest. I sweat to dig a bit deeper to find out, though I didn't want to bother delaying the publishing this patch for a feature not documented, or used anywhere.
Whenever i see a (CE, State) pair, it screams CallEvent to me. That said, i'm worried that State in these callbacks isn't necessarily equal to C.getState() (the latter, by the way, is always equal to the CallEvent's .getState() - that's a relief, right?), so if you'll ever be in the mood to check that, that'd be great :)
I think alloca deserves CDF_MaybeBuiltin. This would also probably allow you to remove the second line.
These are also builtins.
And these too.
I think this is about the (lack of) support for __attribute__((malloc)).
It should be always equal to it. I'll change it.
Actually, BuiltinFunctionChecker uses evalCall to create an AllocaRegion for __builtin_alloca. I spent an hour when writing this patch to track a crash down when I initially made this CDF_MaybeBuiltin :)
Hmmm, I tried making this take a (CallEvent, CheckerContext), but it bloated the code for little gain, since every handler function would start with the retrieval of the state and the call expression. That said, I could cascade these down to FreeMemAux, MallocMemAux, etc, to also take this pair instead, but I'll be honest, I don't see point until something actually breaks.
This is the standard way in the checkers: almost every handler function starts with the retrieval of the state from the CheckerContext. The only reason for an extra State parameter is that sometimes we create more states in the lower level functions but only add the final one to the CheckerContext as a new transition. Does something like this happen here?
Maybe return !!FreeingMemFnMap.lookup(Call)?
Same here: why not just return FreeingMemFnMap.lookup(Call) || NonFreeingMemFn.lookup(Call)?
I think my strdup comments weren't addressed >.<
I agree with @baloghadamsoftware here. If you pass State explicitly, it looks like an indication for the reader that this State may be different from C.getState(). If in practice they're the same, i'd rather do C.getState() in every method than confuse the reader.
Also do you remember what makes us query CallExpr so often? Given that CallEvent includes CallExpr, we should be able to expose everything we need as CallEvent methods. Say, we should be able to replace MallocMemAux(C, CE, CE->getArg(0), ...) with MallocMemAux(C, Call, Call.getArgExpr(0), ...) and only do Call.getOriginExpr() once when we report a bug.