From 7210bac169c32c3845903bb8a06c147f3bba7dcc Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Sat, 28 Feb 2026 10:47:40 +0100 Subject: [PATCH] fix(analyzers): avoid SSA dependency cycle blowups in issue #1555 paths (#1559) Route redirect dependency checks through the cycle-safe dependencyChecker instead of raw recursive valueDependsOn traversal. This ensures Phi-cycle graphs terminate quickly and avoids recursive work amplification that can look like hangs on large/generated codebases. Also remove the nil fallback in dependencyChecker.dependsOn so all analyzer paths consistently use cycle-aware logic. Add regression tests covering raw valueDependsOn behavior on: Phi cycle without target (must return false) Phi cycle with target path (must return true) Self-referential Phi node (must return false) Signed-off-by: Cosmin Cojocar --- analyzers/form_parsing_limits.go | 3 - .../form_parsing_limits_internal_test.go | 47 +++++++++++++++ analyzers/redirect_header_propagation.go | 59 +------------------ 3 files changed, 49 insertions(+), 60 deletions(-) diff --git a/analyzers/form_parsing_limits.go b/analyzers/form_parsing_limits.go index a3e1332..0c4723f 100644 --- a/analyzers/form_parsing_limits.go +++ b/analyzers/form_parsing_limits.go @@ -46,9 +46,6 @@ func newDependencyChecker() *dependencyChecker { } func (c *dependencyChecker) dependsOn(value ssa.Value, target ssa.Value) bool { - if c == nil { - return valueDependsOn(value, target, 0) - } return c.dependsOnDepth(value, target, 0) } diff --git a/analyzers/form_parsing_limits_internal_test.go b/analyzers/form_parsing_limits_internal_test.go index 0458c41..22186ad 100644 --- a/analyzers/form_parsing_limits_internal_test.go +++ b/analyzers/form_parsing_limits_internal_test.go @@ -43,3 +43,50 @@ func TestDependencyCheckerFindsTargetInPhiCycle(t *testing.T) { t.Fatal("expected stable memoized result on repeated call") } } + +func TestValueDependsOnHandlesPhiCycleWithoutTarget(t *testing.T) { + t.Parallel() + + target := ssa.NewConst(constant.MakeInt64(42), types.Typ[types.Int]) + + phiA := &ssa.Phi{} + phiB := &ssa.Phi{} + phiA.Edges = []ssa.Value{phiB} + phiB.Edges = []ssa.Value{phiA} + + if valueDependsOn(phiA, target, 0) { + t.Fatal("expected false for Phi cycle with no path to target") + } +} + +func TestValueDependsOnFindsTargetInPhiCycle(t *testing.T) { + t.Parallel() + + target := ssa.NewConst(constant.MakeInt64(7), types.Typ[types.Int]) + + phiA := &ssa.Phi{} + phiB := &ssa.Phi{} + phiA.Edges = []ssa.Value{phiB, target} + phiB.Edges = []ssa.Value{phiA} + + if !valueDependsOn(phiA, target, 0) { + t.Fatal("expected true when cycle has a path to target") + } + + if !valueDependsOn(phiA, target, 0) { + t.Fatal("expected stable result on repeated call") + } +} + +func TestValueDependsOnSelfReferentialPhi(t *testing.T) { + t.Parallel() + + target := ssa.NewConst(constant.MakeInt64(1), types.Typ[types.Int]) + + phi := &ssa.Phi{} + phi.Edges = []ssa.Value{phi} + + if valueDependsOn(phi, target, 0) { + t.Fatal("expected false for self-referential Phi with no path to target") + } +} diff --git a/analyzers/redirect_header_propagation.go b/analyzers/redirect_header_propagation.go index 9744ac3..d00c3ec 100644 --- a/analyzers/redirect_header_propagation.go +++ b/analyzers/redirect_header_propagation.go @@ -261,61 +261,6 @@ func extractStringConst(v ssa.Value) string { } func valueDependsOn(value ssa.Value, target ssa.Value, depth int) bool { - if value == nil || target == nil || depth > MaxDepth { - return false - } - if value == target { - return true - } - - switch v := value.(type) { - case *ssa.ChangeType: - return valueDependsOn(v.X, target, depth+1) - case *ssa.MakeInterface: - return valueDependsOn(v.X, target, depth+1) - case *ssa.TypeAssert: - return valueDependsOn(v.X, target, depth+1) - case *ssa.UnOp: - return valueDependsOn(v.X, target, depth+1) - case *ssa.FieldAddr: - return valueDependsOn(v.X, target, depth+1) - case *ssa.Field: - return valueDependsOn(v.X, target, depth+1) - case *ssa.IndexAddr: - return valueDependsOn(v.X, target, depth+1) || valueDependsOn(v.Index, target, depth+1) - case *ssa.Index: - return valueDependsOn(v.X, target, depth+1) || valueDependsOn(v.Index, target, depth+1) - case *ssa.Slice: - if valueDependsOn(v.X, target, depth+1) { - return true - } - if v.Low != nil && valueDependsOn(v.Low, target, depth+1) { - return true - } - if v.High != nil && valueDependsOn(v.High, target, depth+1) { - return true - } - return v.Max != nil && valueDependsOn(v.Max, target, depth+1) - case *ssa.Extract: - return valueDependsOn(v.Tuple, target, depth+1) - case *ssa.Phi: - for _, edge := range v.Edges { - if valueDependsOn(edge, target, depth+1) { - return true - } - } - return false - case *ssa.Call: - if v.Call.Value != nil && valueDependsOn(v.Call.Value, target, depth+1) { - return true - } - for _, arg := range v.Call.Args { - if valueDependsOn(arg, target, depth+1) { - return true - } - } - return false - default: - return false - } + checker := newDependencyChecker() + return checker.dependsOnDepth(value, target, depth) }