Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust: Include index in Format.getArgument #18445

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Rust: Include index in Format.getArgument #18445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
033cd17
Rust: Include index in `Format.getArgument`
hvitved Jan 8, 2025
0dccbb9
Rust: Add two more AST consistency checks
hvitved Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions rust/ql/.generated.list
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions rust/ql/lib/codeql/rust/AstConsistency.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,27 @@ query predicate multipleParents(Element child, string childClass, Element parent
parentClass = parent.getPrimaryQlClasses()
}

/** Holds if `parent` has multiple children at the same index. */
query predicate multipleChildren(Element parent, int index, Element child1, Element child2) {
child1 = getChildAndAccessor(parent, index, _) and
child2 = getChildAndAccessor(parent, index, _) and
child1 != child2
}

/**
* Holds if `child` has multiple positions amongst the `accessor` children
* of `parent`.
*
* Children are allowed to have multiple positions for _different_ accessors,
* for example in an array repeat expression `[1; 10]`, `1` has positions for
* both `getRepeatOperand()` and `getExpr()`.
*/
query predicate multiplePositions(Element parent, int pos1, int pos2, string accessor, Element child) {
child = getChildAndAccessor(parent, pos1, accessor) and
child = getChildAndAccessor(parent, pos2, accessor) and
pos1 != pos2
}

/**
* Gets counts of abstract syntax tree inconsistencies of each type.
*/
Expand All @@ -71,4 +92,10 @@ int getAstInconsistencyCounts(string type) {
or
type = "Multiple parents" and
result = count(Element e | multipleParents(e) | e)
or
type = "Multiple children" and
result = count(Element e | multipleChildren(_, _, e, _) | e)
or
type = "Multiple positions" and
result = count(Element e | multiplePositions(_, _, _, _, e) | e)
}
2 changes: 1 addition & 1 deletion rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ final class FormatArgsExprCfgNode extends Nodes::FormatArgsExprCfgNode {
/** Gets a format argument of the `i`th format of this format arguments expression (0-based). */
FormatTemplateVariableAccessCfgNode getFormatTemplateVariableAccess(int i) {
exists(FormatTemplateVariableAccess v |
v.getArgument() = node.getFormat(i).getArgument() and
v.getArgument() = node.getFormat(i).getArgument(_) and
result.getFormatTemplateVariableAccess() = v and
any(ChildMapping mapping).hasCfgChild(node, v, this, result)
)
Expand Down
10 changes: 10 additions & 0 deletions rust/ql/lib/codeql/rust/elements/internal/FormatArgumentImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ module Impl {

override Format getParent() { result = Synth::TFormat(parent, index, _, _) }

/** Gets the position of this argument. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This predicate might be a bit confusing. I think it should be dropped.

There are named and positional arguments which can be mixed to some extent. Not sure what the "position" of a named argument would mean, and it would be strange if getPosition does not coincide with the position of a positional argument. For example:

let width = 2;
println!("Value {value:#precision$.0$}", width, precision = 3, value = 100);

would have getPosition() = 2 for 0 .

int getPosition() {
this =
rank[result + 1](FormatArgument f, int offs |
f = Synth::TFormatArgument(parent, index, _, _, _, offs)
|
f order by offs
)
}

override FormatTemplateVariableAccess getVariable() { result.getArgument() = this }
}

Expand Down
4 changes: 3 additions & 1 deletion rust/ql/lib/codeql/rust/elements/internal/FormatImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ module Impl {

override int getIndex() { result = index }

override FormatArgument getArgument() { result.getParent() = this }
override FormatArgument getArgument(int i) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old definition of getArgument() was clearly wrong. I think I simply forgot to remove the argument field from the schema, so we could just delete this predicate. If you want to keep it, that's fine too.

result.getParent() = this and i = result.getPosition()
}

/**
* Gets the name or position reference of this format, if any. For example `name` and `0` in:
Expand Down
13 changes: 9 additions & 4 deletions rust/ql/lib/codeql/rust/elements/internal/generated/Format.qll
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions rust/ql/lib/codeql/rust/elements/internal/generated/ParentChild.qll
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 11 additions & 11 deletions rust/ql/test/extractor-tests/generated/FormatArgsExpr/Format.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
| gen_format.rs:5:21:5:22 | {} | getParent: | gen_format.rs:5:14:5:32 | FormatArgsExpr | getIndex: | 1 | hasArgument: | no |
| gen_format.rs:7:21:7:46 | {value:#width$.precision$} | getParent: | gen_format.rs:7:14:7:47 | FormatArgsExpr | getIndex: | 1 | hasArgument: | yes |
| gen_format_args_arg.rs:5:26:5:27 | {} | getParent: | gen_format_args_arg.rs:5:17:5:39 | FormatArgsExpr | getIndex: | 1 | hasArgument: | no |
| gen_format_args_expr.rs:6:19:6:20 | {} | getParent: | gen_format_args_expr.rs:6:17:6:37 | FormatArgsExpr | getIndex: | 1 | hasArgument: | no |
| gen_format_args_expr.rs:6:26:6:29 | {:?} | getParent: | gen_format_args_expr.rs:6:17:6:37 | FormatArgsExpr | getIndex: | 3 | hasArgument: | no |
| gen_format_args_expr.rs:7:19:7:21 | {b} | getParent: | gen_format_args_expr.rs:7:17:7:43 | FormatArgsExpr | getIndex: | 1 | hasArgument: | yes |
| gen_format_args_expr.rs:7:27:7:31 | {a:?} | getParent: | gen_format_args_expr.rs:7:17:7:43 | FormatArgsExpr | getIndex: | 3 | hasArgument: | yes |
| gen_format_args_expr.rs:9:19:9:21 | {x} | getParent: | gen_format_args_expr.rs:9:17:9:28 | FormatArgsExpr | getIndex: | 1 | hasArgument: | yes |
| gen_format_args_expr.rs:9:24:9:26 | {y} | getParent: | gen_format_args_expr.rs:9:17:9:28 | FormatArgsExpr | getIndex: | 3 | hasArgument: | yes |
| gen_format_argument.rs:5:21:5:46 | {value:#width$.precision$} | getParent: | gen_format_argument.rs:5:14:5:47 | FormatArgsExpr | getIndex: | 1 | hasArgument: | yes |
| gen_format_argument.rs:7:21:7:30 | {0:#1$.2$} | getParent: | gen_format_argument.rs:7:14:7:56 | FormatArgsExpr | getIndex: | 1 | hasArgument: | yes |
| gen_format.rs:5:21:5:22 | {} | getParent: | gen_format.rs:5:14:5:32 | FormatArgsExpr | getIndex: | 1 | getNumberOfArguments: | 0 |
| gen_format.rs:7:21:7:46 | {value:#width$.precision$} | getParent: | gen_format.rs:7:14:7:47 | FormatArgsExpr | getIndex: | 1 | getNumberOfArguments: | 3 |
| gen_format_args_arg.rs:5:26:5:27 | {} | getParent: | gen_format_args_arg.rs:5:17:5:39 | FormatArgsExpr | getIndex: | 1 | getNumberOfArguments: | 0 |
| gen_format_args_expr.rs:6:19:6:20 | {} | getParent: | gen_format_args_expr.rs:6:17:6:37 | FormatArgsExpr | getIndex: | 1 | getNumberOfArguments: | 0 |
| gen_format_args_expr.rs:6:26:6:29 | {:?} | getParent: | gen_format_args_expr.rs:6:17:6:37 | FormatArgsExpr | getIndex: | 3 | getNumberOfArguments: | 0 |
| gen_format_args_expr.rs:7:19:7:21 | {b} | getParent: | gen_format_args_expr.rs:7:17:7:43 | FormatArgsExpr | getIndex: | 1 | getNumberOfArguments: | 1 |
| gen_format_args_expr.rs:7:27:7:31 | {a:?} | getParent: | gen_format_args_expr.rs:7:17:7:43 | FormatArgsExpr | getIndex: | 3 | getNumberOfArguments: | 1 |
| gen_format_args_expr.rs:9:19:9:21 | {x} | getParent: | gen_format_args_expr.rs:9:17:9:28 | FormatArgsExpr | getIndex: | 1 | getNumberOfArguments: | 1 |
| gen_format_args_expr.rs:9:24:9:26 | {y} | getParent: | gen_format_args_expr.rs:9:17:9:28 | FormatArgsExpr | getIndex: | 3 | getNumberOfArguments: | 1 |
| gen_format_argument.rs:5:21:5:46 | {value:#width$.precision$} | getParent: | gen_format_argument.rs:5:14:5:47 | FormatArgsExpr | getIndex: | 1 | getNumberOfArguments: | 3 |
| gen_format_argument.rs:7:21:7:30 | {0:#1$.2$} | getParent: | gen_format_argument.rs:7:14:7:56 | FormatArgsExpr | getIndex: | 1 | getNumberOfArguments: | 3 |
7 changes: 4 additions & 3 deletions rust/ql/test/extractor-tests/generated/FormatArgsExpr/Format.ql
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 13 additions & 13 deletions rust/ql/test/extractor-tests/generated/FormatArgsExpr/Format_getArgument.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
| gen_format.rs:7:21:7:46 | {value:#width$.precision$} | gen_format.rs:7:22:7:26 | value |
| gen_format.rs:7:21:7:46 | {value:#width$.precision$} | gen_format.rs:7:29:7:33 | width |
| gen_format.rs:7:21:7:46 | {value:#width$.precision$} | gen_format.rs:7:36:7:44 | precision |
| gen_format_args_expr.rs:7:19:7:21 | {b} | gen_format_args_expr.rs:7:20:7:20 | b |
| gen_format_args_expr.rs:7:27:7:31 | {a:?} | gen_format_args_expr.rs:7:28:7:28 | a |
| gen_format_args_expr.rs:9:19:9:21 | {x} | gen_format_args_expr.rs:9:20:9:20 | x |
| gen_format_args_expr.rs:9:24:9:26 | {y} | gen_format_args_expr.rs:9:25:9:25 | y |
| gen_format_argument.rs:5:21:5:46 | {value:#width$.precision$} | gen_format_argument.rs:5:22:5:26 | value |
| gen_format_argument.rs:5:21:5:46 | {value:#width$.precision$} | gen_format_argument.rs:5:29:5:33 | width |
| gen_format_argument.rs:5:21:5:46 | {value:#width$.precision$} | gen_format_argument.rs:5:36:5:44 | precision |
| gen_format_argument.rs:7:21:7:30 | {0:#1$.2$} | gen_format_argument.rs:7:22:7:22 | 0 |
| gen_format_argument.rs:7:21:7:30 | {0:#1$.2$} | gen_format_argument.rs:7:25:7:25 | 1 |
| gen_format_argument.rs:7:21:7:30 | {0:#1$.2$} | gen_format_argument.rs:7:28:7:28 | 2 |
| gen_format.rs:7:21:7:46 | {value:#width$.precision$} | 0 | gen_format.rs:7:22:7:26 | value |
| gen_format.rs:7:21:7:46 | {value:#width$.precision$} | 1 | gen_format.rs:7:29:7:33 | width |
| gen_format.rs:7:21:7:46 | {value:#width$.precision$} | 2 | gen_format.rs:7:36:7:44 | precision |
| gen_format_args_expr.rs:7:19:7:21 | {b} | 0 | gen_format_args_expr.rs:7:20:7:20 | b |
| gen_format_args_expr.rs:7:27:7:31 | {a:?} | 0 | gen_format_args_expr.rs:7:28:7:28 | a |
| gen_format_args_expr.rs:9:19:9:21 | {x} | 0 | gen_format_args_expr.rs:9:20:9:20 | x |
| gen_format_args_expr.rs:9:24:9:26 | {y} | 0 | gen_format_args_expr.rs:9:25:9:25 | y |
| gen_format_argument.rs:5:21:5:46 | {value:#width$.precision$} | 0 | gen_format_argument.rs:5:22:5:26 | value |
| gen_format_argument.rs:5:21:5:46 | {value:#width$.precision$} | 1 | gen_format_argument.rs:5:29:5:33 | width |
| gen_format_argument.rs:5:21:5:46 | {value:#width$.precision$} | 2 | gen_format_argument.rs:5:36:5:44 | precision |
| gen_format_argument.rs:7:21:7:30 | {0:#1$.2$} | 0 | gen_format_argument.rs:7:22:7:22 | 0 |
| gen_format_argument.rs:7:21:7:30 | {0:#1$.2$} | 1 | gen_format_argument.rs:7:25:7:25 | 1 |
| gen_format_argument.rs:7:21:7:30 | {0:#1$.2$} | 2 | gen_format_argument.rs:7:28:7:28 | 2 |
4 changes: 2 additions & 2 deletions rust/ql/test/extractor-tests/generated/FormatArgsExpr/Format_getArgument.ql
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions rust/ql/test/query-tests/diagnostics/AstConsistencyCounts.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
| Multiple children | 0 |
| Multiple locations | 0 |
| Multiple parents | 0 |
| Multiple positions | 0 |
| Multiple primary QL classes | 0 |
| Multiple toStrings | 0 |
| No location | 0 |
2 changes: 1 addition & 1 deletion rust/schema/annotations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1878,7 +1878,7 @@ class Format(Locatable):
"""
parent: FormatArgsExpr
index: int
argument: optional["FormatArgument"] | child
argument: list["FormatArgument"] | child


@synth.on_arguments(parent=FormatArgsExpr, index=int, kind=int, name=string, positional=boolean, offset=int)
Expand Down
Loading