Skip to content

Commit 31356b6

Browse files
authored
fix: make markDepsForAnalyzingSource recursive to fix buildssa panic (#6376)
1 parent cb54f49 commit 31356b6

File tree

2 files changed

+125
-1
lines changed

2 files changed

+125
-1
lines changed

pkg/goanalysis/runner_action.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,10 @@ func (act *action) markDepsForAnalyzingSource() {
6363
// Horizontal deps (analyzer.Requires) must be loaded from source and analyzed before analyzing
6464
// this action.
6565
for _, dep := range act.Deps {
66-
if dep.Package == act.Package {
66+
if dep.Package == act.Package && !dep.needAnalyzeSource {
6767
// Analyze source only for horizontal dependencies, e.g. from "buildssa".
6868
dep.needAnalyzeSource = true // can't be set in parallel
69+
dep.markDepsForAnalyzingSource()
6970
}
7071
}
7172
}
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package goanalysis
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"golang.org/x/tools/go/packages"
8+
)
9+
10+
func Test_action_markDepsForAnalyzingSource(t *testing.T) {
11+
t.Run("marks direct horizontal deps", func(t *testing.T) {
12+
pkg := &packages.Package{PkgPath: "pkg/a"}
13+
14+
dep := &action{Package: pkg}
15+
act := &action{
16+
Package: pkg,
17+
Deps: []*action{dep},
18+
}
19+
20+
act.markDepsForAnalyzingSource()
21+
22+
assert.True(t, dep.needAnalyzeSource)
23+
})
24+
25+
t.Run("marks transitive horizontal deps", func(t *testing.T) {
26+
pkg := &packages.Package{PkgPath: "pkg/a"}
27+
28+
// Chain: act -> dep1 -> dep2
29+
dep2 := &action{Package: pkg}
30+
dep1 := &action{
31+
Package: pkg,
32+
Deps: []*action{dep2},
33+
}
34+
act := &action{
35+
Package: pkg,
36+
Deps: []*action{dep1},
37+
}
38+
39+
act.markDepsForAnalyzingSource()
40+
41+
assert.True(t, dep1.needAnalyzeSource)
42+
assert.True(t, dep2.needAnalyzeSource)
43+
})
44+
45+
t.Run("marks deep transitive horizontal deps", func(t *testing.T) {
46+
pkg := &packages.Package{PkgPath: "pkg/a"}
47+
48+
// Chain: act -> dep1 -> dep2 -> dep3 (simulates nilaway -> buildssa -> ctrlflow -> inspect)
49+
dep3 := &action{Package: pkg}
50+
dep2 := &action{
51+
Package: pkg,
52+
Deps: []*action{dep3},
53+
}
54+
dep1 := &action{
55+
Package: pkg,
56+
Deps: []*action{dep2},
57+
}
58+
act := &action{
59+
Package: pkg,
60+
Deps: []*action{dep1},
61+
}
62+
63+
act.markDepsForAnalyzingSource()
64+
65+
assert.True(t, dep1.needAnalyzeSource)
66+
assert.True(t, dep2.needAnalyzeSource)
67+
assert.True(t, dep3.needAnalyzeSource)
68+
})
69+
70+
t.Run("does not mark cross-package deps", func(t *testing.T) {
71+
pkgA := &packages.Package{PkgPath: "pkg/a"}
72+
pkgB := &packages.Package{PkgPath: "pkg/b"}
73+
74+
dep := &action{Package: pkgB}
75+
act := &action{
76+
Package: pkgA,
77+
Deps: []*action{dep},
78+
}
79+
80+
act.markDepsForAnalyzingSource()
81+
82+
assert.False(t, dep.needAnalyzeSource)
83+
})
84+
85+
t.Run("handles cycles without infinite recursion", func(t *testing.T) {
86+
pkg := &packages.Package{PkgPath: "pkg/a"}
87+
88+
dep1 := &action{Package: pkg}
89+
dep2 := &action{Package: pkg}
90+
91+
dep1.Deps = []*action{dep2}
92+
dep2.Deps = []*action{dep1}
93+
94+
act := &action{
95+
Package: pkg,
96+
Deps: []*action{dep1},
97+
}
98+
99+
// Should not hang or panic
100+
act.markDepsForAnalyzingSource()
101+
102+
assert.True(t, dep1.needAnalyzeSource)
103+
assert.True(t, dep2.needAnalyzeSource)
104+
})
105+
106+
t.Run("skips already marked deps", func(t *testing.T) {
107+
pkg := &packages.Package{PkgPath: "pkg/a"}
108+
109+
dep := &action{
110+
Package: pkg,
111+
needAnalyzeSource: true, // already marked
112+
}
113+
act := &action{
114+
Package: pkg,
115+
Deps: []*action{dep},
116+
}
117+
118+
// Should not recurse into already-marked dep
119+
act.markDepsForAnalyzingSource()
120+
121+
assert.True(t, dep.needAnalyzeSource)
122+
})
123+
}

0 commit comments

Comments
 (0)