[sergo] Sergo Report: Reconcile-Refile + New Alias-Snapshot Bug - 2026-09-27 #63769
Closed
Replies: 1 comment
|
This discussion has been marked as outdated by Sergo - Serena Go Expert. A newer discussion is available at Discussion #63920. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Executive summary
Run R79 of the ongoing Serena-powered Go static-analysis audit of pkg/linters. Registry held steady at 73 analyzers (no new linter since R74). Reconciled 15 pre-run open issues against current code: 2 auto-expired (closed/not_planned) with their bugs still 100% present in the code, so both were refiled with fresh evidence. New-exploration budget went into a first-ever full read of lenstringzero alias tracking, since the never-solo-audited-linter backlog was fully exhausted last run - found a genuinely new bug class. 3 issues filed this run.
Tool and registry status
grep -c Analyzer, pkg/linters/registry.go= 73, unchanged since R74 (uncheckedsliceindex). No new linter to audit this run.Strategy: 50/50 cached vs new
Cached component (reconcile): gh api swept the 15 open sergo-labeled issues. 13 were unchanged. Two had auto-expired closed/not_planned since R78:
This is now the pattern for both lineages: the team auto-expires stale issues faster than the underlying one/two-line fixes land, so a periodic refile with re-verified evidence keeps them visible.
New-exploration component: with the never-solo-audited-linter backlog fully drained as of R78, this run pivoted to re-probing an already-known linter for a bug class it had never been checked against. Grepped for
map[types.Object]state-tracking helpers across pkg/linters and picked lenstringzero, whose alias-tracking feature (added years ago for then := len(s); n == 0idiom) had never had a dedicated bug audit.Findings
lenstringzero: alias table is a single pass-wide snapshot with no position tracking (new)
collectLenStringAliaseswalks every file in the package exactly once viaast.Inspect, building one alias map keyed bytypes.Object, beforerun()ever examines a single comparison. Every later query (analyzeLenStringExpr, invoked once per matchedBinaryExpr) reads that same, already-finalized map - so the map only ever reflects the FINAL state of each variable after all its DEFINE/reassignment/increment events in the file have already happened, never the state at the specific source position of the comparison being checked.Concrete failure: a variable is defined as
len(s), used in a comparison that should be flagged, then reassigned to something unrelated later in the same function. Because the reassignment deletes the alias during the single up-front collection pass (which completes before any comparison is even looked at), the EARLIER, textually-valid comparison is silently never flagged - a false negative. The existing testdata only covers the reassign-then-use order (which happens to produce the right answer for the wrong reason); the use-then-reassign order, which is where the bug actually surfaces, has zero coverage.Named this pattern class
presnapshot_position_blind- distinct from the previously-cataloguedblock_scope_state_reset(loses state too early, per nested block) andbranch_state_merge(conflates sibling branches): this one keeps one global answer for too long, blind to source position entirely.bufferresetbeforereuse: per-block state isolation (6th reverse-phantom refile)
Same root cause first filed at R63 (#60170):
analyzeStraightLineBlockgets fresh write/read maps every call, andcollectEventsrefuses to descend into nested if/for/switch bodies, so a write-then-read in an outer block never links to a reuse-write inside a nested block. Six confirmed cycles of auto-expire-then-refile now.astutil.StringLitValue / IsStringLiteral: missing ParenExpr unwrap (2nd reverse-phantom refile)
Both shared helpers still do a bare
*ast.BasicLittype assertion. Affects 5 call sites transitively (trimleftright, tolowerequalfold, errstringmatch, fmterrorfnoverbs, errorfwrapv) for a 2-line fix.Tasks generated (3 issues filed)
Metrics this run
Historical context
Recommendations
collect*helper called once before aPreorder/Inspectloop) is a candidate.Next run focus (R80)
Verify sg79a1/sg79a2/sg79a3 landed; re-tally open issues (expect 16); if the registry grows to 74 analyzers, audit the new linter fresh; otherwise probe other whole-pass-precomputed lookup tables for the presnapshot_position_blind class.
References:
All reactions