Shared: Make sure getMadRepresentation is unique
#19777
Conversation
github-actions
bot
added
Rust
hvitved
force-pushed
the
shared/summary-stack-mad-repr-unique
branch
from
5cbbc13 to
8549c3b
Compare
hvitved
force-pushed
the
shared/summary-stack-mad-repr-unique
branch
from
8549c3b to
631b14a
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that getMadRepresentation produces unique outputs by appending a delimiter and updates the Rust core model mappings to use a consistent argument identifier case.
- Introduce
getUniqueMadRepresentationto append a trailing slash for uniqueness - Replace direct calls to
getMadRepresentationin the summary stack builder with the unique version - Normalize
"Self"to"self"in iterator entries oflang-core.model.yml
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll | Added getUniqueMadRepresentation and updated summary stack composition logic |
| rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml | Changed iterator argument identifier from Self to self |
Comments suppressed due to low confidence (3)
shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll:690
- There are no tests covering the new
getUniqueMadRepresentationlogic; consider adding unit tests to validate its behavior and ensure uniqueness is enforced correctly.
private string getUniqueMadRepresentation(SummaryComponent c) {
rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml:7
- Ensure the lowercase
selfidentifier matches the case sensitivity expected in all model entries; consider reviewing other mappings for consistency.
- ["lang:core", "<[_]>::iter", "Argument[self].Element", "ReturnValue.Element", "value", "manual"]
shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll:691
- Appending a static
/at the end may introduce unexpected trailing delimiters; consider trimming or choosing a clearer delimiter strategy to avoid formatting issues.
result = strictconcat(string s | s = c.getMadRepresentation() | s, "/")
| @@ -686,6 +686,11 @@ module Make< | |||
| derivedFluentFlowPush(_, _, _, head, tail, _) | |||
| } | |||
|
|
|||
| pragma[nomagic] | |||
There was a problem hiding this comment.
Add a doc comment explaining the purpose of getUniqueMadRepresentation and why it appends a trailing / delimiter to ensure uniqueness.
| pragma[nomagic] | |
| pragma[nomagic] | |
| /** | |
| * Generates a unique string representation of the given `SummaryComponent`. | |
| * | |
| * This representation is used in MaD (Model and Dataflow) models to uniquely | |
| * identify components. A trailing `/` is appended to ensure that the resulting | |
| * string is distinct from other potential string formats and avoids collisions | |
| * when concatenated with other representations. | |
| * | |
| * @param c The `SummaryComponent` to generate a representation for. | |
| * @return A unique string representation of the component. | |
| */ |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
I have to admit I would really appreciate a description of what's going on here. What would a collision look like?
| - ["lang:core", "<[_]>::into_iter", "Argument[Self].Element", "ReturnValue.Element", "value", "manual"] | ||
| - ["lang:core", "<[_]>::iter", "Argument[self].Element", "ReturnValue.Element", "value", "manual"] | ||
| - ["lang:core", "<[_]>::iter_mut", "Argument[self].Element", "ReturnValue.Element", "value", "manual"] | ||
| - ["lang:core", "<[_]>::into_iter", "Argument[self].Element", "ReturnValue.Element", "value", "manual"] |
| @@ -686,6 +686,11 @@ module Make< | |||
| derivedFluentFlowPush(_, _, _, head, tail, _) | |||
| } | |||
|
|
|||
| pragma[nomagic] | |||
There was a problem hiding this comment.
I have to admit I would really appreciate a description of what's going on here. What would a collision look like?
@geoffw0 : For Rust, some |
No description provided.