[llvm] r193412 - Call destroy from ~BasicCallGraph.

[Chris Lattner]

On Oct 30, 2013, at 12:16 AM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Rafael Espindola wrote:
>> Author: rafael
>> Date: Fri Oct 25 10:01:34 2013
>> New Revision: 193412
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=193412&view=rev
>> Log:
>> Call destroy from ~BasicCallGraph.
>> 
>> This fix a memory leak found by valgrind.
>> 
>> Calling it from the base class destructor would not destroy the BasicCallGraph
>> bits.
>> 
>> FIXME: BasicCallGraph is the only thing that inherits from CallGraph. Can
>> we merge the two?
> 
> That's an interesting question. What would a derived CallGraph be allowed to do which BasicCallGraph doesn't?
> 
> Can we have call sites in functions that are declared but not defined? I don't think so because the call edge itself is represented with a CallInst/InvokeInst. The API is written in terms of llvm::CallSite.
> 
> Could we have a CallGraph which does complex analysis to resolve indirect call sites? I don't think so, users of the CallGraph API assume that edges are guaranteed to be direct and will crash if they aren't.
> 
> We could replace getRoot() but does that help? Can I have a Module which would be miscompiled if I used a BasicCG? And what to getExternalCallingNode / getCallsExternalNode do? They return a CallGraphNode but again the API is still written in terms of CallSites, and those CallSites can't refer to things that aren't in the Module anyways?

The idea was that a more sophisticated call graph analysis (e.g. based on points-to, or higher level information encoded in metadata) could provide a more refined callgraph.

This dated back to when I was working on DSA.  I don't know of anything that actually used this though, I'd be perfectly fine with merging them together into a single class.

-Chris