Shared: Make sure getMadRepresentation is unique#19777
Conversation
5cbbc13 to
8549c3b
Compare
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, "/")
| 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. | |
| */ |
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
left a comment
There was a problem hiding this comment.
Approving to get things moving, but I would appreciate more understanding of this issue.
| - ["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"] |
| 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.