CPP: Fix a bug in NewFree.qll

[geoffw0]

See https://discuss.lgtm.com/t/malloc-delete-mismatch-false-positive/1914.

The issue was that the call to tmp.data() was being misrecognized as an allocation call, due to over-enthusiastic data-flow tracking. The fix is to only use local data flow for the purposes of identifying malloc-like functions (whereas global flow is still used to track between the malloc and free).

Diff query: https://lgtm.com/query/3658696700719411428/

[jbj]

How is performance on the snapshots that recently made this query act up?

[jbj]

I must be missing something here. I'd expect myPointer2.getPointer() to return pointer from myPointer2, which is set in the constructor call myPointer2(myBuffer), and myBuffer uses malloc. So why is this // GOOD?

[geoffw0]

The test was supposed to delete myPointer3, sorry (the point being that the query can't tell the difference, so has to be conservative in this case). Updated.

[jbj]

Do you think this bug is new in 1.20?

[geoffw0]

How is performance on the snapshots that recently made this query act up?

I just ran the three queries that use NewDelete.qll on wireshark. The first query goes from 643s -> 809s with these changes. The other two (with a warmed up cache) stay at around 21s and 12s. Similar results for one of the projects we had problems with recently - Singular (167s -> 226s on the first query). And finally 'qtbindings' I don't have and can't get a snapshot of, but qt behaves as expected (379s -> 470s on the first query).

It's likely that with a warmed up cache these queries perform well. In the long run we might replace the remaining SSA in this library with dataflow, which I'd expect to improve performance.

Do you think this bug is new in 1.20?

I don't think so, though we have made some changes to the query lately. It's certainly plausible that the particular FP in the ticket above didn't show up until recently 1.20.

[jbj]

I'm not worried about performance on a cold cache. The data flow library has to be computed at some point in the suite anyway, so the suite won't get slower just because this query uses it. If the second and third queries take 21s and 12s, that means NewFree.qll is likely still fast since there's no cached in that file.

[geoffw0]

Yep I agree.