Rust: Update legacy MaD models 3

[geoffw0]

Update even more legacy MaD models to the new model format (continues from #19942 and should be independent of that).

[geoffw0]

There's currently a large number of test changes caused by this PR (highlighted by CI). I think it would be prudent to get the first two PRs merged before attempting to address this. --- no longer true.

[aibaars]

At a glance the test changes are due to re-ordering of lines, so that looks fine to me.

[geoffw0]

The lost sources in the frameworks/rusqlite test I've added to the issue logging regressions. The rest of the CI failures were just location and model changes. I think this is good to merge now.

[aibaars]

Are you sure the problem is a missing canonical path, or is the problem that getStaticTarget fails (due to type inferencer problem)?

[aibaars]

Just checked the canonical path exists, the problem is the type inferencer, most likely the closure argument.

[geoffw0]

Yep you're right, the getStaticTarget fails (and thus getStaticTarget().(Addressable).getCanonicalPath() fails too) so it's likely a type inference problem.

[aibaars]

Yeah that's why I use a concat in my test predicate:

predicate test(MethodCallExpr e, Function target, string path) {
  target = e.getStaticTarget() and
  path = concat(target.getCanonicalPath())
}

• no result means getStaticTarget() failed
• empty string means: a target was found, but getCanonicalPath() failed

[hvitved]

most likely the closure argument.

Correct; type inference for closures is a known gap.

Yeah that's why I use a concat in my test predicate:

I can also recommend the various debug predicates in TypeInference.qll; change getRelevantLocatable to the relevant file+line, and then quick-eval debugInferType et al.

[geoffw0]

I will try the TypeInference.qll debug predicates, thanks for reminding me those exist.

[aibaars]

Looks mostly fine, the rusqlite test failures can be fixed though.

[geoffw0]

Fixed issues and merged in main. Ready for approval.

[aibaars]

Have you checked the hyper project? DCA reports we are losing a fair number of sources. Just to make sure the hyper models are good

[aibaars]

There is still a test failure:

+| test.rs:483:22:483:50 | file.read_to_end(...) |
+| test.rs:489:22:489:53 | file.read_to_string(...) |
 | test.rs:610:18:610:38 | ...::_print(...) |
 | test.rs:615:18:615:45 | ...::_print(...) |
 | test.rs:619:25:619:49 | address.to_socket_addrs() |
Error: [2/2] [984/989 comp 23.4s eval 41.3s] FAILED(RESULT) /home/runner/work/semmle-code/semmle-code/ql/rust/ql/test/library-tests/dataflow/sources/CONSISTENCY/PathResolutionConsistency.ql

[geoffw0]

Have you checked the hyper project? DCA reports we are losing a fair number of sources. Just to make sure the hyper models are good

I believe this is due to lack of type inference support for tuples. Specifically, we appear to lose most calls to send_request as sources and many (perhaps all) of those lost calls use the let (mut sender, conn) = ... pattern to create the qualifier (sender), i.e. they require tuple support. The fact that a small number of send_request sources are not lost suggests the model itself is fine.

There is still a test failure:

I've just updated the .expected file.

[aibaars]

I believe this is due to lack of type inference support for tuples. Specifically, we appear to lose most calls to send_request as sources and many (perhaps all) of those lost calls use the let (mut sender, conn) = ... pattern to create the qualifier (sender), i.e. they require tuple support. The fact that a small number of send_request sources are not lost suggests the model itself is fine.

Ok good. I expected it to be something like that, thanks for confirming!

[aibaars]

Looks good to me!

[geoffw0]

Thanks for the detailed review and suggestions!