Skip to content

Commit 43eba85

Browse files
author
Paolo Tranquilli
committed
Merge branch 'main' into redsun82/rust-tweaks
2 parents 2a7ce9a + f56e337 commit 43eba85

File tree

19 files changed

+340
-141
lines changed

19 files changed

+340
-141
lines changed

‎.devcontainer/devcontainer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{
2+
"image": "mcr.microsoft.com/devcontainers/base:ubuntu-24.04",
23
"extensions": [
34
"rust-lang.rust-analyzer",
45
"bungcip.better-toml",

‎CODEOWNERS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,6 @@ MODULE.bazel @github/codeql-ci-reviewers
4242
# Misc
4343
/misc/scripts/accept-expected-changes-from-ci.py @RasmusWL
4444
/misc/scripts/generate-code-scanning-query-list.py @RasmusWL
45+
46+
# .devcontainer
47+
/.devcontainer/ @github/codeql-ci-reviewers

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ module ProductFlow {
545545
private predicate outImpl1(Flow1::PathNode pred1, Flow1::PathNode succ1, DataFlowCall call) {
546546
Flow1::PathGraph::edges(pred1, succ1, _, _) and
547547
exists(ReturnKindExt returnKind |
548-
succ1.getNode() = returnKind.getAnOutNode(call) and
548+
succ1.getNode() = getAnOutNodeExt(call, returnKind) and
549549
returnKind = getParamReturnPosition(_, pred1.asParameterReturnNode()).getKind()
550550
)
551551
}
@@ -573,7 +573,7 @@ module ProductFlow {
573573
private predicate outImpl2(Flow2::PathNode pred2, Flow2::PathNode succ2, DataFlowCall call) {
574574
Flow2::PathGraph::edges(pred2, succ2, _, _) and
575575
exists(ReturnKindExt returnKind |
576-
succ2.getNode() = returnKind.getAnOutNode(call) and
576+
succ2.getNode() = getAnOutNodeExt(call, returnKind) and
577577
returnKind = getParamReturnPosition(_, pred2.asParameterReturnNode()).getKind()
578578
)
579579
}

‎ql/ql/src/queries/style/ValidatePredicateGetReturns.ql‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* @name Predicates starting with "get" or "as" should return a value
3+
* @description Checks if predicates that start with "get" or "as" actually return a value.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @id ql/predicates-get-should-return-value
7+
* @tags correctness
8+
* maintainability
9+
* @precision high
10+
*/
11+
12+
import ql
13+
import codeql_ql.ast.Ast
14+
15+
/**
16+
* Identifies predicates whose names start with "get", "as" followed by an uppercase letter.
17+
* This ensures that only predicates like "getValue" are matched, excluding names like "getter".
18+
*/
19+
predicate isGetPredicate(Predicate pred, string prefix) {
20+
prefix = pred.getName().regexpCapture("(get|as)[A-Z].*", 1)
21+
}
22+
23+
/**
24+
* Checks if a predicate has a return type. This is phrased negatively to not flag unresolved aliases.
25+
*/
26+
predicate hasNoReturnType(Predicate pred) {
27+
not exists(pred.getReturnTypeExpr()) and
28+
not pred.(ClasslessPredicate).getAlias() instanceof PredicateExpr
29+
or
30+
hasNoReturnType(pred.(ClasslessPredicate).getAlias().(PredicateExpr).getResolvedPredicate())
31+
}
32+
33+
from Predicate pred, string prefix
34+
where
35+
isGetPredicate(pred, prefix) and
36+
hasNoReturnType(pred)
37+
select pred, "This predicate starts with '" + prefix + "' but does not return a value."

‎ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
| test.qll:4:11:4:18 | ClasslessPredicate getValue | This predicate starts with 'get' but does not return a value. |
2+
| test.qll:25:11:25:28 | ClasslessPredicate getImplementation2 | This predicate starts with 'get' but does not return a value. |
3+
| test.qll:28:11:28:19 | ClasslessPredicate getAlias2 | This predicate starts with 'get' but does not return a value. |
4+
| test.qll:31:11:31:17 | ClasslessPredicate asValue | This predicate starts with 'as' but does not return a value. |
5+
| test.qll:48:11:48:19 | ClasslessPredicate getAlias4 | This predicate starts with 'get' but does not return a value. |
6+
| test.qll:61:11:61:22 | ClasslessPredicate getDistance2 | This predicate starts with 'get' but does not return a value. |

‎ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/style/ValidatePredicateGetReturns.ql

‎ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import ql
2+
3+
// NOT OK -- Predicate starts with "get" but does not return a value
4+
predicate getValue() { none() }
5+
6+
// OK -- starts with get and returns a value
7+
string getData() { result = "data" }
8+
9+
// OK -- starts with get but followed by a lowercase letter, probably should be ignored
10+
predicate getterFunction() { none() }
11+
12+
// OK -- starts with get and returns a value
13+
string getImplementation() { result = "implementation" }
14+
15+
// OK -- is an alias
16+
predicate getAlias = getImplementation/0;
17+
18+
// OK -- Starts with "get" but followed by a lowercase letter, probably be ignored
19+
predicate getvalue() { none() }
20+
21+
// OK -- Does not start with "get", should be ignored
22+
predicate retrieveValue() { none() }
23+
24+
// NOT OK -- starts with get and does not return value
25+
predicate getImplementation2() { none() }
26+
27+
// NOT OK -- is an alias for a predicate which does not have a return value
28+
predicate getAlias2 = getImplementation2/0;
29+
30+
// NOT OK -- starts with as and does not return value
31+
predicate asValue() { none() }
32+
33+
// OK -- starts with as but followed by a lowercase letter, probably should be ignored
34+
predicate assessment() { none() }
35+
36+
// OK -- starts with as and returns a value
37+
string asString() { result = "string" }
38+
39+
// OK -- starts with get and returns a value
40+
HiddenType getInjectableCompositeActionNode() {
41+
exists(HiddenType hidden | result = hidden.toString())
42+
}
43+
44+
// OK
45+
predicate implementation4() { none() }
46+
47+
// NOT OK -- is an alias
48+
predicate getAlias4 = implementation4/0;
49+
50+
// OK -- is an alias
51+
predicate alias5 = implementation4/0;
52+
53+
int root() { none() }
54+
55+
predicate edge(int x, int y) { none() }
56+
57+
// OK -- Higher-order predicate
58+
int getDistance(int x) = shortestDistances(root/0, edge/2)(_, x, result)
59+
60+
// NOT OK -- Higher-order predicate that does not return a value even though has 'get' in the name
61+
predicate getDistance2(int x, int y) = shortestDistances(root/0, edge/2)(_, x, y)
62+
63+
// OK
64+
predicate unresolvedAlias = unresolved/0;
65+
66+
// OK -- unresolved alias
67+
predicate getUnresolvedAlias = unresolved/0;

‎rust/ql/consistency-queries/DataFlowConsistency.ql‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,4 @@
55
* @id rust/diagnostics/data-flow-consistency
66
*/
77

8-
import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
9-
private import rust
10-
private import codeql.rust.dataflow.internal.DataFlowImpl
11-
private import codeql.rust.dataflow.internal.TaintTrackingImpl
12-
private import codeql.dataflow.internal.DataFlowImplConsistency
13-
14-
private module Input implements InputSig<Location, RustDataFlow> { }
15-
16-
import MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>
8+
import codeql.rust.dataflow.internal.DataFlowConsistency

‎rust/ql/lib/codeql/rust/AstConsistency.qll‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ query predicate multipleToStrings(Element e, string cls, string s) {
2121
*/
2222
query predicate multipleLocations(Locatable e) { strictcount(e.getLocation()) > 1 }
2323

24+
/**
25+
* Holds if `e` does not have a `Location`.
26+
*/
27+
query predicate noLocation(Locatable e) { not exists(e.getLocation()) }
28+
2429
private predicate multiplePrimaryQlClasses(Element e) {
2530
strictcount(string cls | cls = e.getAPrimaryQlClass() and cls != "VariableAccess") > 1
2631
}
@@ -58,6 +63,9 @@ int getAstInconsistencyCounts(string type) {
5863
type = "Multiple locations" and
5964
result = count(Element e | multipleLocations(e) | e)
6065
or
66+
type = "No location" and
67+
result = count(Element e | noLocation(e) | e)
68+
or
6169
type = "Multiple primary QL classes" and
6270
result = count(Element e | multiplePrimaryQlClasses(e) | e)
6371
or

‎rust/ql/lib/codeql/rust/dataflow/internal/DataFlowConsistency.qll‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
2+
private import rust
3+
private import codeql.rust.dataflow.internal.DataFlowImpl
4+
private import codeql.rust.dataflow.internal.TaintTrackingImpl
5+
private import codeql.dataflow.internal.DataFlowImplConsistency
6+
7+
private module Input implements InputSig<Location, RustDataFlow> {
8+
predicate uniqueNodeLocationExclude(RustDataFlow::Node n) {
9+
// Exclude nodes where the missing location can be explained by the
10+
// underlying AST node not having a location.
11+
not exists(n.asExpr().getLocation())
12+
}
13+
14+
predicate missingLocationExclude(RustDataFlow::Node n) { not exists(n.asExpr().getLocation()) }
15+
}
16+
17+
import MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>

0 commit comments

Comments
 (0)