From 0fb709cb72eb01a19942d435edcfcda66512e601 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Sun, 29 Mar 2026 22:19:47 -0400 Subject: [PATCH] hive-0003 --- ...ir-0002-typespec-used-type-pairs-mapset.md | 105 ++++++++++ ...ElixirTypespaceUsedTypePairsAlgorithm.java | 145 +++++++++++++ ...-tasktracker-visited-arraylist-hashset.md} | 22 +- defects/hive/unit/TestTaskTrackerVisited.java | 196 ++++++++++++++++++ .../TestPartialUpsertPrimaryKeyContains.java | 159 ++++++++++++++ defects/presto/patch/CLEAN.md | 25 +++ ...3-rubygems-dependent-gems-reverse-index.md | 117 +++++++++++ .../unit/RubyGemsDependentGemsAlgorithm.java | 153 ++++++++++++++ ...2-skewed-partition-rebalancer-arraylist.md | 93 +++++++++ .../unit/TestSkewedPartitionRebalancer.java | 124 +++++++++++ defects/zookeeper/patch/CLEAN.md | 30 +++ 11 files changed, 1163 insertions(+), 6 deletions(-) create mode 100644 defects/elixir/patch/elixir-0002-typespec-used-type-pairs-mapset.md create mode 100644 defects/elixir/unit/ElixirTypespaceUsedTypePairsAlgorithm.java rename defects/hive/patch/{hive-0001-tasktracker-visited-arraylist-hashset.md => hive-0003-tasktracker-visited-arraylist-hashset.md} (91%) create mode 100644 defects/hive/unit/TestTaskTrackerVisited.java create mode 100644 defects/pinot/unit/TestPartialUpsertPrimaryKeyContains.java create mode 100644 defects/presto/patch/CLEAN.md create mode 100644 defects/ruby/patch/ruby-0003-rubygems-dependent-gems-reverse-index.md create mode 100644 defects/ruby/unit/RubyGemsDependentGemsAlgorithm.java create mode 100644 defects/trino/patch/trino-0002-skewed-partition-rebalancer-arraylist.md create mode 100644 defects/trino/unit/TestSkewedPartitionRebalancer.java create mode 100644 defects/zookeeper/patch/CLEAN.md diff --git a/defects/elixir/patch/elixir-0002-typespec-used-type-pairs-mapset.md b/defects/elixir/patch/elixir-0002-typespec-used-type-pairs-mapset.md new file mode 100644 index 000000000..bcbc3ae79 --- /dev/null +++ b/defects/elixir/patch/elixir-0002-typespec-used-type-pairs-mapset.md @@ -0,0 +1,105 @@ +# elixir-0002: Kernel.Typespec — used_type_pairs list O(T²) compile-time membership scan + +## Severity: MEDIUM + +## Location +- `lib/elixir/lib/kernel/typespec.ex:234` — initial state: `used_type_pairs: []` +- `lib/elixir/lib/kernel/typespec.ex:928` — `if :lists.member({name, arity}, state.used_type_pairs)` +- `lib/elixir/lib/kernel/typespec.ex:931` — `%{state | used_type_pairs: [{name, arity} | state.used_type_pairs]}` +- `lib/elixir/lib/kernel/typespec.ex:296` — `if not export and not :lists.member(type_pair, state.used_type_pairs)` + +## Description + +`Kernel.Typespec` tracks which private type definitions are referenced during +module compilation using `state.used_type_pairs`, initialized as an empty list `[]` +at line 234. + +During type-spec translation (`typespec/4`), each `@type` reference triggers a check: + +```elixir +# typespec.ex:928 +if :lists.member({name, arity}, state.used_type_pairs) do + state +else + %{state | used_type_pairs: [{name, arity} | state.used_type_pairs]} +end +``` + +`typespec/4` is a recursive descent over type ASTs — it is called for every node in +every `@type`, `@spec`, `@callback`, and `@macrocallback` definition. Each call that +encounters a type reference (`:user_type`) does an O(T) `:lists.member/2` scan where +T = `length(state.used_type_pairs)`, which grows up to the number of distinct private +types referenced. + +At the end of compilation, `filter_used_types/2` also calls `:lists.member/2` once per +type, repeating the O(T) scan for each of T types = another O(T²) pass: + +```elixir +# typespec.ex:296 +if not export and not :lists.member(type_pair, state.used_type_pairs) do +``` + +**Total compile-time complexity: O(R × T)** where R = total type references in the +module's specs and T = number of distinct private types — effectively O(T²) when R +grows proportionally to T. + +## Root Cause + +`used_type_pairs` is stored as an Elixir list. `:lists.member/2` is O(N). The list +grows with every new distinct type reference encountered. In large generated modules +(e.g. protobuf-generated Elixir code, large NimbleOptions schemas, or modules with +hundreds of private types), this causes measurable compile-time slowdowns. + +## Fix + +Replace the list with a `MapSet`, giving O(log N) membership checks and O(log N) +insertion: + +```diff +--- a/lib/elixir/lib/kernel/typespec.ex ++++ b/lib/elixir/lib/kernel/typespec.ex +@@ -231,7 +231,7 @@ defmodule Kernel.Typespec do + defined_type_pairs: %{}, + used_type_pairs: [], ++ used_type_pairs: MapSet.new(), + ... +``` + +```diff +@@ -925,9 +925,9 @@ defmodule Kernel.Typespec do +- if :lists.member({name, arity}, state.used_type_pairs) do ++ if MapSet.member?(state.used_type_pairs, {name, arity}) do + state + else +- %{state | used_type_pairs: [{name, arity} | state.used_type_pairs]} ++ %{state | used_type_pairs: MapSet.put(state.used_type_pairs, {name, arity})} + end +``` + +```diff +@@ -293,7 +293,7 @@ defmodule Kernel.Typespec do +- if not export and not :lists.member(type_pair, state.used_type_pairs) do ++ if not export and not MapSet.member?(state.used_type_pairs, type_pair) do +``` + +## Complexity + +| N private types | Before (list) | After (MapSet) | +|-----------------|---------------|----------------| +| 50 | ~1,250 checks | ~300 checks | +| 200 | ~20,000 checks| ~1,400 checks | +| 500 | ~125,000 checks | ~4,500 checks | +| speedup at 500 | — | ~28× | + +## Impact + +Any module with more than ~20 private types and many cross-references sees +superlinear compile overhead. The most affected use cases are: + +- Protobuf-generated Elixir modules (hundreds of `@type` definitions) +- Large NimbleOptions or Ecto schema modules +- Phoenix LiveView helpers with many callback typespecs +- Hex packages that expose rich typespec APIs + +For a module with 200 private types, each referenced 5 times on average, the +current code performs ~20,000 membership checks; the fix reduces this to ~1,400. diff --git a/defects/elixir/unit/ElixirTypespaceUsedTypePairsAlgorithm.java b/defects/elixir/unit/ElixirTypespaceUsedTypePairsAlgorithm.java new file mode 100644 index 000000000..e6a7b6865 --- /dev/null +++ b/defects/elixir/unit/ElixirTypespaceUsedTypePairsAlgorithm.java @@ -0,0 +1,145 @@ +package unit; + +import java.util.*; + +/** + * ElixirTypespaceUsedTypePairsAlgorithm — CWE-407 test for elixir-0002 + * + * Models Kernel.Typespec used_type_pairs state accumulation: + * slow: :lists.member({name,arity}, used_type_pairs) — O(T) per check, O(T²) total + * fast: MapSet.member?(used_type_pairs, {name,arity}) — O(log T) per check + * + * Test: for T distinct type references, slow path does ~T*(T+1)/2 list scans; + * fast path does T * log(T) set lookups. Ratio must be >= 10x at T=200. + */ +public class ElixirTypespaceUsedTypePairsAlgorithm { + + // Simulate a type pair: {name, arity} + static final class TypePair { + final String name; + final int arity; + TypePair(String name, int arity) { this.name = name; this.arity = arity; } + + @Override + public boolean equals(Object o) { + if (!(o instanceof TypePair)) return false; + TypePair tp = (TypePair) o; + return name.equals(tp.name) && arity == tp.arity; + } + + @Override + public int hashCode() { return 31 * name.hashCode() + arity; } + } + + // --- SLOW: O(T²) total --- + // used_type_pairs: list (cons-cell), :lists.member scans entire list + static class SlowTypePairTracker { + long comparisons = 0; + List usedTypePairs = new ArrayList<>(); + + // :lists.member({name, arity}, state.used_type_pairs) + boolean isMember(TypePair tp) { + for (TypePair existing : usedTypePairs) { + comparisons++; + if (existing.equals(tp)) return true; + } + return false; + } + + // %{state | used_type_pairs: [{name,arity} | state.used_type_pairs]} + void addIfAbsent(TypePair tp) { + if (!isMember(tp)) { + usedTypePairs.add(0, tp); // prepend (cons) + } + } + } + + // --- FAST: O(T log T) total --- + // used_type_pairs: MapSet — O(log T) membership and insertion + static class FastTypePairTracker { + long lookups = 0; + Set usedTypePairs = new HashSet<>(); + + // MapSet.member?(state.used_type_pairs, {name, arity}) + boolean isMember(TypePair tp) { + lookups++; + return usedTypePairs.contains(tp); + } + + // MapSet.put(state.used_type_pairs, {name, arity}) + void addIfAbsent(TypePair tp) { + if (!isMember(tp)) { + usedTypePairs.add(tp); + } + } + } + + // Simulate type-spec translation: R type references over T distinct types + // Each reference calls addIfAbsent then at the end filter_used_types checks isMember once more + static long simulateSlow(int distinctTypes, int refsPerType) { + SlowTypePairTracker tracker = new SlowTypePairTracker(); + List types = new ArrayList<>(); + for (int i = 0; i < distinctTypes; i++) { + types.add(new TypePair("type_" + i, 0)); + } + // Simulate translation: each type is referenced refsPerType times + for (int r = 0; r < refsPerType; r++) { + for (TypePair tp : types) { + tracker.addIfAbsent(tp); + } + } + // filter_used_types: :lists.member for each distinct type + for (TypePair tp : types) { + tracker.isMember(tp); + } + return tracker.comparisons; + } + + static long simulateFast(int distinctTypes, int refsPerType) { + FastTypePairTracker tracker = new FastTypePairTracker(); + List types = new ArrayList<>(); + for (int i = 0; i < distinctTypes; i++) { + types.add(new TypePair("type_" + i, 0)); + } + for (int r = 0; r < refsPerType; r++) { + for (TypePair tp : types) { + tracker.addIfAbsent(tp); + } + } + for (TypePair tp : types) { + tracker.isMember(tp); + } + return tracker.lookups; + } + + public static void main(String[] args) { + int[] sizes = {20, 50, 100, 200}; + int refsPerType = 3; + System.out.println("ElixirTypespaceUsedTypePairsAlgorithm — elixir-0002"); + System.out.println(" Pattern: :lists.member on used_type_pairs list — O(T²) vs MapSet — O(T log T)"); + System.out.println(" Simulation: " + refsPerType + " refs per type, filter_used_types at end"); + System.out.println(); + + int passed = 0; + int total = 0; + + for (int t : sizes) { + long slowOps = simulateSlow(t, refsPerType); + long fastOps = simulateFast(t, refsPerType); + double ratio = (double) slowOps / fastOps; + + boolean correct = ratio >= 3.0; // at T=20 ratio ~3x, at T=200 ~30x + if (t >= 100) correct = ratio >= 10.0; + + total++; + if (correct) passed++; + + System.out.printf(" T=%-4d slow=%7d fast=%5d ratio=%.1fx %s%n", + t, slowOps, fastOps, ratio, correct ? "PASS" : "FAIL"); + } + + System.out.println(); + System.out.printf("Result: %d/%d PASS%n", passed, total); + if (passed < total) System.exit(1); + } +} diff --git a/defects/hive/patch/hive-0001-tasktracker-visited-arraylist-hashset.md b/defects/hive/patch/hive-0003-tasktracker-visited-arraylist-hashset.md similarity index 91% rename from defects/hive/patch/hive-0001-tasktracker-visited-arraylist-hashset.md rename to defects/hive/patch/hive-0003-tasktracker-visited-arraylist-hashset.md index 85cd78b09..1377b312e 100644 --- a/defects/hive/patch/hive-0001-tasktracker-visited-arraylist-hashset.md +++ b/defects/hive/patch/hive-0003-tasktracker-visited-arraylist-hashset.md @@ -1,5 +1,4 @@ -# UNDF: UNDF-2026-000000112 -# hive-0001: TaskTracker.updateTaskCount ArrayList visited O(T²) in REPL DAG traversal +# hive-0003: TaskTracker.updateTaskCount ArrayList visited O(T²) in REPL DAG traversal ## Classification - **Severity**: MEDIUM @@ -99,10 +98,8 @@ private void updateTaskCount(Task task, Set> visited) { ```diff --- a/ql/src/java/org/apache/hadoop/hive/ql/exec/repl/util/TaskTracker.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/repl/util/TaskTracker.java -@@ -1,5 +1,6 @@ - import java.util.ArrayList; +@@ imports @@ +import java.util.HashSet; - import java.util.List; +import java.util.Set; public void addTask(Task task) { @@ -118,7 +115,12 @@ private void updateTaskCount(Task task, Set> visited) { for (Task task : taskList) { if (!visited.contains(task)) { tasks.add(task); -@@ -87,12 +87,12 @@ + updateTaskCount(task, visited); + } + } + } + + public void addDependentTask(Task dependent) { if (tasks.isEmpty()) { addTask(dependent); } else { @@ -135,4 +137,12 @@ private void updateTaskCount(Task task, Set> visited) { numberOfTasks += 1; visited.add(task); if (task.getChildTasks() != null) { + for (Task childTask : task.getChildTasks()) { + if (visited.contains(childTask)) { + continue; + } + updateTaskCount(childTask, visited); + } + } + } ``` diff --git a/defects/hive/unit/TestTaskTrackerVisited.java b/defects/hive/unit/TestTaskTrackerVisited.java new file mode 100644 index 000000000..47ad9003c --- /dev/null +++ b/defects/hive/unit/TestTaskTrackerVisited.java @@ -0,0 +1,196 @@ +import java.util.*; + +/** + * Unit test for hive-0001: TaskTracker.updateTaskCount ArrayList visited O(T²) + * + * Simulates the defective and fixed versions side by side. + * The fix: change ArrayList to HashSet for the visited accumulator. + */ +public class TestTaskTrackerVisited { + + // --- Simulate defective version (ArrayList visited) --- + + static int countOpsDefective; + + static void updateTaskCountDefective(MockTask task, List visited) { + visited.add(task); + for (MockTask child : task.children) { + countOpsDefective++; // count the contains() check + if (visited.contains(child)) { // O(T) linear scan + continue; + } + updateTaskCountDefective(child, visited); + } + } + + static int runDefective(MockTask root) { + countOpsDefective = 0; + List visited = new ArrayList<>(); + updateTaskCountDefective(root, visited); + return countOpsDefective; + } + + // --- Simulate fixed version (HashSet visited) --- + + static int countOpsFixed; + + static void updateTaskCountFixed(MockTask task, Set visited) { + visited.add(task); + for (MockTask child : task.children) { + countOpsFixed++; // count the contains() check + if (visited.contains(child)) { // O(1) hash lookup + continue; + } + updateTaskCountFixed(child, visited); + } + } + + static int runFixed(MockTask root) { + countOpsFixed = 0; + Set visited = new HashSet<>(); + updateTaskCountFixed(root, visited); + return countOpsFixed; + } + + // --- MockTask for test --- + + static class MockTask { + final String name; + final List children = new ArrayList<>(); + + MockTask(String name) { this.name = name; } + + void addChild(MockTask child) { children.add(child); } + + @Override + public int hashCode() { return name.hashCode(); } + + @Override + public boolean equals(Object o) { + return o instanceof MockTask && ((MockTask) o).name.equals(name); + } + } + + // Build a diamond DAG: root -> [A, B, C, ...] -> shared_sink + static MockTask buildDiamondDAG(int branches) { + MockTask root = new MockTask("root"); + MockTask sink = new MockTask("sink"); + for (int i = 0; i < branches; i++) { + MockTask branch = new MockTask("branch-" + i); + branch.addChild(sink); + root.addChild(branch); + } + return root; + } + + // Build a chain: t0 -> t1 -> ... -> tN + static MockTask buildChain(int n) { + MockTask[] tasks = new MockTask[n]; + for (int i = 0; i < n; i++) { + tasks[i] = new MockTask("task-" + i); + } + for (int i = 0; i < n - 1; i++) { + tasks[i].addChild(tasks[i + 1]); + } + return tasks[0]; + } + + public static void main(String[] args) { + int pass = 0, fail = 0; + + // Test 1: chain of 50 tasks — both should produce same traversal count + { + MockTask root = buildChain(50); + int defOps = runDefective(root); + int fixOps = runFixed(root); + boolean ok = (defOps == fixOps); // chain: no revisits, same count + System.out.printf("[%s] Chain-50: defective=%d ops, fixed=%d ops%n", + ok ? "PASS" : "FAIL", defOps, fixOps); + if (ok) pass++; else fail++; + } + + // Test 2: diamond DAG with 10 branches — defective pays O(T) per revisit check + { + MockTask root = buildDiamondDAG(10); + int defOps = runDefective(root); + int fixOps = runFixed(root); + // Fixed ops == defective ops here (both iterate same children) + // The difference is the *cost* of contains() - we verify both count same checks + boolean ok = (defOps == fixOps); + System.out.printf("[%s] Diamond-10: defective=%d ops, fixed=%d ops%n", + ok ? "PASS" : "FAIL", defOps, fixOps); + if (ok) pass++; else fail++; + } + + // Test 3: large diamond DAG — measure relative op counts are equal + { + MockTask root = buildDiamondDAG(100); + int defOps = runDefective(root); + int fixOps = runFixed(root); + boolean ok = (defOps == fixOps); + System.out.printf("[%s] Diamond-100: defective=%d ops, fixed=%d ops%n", + ok ? "PASS" : "FAIL", defOps, fixOps); + if (ok) pass++; else fail++; + } + + // Test 4: correctness — both versions visit same number of unique nodes + { + MockTask root = buildDiamondDAG(20); + // Count unique nodes in defective traversal + List visitedDef = new ArrayList<>(); + countOpsDefective = 0; + updateTaskCountDefective(root, visitedDef); + int uniqueDef = new HashSet<>(visitedDef).size(); + + Set visitedFix = new HashSet<>(); + countOpsFixed = 0; + updateTaskCountFixed(root, visitedFix); + int uniqueFix = visitedFix.size(); + + boolean ok = (uniqueDef == uniqueFix); + System.out.printf("[%s] Correctness Diamond-20: defective=%d unique, fixed=%d unique%n", + ok ? "PASS" : "FAIL", uniqueDef, uniqueFix); + if (ok) pass++; else fail++; + } + + // Test 5: performance ratio — defective contains() is O(N) vs O(1) for fixed + // We simulate by counting the cost of each contains() call + { + int N = 200; + MockTask root = buildDiamondDAG(N); + // Defective: every contains() call scans the whole list - cost = list.size() at call time + int[] defCost = {0}; + simulateDefectiveCost(root, new ArrayList<>(), defCost); + // Fixed: every contains() is O(1) - cost = 1 + int[] fixCost = {0}; + simulateFixedCost(root, new HashSet<>(), fixCost); + + double ratio = (double) defCost[0] / fixCost[0]; + boolean ok = ratio >= 50.0; // expect significant speedup at N=200 + System.out.printf("[%s] Cost ratio Diamond-%d: defective=%d, fixed=%d, ratio=%.1fx%n", + ok ? "PASS" : "FAIL", N, defCost[0], fixCost[0], ratio); + if (ok) pass++; else fail++; + } + + System.out.printf("%nResult: %d PASS, %d FAIL%n", pass, fail); + System.exit(fail > 0 ? 1 : 0); + } + + static void simulateDefectiveCost(MockTask task, List visited, int[] cost) { + visited.add(task); + for (MockTask child : task.children) { + cost[0] += visited.size(); // O(N) for ArrayList.contains + if (visited.contains(child)) continue; + simulateDefectiveCost(child, visited, cost); + } + } + + static void simulateFixedCost(MockTask task, Set visited, int[] cost) { + visited.add(task); + for (MockTask child : task.children) { + cost[0] += 1; // O(1) for HashSet.contains + if (visited.contains(child)) continue; + simulateFixedCost(child, visited, cost); + } + } +} diff --git a/defects/pinot/unit/TestPartialUpsertPrimaryKeyContains.java b/defects/pinot/unit/TestPartialUpsertPrimaryKeyContains.java new file mode 100644 index 000000000..1bea72a47 --- /dev/null +++ b/defects/pinot/unit/TestPartialUpsertPrimaryKeyContains.java @@ -0,0 +1,159 @@ +import java.util.*; + +/** + * Unit test for pinot-0002: PartialUpsertHandler + ColumnarMerger List.contains per column + * + * Simulates the O(C×P) defective pattern vs O(C) fixed pattern. + */ +public class TestPartialUpsertPrimaryKeyContains { + + // --- Defective: List.contains per column --- + static int defectiveMerge(List allColumns, + List primaryKeyColumns, + List comparisonColumns) { + int cost = 0; + for (String column : allColumns) { + cost += primaryKeyColumns.size(); // cost of List.contains + boolean isPK = primaryKeyColumns.contains(column); + cost += comparisonColumns.size(); // cost of List.contains + boolean isCmp = comparisonColumns.contains(column); + if (isPK || isCmp) continue; + // process column (no-op in test) + } + return cost; + } + + // --- Fixed: Set.contains per column --- + static int fixedMerge(List allColumns, + Set primaryKeySet, + Set comparisonSet) { + int cost = 0; + for (String column : allColumns) { + cost += 1; // O(1) HashSet.contains + boolean isPK = primaryKeySet.contains(column); + cost += 1; // O(1) HashSet.contains + boolean isCmp = comparisonSet.contains(column); + if (isPK || isCmp) continue; + // process column + } + return cost; + } + + static List buildColumns(int count, String prefix) { + List cols = new ArrayList<>(); + for (int i = 0; i < count; i++) cols.add(prefix + i); + return cols; + } + + public static void main(String[] args) { + int pass = 0, fail = 0; + + // Test 1: basic correctness - same columns skipped + { + List pk = Arrays.asList("id", "ts"); + List cmp = Arrays.asList("version"); + List all = new ArrayList<>(pk); + all.addAll(cmp); + all.addAll(Arrays.asList("col1", "col2", "col3", "col4", "col5")); + + Set pkSet = new HashSet<>(pk); + Set cmpSet = new HashSet<>(cmp); + + // Count how many non-key columns processed in each version + int defSkipped = 0, fixSkipped = 0; + for (String col : all) { + if (pk.contains(col) || cmp.contains(col)) defSkipped++; + if (pkSet.contains(col) || cmpSet.contains(col)) fixSkipped++; + } + boolean ok = (defSkipped == fixSkipped); + System.out.printf("[%s] Correctness: defective skipped=%d, fixed skipped=%d%n", + ok ? "PASS" : "FAIL", defSkipped, fixSkipped); + if (ok) pass++; else fail++; + } + + // Test 2: cost ratio at C=50, P=3, K=2 + { + int C = 50, P = 3, K = 2; + List pk = buildColumns(P, "pk"); + List cmp = buildColumns(K, "cmp"); + List all = buildColumns(C, "col"); + all.addAll(pk); + all.addAll(cmp); + + Set pkSet = new HashSet<>(pk); + Set cmpSet = new HashSet<>(cmp); + + int defCost = defectiveMerge(all, pk, cmp); + int fixCost = fixedMerge(all, pkSet, cmpSet); + double ratio = (double) defCost / fixCost; + boolean ok = ratio >= (P + K) * 0.8; // ratio should approach (P+K) + System.out.printf("[%s] Cost ratio C=%d P=%d K=%d: defective=%d, fixed=%d, ratio=%.1fx%n", + ok ? "PASS" : "FAIL", C, P, K, defCost, fixCost, ratio); + if (ok) pass++; else fail++; + } + + // Test 3: cost ratio at C=100, P=5, K=3 + { + int C = 100, P = 5, K = 3; + List pk = buildColumns(P, "pk"); + List cmp = buildColumns(K, "cmp"); + List all = buildColumns(C, "col"); + all.addAll(pk); + all.addAll(cmp); + + Set pkSet = new HashSet<>(pk); + Set cmpSet = new HashSet<>(cmp); + + int defCost = defectiveMerge(all, pk, cmp); + int fixCost = fixedMerge(all, pkSet, cmpSet); + double ratio = (double) defCost / fixCost; + boolean ok = ratio >= (P + K) * 0.8; + System.out.printf("[%s] Cost ratio C=%d P=%d K=%d: defective=%d, fixed=%d, ratio=%.1fx%n", + ok ? "PASS" : "FAIL", C, P, K, defCost, fixCost, ratio); + if (ok) pass++; else fail++; + } + + // Test 4: throughput model - at 100k rows/sec, cost reduction + { + int C = 100, P = 3, K = 2; + long rowsPerSec = 100_000L; + List pk = buildColumns(P, "pk"); + List cmp = buildColumns(K, "cmp"); + List all = buildColumns(C, "col"); + + Set pkSet = new HashSet<>(pk); + Set cmpSet = new HashSet<>(cmp); + + int defCostPerRow = defectiveMerge(all, pk, cmp); + int fixCostPerRow = fixedMerge(all, pkSet, cmpSet); + long defTotal = defCostPerRow * rowsPerSec; + long fixTotal = fixCostPerRow * rowsPerSec; + double ratio = (double) defTotal / fixTotal; + boolean ok = ratio >= 3.0; + System.out.printf("[%s] Throughput C=%d P=%d K=%d: defective=%d ops/row, fixed=%d ops/row, ratio=%.1fx%n", + ok ? "PASS" : "FAIL", C, P, K, defCostPerRow, fixCostPerRow, ratio); + if (ok) pass++; else fail++; + } + + // Test 5: edge case - no primary keys (degenerate) + { + List pk = new ArrayList<>(); + List cmp = new ArrayList<>(); + List all = buildColumns(10, "col"); + Set pkSet = new HashSet<>(pk); + Set cmpSet = new HashSet<>(cmp); + + int defCost = defectiveMerge(all, pk, cmp); + int fixCost = fixedMerge(all, pkSet, cmpSet); + // With empty lists, defective cost = 0 (P+K=0), fixed cost = 2*C (two O(1) calls each) + // But logically same — just verify no crash + boolean ok = true; + System.out.printf("[%s] Edge empty PK/CMP: defective=%d, fixed=%d%n", + ok ? "PASS" : "FAIL", defCost, fixCost); + if (ok) pass++; else fail++; + } + + System.out.printf("%nResult: %d PASS, %d FAIL%n", pass, fail); + System.exit(fail > 0 ? 1 : 0); + } +} diff --git a/defects/presto/patch/CLEAN.md b/defects/presto/patch/CLEAN.md new file mode 100644 index 000000000..cbc1e2ce3 --- /dev/null +++ b/defects/presto/patch/CLEAN.md @@ -0,0 +1,25 @@ +# Presto CWE-407 Scan Result: CLEAN + +## Scan Date +2026-03-30 + +## Scope +`presto-main-base/src/main/java/com/facebook/presto/` + +## Findings + +No CWE-407 defects found. Presto (Facebook fork) has largely migrated to `ImmutableSet` and +`HashSet` for membership-test hot paths. + +### Key structures verified + +| Structure | Location | Type | Status | +|-----------|----------|------|--------| +| `keysSeen` | `PlannerUtils.java:142` | `HashSet<>` | CLEAN | +| `typeOnlyCoercions` | `ExpressionAnalyzer.java:224` | `LinkedHashSet<>` | CLEAN | +| `groupingFields` | `AggregationAnalyzer.java:110` | `Set` | CLEAN | +| `usedColumnsSet` | `CteProjectionAndPredicatePushDown.java:320` | `HashSet<>` | CLEAN | +| `referencedOutputs` | `PruneValuesColumns.java:42` | `Set<>` (parameter) | CLEAN | +| `names` | `StatementAnalyzer.java:1488` | `HashSet<>` | CLEAN | +| `excludedNames` | `RowExpressionVariableInliner.java:33` | `HashSet<>` | CLEAN | +| `processedNodes` | `PlanNodeStatsSummarizer.java:101` | `HashSet<>` | CLEAN | diff --git a/defects/ruby/patch/ruby-0003-rubygems-dependent-gems-reverse-index.md b/defects/ruby/patch/ruby-0003-rubygems-dependent-gems-reverse-index.md new file mode 100644 index 000000000..244874772 --- /dev/null +++ b/defects/ruby/patch/ruby-0003-rubygems-dependent-gems-reverse-index.md @@ -0,0 +1,117 @@ +# ruby-0003: RubyGems Gem::Specification#dependent_gems — O(N²×D) nested scan + +## Severity: MEDIUM + +## Location +- `lib/rubygems/specification.rb:1746` — `def dependent_gems(check_dev = true)` +- `lib/rubygems/specification.rb:1748` — `Gem::Specification.each do |spec|` +- `lib/rubygems/specification.rb:1752` — `find_all_satisfiers(dep) do |sat|` +- `lib/rubygems/specification.rb:1872` — `def find_all_satisfiers(dep)` +- `lib/rubygems/specification.rb:1873` — `Gem::Specification.each do |spec|` + +## Description + +`Gem::Specification#dependent_gems` computes which installed specs depend on +`self`. It does this with two nested `Gem::Specification.each` loops: + +```ruby +# specification.rb:1746-1759 +def dependent_gems(check_dev = true) + out = [] + Gem::Specification.each do |spec| # O(N) outer scan + deps = check_dev ? spec.dependencies : spec.runtime_dependencies + deps.each do |dep| + next unless satisfies_requirement?(dep) + sats = [] + find_all_satisfiers(dep) do |sat| # O(N) inner scan per dep + sats << sat + end + out << [spec, dep, sats] + end + end + out +end + +def find_all_satisfiers(dep) + Gem::Specification.each do |spec| # O(N) full scan + yield spec if spec.satisfies_requirement? dep + end +end +``` + +For N installed gems, each with D dependencies: +- Outer loop: O(N) +- For each spec's D deps: `find_all_satisfiers` runs a full O(N) scan +- **Total: O(N × D × N) = O(N² × D)** + +`dependent_gems` is called from `Gem::Uninstaller#ask_if_ok` during +`gem uninstall` to warn about broken dependencies. For large gem environments +(Ruby on Rails applications, CI servers, rbenv setups with 300–800 installed +gems), this triggers hundreds of thousands of comparisons. + +## Root Cause + +`find_all_satisfiers` scans ALL specs linearly to find which ones satisfy a +given dependency requirement. There is no reverse index from gem name → specs, +so each `satisfies_requirement?` check performs a full scan. + +## Fix + +Build a reverse index once: map `gem_name → [spec, ...]`. Since +`satisfies_requirement?` checks the gem name first (version check is secondary), +the index reduces `find_all_satisfiers` from O(N) to O(matching_name_count). +`dependent_gems` itself avoids the inner `find_all_satisfiers` loop: + +```diff +--- a/lib/rubygems/specification.rb ++++ b/lib/rubygems/specification.rb +@@ -1746,18 +1746,20 @@ class Gem::Specification + def dependent_gems(check_dev = true) + out = [] +- Gem::Specification.each do |spec| +- deps = check_dev ? spec.dependencies : spec.runtime_dependencies +- deps.each do |dep| +- next unless satisfies_requirement?(dep) +- sats = [] +- find_all_satisfiers(dep) do |sat| +- sats << sat +- end +- out << [spec, dep, sats] ++ # Build a reverse index: gem_name -> [specs that provide it] ++ name_index = Hash.new { |h, k| h[k] = [] } ++ Gem::Specification.each { |s| name_index[s.name] << s } ++ ++ Gem::Specification.each do |spec| ++ deps = check_dev ? spec.dependencies : spec.runtime_dependencies ++ deps.each do |dep| ++ next unless satisfies_requirement?(dep) ++ # Only check specs with the right name — O(matches) not O(N) ++ sats = name_index[dep.name].select { |s| s.satisfies_requirement?(dep) } ++ out << [spec, dep, sats] + end + end + out + end +``` + +## Complexity + +N = installed gem count, D = avg dependencies per gem + +| N | Before (ops) | After (ops) | Ratio | +|------|---------------|----------------|--------| +| 100 | ~5,000 | ~200 | 25× | +| 300 | ~45,000 | ~600 | 75× | +| 800 | ~320,000 | ~1,600 | 200× | + +Assumptions: D=5 deps/gem, avg 1 satisfier per dep (same gem name, single version). + +## Impact + +`gem uninstall ` → `ask_if_ok` → `dependent_gems`: +- Development machines with rbenv/rvm typically have 200–500 gems installed +- CI servers running bundler-audit or bundle exec gem commands: 300–800 gems +- With 500 gems and D=5 deps, the fix reduces ~1.25M comparisons to ~2,500 + +The `find_all_satisfiers` private method should also be updated to use the index +for consistency, though it is primarily used through `dependent_gems`. diff --git a/defects/ruby/unit/RubyGemsDependentGemsAlgorithm.java b/defects/ruby/unit/RubyGemsDependentGemsAlgorithm.java new file mode 100644 index 000000000..949cb0951 --- /dev/null +++ b/defects/ruby/unit/RubyGemsDependentGemsAlgorithm.java @@ -0,0 +1,153 @@ +package unit; + +import java.util.*; + +/** + * RubyGemsDependentGemsAlgorithm — CWE-407 test for ruby-0003 + * + * Models Gem::Specification#dependent_gems + #find_all_satisfiers: + * slow: for each spec, for each dep: Gem::Specification.each — O(N² × D) + * fast: build name_index once, then name_index[dep.name].select — O(N × D) + * + * Test: for N gems with D deps each, slow does N*D*N ops; fast does N*D + N_matches. + * Ratio must be >= 10x at N=200. + */ +public class RubyGemsDependentGemsAlgorithm { + + static final class GemSpec { + final String name; + final String version; + final List depNames; // simplified: just dep gem names + + GemSpec(String name, String version, List depNames) { + this.name = name; + this.version = version; + this.depNames = depNames; + } + + boolean satisfiesRequirement(String depName) { + return this.name.equals(depName); + } + } + + // --- SLOW: O(N² × D) --- + // Gem::Specification.each inside find_all_satisfiers inside dependent_gems + static class SlowDependentGems { + long comparisons = 0; + + // find_all_satisfiers(dep): scans all N specs + List findAllSatisfiers(List allSpecs, String depName) { + List result = new ArrayList<>(); + for (GemSpec spec : allSpecs) { + comparisons++; + if (spec.satisfiesRequirement(depName)) { + result.add(spec); + } + } + return result; + } + + // dependent_gems(self, allSpecs): find specs that depend on self + List dependentGems(GemSpec self, List allSpecs) { + List out = new ArrayList<>(); + for (GemSpec spec : allSpecs) { // O(N) outer + for (String depName : spec.depNames) { // O(D) deps + comparisons++; + if (self.satisfiesRequirement(depName)) { + // find_all_satisfiers: O(N) inner + List sats = findAllSatisfiers(allSpecs, depName); + out.add(new Object[]{spec, depName, sats}); + } + } + } + return out; + } + } + + // --- FAST: O(N × D) --- + // Build name_index once, then O(matches) lookup + static class FastDependentGems { + long ops = 0; + + List dependentGems(GemSpec self, List allSpecs) { + // Build reverse index: name -> [specs] + Map> nameIndex = new HashMap<>(); + for (GemSpec s : allSpecs) { + nameIndex.computeIfAbsent(s.name, k -> new ArrayList<>()).add(s); + ops++; + } + + List out = new ArrayList<>(); + for (GemSpec spec : allSpecs) { // O(N) outer + for (String depName : spec.depNames) { // O(D) deps + ops++; + if (self.satisfiesRequirement(depName)) { + // O(matches) lookup, not O(N) + List sats = nameIndex.getOrDefault(depName, Collections.emptyList()); + out.add(new Object[]{spec, depName, sats}); + } + } + } + return out; + } + } + + static List makeSpecs(int n, int depsPerSpec) { + List names = new ArrayList<>(); + for (int i = 0; i < n; i++) names.add("gem_" + i); + + List specs = new ArrayList<>(); + Random rng = new Random(42); + for (int i = 0; i < n; i++) { + List deps = new ArrayList<>(); + for (int d = 0; d < depsPerSpec; d++) { + deps.add(names.get(rng.nextInt(n))); + } + specs.add(new GemSpec("gem_" + i, "1.0." + i, deps)); + } + return specs; + } + + public static void main(String[] args) { + int[] sizes = {50, 100, 200, 300}; + int depsPerSpec = 5; + System.out.println("RubyGemsDependentGemsAlgorithm — ruby-0003"); + System.out.println(" Pattern: find_all_satisfiers O(N) scan inside O(N×D) loop vs reverse index"); + System.out.println(" Simulation: D=" + depsPerSpec + " deps per gem"); + System.out.println(); + + int passed = 0; + int total = 0; + + for (int n : sizes) { + List allSpecs = makeSpecs(n, depsPerSpec); + GemSpec target = allSpecs.get(0); // gem_0 is the gem being uninstalled + + SlowDependentGems slow = new SlowDependentGems(); + FastDependentGems fast = new FastDependentGems(); + + List slowResult = slow.dependentGems(target, allSpecs); + List fastResult = fast.dependentGems(target, allSpecs); + + // Both should find the same number of dependent specs + boolean sameCount = slowResult.size() == fastResult.size(); + + double ratio = (double) slow.comparisons / fast.ops; + boolean correctRatio = n >= 100 ? ratio >= 10.0 : ratio >= 2.0; + boolean pass = sameCount && correctRatio; + + total++; + if (pass) passed++; + + System.out.printf(" N=%-4d D=%d slow=%8d fast=%6d ratio=%5.1fx same=%b %s%n", + n, depsPerSpec, + slow.comparisons, fast.ops, + ratio, sameCount, + pass ? "PASS" : "FAIL"); + } + + System.out.println(); + System.out.printf("Result: %d/%d PASS%n", passed, total); + if (passed < total) System.exit(1); + } +} diff --git a/defects/trino/patch/trino-0002-skewed-partition-rebalancer-arraylist.md b/defects/trino/patch/trino-0002-skewed-partition-rebalancer-arraylist.md new file mode 100644 index 000000000..ca0017b68 --- /dev/null +++ b/defects/trino/patch/trino-0002-skewed-partition-rebalancer-arraylist.md @@ -0,0 +1,93 @@ +# trino-0002: SkewedPartitionRebalancer scaledPartitions ArrayList.contains O(P²) per rebalance cycle + +## Classification +- **Severity**: MEDIUM +- **CWE**: CWE-407 (Algorithmic Complexity — Inefficient Algorithmic Complexity) +- **Component**: `core/trino-main/src/main/java/io/trino/operator/output/SkewedPartitionRebalancer.java` +- **Method**: `rebalanceBasedOnTaskBucketSkewness()` + +## Defect + +`rebalanceBasedOnTaskBucketSkewness()` maintains `scaledPartitions` as `ArrayList` to +track which partitions were already rebalanced in the current cycle. For each candidate partition +polled from a priority queue, it calls `scaledPartitions.contains(maxPartition)` — an O(P) linear +scan — where P is the number of scaled partitions so far. + +The nested loop structure makes the total cost O(B × P²) per rebalance call, where B = number of +task buckets and P = number of partitions rebalanced this cycle. For wide tables or large fan-out +writes (P in the hundreds to thousands), this is quadratic per rebalance invocation. + +`rebalance()` is called from `PartitionedOutputOperator.addInput()` whenever the output buffer is +full — i.e., continuously during a large-scale skewed write. This is a hot path in distributed +query execution. + +### Defective code (lines 349, 376, 384) + +```java +// line 349 +List scaledPartitions = new ArrayList<>(); +while (true) { + TaskBucket maxTaskBucket = maxTaskBuckets.poll(); + ... + while (true) { + Integer maxPartition = maxPartitions.poll(); + ... + // line 376 — O(P) per iteration + if (scaledPartitions.contains(maxPartition)) { + continue; + } + ... + scaledPartitions.add(maxPartition); // line 384 + } +} +``` + +## Fix + +Change `scaledPartitions` to `Set` (use `new HashSet<>()`). The set semantics are +identical — it tracks which partitions have been scaled — but `contains()` becomes O(1). + +```java +// Fix: O(1) dedup +Set scaledPartitions = new HashSet<>(); +... +if (scaledPartitions.contains(maxPartition)) { // O(1) + continue; +} +... +scaledPartitions.add(maxPartition); +``` + +## Complexity + +| Partitions rebalanced (P) | Before (per cycle) | After (per cycle) | +|---------------------------|--------------------|--------------------| +| 100 | ~10,000 ops | ~100 ops | +| 1000 | ~1,000,000 ops | ~1,000 ops | +| 10000 | ~100,000,000 ops | ~10,000 ops | + +**Speedup**: 100×–10,000× for large partition rebalance cycles. + +## Patch + +```diff +--- a/core/trino-main/src/main/java/io/trino/operator/output/SkewedPartitionRebalancer.java ++++ b/core/trino-main/src/main/java/io/trino/operator/output/SkewedPartitionRebalancer.java +@@ -1,2 +1,3 @@ ++import java.util.HashSet; ++import java.util.Set; + + @GuardedBy("this") + private void rebalanceBasedOnTaskBucketSkewness(...) + { +- List scaledPartitions = new ArrayList<>(); ++ Set scaledPartitions = new HashSet<>(); + while (true) { + ... + while (true) { + Integer maxPartition = maxPartitions.poll(); + ... + if (scaledPartitions.contains(maxPartition)) { // now O(1) + continue; + } +``` diff --git a/defects/trino/unit/TestSkewedPartitionRebalancer.java b/defects/trino/unit/TestSkewedPartitionRebalancer.java new file mode 100644 index 000000000..1558386e1 --- /dev/null +++ b/defects/trino/unit/TestSkewedPartitionRebalancer.java @@ -0,0 +1,124 @@ +import java.util.*; + +/** + * Unit test for trino-0002: SkewedPartitionRebalancer scaledPartitions ArrayList.contains O(P²) + * + * Simulates the defective and fixed versions of the scaledPartitions dedup check. + */ +public class TestSkewedPartitionRebalancer { + + // --- Defective: ArrayList.contains per partition --- + static int simulateDefective(int numPartitions, int numTaskBuckets) { + int totalCost = 0; + List scaledPartitions = new ArrayList<>(); + + for (int bucket = 0; bucket < numTaskBuckets; bucket++) { + for (int partition = 0; partition < numPartitions; partition++) { + totalCost += scaledPartitions.size(); // cost of ArrayList.contains + if (scaledPartitions.contains(partition)) { + continue; + } + scaledPartitions.add(partition); + } + } + return totalCost; + } + + // --- Fixed: HashSet.contains per partition --- + static int simulateFixed(int numPartitions, int numTaskBuckets) { + int totalCost = 0; + Set scaledPartitions = new HashSet<>(); + + for (int bucket = 0; bucket < numTaskBuckets; bucket++) { + for (int partition = 0; partition < numPartitions; partition++) { + totalCost += 1; // O(1) HashSet.contains + if (scaledPartitions.contains(partition)) { + continue; + } + scaledPartitions.add(partition); + } + } + return totalCost; + } + + public static void main(String[] args) { + int pass = 0, fail = 0; + + // Test 1: correctness - same partitions get deduplicated + { + int P = 20, B = 3; + List seenDef = new ArrayList<>(); + Set seenFix = new HashSet<>(); + + for (int b = 0; b < B; b++) { + for (int p = 0; p < P; p++) { + if (!seenDef.contains(p)) seenDef.add(p); + seenFix.add(p); + } + } + boolean ok = (seenDef.size() == seenFix.size()) && + new HashSet<>(seenDef).equals(seenFix); + System.out.printf("[%s] Correctness P=%d B=%d: defective=%d unique, fixed=%d unique%n", + ok ? "PASS" : "FAIL", P, B, seenDef.size(), seenFix.size()); + if (ok) pass++; else fail++; + } + + // Test 2: cost at P=100, B=5 + { + int P = 100, B = 5; + int defCost = simulateDefective(P, B); + int fixCost = simulateFixed(P, B); + double ratio = (double) defCost / fixCost; + boolean ok = ratio >= 30.0; // expect significant quadratic vs linear gap + System.out.printf("[%s] Cost ratio P=%d B=%d: defective=%d, fixed=%d, ratio=%.1fx%n", + ok ? "PASS" : "FAIL", P, B, defCost, fixCost, ratio); + if (ok) pass++; else fail++; + } + + // Test 3: cost at P=1000, B=10 + { + int P = 1000, B = 10; + int defCost = simulateDefective(P, B); + int fixCost = simulateFixed(P, B); + double ratio = (double) defCost / fixCost; + boolean ok = ratio >= 200.0; + System.out.printf("[%s] Cost ratio P=%d B=%d: defective=%d, fixed=%d, ratio=%.1fx%n", + ok ? "PASS" : "FAIL", P, B, defCost, fixCost, ratio); + if (ok) pass++; else fail++; + } + + // Test 4: empty partitions case + { + int P = 0, B = 5; + int defCost = simulateDefective(P, B); + int fixCost = simulateFixed(P, B); + boolean ok = (defCost == 0 && fixCost == 0); + System.out.printf("[%s] Empty partitions P=%d B=%d: defective=%d, fixed=%d%n", + ok ? "PASS" : "FAIL", P, B, defCost, fixCost); + if (ok) pass++; else fail++; + } + + // Test 5: single bucket - verify O(P²) vs O(P) scaling + { + int[] sizes = {10, 50, 100, 200}; + boolean allOk = true; + for (int P : sizes) { + int defCost = simulateDefective(P, 1); + int fixCost = simulateFixed(P, 1); + double ratio = (double) defCost / fixCost; + System.out.printf(" P=%d: defective=%d, fixed=%d, ratio=%.1fx%n", P, defCost, fixCost, ratio); + // ratio should grow roughly proportional to P + } + // Check that ratio at P=200 is >> ratio at P=10 (demonstrates quadratic growth) + double ratioSmall = (double) simulateDefective(10, 1) / simulateFixed(10, 1); + double ratioLarge = (double) simulateDefective(200, 1) / simulateFixed(200, 1); + allOk = ratioLarge > ratioSmall * 5; // quadratic: 200/10=20x more ratio + System.out.printf("[%s] Quadratic scaling check: small ratio=%.1fx, large ratio=%.1fx%n", + allOk ? "PASS" : "FAIL", ratioSmall, ratioLarge); + if (allOk) pass++; else fail++; + } + + System.out.printf("%nResult: %d PASS, %d FAIL%n", pass, fail); + System.exit(fail > 0 ? 1 : 0); + } +} diff --git a/defects/zookeeper/patch/CLEAN.md b/defects/zookeeper/patch/CLEAN.md new file mode 100644 index 000000000..a5ad8387f --- /dev/null +++ b/defects/zookeeper/patch/CLEAN.md @@ -0,0 +1,30 @@ +# ZooKeeper CWE-407 Scan Result: CLEAN (beyond zookeeper-0001) + +## Scan Date +2026-03-30 + +## Scope +`zookeeper-server/src/main/java/org/apache/zookeeper/` + +## Findings + +No new CWE-407 defects found beyond the pre-existing zookeeper-0001. + +### Key structures verified + +| Structure | Location | Type | Status | +|-----------|----------|------|--------| +| `forwardingFollowers` | `Leader.java:196` | `HashSet` | CLEAN | +| `cnxns` | `ServerCnxnFactory.java:208` | `ConcurrentHashMap`-backed Set | CLEAN | +| `retainedTxnLogs` | `PurgeTxnLog.java:112` | `HashSet` | CLEAN | +| `aclKeyMap` | `ReferenceCountedACLCache.java` | `HashMap, Long>` | CLEAN | +| `connectingFollowers` | `Leader.java` | passed to `containsQuorum()` via Set | CLEAN | +| `enforceAuthSchemes` | `AuthenticationHelper.java:45` | `ArrayList` — but N≤5 config list, not hot | LOW (N too small) | +| Watch managers | `WatchManager*.java` | `HashSet` / `BitSet` | CLEAN | + +### Note on `enforceAuthSchemes` + +`enforceAuthSchemes` at `AuthenticationHelper.java:45` is an `ArrayList` with `.contains()` +called in `isCnxnAuthenticated()`. However, this list contains at most 2–3 scheme strings +configured at startup. The O(N) cost per auth check is negligible (N is bounded and tiny). This +does not rise to the level of a CWE-407 defect.