From 79f4f78fc2ab974e3d8ab4a901c392779690efe4 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 15 Nov 2024 11:07:01 +0000 Subject: [PATCH 1/3] Make separate classes for control flow node kinds This puts all the logic of a particular control flow node kind into one place and makes it easier to add new kinds. --- .../lib/semmle/code/java/ControlFlowGraph.qll | 100 ++++++++++++------ 1 file changed, 67 insertions(+), 33 deletions(-) diff --git a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll index e6ba8485eeee..6473d4572a5f 100644 --- a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll +++ b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll @@ -105,29 +105,19 @@ module ControlFlow { /** A node in the expression-level control-flow graph. */ class Node extends TNode { /** Gets the statement containing this node, if any. */ - Stmt getEnclosingStmt() { - result = this.asStmt() or - result = this.asExpr().getEnclosingStmt() - } + Stmt getEnclosingStmt() { none() } /** Gets the immediately enclosing callable whose body contains this node. */ - Callable getEnclosingCallable() { - this = TExitNode(result) or - result = this.asStmt().getEnclosingCallable() or - result = this.asExpr().getEnclosingCallable() - } + Callable getEnclosingCallable() { none() } /** Gets the statement this `Node` corresponds to, if any. */ - Stmt asStmt() { this = TStmtNode(result) } + Stmt asStmt() { none() } /** Gets the expression this `Node` corresponds to, if any. */ - Expr asExpr() { this = TExprNode(result) } + Expr asExpr() { none() } /** Gets the call this `Node` corresponds to, if any. */ - Call asCall() { - result = this.asExpr() or - result = this.asStmt() - } + Call asCall() { none() } /** Gets an immediate successor of this node. */ Node getASuccessor() { result = succ(this) } @@ -148,33 +138,77 @@ module ControlFlow { BasicBlock getBasicBlock() { result.getANode() = this } /** Gets a textual representation of this element. */ - string toString() { - result = this.asExpr().toString() - or - result = this.asStmt().toString() - or - result = "Exit" and this instanceof ExitNode - } + string toString() { none() } /** Gets the source location for this element. */ - Location getLocation() { - result = this.asExpr().getLocation() or - result = this.asStmt().getLocation() or - result = this.(ExitNode).getEnclosingCallable().getLocation() - } + Location getLocation() { none() } /** * Gets the most appropriate AST node for this control flow node, if any. */ - ExprParent getAstNode() { - result = this.asExpr() or - result = this.asStmt() or - this = TExitNode(result) - } + ExprParent getAstNode() { none() } + } + + /** A control-flow node that represents the evaluation of an expression. */ + class ExprNode extends Node, TExprNode { + Expr e; + + ExprNode() { this = TExprNode(e) } + + override Stmt getEnclosingStmt() { result = e.getEnclosingStmt() } + + override Callable getEnclosingCallable() { result = e.getEnclosingCallable() } + + override Expr asExpr() { result = e } + + override Call asCall() { result = e } + + override ExprParent getAstNode() { result = e } + + /** Gets a textual representation of this element. */ + override string toString() { result = e.toString() } + + /** Gets the source location for this element. */ + override Location getLocation() { result = e.getLocation() } + } + + /** A control-flow node that represents a statement. */ + class StmtNode extends Node, TStmtNode { + Stmt s; + + StmtNode() { this = TStmtNode(s) } + + override Stmt getEnclosingStmt() { result = s } + + override Callable getEnclosingCallable() { result = s.getEnclosingCallable() } + + override Stmt asStmt() { result = s } + + override Call asCall() { result = s } + + override ExprParent getAstNode() { result = s } + + override string toString() { result = s.toString() } + + override Location getLocation() { result = s.getLocation() } } /** A control flow node indicating the termination of a callable. */ - class ExitNode extends Node, TExitNode { } + class ExitNode extends Node, TExitNode { + Callable c; + + ExitNode() { this = TExitNode(c) } + + override Callable getEnclosingCallable() { result = c } + + override ExprParent getAstNode() { result = c } + + /** Gets a textual representation of this element. */ + override string toString() { result = "Exit" } + + /** Gets the source location for this element. */ + override Location getLocation() { result = c.getLocation() } + } } class ControlFlowNode = ControlFlow::Node; From aaa4361120c966fadadc947a710e7d0737256619 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 15 Nov 2024 11:21:58 +0000 Subject: [PATCH 2/3] Rearrange member predicates in ControlFlow::Node Put all the ones which might need to be overrridden by subclasses together for ease of reading. --- .../lib/semmle/code/java/ControlFlowGraph.qll | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll index 6473d4572a5f..4702fb45b092 100644 --- a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll +++ b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll @@ -104,21 +104,6 @@ module ControlFlow { /** A node in the expression-level control-flow graph. */ class Node extends TNode { - /** Gets the statement containing this node, if any. */ - Stmt getEnclosingStmt() { none() } - - /** Gets the immediately enclosing callable whose body contains this node. */ - Callable getEnclosingCallable() { none() } - - /** Gets the statement this `Node` corresponds to, if any. */ - Stmt asStmt() { none() } - - /** Gets the expression this `Node` corresponds to, if any. */ - Expr asExpr() { none() } - - /** Gets the call this `Node` corresponds to, if any. */ - Call asCall() { none() } - /** Gets an immediate successor of this node. */ Node getASuccessor() { result = succ(this) } @@ -137,6 +122,21 @@ module ControlFlow { /** Gets the basic block that contains this node. */ BasicBlock getBasicBlock() { result.getANode() = this } + /** Gets the statement containing this node, if any. */ + Stmt getEnclosingStmt() { none() } + + /** Gets the immediately enclosing callable whose body contains this node. */ + Callable getEnclosingCallable() { none() } + + /** Gets the statement this `Node` corresponds to, if any. */ + Stmt asStmt() { none() } + + /** Gets the expression this `Node` corresponds to, if any. */ + Expr asExpr() { none() } + + /** Gets the call this `Node` corresponds to, if any. */ + Call asCall() { none() } + /** Gets a textual representation of this element. */ string toString() { none() } From 8e1178918630fddaaaabb4157b21f8da8b62eee2 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 12 Dec 2024 12:30:01 +0000 Subject: [PATCH 3/3] Restore `asStmt`, `asExpr` and `asCall` to `Node` It doesn't really make sense to define them in terms of dispatch. --- .../lib/semmle/code/java/ControlFlowGraph.qll | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll index 4702fb45b092..e3c7ed6e5d9e 100644 --- a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll +++ b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll @@ -129,13 +129,16 @@ module ControlFlow { Callable getEnclosingCallable() { none() } /** Gets the statement this `Node` corresponds to, if any. */ - Stmt asStmt() { none() } + Stmt asStmt() { this = TStmtNode(result) } /** Gets the expression this `Node` corresponds to, if any. */ - Expr asExpr() { none() } + Expr asExpr() { this = TExprNode(result) } /** Gets the call this `Node` corresponds to, if any. */ - Call asCall() { none() } + Call asCall() { + result = this.asExpr() or + result = this.asStmt() + } /** Gets a textual representation of this element. */ string toString() { none() } @@ -159,10 +162,6 @@ module ControlFlow { override Callable getEnclosingCallable() { result = e.getEnclosingCallable() } - override Expr asExpr() { result = e } - - override Call asCall() { result = e } - override ExprParent getAstNode() { result = e } /** Gets a textual representation of this element. */ @@ -182,10 +181,6 @@ module ControlFlow { override Callable getEnclosingCallable() { result = s.getEnclosingCallable() } - override Stmt asStmt() { result = s } - - override Call asCall() { result = s } - override ExprParent getAstNode() { result = s } override string toString() { result = s.toString() }