Skip to content

Commit 3487858

Browse files
Copilotowen-mc
andauthored
Fix shared Go CFG expression and return-edge regressions
Agent-Logs-Url: https://github.com/github/codeql/sessions/3f96ead2-cda4-479c-9e37-f38ace035870 Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com>
1 parent 66a191d commit 3487858

2 files changed

Lines changed: 22 additions & 3 deletions

File tree

go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,10 @@ module ControlFlow {
340340
* cannot return normally, but never fails to hold of a function that can return normally.
341341
*/
342342
predicate mayReturnNormally(FuncDecl f) {
343-
exists(GoCfg::ControlFlow::NormalExitNode exit | exit.getEnclosingCallable() = f)
343+
exists(GoCfg::ControlFlow::NormalExitNode exit |
344+
exit.getEnclosingCallable() = f and
345+
exists(exit.getAPredecessor())
346+
)
344347
}
345348

346349
/**

go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@ module GoCfg {
7979
or
8080
not node instanceof Callable and
8181
not exists(node.getEnclosingFunction()) and
82-
result = node.getFile() and
83-
result instanceof Callable
82+
result = node.getFile()
8483
}
8584

8685
class Stmt = Go::Stmt;
@@ -292,6 +291,10 @@ module GoCfg {
292291
predicate preOrderExpr(Ast::Expr e) { none() }
293292

294293
predicate postOrInOrder(Ast::AstNode n) {
294+
n instanceof Go::ReferenceExpr
295+
or
296+
n instanceof Go::BasicLit
297+
or
295298
n instanceof Go::CallExpr and
296299
not n = any(Go::DeferStmt defer).getCall() and
297300
not n = any(Go::GoStmt go_).getCall()
@@ -633,6 +636,12 @@ module GoCfg {
633636
c.hasLabel(TGoLabel(lbl.getLabel()))
634637
)
635638
or
639+
exists(Go::FuncDef fd |
640+
ast = fd.getBody() and
641+
c.getSuccessorType() instanceof ReturnSuccessor and
642+
n.isAfter(fd.getBody())
643+
)
644+
or
636645
exists(Go::LabeledStmt lbl, Go::FuncDef fd |
637646
ast = fd.getBody() and
638647
n.isBefore(lbl) and
@@ -841,6 +850,13 @@ module GoCfg {
841850
n1.isAfter(lastChild) and n2.isAdditional(ret, getFirstReturnEpilogueTag(ret))
842851
)
843852
or
853+
exists(Ast::AstNode lastChild | lastChild = getLastRankedChild(ret) |
854+
// After last expr → return node directly if there is no return epilogue
855+
not exists(getFirstReturnEpilogueTag(ret)) and
856+
n1.isAfter(lastChild) and
857+
n2.isIn(ret)
858+
)
859+
or
844860
// No expressions → before → return node directly
845861
not exists(getRankedChild(ret, _)) and
846862
n1.isBefore(ret) and

0 commit comments

Comments
 (0)