Opened as draft since it builds on #8515. Only 45a29cb is new at the moment.

@geoffw0 there's some new false positives in the CWE-611 tests, and I'm not sure whether they're an issue with this PR or just exposing a known issue with the CWE-611 query. Could you take a look at those?

The performance on vim/vim looks like something we should investigate before we merge this.

Looking at the DCA experiment: did you look whether the alert changes made sense?

@rdmarsh2 I think there's still the following to resolve:

• code scanning alerts
• open question regarding DCA
• merge conflicts
• C++: IR data flow through global variables #8596 (comment)

There's also still: #8596 (comment) Is this something that needs to be looked at on the extractor side?

The DCA alert changes made sense, yes. Responded to that last comment - I'm not sure if they need work on the extractor side or if they need special handling in IR generation.

LGTM. I've kicked off a final DCA experiment, as we changed quite a few things related to (global) variables on the extractor side since this PR was opened. I'll approve once the experiment is done (unless we find some serious problem that is new compared to the previous DCA experiment, which probably isn't very likely).

New DCA results are in:

• There's a significant increase in the number of alerts (compared to the earlier DCA experiment), which makes sense given the extractor related issues we fixed and as we added/enabled systemd and Samate. The alerts look ok to me, but a second pair of eyes would be welcome.
• The second vim run OOM'ed. This might either be related to the OOM problems we were having earlier (although those should be fixed), due to a bad join order, or simply because we now need a more powerful runner. Have the join orders been looked at?

There's a significant increase in the number of alerts (compared to the earlier DCA experiment), which makes sense given the extractor related issues we fixed and as we added/enabled systemd and Samate. The alerts look ok to me, but a second pair of eyes would be welcome.

So far:

• All the new results on cpp/command-line-injection are new TPs involving global variables 🎉
• There are a lot of new cpp/path-injection on Samate. I've picked an arbitrary set of results, and they're all new TPs involving global variables. So I'm gonna guess that these are all changes that we're happy with
• The two new results for cpp/path-injection on git/git are TPs involving writes to a global variable global_argv.
• The 6 new results for cpp/path-injection on kamailio are also TPs involving global variables.

If anyone wants to look at the remaining results feel free to do so. I'm pretty confident in the new results.

Edit: According to people smarter than me the following might not actually be an issue.

The OOM on vim/vim looks real: here's the relevant log output from the ExecTainted query:

[2022-08-02 11:31:29] Evaluated non-recursive predicate boundedFastTC(DataFlowImpl::pathSucc#9021cc4c#ff,DataFlowImpl::Configuration::hasFlowPath#dispred#f0820431#fff#higher_order_body)@c2650acp in 2219ms (size: 1126884102485).
[2022-08-02 11:31:30] Evaluated non-recursive predicate DataFlowImpl::Configuration::hasFlowPath#dispred#f0820431#fff@88c1067n in 1777ms (size: 181).
Evaluated relational algebra for predicate DataFlowImpl::Configuration::hasFlowPath#dispred#f0820431#fff@88c1067n with tuple counts:
        11   ~0%    {2} r1 = JOIN DataFlowImpl::PathNode::getConfiguration#dispred#f0820431#ff WITH DataFlowImpl::PathNode::isSource#dispred#f0820431#f ON FIRST 1 OUTPUT Lhs.0, Lhs.1
        11   ~0%    {3} r2 = JOIN r1 WITH DataFlowImpl::PathNodeImpl::getNodeEx#dispred#f0820431#ff ON FIRST 1 OUTPUT Rhs.1, Lhs.0, Lhs.1
                
        11   ~0%    {3} r3 = JOIN r2 WITH num#DataFlowImpl::TNodeNormal#9021cc4c#ff_1#join_rhs ON FIRST 1 OUTPUT Lhs.1, Lhs.1, Lhs.2
                
        11   ~0%    {2} r4 = JOIN r2 WITH num#DataFlowImpl::TNodeNormal#9021cc4c#ff_1#join_rhs ON FIRST 1 OUTPUT Lhs.1, Lhs.2
  15068152   ~0%    {3} r5 = JOIN r4 WITH boundedFastTC(DataFlowImpl::pathSucc#9021cc4c#ff,DataFlowImpl::Configuration::hasFlowPath#dispred#f0820431#fff#higher_order_body) ON FIRST 1 OUTPUT Rhs.1, Lhs.0, Lhs.1
                
  15068163   ~0%    {3} r6 = r3 UNION r5
        181   ~2%    {4} r7 = JOIN r6 WITH DataFlowImpl::PathNodeSink#class#9021cc4c#ffff ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.0
        181   ~0%    {3} r8 = JOIN r7 WITH num#DataFlowImpl::TNodeNormal#9021cc4c#ff_1#join_rhs ON FIRST 1 OUTPUT Lhs.2, Lhs.1, Lhs.3
                    return r8

Yes, that's a relation of size 1.126.884.102.485 😱

Looks like the OOM is on ExecTainted.ql - all other queries finish before the error. I'll take a deeper look at it.

Looks like the OOM is on ExecTainted.ql - all other queries finish before the error. I'll take a deeper look at it.

You might want to have a brief sync with Mathias about it. If you can't track it down I would suggest repeating the vim part of the DCA experiment with a lower --ram option than the 14979 currently used. I can explain via our internal channels.

I if see correctly, the re-run of DCA on vim was without the --ram option, because my directions were slightly wrong. Sorry for this! Nevertheless, the re-run was successful, and the slowdown doesn't seem to have changed compared to the previous successful run. I propose we go ahead and merge this, and that if we see DCA OOMs again on vim, we move vim to a slightly bigger runner.