From 85f2582afd4d8f5f1fd2ea3e0f24dce65c6ba210 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 8 Oct 2024 10:26:14 +0100 Subject: [PATCH 1/8] Rust: Move CFG consistency logic into a library. --- rust/ql/consistency-queries/CfgConsistency.ql | 38 +---------------- .../controlflow/internal/CfgConsistency.qll | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 37 deletions(-) create mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll diff --git a/rust/ql/consistency-queries/CfgConsistency.ql b/rust/ql/consistency-queries/CfgConsistency.ql index 3a95fde507b4..7f5e1649b22d 100644 --- a/rust/ql/consistency-queries/CfgConsistency.ql +++ b/rust/ql/consistency-queries/CfgConsistency.ql @@ -1,37 +1 @@ -import rust -import codeql.rust.controlflow.internal.ControlFlowGraphImpl::Consistency as Consistency -import Consistency -import codeql.rust.controlflow.ControlFlowGraph -import codeql.rust.controlflow.internal.ControlFlowGraphImpl as CfgImpl -import codeql.rust.controlflow.internal.Completion - -/** - * All `Expr` nodes are `PostOrderTree`s - */ -query predicate nonPostOrderExpr(Expr e, string cls) { - cls = e.getPrimaryQlClasses() and - not e instanceof LetExpr and - not e instanceof ParenExpr and - exists(AstNode last, Completion c | - CfgImpl::last(e, last, c) and - last != e and - c instanceof NormalCompletion - ) -} - -query predicate scopeNoFirst(CfgScope scope) { - Consistency::scopeNoFirst(scope) and - not scope = any(Function f | not exists(f.getBody())) and - not scope = any(ClosureExpr c | not exists(c.getBody())) -} - -/** Holds if `be` is the `else` branch of a `let` statement that results in a panic. */ -private predicate letElsePanic(BlockExpr be) { - be = any(LetStmt let).getLetElse().getBlockExpr() and - exists(Completion c | CfgImpl::last(be, _, c) | completionIsNormal(c)) -} - -query predicate deadEnd(CfgImpl::Node node) { - Consistency::deadEnd(node) and - not letElsePanic(node.getAstNode()) -} +import codeql.rust.controlflow.internal.CfgConsistency diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll b/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll new file mode 100644 index 000000000000..dafbb327237e --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll @@ -0,0 +1,41 @@ +/** + * Provides classes for recognizing control flow graph inconsistencies. + */ + +private import rust +private import codeql.rust.controlflow.internal.ControlFlowGraphImpl::Consistency as Consistency +import Consistency +private import codeql.rust.controlflow.ControlFlowGraph +private import codeql.rust.controlflow.internal.ControlFlowGraphImpl as CfgImpl +private import codeql.rust.controlflow.internal.Completion + +/** + * All `Expr` nodes are `PostOrderTree`s + */ +query predicate nonPostOrderExpr(Expr e, string cls) { + cls = e.getPrimaryQlClasses() and + not e instanceof LetExpr and + not e instanceof ParenExpr and + exists(AstNode last, Completion c | + CfgImpl::last(e, last, c) and + last != e and + c instanceof NormalCompletion + ) +} + +query predicate scopeNoFirst(CfgScope scope) { + Consistency::scopeNoFirst(scope) and + not scope = any(Function f | not exists(f.getBody())) and + not scope = any(ClosureExpr c | not exists(c.getBody())) +} + +/** Holds if `be` is the `else` branch of a `let` statement that results in a panic. */ +private predicate letElsePanic(BlockExpr be) { + be = any(LetStmt let).getLetElse().getBlockExpr() and + exists(Completion c | CfgImpl::last(be, _, c) | completionIsNormal(c)) +} + +query predicate deadEnd(CfgImpl::Node node) { + Consistency::deadEnd(node) and + not letElsePanic(node.getAstNode()) +} From 4398c83a67bdb8b1f2ede221825a52ab804d932b Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:13:28 +0100 Subject: [PATCH 2/8] Rust: Add more QLDoc to the CFG consistency library. --- .../lib/codeql/rust/controlflow/internal/CfgConsistency.qll | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll b/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll index dafbb327237e..849c2a824adc 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll @@ -23,6 +23,9 @@ query predicate nonPostOrderExpr(Expr e, string cls) { ) } +/** + * Holds if CFG scope `scope` lacks an initial AST node. Overrides shared consistency predicate. + */ query predicate scopeNoFirst(CfgScope scope) { Consistency::scopeNoFirst(scope) and not scope = any(Function f | not exists(f.getBody())) and @@ -35,6 +38,9 @@ private predicate letElsePanic(BlockExpr be) { exists(Completion c | CfgImpl::last(be, _, c) | completionIsNormal(c)) } +/** + * Holds if `node` is lacking a successor. Overrides shared consistency predicate. + */ query predicate deadEnd(CfgImpl::Node node) { Consistency::deadEnd(node) and not letElsePanic(node.getAstNode()) From 7b712f3d65c901235847b46b017bbf6c212f76c0 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 8 Oct 2024 09:48:50 +0100 Subject: [PATCH 3/8] Rust: Calculate a total of CFG inconsistencies. --- .../controlflow/internal/CfgConsistency.qll | 47 +++++++++++++++++++ rust/ql/src/queries/summary/Stats.qll | 12 +++++ rust/ql/src/queries/summary/SummaryStats.ql | 2 + .../diagnostics/SummaryStats.expected | 1 + 4 files changed, 62 insertions(+) diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll b/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll index 849c2a824adc..e463f5776652 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll @@ -45,3 +45,50 @@ query predicate deadEnd(CfgImpl::Node node) { Consistency::deadEnd(node) and not letElsePanic(node.getAstNode()) } + +/** + * Gets counts of control flow graph inconsistencies of each type. + */ +int getCfgInconsistencyCounts(string type) { + // total results from all the CFG consistency query predicates in: + // - `codeql.rust.controlflow.internal.CfgConsistency` (this file) + // - `shared.controlflow.codeql.controlflow.Cfg` + type = "Non-unique set representation" and + result = count(CfgImpl::Splits ss | Consistency::nonUniqueSetRepresentation(ss, _) | ss) + or + type = "Splitting invariant 2" and + result = count(AstNode n | Consistency::breakInvariant2(n, _, _, _, _, _) | n) + or + type = "Splitting invariant 3" and + result = count(AstNode n | Consistency::breakInvariant3(n, _, _, _, _, _) | n) + or + type = "Splitting invariant 4" and + result = count(AstNode n | Consistency::breakInvariant4(n, _, _, _, _, _) | n) + or + type = "Splitting invariant 5" and + result = count(AstNode n | Consistency::breakInvariant5(n, _, _, _, _, _) | n) + or + type = "Multiple successors of the same type" and + result = count(CfgNode n | Consistency::multipleSuccessors(n, _, _) | n) + or + type = "Simple and normal successors" and + result = count(CfgNode n | Consistency::simpleAndNormalSuccessors(n, _, _, _, _) | n) + or + type = "Dead end" and + result = count(CfgNode n | Consistency::deadEnd(n) | n) + or + type = "Non-unique split kind" and + result = count(CfgImpl::SplitImpl si | Consistency::nonUniqueSplitKind(si, _) | si) + or + type = "Non-unique list order" and + result = count(CfgImpl::SplitKind sk | Consistency::nonUniqueListOrder(sk, _) | sk) + or + type = "Multiple toStrings" and + result = count(CfgNode n | Consistency::multipleToString(n, _) | n) + or + type = "CFG scope lacks initial AST node" and + result = count(CfgScope s | Consistency::scopeNoFirst(s) | s) + or + type = "Non-PostOrderTree Expr node" and + result = count(Expr e | nonPostOrderExpr(e, _) | e) +} diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll index 2f3992303f3b..c1f0bdd08961 100644 --- a/rust/ql/src/queries/summary/Stats.qll +++ b/rust/ql/src/queries/summary/Stats.qll @@ -3,9 +3,21 @@ */ import rust +private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency +/** + * Gets a count of the total number of lines of code in the database. + */ int getLinesOfCode() { result = sum(File f | | f.getNumberOfLinesOfCode()) } +/** + * Gets a count of the total number of lines of code from the source code directory in the database. + */ int getLinesOfUserCode() { result = sum(File f | exists(f.getRelativePath()) | f.getNumberOfLinesOfCode()) } + +/** + * Gets a count of the total number of control flow graph inconsistencies in the database. + */ +int getTotalCfgInconsistencies() { result = sum(CfgConsistency::getCfgInconsistencyCounts(_)) } diff --git a/rust/ql/src/queries/summary/SummaryStats.ql b/rust/ql/src/queries/summary/SummaryStats.ql index a3494ccb769c..83429fae6d7d 100644 --- a/rust/ql/src/queries/summary/SummaryStats.ql +++ b/rust/ql/src/queries/summary/SummaryStats.ql @@ -33,4 +33,6 @@ where key = "Lines of code extracted" and value = getLinesOfCode().toString() or key = "Lines of user code extracted" and value = getLinesOfUserCode().toString() + or + key = "Inconsistencies - CFG" and value = getTotalCfgInconsistencies().toString() select key, value diff --git a/rust/ql/test/query-tests/diagnostics/SummaryStats.expected b/rust/ql/test/query-tests/diagnostics/SummaryStats.expected index 100ade0c0203..0a1312ad5d02 100644 --- a/rust/ql/test/query-tests/diagnostics/SummaryStats.expected +++ b/rust/ql/test/query-tests/diagnostics/SummaryStats.expected @@ -5,5 +5,6 @@ | Files extracted - total | 7 | | Files extracted - with errors | 2 | | Files extracted - without errors | 5 | +| Inconsistencies - CFG | 0 | | Lines of code extracted | 59 | | Lines of user code extracted | 59 | From d4c3e3323f8251bfcfe1cd930128aadc4c520164 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:16:05 +0100 Subject: [PATCH 4/8] Rust: Add diagnostic query for CFG inconsistency counts. --- .../queries/diagnostics/CfgConsistencyCounts.ql | 15 +++++++++++++++ .../diagnostics/CfgConsistencyCounts.expected | 13 +++++++++++++ .../diagnostics/CfgConsistencyCounts.qlref | 1 + 3 files changed, 29 insertions(+) create mode 100644 rust/ql/src/queries/diagnostics/CfgConsistencyCounts.ql create mode 100644 rust/ql/test/query-tests/diagnostics/CfgConsistencyCounts.expected create mode 100644 rust/ql/test/query-tests/diagnostics/CfgConsistencyCounts.qlref diff --git a/rust/ql/src/queries/diagnostics/CfgConsistencyCounts.ql b/rust/ql/src/queries/diagnostics/CfgConsistencyCounts.ql new file mode 100644 index 000000000000..ecc1833176aa --- /dev/null +++ b/rust/ql/src/queries/diagnostics/CfgConsistencyCounts.ql @@ -0,0 +1,15 @@ +/** + * @name Control flow graph inconsistency counts + * @description Counts the number of control flow graph inconsistencies of each type. This query is intended for internal use. + * @kind diagnostic + * @id rust/diagnostics/cfg-consistency-counts + */ + +import rust +import codeql.rust.controlflow.internal.CfgConsistency as Consistency + +// see also `rust/diagnostics/cfg-consistency`, which lists the +// individual inconsistency results. +from string type, int num +where num = Consistency::getCfgInconsistencyCounts(type) +select type, num diff --git a/rust/ql/test/query-tests/diagnostics/CfgConsistencyCounts.expected b/rust/ql/test/query-tests/diagnostics/CfgConsistencyCounts.expected new file mode 100644 index 000000000000..7df2863da9e8 --- /dev/null +++ b/rust/ql/test/query-tests/diagnostics/CfgConsistencyCounts.expected @@ -0,0 +1,13 @@ +| CFG scope lacks initial AST node | 0 | +| Dead end | 0 | +| Multiple successors of the same type | 0 | +| Multiple toStrings | 0 | +| Non-PostOrderTree Expr node | 0 | +| Non-unique list order | 0 | +| Non-unique set representation | 0 | +| Non-unique split kind | 0 | +| Simple and normal successors | 0 | +| Splitting invariant 2 | 0 | +| Splitting invariant 3 | 0 | +| Splitting invariant 4 | 0 | +| Splitting invariant 5 | 0 | diff --git a/rust/ql/test/query-tests/diagnostics/CfgConsistencyCounts.qlref b/rust/ql/test/query-tests/diagnostics/CfgConsistencyCounts.qlref new file mode 100644 index 000000000000..6e7ffa8aaa9d --- /dev/null +++ b/rust/ql/test/query-tests/diagnostics/CfgConsistencyCounts.qlref @@ -0,0 +1 @@ +queries/diagnostics/CfgConsistencyCounts.ql From ac9a8d602c21b88f216658e51d4bf4b37611af98 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 10 Oct 2024 11:35:22 +0100 Subject: [PATCH 5/8] Rust: Add metadata to the original CFG consistency query. --- rust/ql/consistency-queries/CfgConsistency.ql | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/rust/ql/consistency-queries/CfgConsistency.ql b/rust/ql/consistency-queries/CfgConsistency.ql index 7f5e1649b22d..9f766a9f6687 100644 --- a/rust/ql/consistency-queries/CfgConsistency.ql +++ b/rust/ql/consistency-queries/CfgConsistency.ql @@ -1 +1,8 @@ -import codeql.rust.controlflow.internal.CfgConsistency +/** + * @name Control flow graph inconsistencies + * @description Lists the control flow graph inconsistencies in the database. This query is intended for internal use. + * @kind table + * @id rust/diagnostics/cfg-consistency + */ + + import codeql.rust.controlflow.internal.CfgConsistency From 983179b84e6dd1acf72f565b7fcfb26325e5716c Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 15 Oct 2024 11:23:16 +0100 Subject: [PATCH 6/8] Rust: Autoformat. --- rust/ql/consistency-queries/CfgConsistency.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/ql/consistency-queries/CfgConsistency.ql b/rust/ql/consistency-queries/CfgConsistency.ql index 9f766a9f6687..4e155ca307b0 100644 --- a/rust/ql/consistency-queries/CfgConsistency.ql +++ b/rust/ql/consistency-queries/CfgConsistency.ql @@ -5,4 +5,4 @@ * @id rust/diagnostics/cfg-consistency */ - import codeql.rust.controlflow.internal.CfgConsistency +import codeql.rust.controlflow.internal.CfgConsistency From abc498130083832dc592bd7b57bc581708cbcc0f Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 15 Oct 2024 11:30:03 +0100 Subject: [PATCH 7/8] Rust: Address QL-for-QL complaint. --- rust/ql/src/queries/summary/Stats.qll | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll index c1f0bdd08961..242bb990e04a 100644 --- a/rust/ql/src/queries/summary/Stats.qll +++ b/rust/ql/src/queries/summary/Stats.qll @@ -20,4 +20,6 @@ int getLinesOfUserCode() { /** * Gets a count of the total number of control flow graph inconsistencies in the database. */ -int getTotalCfgInconsistencies() { result = sum(CfgConsistency::getCfgInconsistencyCounts(_)) } +int getTotalCfgInconsistencies() { + result = sum(string type | | CfgConsistency::getCfgInconsistencyCounts(type)) +} From 79c5adfc9a1f90cee974ce155294172c9008a12c Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 15 Oct 2024 14:10:51 +0100 Subject: [PATCH 8/8] Rust: Use correct versions of the consistency predicates. --- .../controlflow/internal/CfgConsistency.qll | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll b/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll index e463f5776652..378062e16b41 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/CfgConsistency.qll @@ -54,40 +54,40 @@ int getCfgInconsistencyCounts(string type) { // - `codeql.rust.controlflow.internal.CfgConsistency` (this file) // - `shared.controlflow.codeql.controlflow.Cfg` type = "Non-unique set representation" and - result = count(CfgImpl::Splits ss | Consistency::nonUniqueSetRepresentation(ss, _) | ss) + result = count(CfgImpl::Splits ss | nonUniqueSetRepresentation(ss, _) | ss) or type = "Splitting invariant 2" and - result = count(AstNode n | Consistency::breakInvariant2(n, _, _, _, _, _) | n) + result = count(AstNode n | breakInvariant2(n, _, _, _, _, _) | n) or type = "Splitting invariant 3" and - result = count(AstNode n | Consistency::breakInvariant3(n, _, _, _, _, _) | n) + result = count(AstNode n | breakInvariant3(n, _, _, _, _, _) | n) or type = "Splitting invariant 4" and - result = count(AstNode n | Consistency::breakInvariant4(n, _, _, _, _, _) | n) + result = count(AstNode n | breakInvariant4(n, _, _, _, _, _) | n) or type = "Splitting invariant 5" and - result = count(AstNode n | Consistency::breakInvariant5(n, _, _, _, _, _) | n) + result = count(AstNode n | breakInvariant5(n, _, _, _, _, _) | n) or type = "Multiple successors of the same type" and - result = count(CfgNode n | Consistency::multipleSuccessors(n, _, _) | n) + result = count(CfgNode n | multipleSuccessors(n, _, _) | n) or type = "Simple and normal successors" and - result = count(CfgNode n | Consistency::simpleAndNormalSuccessors(n, _, _, _, _) | n) + result = count(CfgNode n | simpleAndNormalSuccessors(n, _, _, _, _) | n) or type = "Dead end" and - result = count(CfgNode n | Consistency::deadEnd(n) | n) + result = count(CfgNode n | deadEnd(n) | n) or type = "Non-unique split kind" and - result = count(CfgImpl::SplitImpl si | Consistency::nonUniqueSplitKind(si, _) | si) + result = count(CfgImpl::SplitImpl si | nonUniqueSplitKind(si, _) | si) or type = "Non-unique list order" and - result = count(CfgImpl::SplitKind sk | Consistency::nonUniqueListOrder(sk, _) | sk) + result = count(CfgImpl::SplitKind sk | nonUniqueListOrder(sk, _) | sk) or type = "Multiple toStrings" and - result = count(CfgNode n | Consistency::multipleToString(n, _) | n) + result = count(CfgNode n | multipleToString(n, _) | n) or type = "CFG scope lacks initial AST node" and - result = count(CfgScope s | Consistency::scopeNoFirst(s) | s) + result = count(CfgScope s | scopeNoFirst(s) | s) or type = "Non-PostOrderTree Expr node" and result = count(Expr e | nonPostOrderExpr(e, _) | e)