github / github/gh-aw

bufferresetbeforereuse: per-block state isolation misses reuse across if/for/switch boundaries

Open
#60,170 0 comments 0 reactions 0 assignees View on GitHub
cookie sergo
Dominant language
Go
Stars
5.1k
Forks
541
Avg merge
5h 48m
Merged PRs (30d)
773

Description

**Summary**

bufferresetbeforereuse (the newest, 68th registered analyzer -- pkg/linters/bufferresetbeforereuse/bufferresetbeforereuse.go) is supposed to flag bytes.Buffer/strings.Builder reuse (write, then read via String()/Bytes()/Len(), then write again) without an intervening Reset(). The detector never links a write/read pair in an outer block with a later write inside a nested if/for/switch/select block, so the exact pattern the linter is designed to catch escapes detection whenever it spans a control-flow boundary -- arguably the most common real-world shape of this bug.

**Root cause**

- analyzeBlockForBufferReuse (bufferresetbeforereuse.go:74-84) walks the whole function body with ast.Inspect and calls analyzeStraightLineBlock once per BlockStmt node it encounters (the function body itself, plus every nested if/for/switch body, since ast.Inspects default return true lets it descend into those nested blocks).
- analyzeStraightLineBlock (bufferresetbeforereuse.go:86-130) creates brand-new written/read maps on each call (line 92-93) and calls collectEvents, which explicitly stops descent at IfStmt/ForStmt/RangeStmt/SwitchStmt/TypeSwitchStmt/SelectStmt/nested BlockStmt (collectEvents, line 133-155, the return false cases at line 137-139).
- Net effect: each blocks write/read/reset history is analyzed in total isolation from its enclosing block. A write in the outer scope, followed by a read, followed by a reuse-write inside a nested if/for block, is invisible to the linter because the inner blocks local written/read maps start empty -- the reuse-write is treated as if it were the first write ever seen.

**Reproduction (not in testdata)**

```
func reuseCrossIf(cond bool) {
var buf bytes.Buffer
buf.WriteString("first")
_ = buf.String()
if cond {
buf.WriteString("second") // reused without Reset -- NOT flagged
}
}
```

Trace: the outer blocks collectEvents treats the whole IfStmt as opaque (stops before descending into it), so it only sees write, read for buf at the outer level -- no violation (correct, in isolation). Separately, analyzeBlockForBufferReuses outer ast.Inspect also visits the ifs BlockStmt body directly and re-invokes analyzeStraightLineBlock on it with fresh maps; that isolated call sees only buf.WriteString("second"), a first write with no prior read recorded, so it is never reported. The same escape applies to for/switch/select bodies.

**Impact**

This is a false negative on the linters core, advertised purpose. The existing testdata (testdata/src/a/a.go) only tests same-block reuse and one true-negative branch case (okMutuallyExclusiveBranches, an early-return before the reuse point) -- there is no test at all for the (very common) case where a write+read happens before a conditional/loop and the buffer is written again inside it. Since the analyzer is newly registered and not yet in cgo.yml LINTER_FLAGS, this is latent rather than a live CI gap, but the gap will persist invisibly once enforcement is turned on.

**Recommendation**

Track write/read/reset state per-variable across the whole function scope (not per-BlockStmt), the same way the existing resourcetracker framework (used by fileclosenotdeferred/contextcancelnotdeferred/manualmutexunlock) tracks acquisitions across a FuncDecl, only breaking at FuncLit boundaries (which bufferresetbeforereuse already does correctly at the top level). A single ast.Inspect per FuncDecl/FuncLit that accumulates one written/read map for the whole function (still stopping only at FuncLit, not at every control-flow block) would catch the cross-block case while keeping the existing single-block tests passing.

**Validation checklist**

- Add a testdata case reproducing reuseCrossIf above (write+read outside, reuse-write inside if) and confirm it currently passes silently, then add a want comment after the fix
- Add a for-loop-body variant (write+read before the loop, reuse-write inside the loop) to same testdata
- Verify okMutuallyExclusiveBranches (early-return case) still reports no diagnostic after the fix
- Confirm existing same-block cases (notOkReuseWithoutReset, etc.) still report correctly

**Effort:** small-medium -- the state-tracking restructure is localized to analyzeBlockForBufferReuse/analyzeStraightLineBlock/collectEvents; no changes needed to run, checkBufferReuse, or the object-identification helpers.

> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>

> Generated by [🤖 Sergo - Serena Go Expert](https://github.com/github/gh-aw/actions/runs/34559696103) · claude · agent · 252.5 AIC · ⌖ 6.37 AIC · ⊞ 6.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Fsergo%22&type=issues)
> - [x] expires on Sep 17, 2026, 7:58 PM UTC-08:00

Contributor guide

Open the contributing guide

Research direction

Start in pkg/linters/bufferresetbeforereuse/bufferresetbeforereuse.go by reading analyzeBlockForBufferReuse, analyzeStraightLineBlock, and collectEvents. Add testdata cases for reuseCrossIf and the for-loop variant in testdata/src/a/a.go, then verify the new cases report diagnostics while okMutuallyExclusiveBranches and existing same-block cases retain their current behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.