8000 fix(sa5009): avoid false positives for loop-populated slices by scnsh · Pull Request #1846 · dominikh/go-tools · GitHub
[go: up one dir, main page]
Skip to content

fix(sa5009): avoid false positives for loop-populated slices - #1846

Closed
scnsh wants to merge 1 commit into
dominikh:masterfrom
scnsh:fix/sa5009-slice-arguments
Closed

fix(sa5009): avoid false positives for loop-populated slices#1846
scnsh wants to merge 1 commit into
dominikh:masterfrom
scnsh:fix/sa5009-slice-arguments

Conversation

@scnsh
@scnsh scnsh commented Oct 21, 2026
Copy link
Copy Markdown

Fixes #1845

Summary

irutil.Vararg recovers arguments from stores through the backing array. Stores through the slice itself are not among that allocation's referrers, so a nonempty slice can incorrectly be reported as a known empty argument list. SA5009 then reports missing arguments for valid fmt.Sprintf calls.

The loop argument used to reach the check through Phi/Sigma nodes in 2026.1. The newer SSA representation exposes the slice directly, making this incorrect empty-list result observable.

Only accept recovered arguments when their count matches the backing array length. Otherwise, let SA5009 skip the call because its argument values are unknown.

 recover stores through the backing array
+if recovered count does not match array length
+    return unknown
 return recovered arguments

Evidence

  • Before: the regression fixture reports unexpected SA5009 diagnostics for both a loop-populated slice and a zero-initialized nonempty slice.
    After: both calls are accepted, while an actually empty slice still produces the expected missing-argument diagnostic.
  • Regression tests: go test ./staticcheck/sa5009 ./go/ir/irutil on Go 1.26.0 and Go 1.27.2.
  • Full tests: go test ./... on Go 1.27.2.
  • Static analysis: go vet ./... and the rebuilt staticcheck ./... on Go 1.27.2.
  • A rebuilt Staticcheck also accepts the standalone reproduction from SA5009: false positive reporting zero arguments for a slice populated in a loop #1845 and the original affected tests without any ignore directives.

Merge Danger

Door: two-way

Blast Radius: SA5009

SA5009 is the only in-repository caller of irutil.Vararg. Incomplete recovery now returns an unknown result instead of a misleading argument list. Existing known-argument diagnostics remain covered by the analyzer fixtures.

@scnsh
fix(irutil): reject incomplete variadic argument recovery
ee2c7aa 
Stores through a slice are not visible in the backing allocation's referrers, so nonempty slices can be mistaken for known empty argument lists

Treat unrecovered elements as unknown to avoid SA5009 false positives while retaining diagnostics for actual empty argument lists
@scnsh
scnsh marked this pull request as draft October 21, 2026 09:34
@scnsh scnsh closed this Oct 21, 2026
@scnsh
scnsh commented Oct 21, 2026
Copy link
Copy Markdown
Author

Closing this PR to respect the contribution policy in CONTRIBUTING.md, which asks that problems be reported as issues rather than pull requests.

The bug report, minimal reproduction, and version comparison remain available in #1845. Apologies for overlooking the contribution policy before opening this PR.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SA5009: false positive reporting zero arguments for a slice populated in a loop

1 participant

@scnsh
0