Rust: Initial model generation setup #18628

paldepind · 2025-01-29T16:45:21Z

Adds the setup for model generation based on C# and Java.
Instantiates the model generation signature. Some parts are not implemented yet, but enough is there to get simple models and models using variants to work.

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

paldepind · 2025-01-30T10:56:31Z

rust/ql/src/utils/modelgenerator/debug/CaptureSummaryModelsPartialPath.ql

+  p = source.getNode() and
+  p.asParameter() = api.getAParameter()
+select sink.getNode(), source, sink, "There is flow from a $@ to $@.", source.getNode(),
+  "parameter", sink.getNode(), "intermediate value"


These are debug queries copy pasted from C#, so I'm inclined to ignore this?

I wouldn't worry about this too much, assuming we're not expecting users to use these queries directly. We might want to make it clearer what these queries are for though (e.g. in the @description) in case users do find them?

rust/ql/src/utils/modelgenerator/debug/CaptureSummaryModelsPath.ql

+  returnNodeExt = sink.getNode() and
+  exists(captureThroughFlow0(api, p, returnNodeExt))
+select sink.getNode(), source, sink, "There is flow from $@ to the $@.", source.getNode(),
+  "parameter", sink.getNode(), "return value"


rust/ql/src/utils/modelgenerator/internal/CaptureModels.qll

+  /**
+   * Gets the underlying type of the content `c`.
+   */
+  Type getUnderlyingContentType(DataFlow::ContentSet c) { result = any(Type t) and exists(c) }


rust/ql/src/utils/modelgenerator/internal/CaptureModels.qll

rust/ql/test/utils/modelgenerator/CaptureSummaryModels.ql

rust/ql/src/utils/modelgenerator/internal/CaptureModels.qll

paldepind · 2025-01-30T10:57:42Z

rust/ql/test/utils/modelgenerator/summaries.rs

+    }
+
+    // MISSING: summary=repo::test;<crate::summaries::MyStruct>::get_foo;Argument[self].Struct[crate::summaries::MyStruct::foo];ReturnValue;value;dfc-generated
+    pub fn get_foo(self) -> i64 {


I'd have expected this to work, but I'll investigate later. Right now variants are more interesting, as generating models for Option is the current goal.

I like the way these tests are set up, by the way. I suspect we'll want more of them as we debug things, but it's a really nice setup.

Absolutely, and I completely agree. It's all thanks to @michaelnebel 🎉

rust/ql/test/utils-tests/modelgenerator/CaptureSummaryModels.ql

paldepind · 2025-02-03T08:24:01Z

CI complains about missing QLdoc, but it's the same in C# and Java, so I think it's ok to ignore.

hvitved

Looks great, oh my that ModelGeneratorInputSig interface is complex... My main comment is about only using content-based (precise) summary generation.

hvitved · 2025-02-03T12:28:06Z

rust/ql/lib/codeql/rust/dataflow/DataFlow.qll

-    /** Gets the parameter that this node corresponds to. */
-    ParamBase getParameter() { result = super.getParameter().getParamBase() }
-  }
+  final class ParameterNode = Node::SourceParameterNode;


This made me realize that we are already now exposing internal implementation details in the public DataFlow::Node class, which I want to avoid. So I would like for this to be reverted back 🙏

hvitved · 2025-02-03T13:05:27Z

rust/ql/src/utils/modelgenerator/CaptureMixedSummaryModels.ql

@@ -0,0 +1,13 @@
+/**


I'm not sure we'll want this query, I hope that CaptureContentSummaryModels.ql should be enough for us.

I don't have strong feelings but I'd be inclined to leave the files in places because:
1/ The shared Python script assumes that these files are there.
2/ For consistency with C# and Java
3/ We might never have to ever use them, but perhaps it could come in handy for debugging or experimenting through the generation script.

hvitved · 2025-02-03T13:05:49Z

rust/ql/src/utils/modelgenerator/CaptureSummaryModels.ql

@@ -0,0 +1,13 @@
+/**


Same comment.

hvitved · 2025-02-03T13:10:51Z

rust/ql/test/utils-tests/modelgenerator/CaptureSummaryModels.ql

+import utils.test.InlineMadTest
+
+module InlineMadTestConfig implements InlineMadTestConfigSig {
+  string getCapturedModel(Function c) { result = captureMixedFlow(c, _) }


Again, I think we just want ContentSensitive::captureFlow for Rust.

Makes no difference in the tests, so let's just do that yes 👍

geoffw0 · 2025-02-03T17:20:45Z

I'm looking forward to seeing early results from the model generator, which I gather is going to come a bit later.

paldepind · 2025-02-04T08:51:43Z

I'm looking forward to seeing early results from the model generator, which I gather is going to come a bit later.

After this PR I'll create one adding flow for higher-order functions. With that we get around 50 lines of yaml for Option that it would probably make sense to commit at that point! 👍

github-actions bot added documentation Rust Pull requests that update Rust code labels Jan 29, 2025

github-advanced-security bot found potential problems Jan 29, 2025

View reviewed changes

paldepind force-pushed the rust-flow-summary-generation branch from ecf32a1 to 360c5eb Compare January 30, 2025 07:54

github-advanced-security bot found potential problems Jan 30, 2025

View reviewed changes

rust/ql/src/utils/modelgenerator/internal/CaptureModels.qll Fixed Show fixed Hide fixed

Rust: Initial model generation setup

f76647f

paldepind force-pushed the rust-flow-summary-generation branch from 360c5eb to f76647f Compare January 30, 2025 10:45

paldepind marked this pull request as ready for review January 30, 2025 10:53

paldepind commented Jan 30, 2025

View reviewed changes

paldepind removed the documentation label Jan 30, 2025

github-actions bot added the documentation label Feb 3, 2025

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

rust/ql/test/utils-tests/modelgenerator/CaptureSummaryModels.ql Fixed Show fixed Hide fixed

rust/ql/test/utils-tests/modelgenerator/CaptureSummaryModels.ql Fixed Show fixed Hide fixed

Rust: Fix model generation test

0a9b864

paldepind force-pushed the rust-flow-summary-generation branch from 03ec4ee to 0a9b864 Compare February 3, 2025 08:06

paldepind requested a review from hvitved February 3, 2025 08:24

paldepind removed the documentation label Feb 3, 2025

hvitved requested changes Feb 3, 2025

View reviewed changes

paldepind added 2 commits February 4, 2025 10:10

Merge branch 'main' into rust-flow-summary-generation

fc15c0d

Rust: Address PR comments

0741331

github-actions bot added the documentation label Feb 4, 2025

paldepind removed the documentation label Feb 4, 2025

Merge branch 'main' into rust-flow-summary-generation

cf4f657

paldepind force-pushed the rust-flow-summary-generation branch from ffaad99 to cf4f657 Compare February 4, 2025 09:42

github-actions bot added the documentation label Feb 4, 2025

paldepind requested a review from hvitved February 4, 2025 09:51

hvitved approved these changes Feb 4, 2025

View reviewed changes

paldepind merged commit 8d01bbc into github:main Feb 4, 2025
16 of 17 checks passed

paldepind deleted the rust-flow-summary-generation branch February 4, 2025 11:37

paldepind mentioned this pull request Feb 4, 2025

Rust: Accept test changes #18677

Merged

Rust: Initial model generation setup #18628

Rust: Initial model generation setup #18628

Uh oh!

Conversation

paldepind commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Check warning

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check warning

Check warning

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

paldepind commented Feb 3, 2025

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoffw0 commented Feb 3, 2025

Uh oh!

paldepind commented Feb 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paldepind commented Jan 29, 2025 •

edited

Loading