Improve taint analyzer performance with shared SSA cache, parallel analyzer execution, and CI regression guard (#1530)

* Improve taint analyzer performance with shared SSA cache, parallel analyzer execution, and CI regression guard * Added a shared per-package SSA analysis cache with lazy, concurrency-safe call graph reuse across analyzers. * Updated taint analyzers to consume the shared cache instead of recomputing expensive artifacts per rule run. * Parallelized analyzer execution at package level while preserving deterministic issue aggregation. * Added a package-level taint benchmark to measure real end-to-end taint analyzer pass performance. * Introduced a CI benchmark regression guard with configurable thresholds for ns/op, B/op, and allocs/op. * Documented the performance guard workflow, local run command, and baseline update process in the README. Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch> * Fix script Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch> --------- Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
2026-06-20 00:15:59 +02:00 · 2026-02-19 16:50:41 +01:00
parent bd11fbe2ba
commit caf93d07f1
10 changed files with 447 additions and 44 deletions
@@ -0,0 +1,10 @@
+# Baseline metrics for BenchmarkTaintPackageAnalyzers_SharedCache
+# Update with: BENCH_COUNT=10 tools/check_taint_benchmark.sh --update-baseline
+BASE_NS_OP=33593865
+BASE_B_PER_OP=8641204
+BASE_ALLOCS_PER_OP=51374
+
+# Allowed regressions (%) relative to baseline
+NS_OP_REGRESSION_PCT=15
+B_PER_OP_REGRESSION_PCT=10
+ALLOCS_PER_OP_REGRESSION_PCT=10
@@ -43,8 +43,28 @@ jobs:
        run: make test
      - name: Perf Diff
        run: make perf-diff
+  taint-perf-guard:
+    runs-on: ubuntu-latest
+    env:
+      GO111MODULE: on
+      BENCH_COUNT: "5"
+    steps:
+      - name: Setup go
+        uses: actions/setup-go@v6
+        with:
+          go-version: "1.26.0"
+      - name: Checkout Source
+        uses: actions/checkout@v6
+      - uses: actions/cache@v5
+        with:
+          path: ~/go/pkg/mod
+          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go-
+      - name: Check taint benchmark regression
+        run: bash tools/check_taint_benchmark.sh
  coverage:
-    needs: [test]
+    needs: [test, taint-perf-guard]
    runs-on: ubuntu-latest
    env:
      GO111MODULE: on
@@ -171,6 +171,27 @@ nogo(
 go install github.com/securego/gosec/v2/cmd/gosec@latest
 ```

+## Performance regression guard
+
+CI includes a taint-analysis benchmark guard based on `BenchmarkTaintPackageAnalyzers_SharedCache`.
+
+- Baseline values and allowed regression thresholds are stored in [.github/benchmarks/taint_benchmark_baseline.env](.github/benchmarks/taint_benchmark_baseline.env).
+- CI runs the guard script [tools/check_taint_benchmark.sh](tools/check_taint_benchmark.sh), averages several benchmark samples, and fails if `ns/op`, `B/op`, or `allocs/op` exceed configured thresholds.
+
+Run the guard locally:
+
+```bash
+bash tools/check_taint_benchmark.sh
+```
+
+Update the baseline after intentional performance changes:
+
+```bash
+BENCH_COUNT=10 bash tools/check_taint_benchmark.sh --update-baseline
+```
+
+When baseline updates are intentional, commit both the benchmark-related code changes and the updated baseline file.
+
 ## Usage

 Gosec can be configured to only run a subset of rules, to exclude certain file
@@ -559,55 +559,77 @@ func (gosec *Analyzer) CheckAnalyzersWithSSA(pkg *packages.Package, ssaResult *b

 // checkAnalyzersWithSSA runs analyzers on a given package using an existing SSA result (Stateless API).
 func (gosec *Analyzer) checkAnalyzersWithSSA(pkg *packages.Package, ssaResult *buildssa.SSA, allIgnores ignores) ([]*issue.Issue, *Metrics) {
-	resultMap := map[*analysis.Analyzer]any{
-		buildssa.Analyzer: &ssautil.SSAAnalyzerResult{
-			Config: gosec.Config(),
-			Logger: gosec.logger,
-			SSA:    ssaResult,
-		},
+	sharedCache := ssautil.NewPackageAnalysisCache(ssaResult)
+	ssaAnalyzerResult := &ssautil.SSAAnalyzerResult{
+		Config: gosec.Config(),
+		Logger: gosec.logger,
+		SSA:    ssaResult,
+		Shared: sharedCache,
 	}

 	generatedFiles := gosec.generatedFiles(pkg)
 	issues := make([]*issue.Issue, 0)
 	stats := &Metrics{}
+	analyzerRuns := make([][]*issue.Issue, len(gosec.analyzerSet.Analyzers))
+
+	runner := errgroup.Group{}
+	runner.SetLimit(max(gosec.concurrency, 1))
+
+	for index, analyzer := range gosec.analyzerSet.Analyzers {
+		runner.Go(func() error {
+			pass := &analysis.Pass{
+				Analyzer:     analyzer,
+				Fset:         pkg.Fset,
+				Files:        pkg.Syntax,
+				OtherFiles:   pkg.OtherFiles,
+				IgnoredFiles: pkg.IgnoredFiles,
+				Pkg:          pkg.Types,
+				TypesInfo:    pkg.TypesInfo,
+				TypesSizes:   pkg.TypesSizes,
+				ResultOf: map[*analysis.Analyzer]any{
+					buildssa.Analyzer: ssaAnalyzerResult,
+				},
+				Report:            func(d analysis.Diagnostic) {},
+				ImportObjectFact:  nil,
+				ExportObjectFact:  nil,
+				ImportPackageFact: nil,
+				ExportPackageFact: nil,
+				AllObjectFacts:    nil,
+				AllPackageFacts:   nil,
+			}
+
+			result, err := pass.Analyzer.Run(pass)
+			if err != nil {
+				gosec.logger.Printf("Error running analyzer %s: %s\n", analyzer.Name, err)
+				return nil
+			}
+
+			if result == nil {
+				return nil
+			}

-	for _, analyzer := range gosec.analyzerSet.Analyzers {
-		pass := &analysis.Pass{
-			Analyzer:          analyzer,
-			Fset:              pkg.Fset,
-			Files:             pkg.Syntax,
-			OtherFiles:        pkg.OtherFiles,
-			IgnoredFiles:      pkg.IgnoredFiles,
-			Pkg:               pkg.Types,
-			TypesInfo:         pkg.TypesInfo,
-			TypesSizes:        pkg.TypesSizes,
-			ResultOf:          resultMap,
-			Report:            func(d analysis.Diagnostic) {},
-			ImportObjectFact:  nil,
-			ExportObjectFact:  nil,
-			ImportPackageFact: nil,
-			ExportPackageFact: nil,
-			AllObjectFacts:    nil,
-			AllPackageFacts:   nil,
-		}
-		result, err := pass.Analyzer.Run(pass)
-		if err != nil {
-			gosec.logger.Printf("Error running analyzer %s: %s\n", analyzer.Name, err)
-			continue
-		}
-		if result != nil {
 			if passIssues, ok := result.([]*issue.Issue); ok {
-				for _, iss := range passIssues {
-					if gosec.excludeGenerated {
-						if _, ok := generatedFiles[iss.File]; ok {
-							continue
-						}
-					}
+				analyzerRuns[index] = passIssues
+			}

-					// issue filtering logic
-					issues = gosec.updateIssues(iss, issues, stats, allIgnores)
+			return nil
+		})
+	}
+
+	if err := runner.Wait(); err != nil {
+		gosec.logger.Printf("Error waiting for analyzers: %s\n", err)
+	}
+
+	for _, passIssues := range analyzerRuns {
+		for _, iss := range passIssues {
+			if gosec.excludeGenerated {
+				if _, ok := generatedFiles[iss.File]; ok {
+					continue
 				}
 			}
+
+			// issue filtering logic
+			issues = gosec.updateIssues(iss, issues, stats, allIgnores)
 		}
 	}
 	return issues, stats
@@ -0,0 +1,120 @@
+package gosec
+
+import (
+	"fmt"
+	"io"
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"golang.org/x/tools/go/packages"
+
+	"github.com/securego/gosec/v2/analyzers"
+)
+
+func BenchmarkTaintPackageAnalyzers_SharedCache(b *testing.B) {
+	pkg := createTaintBenchmarkPackage(b, generateTaintStressProgram(180))
+
+	logger := log.New(io.Discard, "", 0)
+	analyzer := NewAnalyzer(NewConfig(), false, false, false, 6, logger)
+	analyzer.LoadAnalyzers(analyzers.Generate(false,
+		analyzers.NewAnalyzerFilter(false, "G701", "G702", "G703", "G704", "G705", "G706"),
+	).AnalyzersInfo())
+
+	ssaResult, err := analyzer.buildSSA(pkg)
+	if err != nil {
+		b.Fatalf("failed to build SSA: %v", err)
+	}
+
+	b.ResetTimer()
+	for range b.N {
+		issues, stats := analyzer.checkAnalyzersWithSSA(pkg, ssaResult, nil)
+		if stats == nil {
+			b.Fatal("stats is nil")
+		}
+		if issues == nil {
+			b.Fatal("issues slice is nil")
+		}
+	}
+}
+
+func createTaintBenchmarkPackage(b *testing.B, source string) *packages.Package {
+	b.Helper()
+
+	tmpDir, err := os.MkdirTemp("", "gosec_taint_bench")
+	if err != nil {
+		b.Fatalf("failed to create temp dir: %v", err)
+	}
+	b.Cleanup(func() { _ = os.RemoveAll(tmpDir) })
+
+	mainGo := filepath.Join(tmpDir, "main.go")
+	if err := os.WriteFile(mainGo, []byte(source), 0o600); err != nil {
+		b.Fatalf("failed to write source file: %v", err)
+	}
+
+	goMod := filepath.Join(tmpDir, "go.mod")
+	if err := os.WriteFile(goMod, []byte("module bench\n\ngo 1.25\n"), 0o600); err != nil {
+		b.Fatalf("failed to write go.mod: %v", err)
+	}
+
+	conf := &packages.Config{
+		Mode: LoadMode,
+		Dir:  tmpDir,
+	}
+
+	pkgs, err := packages.Load(conf, ".")
+	if err != nil {
+		b.Fatalf("failed to load package: %v", err)
+	}
+	if len(pkgs) == 0 {
+		b.Fatal("no packages loaded")
+	}
+	if len(pkgs[0].Errors) > 0 {
+		b.Fatalf("errors loading package: %v", pkgs[0].Errors)
+	}
+
+	return pkgs[0]
+}
+
+func generateTaintStressProgram(functionCount int) string {
+	var sb strings.Builder
+
+	sb.WriteString("package main\n")
+	sb.WriteString("\nimport (\n")
+	sb.WriteString("\t\"database/sql\"\n")
+	sb.WriteString("\t\"fmt\"\n")
+	sb.WriteString("\t\"log\"\n")
+	sb.WriteString("\t\"net/http\"\n")
+	sb.WriteString("\t\"os\"\n")
+	sb.WriteString("\t\"os/exec\"\n")
+	sb.WriteString(")\n\n")
+
+	sb.WriteString("var globalDB *sql.DB\n\n")
+
+	for i := range functionCount {
+		fmt.Fprintf(&sb, "func sinkFanout%d(w http.ResponseWriter, r *http.Request) {\n", i)
+		sb.WriteString("\tq := r.URL.Query().Get(\"q\")\n")
+		sb.WriteString("\tenv := os.Getenv(\"TAINT_ENV\")\n")
+		sb.WriteString("\tjoined := q + env\n")
+		sb.WriteString("\t_, _ = globalDB.Query(joined)\n")
+		sb.WriteString("\t_ = exec.Command(\"sh\", \"-c\", joined)\n")
+		sb.WriteString("\t_, _ = os.Open(joined)\n")
+		sb.WriteString("\t_, _ = http.Get(joined)\n")
+		sb.WriteString("\t_, _ = fmt.Fprintf(w, \"%s\", joined)\n")
+		sb.WriteString("\t_, _ = w.Write([]byte(joined))\n")
+		sb.WriteString("\tlog.Print(joined)\n")
+		sb.WriteString("}\n\n")
+	}
+
+	sb.WriteString("func main() {\n")
+	sb.WriteString("\thttp.HandleFunc(\"/\", func(w http.ResponseWriter, r *http.Request) {\n")
+	for i := range functionCount {
+		fmt.Fprintf(&sb, "\t\tsinkFanout%d(w, r)\n", i)
+	}
+	sb.WriteString("\t})\n")
+	sb.WriteString("}\n")
+
+	return sb.String()
+}
@@ -0,0 +1,40 @@
+package ssautil
+
+import (
+	"sync"
+
+	"golang.org/x/tools/go/analysis/passes/buildssa"
+	"golang.org/x/tools/go/callgraph"
+	"golang.org/x/tools/go/callgraph/cha"
+)
+
+// PackageAnalysisCache stores expensive SSA-derived artifacts that can be
+// shared by multiple analyzers running on the same package.
+type PackageAnalysisCache struct {
+	ssa *buildssa.SSA
+
+	callGraphOnce sync.Once
+	callGraph     *callgraph.Graph
+}
+
+// NewPackageAnalysisCache builds a cache object for a package-level SSA result.
+func NewPackageAnalysisCache(ssaResult *buildssa.SSA) *PackageAnalysisCache {
+	return &PackageAnalysisCache{ssa: ssaResult}
+}
+
+// CallGraph returns a lazily initialized CHA call graph for the package.
+// It is safe for concurrent use by multiple analyzers.
+func (c *PackageAnalysisCache) CallGraph() *callgraph.Graph {
+	if c == nil {
+		return nil
+	}
+
+	c.callGraphOnce.Do(func() {
+		if c.ssa == nil || len(c.ssa.SrcFuncs) == 0 || c.ssa.SrcFuncs[0] == nil {
+			return
+		}
+		c.callGraph = cha.CallGraph(c.ssa.SrcFuncs[0].Prog)
+	})
+
+	return c.callGraph
+}
@@ -20,6 +20,7 @@ type SSAAnalyzerResult struct {
 	Config map[string]any
 	Logger *log.Logger
 	SSA    *buildssa.SSA
+	Shared *PackageAnalysisCache
 }

 // GetSSAResult retrieves the SSA result from analysis pass
@@ -56,6 +56,9 @@ func makeAnalyzerRunner(rule *RuleInfo, config *Config) func(*analysis.Pass) (in

 		// Run taint analysis
 		analyzer := New(config)
+		if ssaResult.Shared != nil {
+			analyzer.SetCallGraph(ssaResult.Shared.CallGraph())
+		}
 		results := analyzer.Analyze(srcFuncs[0].Prog, srcFuncs)

 		// Convert results to gosec issues
@@ -102,6 +102,11 @@ type Analyzer struct {
 	callGraph  *callgraph.Graph
 }

+// SetCallGraph injects a precomputed call graph.
+func (a *Analyzer) SetCallGraph(cg *callgraph.Graph) {
+	a.callGraph = cg
+}
+
 // New creates a new taint analyzer with the given configuration.
 func New(config *Config) *Analyzer {
 	a := &Analyzer{
@@ -176,10 +181,12 @@ func (a *Analyzer) Analyze(prog *ssa.Program, srcFuncs []*ssa.Function) []Result
 		return nil
 	}

-	// Build call graph using Class Hierarchy Analysis (CHA).
-	// CHA is fast and sound (no false negatives) but may have false positives.
-	// For more precision, use VTA (Variable Type Analysis) instead.
-	a.callGraph = cha.CallGraph(prog)
+	if a.callGraph == nil {
+		// Build call graph using Class Hierarchy Analysis (CHA).
+		// CHA is fast and sound (no false negatives) but may have false positives.
+		// For more precision, use VTA (Variable Type Analysis) instead.
+		a.callGraph = cha.CallGraph(prog)
+	}

 	var results []Result

@@ -0,0 +1,159 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+BASELINE_FILE="${ROOT_DIR}/.github/benchmarks/taint_benchmark_baseline.env"
+BENCH_NAME="BenchmarkTaintPackageAnalyzers_SharedCache"
+BENCH_COUNT="${BENCH_COUNT:-5}"
+
+if [[ ! -f "${BASELINE_FILE}" ]]; then
+  echo "Baseline file not found: ${BASELINE_FILE}" >&2
+  exit 1
+fi
+
+# shellcheck disable=SC1090
+source "${BASELINE_FILE}"
+
+extract_metrics() {
+  local json_file="$1"
+
+  awk -v bench="${BENCH_NAME}" '
+    BEGIN {
+      count = 0
+      ns_sum = 0
+      b_sum = 0
+      allocs_sum = 0
+    }
+    {
+      if (index($0, "\"Output\":\"") == 0) {
+        next
+      }
+
+      line = $0
+      sub(/^.*"Output":"/, "", line)
+      sub(/".*$/, "", line)
+
+      gsub(/\\t/, "\t", line)
+      gsub(/\\n/, "", line)
+
+      if (line !~ ("^" bench "-[0-9]+")) {
+        next
+      }
+
+      split(line, fields, "\t")
+      if (length(fields) < 5) {
+        next
+      }
+
+      ns = fields[3]
+      b = fields[4]
+      allocs = fields[5]
+
+      gsub(/ ns\/op/, "", ns)
+      gsub(/ B\/op/, "", b)
+      gsub(/ allocs\/op/, "", allocs)
+      gsub(/ /, "", ns)
+      gsub(/ /, "", b)
+      gsub(/ /, "", allocs)
+
+      if (ns == "" || b == "" || allocs == "") {
+        next
+      }
+
+      ns_sum += ns + 0
+      b_sum += b + 0
+      allocs_sum += allocs + 0
+      count++
+    }
+    END {
+      if (count == 0) {
+        exit 1
+      }
+      printf "%d %d %d %d\n", int(ns_sum / count), int(b_sum / count), int(allocs_sum / count), count
+    }
+  ' "${json_file}"
+}
+
+run_benchmark() {
+  local json_file="$1"
+  go test -run '^$' -bench "^${BENCH_NAME}$" -benchmem -count="${BENCH_COUNT}" -json ./ > "${json_file}"
+}
+
+update_baseline() {
+  local json_file
+  json_file="$(mktemp)"
+
+  echo "Running benchmark ${BENCH_NAME} to refresh baseline (count=${BENCH_COUNT})..."
+  run_benchmark "${json_file}"
+
+  read -r ns b allocs count < <(extract_metrics "${json_file}")
+
+  awk -v ns="${ns}" -v b="${b}" -v allocs="${allocs}" '
+    /^BASE_NS_OP=/ { print "BASE_NS_OP=" ns; next }
+    /^BASE_B_PER_OP=/ { print "BASE_B_PER_OP=" b; next }
+    /^BASE_ALLOCS_PER_OP=/ { print "BASE_ALLOCS_PER_OP=" allocs; next }
+    { print }
+  ' "${BASELINE_FILE}" > "${BASELINE_FILE}.tmp"
+  mv "${BASELINE_FILE}.tmp" "${BASELINE_FILE}"
+
+  echo "Baseline updated from ${count} samples:"
+  echo "  BASE_NS_OP=${ns}"
+  echo "  BASE_B_PER_OP=${b}"
+  echo "  BASE_ALLOCS_PER_OP=${allocs}"
+
+  rm -f "${json_file}"
+}
+
+check_regression() {
+  local json_file
+  json_file="$(mktemp)"
+
+  echo "Running benchmark ${BENCH_NAME} (count=${BENCH_COUNT})..."
+  run_benchmark "${json_file}"
+
+  read -r ns b allocs count < <(extract_metrics "${json_file}")
+
+  local ns_limit b_limit allocs_limit
+  ns_limit=$(( BASE_NS_OP + (BASE_NS_OP * NS_OP_REGRESSION_PCT / 100) ))
+  b_limit=$(( BASE_B_PER_OP + (BASE_B_PER_OP * B_PER_OP_REGRESSION_PCT / 100) ))
+  allocs_limit=$(( BASE_ALLOCS_PER_OP + (BASE_ALLOCS_PER_OP * ALLOCS_PER_OP_REGRESSION_PCT / 100) ))
+
+  echo "Averaged over ${count} samples:"
+  echo "  ns/op:     ${ns} (baseline ${BASE_NS_OP}, limit ${ns_limit})"
+  echo "  B/op:      ${b} (baseline ${BASE_B_PER_OP}, limit ${b_limit})"
+  echo "  allocs/op: ${allocs} (baseline ${BASE_ALLOCS_PER_OP}, limit ${allocs_limit})"
+
+  local failed=0
+  if (( ns > ns_limit )); then
+    echo "Regression detected: ns/op exceeded threshold" >&2
+    failed=1
+  fi
+  if (( b > b_limit )); then
+    echo "Regression detected: B/op exceeded threshold" >&2
+    failed=1
+  fi
+  if (( allocs > allocs_limit )); then
+    echo "Regression detected: allocs/op exceeded threshold" >&2
+    failed=1
+  fi
+
+  if (( failed != 0 )); then
+    rm -f "${json_file}"
+    exit 1
+  fi
+
+  rm -f "${json_file}"
+}
+
+case "${1:-}" in
+  --update-baseline)
+    update_baseline
+    ;;
+  "")
+    check_regression
+    ;;
+  *)
+    echo "Usage: $0 [--update-baseline]" >&2
+    exit 2
+    ;;
+esac