CPP: Fix FPs for 'Resource not released in destructor' involving virtual method calls

[geoffw0]

Fixes https://jira.semmle.com/browse/CPP-308. The fix is to account for calls to virtual methods releasing memory via overriding function definitions in derived classes.

@jbj I'm tempted to move the fix into Function.qll with the addition of a function:

  Function getAPossibleTarget() {
    result = getTarget() or
    (
      result = getTarget().(MemberFunction).getAnOverridingFunction*() and
      exists(getQualifier()) // exclude overrides when the target is fixed
    )
  }

This comes very close to your ideas about a virtual dispatch library, however my solution appears rather trivial (and I'm not completely sure about the last line). How much complexity do you think there would need to be in such a library? What can we do better than the above?

[jbj]

Without a proper virtual-dispatch library you'll get false negatives in the test if you define a class MyClass5b : MyClass4 that overrides Release to do nothing. When you then release a MyClass5b that has static type MyClass4, it'll appear to be correctly deleted because a correct delete exists in a subclass of MyClass4.

I can probably be persuaded to merge a stub virtual-dispatch library since it'll be better than having nothing at all. I'm wary of adding it to Function.qll, though. I find that all heuristic analysis we've added to the basic AST classes so far has become out of date quickly and is hard to change without breaking existing code that relies on the bad behaviour. Earlier this year I improved Call.passesByReference, and it took a lot of iterations to fix all the code that was relying on its old broken behaviour. C# has a getARuntimeTarget predicate on Call, but Java requires you to explicitly call the virtual-dispatch library, and I think we should follow Java.

[jbj]

How is performance?

[jbj]

These two * can be +, right?

[geoffw0]

Yes. Done.

[geoffw0]

When you then release a MyClass5b that has static type MyClass4, it'll appear to be correctly deleted because a correct delete exists in a subclass of MyClass4.

I've added some test cases that verify this behaviour.

I can probably be persuaded to merge a stub virtual-dispatch library
...
I'm wary of adding it to Function.qll, though

OK, let's do a proper library at a later date then.

How is performance?

Not too bad:

qt before: 137s, 233 results
qt after: 153s, 228 results
wireshark before: 203s, 445 results
wireshark after: 192s, 445 results

Note that leakedInSameMethod is quite slow:

	AV Rule 79.ql-3:AV Rule 79::leakedInSameMethod#ff ............................... 1m21s

due to the a.getEnclosingFunction() = b.getEnclosingFunction() pattern. This is just because I don't yet have #580.