Skip to content

Go: fix CFG for nested labels - #22684

Open
owen-mc wants to merge 2 commits into
github:mainfrom
owen-mc:go/fix/cfg-nested-labels
Open

owen-mc wants to merge 2 commits into
github:mainfrom
owen-mc:go/fix/cfg-nested-labels

Conversation

@owen-mc

@owen-mc owen-mc commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Fix Go CFG construction for goto statements targeting an enclosing label, including the inner label in stacked labels.

Problem

The shared CFG logic resolves goto targets that are sibling statements in the same block. It did not handle this pattern:

outer:
inner:
{
    goto inner
}

Here, inner encloses the current block rather than appearing inside it. The goto completion therefore failed to reach its target.

This kind of code is rarely written by hand, but is commonly generated by parser generators.

Fix

We already have Go-specific goto handling to cover targets that the shared block logic cannot see. Currently it just covers Top-level statements in function bodies.. In this PR we extend it to labels enclosing the current node.

The target is matched against the direct LabeledStmt, ensuring stacked labels such as inner and outer remain distinct.

A focused CFG regression test covers all gotos in the stacked-label example and demonstrates the missing inner edge before the fix.

I have validated this fixed some incorrect alert changes in real projects.

This does not need a change note because it fixes FPs that have never been in a release.

@owen-mc
owen-mc requested a review from a team September 25, 2026 14:03
@owen-mc
owen-mc requested a review from a team as a code owner September 25, 2026 14:03
@owen-mc owen-mc added the no-change-note-required This PR does not need a change note label Sep 25, 2026
Copilot AI balanced review requested due to automatic review settings September 25, 2026 14:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The new guard still prevents resolution when an enclosing label directly labels the goto statement.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes Go CFG resolution for goto statements targeting enclosing and stacked labels.

Changes:

  • Extends Go-specific goto target handling to enclosing labels.
  • Adds an inline CFG regression test for stacked labels.
File Description
ControlFlowGraphImpl.qll Resolves additional enclosing goto targets.
GotoTarget.ql Defines inline CFG assertions.
gotos.go Adds stacked-label test cases.
GotoTarget.expected Adds the generated expected baseline.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll Outdated
@github-actions github-actions Bot added the Go label Sep 25, 2026
@owen-mc
owen-mc force-pushed the go/fix/cfg-nested-labels branch from 7de00b0 to cb2024e Compare September 25, 2026 15:51
@owen-mc
owen-mc force-pushed the go/fix/cfg-nested-labels branch from cb2024e to fba894a Compare September 25, 2026 15:53

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Go no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants